feat(assistant): add web and IM access and enable by default

[xxhZs]

Summary

Make the personal assistant available by default and let connected IM channels choose Personal Assistant alongside ordinary sessions and WorkHub. Preserve an explicit disabled setting and keep the default IM routing mode as ordinary sessions.

• Add an opt-in web chat surface with shared assistant history, message summaries, avatars, and live updates. The server is off unless configured, binds to loopback by default, and does not provide built-in authentication; remote exposure requires access protection.
• Generalize assistant routing across all eight bot providers, with per-channel overrides, private-user checks, durable delivery receipts, and real private-chat IDs. Preserve existing WeChat/Feishu receipts, reply context, and typing/read feedback.
• Add a compact task-status float while retaining the existing assistant chat presentation and WorkHub.
• Review only waiting/completion settlements; allow continued work through the existing revision and idempotency guards. Preserve meaningful execution evidence and reject material gaps without requiring repeated evidence formatting.
• Merge current main so previously merged Bot, typing, and shared-chat fixes are retained.

Validation

• Workspace dependency builds and full desktop build passed (including the web bundle).
• Desktop type checks passed for main, preload, renderer, Storybook, and assistant web.
• 65 core settings, IM adapter, assistant service, and Bot tests passed.
• 85 desktop web/IM, onboarding, task-float, routing, and WorkHub bridge tests passed.
• 184 assistant component tests passed, including settlement-review regression coverage.
• Diff whitespace checks passed.

These are controlled tests; this submission did not run new paid model tests, send messages through real IM accounts, or deploy/restart the daily application. Build output retains the existing large-bundle warnings.

AI use

• Generative tooling made a substantive contribution

Tool: OpenAI Codex. Codex assisted with implementation, review-fix analysis, code edits, tests, and PR documentation. These contributions include material AI-generated code. Preserve Generated-by: OpenAI Codex in the final squash commit; the latest integration commit includes this trailer.

Latest integration verification

Merged main at 21f1cc1c2 and regenerated both Astryx inventory artifacts. On the merged tree: the Astryx surface check passed (344 files), renderer architecture checks passed (168 tests), 15 assistant task/chat/navigation tests passed, and the UI dependency build, renderer typecheck, and production renderer build passed. The renderer retains existing large-bundle warnings. Full-repository tests, live IM-provider tests, and paid-model tests were not rerun for this integration; remote CI is separate from these local results.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

First review of 57cde86b..74c8f378 (the merge-base is the current main tip, the #6124 merge; 52 files, +4302/-86). I focused on security for the new IM/web ingress and on the default-enable change. No earlier findings to track.

P2: Default-on silently enables the assistant for existing users and can now break Host boot for them. PersonalAssistantService.recover changes let enabled = false to true when enablement.json is missing (service.ts:359). On upgrade, every user who never opted in therefore gets legacy plugin config/data directories adopted and assistant.json written. They also get the memory-network/proactive-matters/index-initiative modules initialized with their schedulers started, and the assistant's prompt sections and tools active. The test that guarded this ("existing component configuration is not implicit consent") was rewritten to drop that guarantee. There is also a failure mode: loadSettings/initialize throw on ambiguous legacy config ("Multiple legacy configurations ... cannot choose a data authority"), an invalid data directory, or a malformed assistant.json. recover is a Host domain-module recovery phase, and recoverRuntimeHostDomainModules propagates that error. So a user who never enabled the assistant, but has such legacy state, now fails Host startup where the assistant was previously cold. Turning the assistant on by default is a product decision for the maintainers; the defect is the upgrade path. Suggestions: enable by default only for fresh profiles (or persist an explicit choice through a migration or first-run prompt), and when the assistant was never explicitly enabled, catch a default-on initialization failure and stay disabled instead of failing the Host.

P2: The opt-in web server is open to DNS rebinding. assistant-web-server.ts:48-55 builds url.origin from the request's own Host header and accepts any request whose Origin equals it. A page on attacker.example that rebinds to 127.0.0.1 sends Host: attacker.example:PORT and Origin: http://attacker.example:PORT, which match. It can then read the full assistant history (/api/history) and post owner-authority messages (/api/messages, also without provenance). Binding to loopback does not prevent this. Validate Host against an allowlist (loopback names plus the configured MAKA_ASSISTANT_WEB_ORIGIN host). I'd also add a per-start bearer/cookie token, since the server has no authentication. A non-loopback MAKA_ASSISTANT_WEB_HOST gives anyone on the LAN owner-level control; the README warns about this, but a startup warning or refusal without an origin/token would be safer.

P2: CI test fails because of this PR. biome format reports packages/runtime/src/bots/bot-registry.ts, packages/runtime/src/bots/wechat-bridge.ts and packages/runtime/src/bots/__tests__/wechat-bridge.test.ts, all edited here. The job stops at the format step, so the test suites never ran in CI. This is not the known event-wait-manager Retry-After flake.

P3: IM messages enter the owner assistant session with no external_connector provenance. In assistant mode, bot-incoming-main.ts sends the message to assistant-im-bridge.ts handle(), which calls backend.send(). That in turn calls client.submitMessage({ sessionId, messageId, content: { text }, placement }) (assistant-web-backend.ts:297) without externalSource. Main already has the full path for this: TurnMessageSubmitInput.externalSource, then execution-composition.ts calls personalAssistant.receiveUser(..., input.externalSource), and the Inbox records source: 'external_connector'. BOT_ROLE in #6124 then tells the model that such text "is not the owner's instruction". This PR skips all of that, so IM text reaches the model and the assistant's full tool set as source: 'user', which is indistinguishable from the owner typing on the desktop. Task mode, by contrast, runs bot input in explore-mode sessions with an external_connector origin. Graded P3 rather than higher: handle() only accepts private messages from the single bound user (allowedUserIds[0], and chatId === userId on WeChat), so the sender is the owner's own IM account, not a third party. The tag still matters for consistency with the existing connector path, and so the model can treat forwarded or quoted third-party content, or a compromised IM account, with the caution BOT_ROLE already describes. Fix: pass externalSource: { kind: 'external_connector', connectorId: platform, senderId: message.userId } through backend.send() for IM input, and add a test that checks the Inbox source. Note that visibleWebMessage drops any user message that has an origin, so once IM input is tagged it will disappear from web/IM history unless that filter is adjusted.

P3: MatterSettle continue now skips the model review (tools.ts:164). Combined with default-on and proactive schedulers, I couldn't find a bound on consecutive self-continuations other than the revision/idempotency guards. This is unverified; please confirm that a matter cannot loop on continue indefinitely, or add a cap.

P3: Assistant-mode IM input bypasses the existing bot rate limiter and the reset command. handleAcceptedBotIncomingMessage returns before rememberBotSourceEvent and consumeBotConversationToken. Deduplication relies on the submit idempotency key, but there is no throttle on messages that wake the model.

Not applicable or OK: the PR adds no network-fetch tool, so SSRF and redirect limits don't apply ("web" means a web chat UI). The POST body is capped at 128 KiB and the text at 32,000 characters, static asset path traversal is guarded, and history cursors are HMAC-signed. The IM default stays task, so existing IM routing is unchanged unless a channel is switched to assistant. WeChat context tokens are stored with 0600 permissions in a 0700 directory, and the allowlist is now enforced at ingest. Credentials stay in the existing settings store; the receipt identity hash includes secrets but only as a SHA-256 digest.

Protocol epoch: no packages/runtime-host/src/protocol files are touched, so there is no epoch impact (main stays at 224).

Mergeability: GitHub reports MERGEABLE/BLOCKED. Checks: test fails (format, PR-caused); audit, label, package and package-linux pass.

[Astro-Han]

P3: IM text is submitted with no externalSource, so the assistant Inbox records it as source: 'user' (owner) instead of external_connector. Pass { kind: 'external_connector', connectorId: platform, senderId: message.userId } through backend.send() into submitMessage.

[xxhZs]

Deferring provenance changes in this patch at the author's request. The assistant channel is an alternative interface for the single explicitly bound owner; the current BOT_ROLE interpretation of external_connector as not the owner's instruction would change that intended behavior. A future change should distinguish transport provenance from authority and preserve tagged owner messages in history, rather than simply attaching the existing tag. We acknowledge the missing transport metadata; this is not a claim that forwarded content or account compromise is harmless.

[Astro-Han]

P3 (IM) / P2 (web, see DNS rebinding): submitMessage is called without externalSource for both IM and unauthenticated web input. Callers need a way to supply connector provenance here.

[xxhZs]

For IM, deferring until transport provenance can be represented without treating the bound owner's cross-device instructions as non-owner input (details in the adjacent bridge thread). For web, 9191759 independently validates Host/Origin and refuses direct LAN/public binding. Remote access must use an authenticated reverse proxy; publicOrigin alone does not authenticate a client. No provenance tagging change is included in this revision.

[Astro-Han]

P2: url.origin comes from the attacker-controlled Host header, so a DNS-rebinding page passes this check. Validate Host against an allowlist (loopback names plus the publicOrigin host), and ideally require a per-start token.

[xxhZs]

Fixed in 9191759. Host is now checked independently against loopback authorities at the actual listening port and the explicitly configured public-origin authority. Origin uses its own trusted allowlist; forwarded headers grant no access. Regression requests with matching attacker Host/Origin are rejected for history, events, avatar, messages, and static paths, including requests with no Origin. Direct non-loopback binding is now refused. Remote/phone access requires an authenticated reverse proxy protecting the page and all API routes; configuring publicOrigin is not authentication. We did not add a second built-in login/token system in this change.

[Astro-Han]

P2: Defaulting to true when enablement.json is missing enables the assistant (legacy adoption, schedulers) for existing users without consent. A legacy-config error from initialize() now also fails Host recovery for users who never opted in. Consider applying default-on only to fresh profiles, or falling back to disabled on failure when the assistant was never explicitly enabled.

[xxhZs]

Fixed the Host-boot failure in 9191759. Assistant recovery errors now leave the assistant unavailable with an error exposed in Settings, while Host startup can proceed. Saved preferences and data are not rewritten on recovery failure; after configuration repair, Settings can retry initialization. Tests cover ambiguous adoption, malformed settings, malformed enablement, explicit enabled state, and successful retry. Default-on remains an explicit product requirement from the author; an existing explicit false remains respected.

[Astro-Han]

P3: continue now skips the review entirely. Is there a bound on consecutive self-continuations?

[xxhZs]

Leaving this behavior unchanged intentionally, as requested by the author. There is no new consecutive-continue cap: continue means the worker still has work it can execute in the current run. The requested policy reviews wait and complete, while continue retains state/revision/idempotency guards. A hard continuation count previously caused unfinished work to stop without a useful outcome. This does not prove a model can never loop; it is a deliberate execution-policy choice, not a claim that the existing guards enforce progress.

[xxhZs]

Addressed the selected review findings in 9191759:

• Isolate assistant initialization failures from Host recovery, surface the error in Settings, retain data/preferences, and support retry after repair. Default-on is retained by explicit product decision.
• Validate web Host and Origin independently. Refuse direct non-loopback bindings; remote access requires an authenticated reverse proxy. Added DNS-rebinding and binding-policy regressions.
• Apply the repository formatter to the three reported runtime files. Full npm run format:check now passes locally.

Validation: 67 targeted assistant/Bot/web/IM tests passed; runtime-host and desktop-main builds, desktop-renderer build, desktop type checks, and diff whitespace checks passed. The renderer retains large-bundle warnings. The new remote CI run is separate from these local results; the earlier CI run did stop before tests at formatting.

Deferred intentionally: IM rate limiting/reset changes, transport provenance, and a continuation cap. Assistant input already enters the durable Inbox and is batch-delivered to the same Bot; it does not create a separate Agent per message. The author requested keeping that behavior instead of importing ordinary-task throttling/reset semantics in this patch. Inbox batching is not an abuse-rate limit, so flooding remains a separate hardening question. A reset for a channel must not accidentally reset the shared assistant conversation. Provenance and continuation-policy reasoning are also recorded in their inline threads.

No daily-app restart or new real-model/account-message testing was performed for this revision.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of 74c8f378..1250bda9 (head 1250bda9c8fbb36fe1c9d0c4848f5241a4304518). The branch was not rebased: the merge-base with main is still 57cde86b, so the delta is exactly the three new commits (9191759, 6394128 and 1250bda; 21 files, +932/-222).

Prior findings

• Default-on upgrade path (P2): Host boot failure fixed; the default-on behaviour itself remains a maintainer decision. PersonalAssistantService.recover now catches failures from reading the enablement preference and from initialize(). It records them as initializationError, which assistant.enablement exposes to Settings, and leaves enablement.json and assistant.json untouched. initialize() already closes a partially recovered runtime. So ambiguous legacy config or a malformed assistant.json no longer fails Host recovery, and the new tests cover both cases. Users who never opted in are still enabled silently on upgrade. I'm leaving that to the maintainers as a product decision rather than a defect.

• Web server DNS rebinding (P2): fixed. Host is now checked against a fixed allowlist: 127.0.0.1:PORT, localhost:PORT and [::1]:PORT for the actual bound port, plus the configured public origin's host, compared lowercase. Origin is checked separately against the matching origins. I checked these Hosts against the allowlist:

  • Rejected: attacker.example:PORT, the wrong port, a missing port on a non-80 port, localhost. (trailing dot), [0:0:0:0:0:0:0:1], [::ffff:127.0.0.1] and 127.1.
  • Accepted: LOCALHOST:PORT, [::1]:PORT and assistant.example.

  A rebinding page cannot produce an allowed Host, because the browser sets Host from the attacker's own domain. Non-loopback binds now refuse to start, and that failure is caught and logged in runtime-host-boot.ts, so it does not crash the app. One minor note: a proxy that forwards an explicit default port (assistant.example:443) will get a 403. That fails closed and is acceptable. The server still has no authentication of its own, so any local process can use it; the README now says this clearly.

• CI format failure (P2): fixed, but CI now fails on a different PR-caused check (below). biome format is clean on the changed files; the bot-registry and wechat-bridge changes in 9191759 are formatting only.

• IM input without external_connector provenance (P3): open. assistant-im-bridge.ts, assistant-web-backend.ts and bot-incoming-main.ts are not touched in this range.

• MatterSettle continue skips review / unbounded (P3): open. tools.ts is unchanged.

• Assistant-mode IM bypasses the bot rate limiter and reset (P3): open. Unchanged.

New findings

P2: CI test fails at the renderer architecture check, and this PR causes it. check:architecture reports two errors. First, src/renderer/app-shell.tsx now uses PersonalAssistantTaskLayout, which rootSymbolUses does not record. Second, the existing PersonalAssistantChatLayout entry is stale. The PR changed app-shell.tsx (lines 20, 1307 and 1477) but not apps/desktop/renderer-architecture.json (line 422 still lists PersonalAssistantChatLayout). I reproduced the failure locally with node scripts/check-renderer-architecture.mjs --base 57cde86b --strict-base. The earlier format failure hid it, and the job still stops before the test suites run. This is not the event-wait-manager Retry-After flake.

P2: On the shared maintenance schedule, one index that never finishes stops scheduled maintenance for every other index. In tick(), a new round starts only when schedule.remainingIds is empty (controller.ts:1025). A member that does not finish stays in remainingIds, and tick() then just retries it after retryAt, with backoff capped at 1 h. Each retry is limited to the owner of eligible[0] (:1045). So while any member keeps failing, nextCheckAt is never consulted again, and no other index (same owner or another owner) gets another scheduled round. Before this commit each index had its own clock, so a failing index only affected itself.

I verified this with a test added to test/assistant/memory-network/test/ using the existing setup() fixture with retryMs: 20. The worker runner checkpoints ids[1] but never ids[0]. After the first round I set nextCheckAt to 0. The batches were [[0,1],[0],[0],[0],[0],[0],[0],[0]] and remainingIds stayed [ids[0]]: ids[1] was never maintained again, even though the shared deadline had passed.

Realistic triggers are any persistent per-member failure: the model not checkpointing one index, an observation or visibility error (throw result.reason aborts the whole group), or Host rejected group maintenance. Suggested fix: when nextCheckAt passes, start a new round anyway and merge it with the leftover members. Alternatively, drop members after N consecutive failures, or rotate owners and members so the round moves past a failing one. Also add a regression test like the one above.

P3: Per-index intervals configured before this commit are silently dropped. schedule() adopts the earliest existing nextCheckAt once, but takes intervalMs from config.intervalMs (default 12 h) and ignores any worker.intervalMs that MemoryIndexControl configure set earlier (controller.ts:772). A user who set, say, 1 h on one index silently moves to 12 h. Consider adopting the minimum configured interval, or reporting the change. Related: MemoryIndexControl with an indexId and intervalMs now changes the global interval while checking visibility of only that one index; the indexId-less path checks all indexes. This is low impact for a single-owner assistant, but the two paths are inconsistent.

P3: The shared maintenance session is persisted per owner and never re-created. schedule.sessions[owner] is created once and reused for every later round. If that session is deleted or can no longer be resumed, every scheduled round for that owner fails, and through the finding above that stops all scheduled maintenance. The per-index path had the same no-recreate pattern before, so I'm grading this P3. Consider clearing sessions[owner] when agents.resume fails.

Transport-failure recovery (6394128): no defect found. ERR_HTTP2_STREAM_ERROR is added to both the pre-response and the 2xx-response transport sets. Model calls are replayed, but the existing 2xx durable-tool-result guard prevents repeating tool effects; tests cover both codes across recover, exhaust, stop and step-limit. Note that this classification change applies to all providers and sessions, not only the assistant.

In the bot controller, an error is now recorded only up to errorThrough = head at the time of the attempt. New mail after a failure triggers one fresh attempt, with no timer-driven retry storm; the tests cover this. A blocked worker is restarted only when pendingInteractions === 0, and production snapshots do expose pendingInteractions (execution-composition.ts:2351, same pattern as execution-outcome.ts). Replaying unacknowledged inbox items after a failed turn is the existing at-least-once design. A reply the model sent before the transport failure can therefore be sent again under a new messageId; that is inherent to the replay design and not new here.

Shared schedule, concurrency and shutdown: otherwise OK. tick() is serialized through this.ticking and returns while any run is active. launch re-checks runs for every member. close() clears both intervals, aborts and awaits ticking and the runs, then releases the lease. The existing shared-schedule tests pass locally.

Protocol epoch: no packages/runtime-host/src/protocol files are touched, so there is no epoch impact.

Mergeability: GitHub reports MERGEABLE/BLOCKED. Checks on 1250bda9: test fails (renderer architecture check, PR-caused, see above); audit, package and package-linux pass.

[Astro-Han]

P2: A new round starts only when remainingIds is empty. A member that never finishes stays here, and tick() keeps retrying it with backoff, so nextCheckAt is never consulted again and no other index gets another scheduled round. I verified this with a fixture test: after nextCheckAt was set to 0 the batches were [[0,1],[0],[0],...]. Consider starting a new round once the deadline passes even when members are left over, or dropping or rotating members that keep failing.

[xxhZs]

We consider this a reasonable trade-off for the shared maintenance design: an index maintenance failure is itself something that needs to be addressed. Incremental reads can still access history beyond the indexed coverage (MemoryHistory from coveredCursor to observedCursor, or direct original-source reads), so stalled maintenance does not by itself make new information unreadable. This is about read availability, not indexing freshness: other indexes' scheduled maintenance can still be delayed by the stalled round.

[Astro-Han]

P3: Interval adoption ignores per-index worker.intervalMs set by an earlier MemoryIndexControl configure, so a custom interval (for example 1 h) silently becomes 12 h after upgrade.

[Astro-Han]

P2 (CI): check:architecture fails. The appShell root now uses PersonalAssistantTaskLayout, but apps/desktop/renderer-architecture.json rootSymbolUses still lists PersonalAssistantChatLayout. Please update the manifest.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Summary: two independent automated reviews of b7aa5cab (different model families). Not ready to merge: one new P1 (Feishu drops inbound messages for channels that follow the global routing mode), and two open P2s (PR-caused check:architecture CI failure; memory maintenance stall, disputed by the author). Both reviews independently found no new exposure from "enabled by default".

Incremental review of 1250bda9..b7aa5cab (head b7aa5cabc44ad2af54fdf6cacaee5c0d81aacc4c), plus a pass over the whole PR (57cde86b..b7aa5cab, 63 files). There was no rebase: the merge-base is still 57cde86b, and the increment is one commit (b7aa5ca, "hide archived tasks from the active list"; 5 renderer files). That commit puts paused ahead of the error states in projectTasks and hides paused, completed and cancelled tasks behind the "archived" toggle. The test covers it and I found no defect.

Prior findings

• CI test fails at check:architecture (P2): open. CI on b7aa5cab fails with the same two errors: app-shell.tsx uses PersonalAssistantTaskLayout, which rootSymbolUses does not record, and the PersonalAssistantChatLayout entry is stale. apps/desktop/renderer-architecture.json is still not updated. Main CI is green (2333c1ab, 0b9204b1), so this PR causes it. The job still stops before the test suites run.
• Shared memory maintenance round stalls on one failing member (P2): open; the author disputes it. memory-network/controller.ts is unchanged. The author's reply says reads still work through MemoryHistory/source reads and acknowledges that other indexes' scheduled maintenance "can still be delayed by the stalled round". That delay has no bound for as long as the member keeps failing. I'm keeping this open for the maintainers to decide; the fix suggested earlier (start a new round once nextCheckAt passes, or drop or rotate members that keep failing) still applies.
• Default-on upgrade path: the Host boot failure is fixed (as at 1250bda). Default-on itself is a product decision.
• Web DNS rebinding: fixed (as at 1250bda).
• P3s still open, with the author's stated rationale: IM input carries no external_connector provenance, MatterSettle continue has no cap, assistant-mode IM skips the bot rate limiter and reset, per-index intervals are dropped, and the per-owner maintenance session is never re-created.

New finding

P1: After upgrade, Feishu drops every incoming message for users on the default task routing. feishu-bridge.ts:354 computes const assistant = (this.settings.messageHandling ?? 'assistant') === 'assistant'. The bridge receives the raw channels.feishu object from BotRegistry.applySettings. In that object, messageHandling is undefined unless the user picked a per-channel override; "Follow default" stores undefined, and normalizeBotChannel keeps it that way. So a channel that inherits the global mode is always treated as assistant mode, even though resolveBotMessageHandling routes it to task (or workhub). With assistant true, the bridge drops all group messages, and it drops all DMs unless allowedUserIds has exactly one entry. Before this PR the Feishu settings had no allowlist field, and the bridge ran with dmMode: 'open' and no extra filter (main feishu-bridge.ts:183-187). So every existing Feishu user in the default task or WorkHub mode stops receiving bot messages after upgrade, with no error shown. Users who set one allowed user still lose group chats.

Reproduced with a test that uses the same fake-SDK fixture as feishu-bridge.test.ts:

• Global messageHandling: 'task', a Feishu channel with no override and no allowlist: resolveBotMessageHandling returns task, and one DM plus one group message produce 0 incoming events.
• The same channel with allowedUserIds: ['ou_any']: the group message produces 0 events.
• Control, with an explicit channel messageHandling: 'task': both messages arrive.

The PR's own test (assistant DM policy rejects groups...) builds the channel with createDefaultBotChannel('feishu'), so it relies on this default without saying so.

Fix: don't default to assistant in the bridge. Either pass the resolved mode into the bridge (for example, applySettings sets each channel's messageHandling from resolveBotMessageHandling, and changes to the global mode reach updateSettings), or apply the single-user DM rule only in the assistant routing path, which assistant-im-bridge.ts handle() already enforces. Please add a test for the inherited task mode.

Exposure from "enabled by default" (checked, no further defect)

• Default-on enables only the in-app assistant runtime (modules, schedulers, prompt and tools). It opens no new inbound surface.
• The web chat starts only when MAKA_ASSISTANT_WEB_PORT is set. It refuses to bind anywhere other than loopback and checks Host against fixed loopback authorities plus the configured origin. Cross-origin POST /api/messages requires an application/json preflight, and GET responses cannot be read cross-origin. It has no authentication of its own, so any local process can drive the owner's assistant; the README documents this.
• IM assistant routing is opt-in per channel (the global default stays task). handle() accepts only private, non-group messages whose platform-reported userId equals the single allowedUserIds[0], and on WeChat it also requires chatId === userId. Sender identity comes from the authenticated Feishu websocket and the WeChat iLink long-poll, not from an HTTP endpoint the PR opens. IM and web text reaches the owner session as source: 'user' (the P3 above).
• Onboarding now pre-fills allowedUserIds with the onboarding user for new WeChat and Feishu bindings, which also restricts task mode for new setups. That restriction is intentional and safer.
• SSRF: the PR adds no fetch of URLs from message content. The outbound calls go to the configured iLink base URL and the Feishu SDK only, and Feishu resources become an attachmentKind without being downloaded.

Persistence

• New files: assistant-wechat/<platform->sha256(rootId).json (delivery receipts) and wechat-reply-context/<sha256(url,bot,token)>.json (iLink context tokens). Both are written atomically with mode 0600.
• Context files keyed on an old token are never removed after a token rotation. That is a minor cleanup item (P3).
• The settings additions (messageHandling: 'assistant', the per-channel override) degrade safely on older builds, which normalize unknown values to task or ignore them.
• No schema migrations.

Protocol epoch: no packages/runtime-host/src/protocol files are touched, so there is no epoch impact and no collision with the open protocol PRs. #5623 (Feishu approvals) also edits bot-incoming-main.ts; the assistant-mode early return here would bypass whatever it adds for assistant-routed messages (P3, coordination only).

Mergeability: GitHub reports MERGEABLE/BLOCKED. Checks on b7aa5cab: test fails (check:architecture, PR-caused); audit, package and package-linux pass.

Second independent review (confirms the P1 and the CI P2)

A second reviewer, working without the first review's conclusions, reproduced the check:architecture failure locally (fresh main passes) and checked all eight IM receivers end to end (adapter filter → assistant bridge / web backend → Host submit): unknown senders, groups, empty or multi-user bindings, disabled channels and non-assistant modes are all rejected from the assistant path, and the web server does not listen without a configured port. Asked to adjudicate the Feishu routing finding, it confirmed it as P1 with a real BotRegistry → Feishu receive-callback matrix, swapping in only main's Feishu module as the control:

• Global task, inherited channel, no allowlist: 0 events at this head; both messages with main's Feishu module.
• Global workhub, inherited: also 0 events. Switching the global mode from task to workhub at runtime leaves the same channel dropping messages.
• One allowed sender: group messages still dropped.
• Explicit channel task / workhub overrides: both messages delivered.
• The other seven provider receivers have no messageHandling branch, so the issue is Feishu-only.

[Astro-Han]

P1: settings.messageHandling is undefined when the channel inherits the global mode (the default), so this treats default task/WorkHub channels as assistant mode. It then drops all group messages, and all DMs unless exactly one user is allowed. Existing Feishu users had no allowlist (dmMode: 'open'), so they stop receiving messages after upgrade. I reproduced this with the fake-SDK fixture: with inherited task and no allowlist, a DM and a group message produce 0 events, while an explicit task delivers both. Pass the resolved mode into the bridge, or leave the single-user rule to assistant-im-bridge handle().

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Reviewed f1af55a9b6572334187ff29ed015d44cce385387, including the two commits after b7aa5cab.

The previous Feishu P1, renderer architecture P2, and shared maintenance P2 are fixed. The registry resolves inherited message handling for both bridge creation and settings updates. Actual Feishu message delivery now preserves inherited task/WorkHub reception, while switching the same bridge to assistant mode enforces its owner requirement. Loading only the previous registry/bridge restores the message loss. The strict architecture check also passes. Failed maintenance members now retain their outstanding ranges without retaining the global round indefinitely; healthy indexes and other owner batches continue. The current memory suite passes 83 tests, and replacing only the production controller with its previous version makes both failure/recovery regressions fail again.

P2 — Task float still fails the required Astryx surface check. In task-float.tsx:53–62, the expand/collapse, task selection, archive and paging controls use raw <button> elements. CI run 39214057862 fails at “Astryx surface inventory” on this file. I reproduced the same blocker locally; the same checker passes against current main's product sources and committed inventory. The PR also leaves three new product files out of the committed .paths inventory (task-float.css, task-float.tsx, task-layout.tsx). Replace the handwritten controls with Astryx primitives and regenerate both inventory artifacts. This is a remaining PR failure exposed after fixing the earlier architecture failure; it was not introduced by these last two commits.

Validation: fresh dependency installation and normal root build:test passed; 99 focused tests passed (83 memory network, four Host feedback, 12 registry/Feishu), plus the actual message delivery/update checks and previous-code controls. The strict renderer architecture check passed. The reviewed head is still open and mergeable; audit and Linux/Windows packaging checks passed, while the test check remains red for the issue above. This incremental pass does not claim new live IM-provider, paid-model, or full desktop end-to-end coverage.

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Reviewed f1af55a9..5fab9edb6ee0b7552feb87b03ac52c8f56d17b14: one appended commit, four files, with no rebase.

The previous task-float CI P2 is fixed. task-float.tsx now uses Astryx Button for each control, and both generated inventory artifacts include the three new product files. The inventory check passes with 348 files. Running the same checker against the previous head's sources reproduces the raw-button blocker.

I found no new P0–P2 issue in this increment. I checked the real Button implementation's children/label behavior and exercised the production task/chat frame in Chromium at 480 and 1280 pixels in English, Simplified Chinese and Traditional Chinese. Progress and timestamps remain visible; task detail/back, paging and disabled controls, archive state, and keyboard collapse/expand work. The five task projection, Host-selection and polling tests also pass. The earlier Feishu and maintenance fixes are outside these four changed files and are not being claimed as newly retested here.

Validation: fresh dependencies, normal root build:test, a clean renderer typecheck and production renderer build, strict renderer architecture, repository lint/format, and the surface inventory passed. There is no protocol change; the epoch remains 224. These are controlled component/browser checks, not a live IM/provider or full-application test.

P3 integration follow-up: the head is open but conflicts with current main only in docs/astryx-surface-file-inventory.md; merge main and regenerate the inventory before integration. No CI results are attached to this head, so the local results do not establish that the eventual merge tree passes CI. The PR body also still needs the required declaration about material generative-tool contributions.

[xxhZs]

Automated update posted by OpenAI Codex at the author’s request.

@Astro-Han Thanks for the review. I merged current main (21f1cc1c2), resolved the generated inventory conflict by regenerating both artifacts, and pushed e81b471d2. The PR body now includes the material generative-tool contribution declaration and the final squash trailer reminder.

On the merged tree, the Astryx surface check (344 files), strict renderer architecture checks (168 tests), 15 assistant task/chat/navigation tests, UI dependency build, renderer typecheck, and production renderer build passed. Full-repository/live-provider/paid-model tests were not rerun for this integration; remote CI still needs to finish.

Could you please take another look and, if there are no remaining concerns and the checks are satisfactory, approve the PR?

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of 5fab9edb..e81b471d. The only new commit is a merge of main plus a regenerated Astryx surface inventory; git show --remerge-diff shows the conflict resolution touches only docs/astryx-surface-file-inventory.md (+1/-5). No author runtime code changed. That resolves the P3 from the previous review.

P2: CI test fails at Knip (apps/desktop). Run 39240535098 reports one unused export: PersonalAssistantChatLayout at apps/desktop/src/renderer/features/personal-assistant/index.tsx:465. Since app-shell.tsx now uses PersonalAssistantTaskLayout, the only remaining consumer is task-layout.tsx, which imports it directly from ./chat-presentation.js, so the barrel re-export is dead. Dropping PersonalAssistantChatLayout from that export { … } line should make Knip pass. This is the first CI run on a head containing the earlier fixes, so it was not visible before.

No other changes since the last review; prior conclusions stand (no other open P0–P2).

[Astro-Han]

Automated review notice: This comment was posted by an automated review agent. It is not an independent human review and does not replace one.

Incremental review of e81b471d2ba51459fcc2af1252b3c104751fdb82 → f06385a1dc8032270d8fa6a80b5fc836f0b14cfd: three appended commits, eight files, 217 additions/11 deletions, with no rebase or main merge. This covers the unused export, explicit Host test enablement, and the latest Bot history-compaction change.

The previous Knip P2 is fixed; no new verified P0–P2 findings. PersonalAssistantChatLayout is no longer re-exported from apps/desktop/src/renderer/features/personal-assistant/index.tsx:465; its implementation and direct task-layout.tsx import remain. Desktop and UI Knip passed again at this head. In the preceding verification at 06626e60, restoring only the old re-export reproduced exactly one unused-export finding, after which the source was restored.

The fixture changes retain their coverage: the master-switch test first checks the enabled default, explicitly disables it, then checks execution gating; the ordinary provider-wire fixture explicitly disables assistant tools. Those two cases and the adjacent Host Inbox case passed again at this head: three passed, none skipped or cancelled. Loading the previous two test modules against 06626e60 had reproduced the disabled-default and ordinary-tool-list failures.

The additional compaction commit preserves ordinary-session policy and durable Inbox contents in the exercised paths. packages/runtime-host/src/server/execution-model-composition.ts:414 supplies an 80% fallback target only when both background-assistant and Bot labels are present and the selected model has a known effective capacity. packages/runtime/src/ai-sdk-compaction.ts:799 gives an explicit user compaction threshold precedence. Ordinary sessions, discovery sessions, single-label sessions, unknown-capacity models, and a below-threshold Bot do not acquire this automatic fold. The actual backend tests cover wake-time compaction and in-turn compaction while retaining the current wake anchor and tool execution. Loading only the previous production compaction modules makes both new fallback tests fail with zero checkpoints instead of one.

I also extended the actual backend fixture with the actual SQLite BotStore and BotController in 12 configurations, including higher/lower explicit thresholds. A message delivered earlier but left unfinished remained readable; unread messages posted before a fold and during checkpoint persistence remained available through InboxCheck; unread mail still prevented Sleep. Closing and reopening the Inbox database retained every message and replayed uncommitted delivery. Compaction shapes model history rather than deleting Inbox rows or acknowledging task completion (assistant/bot/store.ts:78, controller.ts:40). These are controlled backend/store/controller checks, not a full application restart with a live IM provider. Summaries remain lossy; this does not guarantee that a model remembers every unfinished requirement without consulting retained Inbox history.

At the new head, normal root build:test, workspace typechecks including assistant web, both Knip checks, formatting/lint checks for all six additional files, and the protocol guard passed. Epoch remains 224 with no protocol change. Seven relevant test files produced 202 passes and one failure, with no skips or cancellations. The failure is the managed-Bash case: this machine reports “Command sandbox is required but unavailable on platform linux.” The same case fails with the identical result when only the preceding compaction implementation is loaded. The separate three-case Host run and the 12-configuration Inbox check passed. Production renderer/web builds, strict architecture, inventory, ASF and repository-wide lint/format checks passed in the preceding 06626e60 verification; they were not all repeated after the compaction-only commit.

The exact head is OPEN/MERGEABLE, and a trial merge with current main was clean; I did not build that merged tree. CI 39250334952 and Windows check 39250334967 are still running at this check; Linux release and dependency-audit checks have succeeded. The earlier green runs at 06626e60 do not establish CI success for this head. I did not repeat full application/IM/browser coverage, paid-model tests, deployment, or the complete local test suite.