Cut legacy lifecycle and comms authority paths#28
Merged
Conversation
added 23 commits
May 25, 2026 12:49
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdopts a runtime-topology YAML store for session lifecycle and graveyard state; removes legacy agent input/history and dispatch writers; exposes a disabled runtime-core for mutation attempts; rewires multiplexer, metadata server, CLI, plugin, and client UI toward read-only transcript output and file-backed task handoffs. ChangesRuntime topology & core
Multiplexer & runtime wiring
Metadata server, CLI, plugin
Client UI and app changes
Attachments, inputs, collaboration, and tests
Estimated code review effort: Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
There was a problem hiding this comment.
Pull request overview
This PR removes legacy “authority” paths for session lifecycle/comms (metadata-backed backend session IDs, task/message dispatch injection, takeover wiring, chat history/input ops) and pivots session truth to the runtime-topology YAML while stubbing lifecycle mutations behind a disabled runtime-core boundary.
Changes:
- Cut legacy dispatch/takeover/session-history/input-operation plumbing and associated UI/SDK surfaces (desktop + app).
- Introduce/expand runtime-core topology storage + session lifecycle helpers and wire various readers to topology.
- Add YAML dependency and new dev-lane script for the OpenRig runtime-core lane.
Reviewed changes
Copilot reviewed 105 out of 108 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Adds yaml@^2.9.0 resolution. |
| package.json | Adds yaml dependency + OpenRig dev scripts. |
| scripts/dev-lanes/openrig-runtime-core.sh | New dev lane wrapper script for OpenRig runtime-core. |
| src/runtime-core/index.ts | Adds disabled runtime-core boundary + readTopology. |
| src/runtime-core/index.test.ts | Tests disabled runtime-core error behavior. |
| src/runtime-core/topology-store.ts | YAML-backed runtime topology store + coercion. |
| src/runtime-core/topology-store.test.ts | Tests topology YAML round-trip + corruption rejection. |
| src/runtime-core/topology-sessions.ts | Topology ↔ session-state mapping + lifecycle helpers. |
| src/runtime-core/topology-sessions.test.ts | Tests topology session lifecycle + pruning behavior. |
| src/paths.ts | Replaces graveyard path with runtime-topology path; removes session-messages/input-ops dirs. |
| src/metadata-store.ts | Removes backendSessionId from persisted metadata; scrubs on load/save. |
| src/main.ts | Removes raw agent input + backend-session recording; adds aimux topology command; graveyard uses topology. |
| src/config.ts | Removes sessionCapture config; updates .gitignore template notes. |
| src/multiplexer/index.ts | Removes task/orchestration dispatchers; aligns state to topology-owned lifecycle. |
| src/multiplexer/runtime-sync.ts | Heartbeat becomes liveness-only; topology sync replaces state sync. |
| src/multiplexer/session-launch.ts | Resume/restore now source from topology; removes backend session capture scheduling. |
| src/multiplexer/session-capture.ts | Removes filesystem capture logic; keeps Codex arg extraction. |
| src/multiplexer/session-runtime-core.ts | Deletes legacy agent input/history IO paths; stops using metadata backendSessionId. |
| src/multiplexer/dashboard-tail-methods.ts | Lifecycle mutations now route to disabled runtime-core methods; takeover removed. |
| src/multiplexer/dashboard-ops.ts | Removes session “seed” payloads for kill/resume; removes takeover op wrapper. |
| src/multiplexer/dashboard-ops.test.ts | Updates expectations for resume/kill payloads. |
| src/multiplexer/dashboard-model.ts | Offline session resolution now uses topology; removes agent input/history APIs from project service surface. |
| src/multiplexer/dashboard-model.test.ts | Updates mocks for removed project-service agent IO routes. |
| src/multiplexer/dashboard-interaction.ts | Review request import moved; removes remote takeover “Enter” behavior. |
| src/multiplexer/archives.ts | Graveyard now sourced from topology instead of graveyard.json. |
| src/multiplexer/worktree-graveyard.ts | Removes write helper for graveyard entries. |
| src/multiplexer/service-state-snapshot.ts | State.json becomes services-only snapshot; removes agent snapshotting. |
| src/multiplexer/service-state-snapshot.test.ts | Updates tests for services-only state snapshot behavior. |
| src/multiplexer/agent-io-methods.ts | Disables legacy orchestration delivery-to-agent-input path; interrupt now disabled runtime-core. |
| src/multiplexer/agent-io-methods.test.ts | Updates orchestration delivery test to assert no legacy delivery. |
| src/project-scanner.ts | Scans sessions from runtime topology + statusline enrichment; removes PID ownership field. |
| src/project-scanner.test.ts | Updates tests to seed topology instead of instances/state sessions. |
| src/project-events.ts | Removes history_update event type and publisher. |
| src/plugin-runtime.ts | Session listing now comes from topology session states. |
| src/project-service-manifest.ts | Capability flags updated (removes writer/history; keeps attachment read). |
| src/default-plugins/gh-pr-context.ts | PR target collection now driven by topology sessions; uses statusline/metadata only to fill paths. |
| src/default-plugins/gh-pr-context.test.ts | Updates PR target tests for topology-based selection. |
| src/debug-state.ts | Adds runtime-topology source; graveyard derived from topology sessions. |
| src/debug-state.test.ts | Updates debug-state tests to write topology instead of saved sessions. |
| src/dashboard/pending-actions.ts | Stops synthesizing “starting/stopping/migrating” missing session rows; only create/fork. |
| src/dashboard/pending-actions.test.ts | Updates expectations for removed synthesized session rows. |
| src/tmux/window-open.ts | Removes backendSessionId match when opening managed windows. |
| src/tmux/runtime-manager.ts | Removes backendSessionId match in managed window lookup. |
| src/tui/screens/dashboard-renderers.ts | Removes “Enter takeover” label path. |
| src/tui/screens/overlay-renderers.ts | Help text updated (“open, resume, or focus”). |
| src/tool-output-watchers.ts | Removes tool-output watcher observation + service extraction logic, keeps classification. |
| src/tool-output-watchers.test.ts | Trims tests to classify-only cases. |
| src/task-dispatcher.ts | Deleted legacy task dispatcher loop and requestReview duplicate. |
| src/task-workflow.ts | Retains review workflow handling + moves requestReview here. |
| src/orchestration-dispatcher.ts | Deleted legacy durable orchestration delivery loop. |
| src/orchestration-dispatcher.test.ts | Deleted tests for removed dispatcher. |
| src/session-message-history.ts | Deleted file-backed session chat history storage. |
| src/session-input-operations.ts | Deleted file-backed “agent input op” queue. |
| src/agent-message-parts.ts | Deleted structured agent input serialization (text/image parts). |
| src/agent-message-parts.test.ts | Deleted tests for removed serializer. |
| src/collaboration.ts | Deleted multi-user writer prefixing logic. |
| src/collaboration.test.ts | Deleted tests for removed collaboration module. |
| src/attachment-store.ts | Removes attachment ingestion/write paths; keeps read-side accessors. |
| src/instance-registry.ts | Heartbeat becomes liveness-only; stops persisting session refs; removes claim/takeover. |
| src/instance-registry.test.ts | Updates tests for liveness-only registry behavior. |
| src/instance-directory.ts | Removes claim proxy; renames sessions directory entry builder. |
| src/instance-directory.test.ts | Updates tests for renamed builder + removed claim proxy. |
| src/session-bootstrap.ts | Updates agent instructions: no metadata API usage unless explicitly commanded; tasks as handoff records. |
| src/session-bootstrap.test.ts | Updates assertions for new instructions content. |
| docs/tool-integration.md | Updates guidance: prompt pushing must live in runtime-core replacement. |
| docs/project-host-model.md | Notes takeover removal and topology-owned lifecycle. |
| docs/orchestration-prd.md | Updates language to “handoff records” + durable state (not delivery). |
| docs/multi-user-chat.md | Reframes to read-only session sharing; removes writer/attachments scope. |
| docs/latency-architecture-rfc.md | Terminology updates (payloads, orchestration writes). |
| docs/global-control-plane-rfc.md | Documents topology as lifecycle authority; instances.json liveness-only. |
| docs/desktop-ui-contract.md | Updates orchestration UI scope to read-only during runtime-core replacement. |
| docs/current-architecture.md | Updates canonical state surfaces: topology authoritative for lifecycle. |
| CODEX.md | Updates delegation protocol and removes sessions.json mention. |
| app/yarn.lock | Removes expo image-picker deps. |
| app/package.json | Removes expo-image-picker. |
| app/app.config.js | Removes photo permissions + image-picker plugin. |
| app/stores/projects.ts | Selected session now resolves from desktop-state cache, not daemon project payload. |
| app/stores/chat.ts | Removes chat history + pending message client state; transcript becomes output-only. |
| app/lib/events.ts | Removes history_update + agent input part types; trims message delivery fields. |
| app/lib/heartbeat.ts | Removes history_update from SSE subscription. |
| app/lib/api.ts | Removes agent history/input/attachments + agent lifecycle methods from app API client. |
| app/lib/api.test.ts | Updates API routing tests for removed routes; keeps output/plan/service calls. |
| app/lib/parsed-transcript.ts | Removes pending prompt de-dupe helper. |
| app/lib/parsed-transcript.test.ts | Removes tests for pending prompt de-dupe helper. |
| app/components/MessageBlock.tsx | Removes delivery-state UI rendering. |
| app/components/ChatComposer.tsx | Deleted message composer (no writer path). |
| app/lib/image-picker.web.ts | Deleted web image picker. |
| app/lib/image-picker.native.ts | Deleted native image picker. |
| app/lib/image-picker.d.ts | Deleted picker typings. |
| app/app/(main)/(tabs)/(dashboard)/agent/[sessionId]/chat.tsx | Chat screen becomes transcript/output viewer; removes history fetch + composer. |
| app/app/(main)/_layout.tsx | Renames “Shared chat” label to “Shared session”. |
| app/app/shares/invite/[ownerUserId]/[token]/accept.tsx | UI text updated (“Shared session invite”). |
| app/app/landing.tsx | Marketing copy updated to “Remote visibility”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+132
to
+137
| export function loadMetadataState(projectRoot?: string): MetadataState { | ||
| return loadJson<MetadataState>(metadataPathFor(projectRoot), { version: 1, sessions: {} }); | ||
| const state = loadJson<MetadataState>(metadataPathFor(projectRoot), { version: 1, sessions: {} }); | ||
| for (const session of Object.values(state.sessions)) { | ||
| delete (session as SessionMetadata & { backendSessionId?: string }).backendSessionId; | ||
| } | ||
| return state; |
Owner
Author
There was a problem hiding this comment.
Fixed in dd3e88d - metadata backend-session-id scrubbing now guards malformed session entries when loading persisted metadata.
Comment on lines
140
to
145
| export function saveMetadataState(state: MetadataState, projectRoot?: string): void { | ||
| for (const session of Object.values(state.sessions)) { | ||
| delete (session as SessionMetadata & { backendSessionId?: string }).backendSessionId; | ||
| } | ||
| saveJson(metadataPathFor(projectRoot), state); | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in dd3e88d - saveMetadataState uses the same guarded scrubber so malformed session entries do not throw during writes.
Comment on lines
+25
to
+28
| function listLaunchableTopologySessions(toolFilter?: string): any[] { | ||
| const sessions = listTopologySessionStates({ statuses: ["running", "idle", "offline"] }); | ||
| return toolFilter ? sessions.filter((s: any) => s.tool === toolFilter || s.toolConfigKey === toolFilter) : sessions; | ||
| } |
Owner
Author
There was a problem hiding this comment.
Fixed in dd3e88d - resume and restore now launch only offline topology sessions, with regression coverage for running rows.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/attachment-store.ts (1)
46-57:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTreat malformed attachment metadata as a cache miss.
loadAttachmentRecord()already returnsnullfor missing records, butreadFileSync()/JSON.parse()can still throw here. That turns one corrupt${id}.jsonfile into a hard failure for both exported read APIs instead of a graceful miss.🛡️ Proposed fix
function loadAttachmentRecord(id: string): AttachmentRecord | null { const normalizedId = id.trim(); if (!normalizedId) return null; const metadataPath = join(getAttachmentsDir(), `${normalizedId}.json`); - if (!existsSync(metadataPath)) { - return null; - } - const parsed = JSON.parse(readFileSync(metadataPath, "utf8")) as AttachmentRecord; - if (!parsed.contentPath || !existsSync(parsed.contentPath)) { - return null; - } - return parsed; + try { + if (!existsSync(metadataPath)) { + return null; + } + const parsed = JSON.parse(readFileSync(metadataPath, "utf8")) as AttachmentRecord; + if (!parsed.contentPath || !existsSync(parsed.contentPath)) { + return null; + } + return parsed; + } catch { + return null; + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/attachment-store.ts` around lines 46 - 57, loadAttachmentRecord currently calls readFileSync and JSON.parse which can throw on malformed/corrupt JSON and escalate to a hard failure; wrap the metadata read/parse in a try/catch inside loadAttachmentRecord and return null on any error (treat as cache miss) while preserving existing checks for parsed.contentPath and existsSync; reference the metadataPath/getAttachmentsDir/AttachmentRecord symbols when locating the code to modify.src/multiplexer/runtime-state.ts (1)
295-357:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore recovered runtimes'
backendSessionIdfrom topology.
savedByIdis only used for the label fallback right now. If a live tmux window is recovered before its metadata carries the backend id, the new runtime comes back withbackendSessionId === undefined, and the nextsaveState()can overwrite the resumable id already preserved in topology.🩹 Proposed fix
for (const { target, metadata } of liveAgentWindows) { const existing = host.sessions.find((session: any) => session.id === metadata.sessionId); if (existing) continue; + const saved = savedById.get(metadata.sessionId); const transport = new TmuxSessionTransport( metadata.sessionId, metadata.command, target, host.tmuxRuntimeManager, cols, rows, ); host.sessionTmuxTargets.set(metadata.sessionId, target); - host.registerManagedSession( + const runtime = host.registerManagedSession( transport, metadata.args, metadata.toolConfigKey, metadata.worktreePath, metadata.role, metadata.createdAt ? Date.parse(metadata.createdAt) : undefined, metadata.team, ); - const saved = savedById.get(metadata.sessionId); + if (saved?.backendSessionId && !runtime.backendSessionId) { + runtime.backendSessionId = saved.backendSessionId; + } const label = metadata.label ?? saved?.label; if (label) { host.sessionLabels.set(metadata.sessionId, label); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/multiplexer/runtime-state.ts` around lines 295 - 357, The recovered tmux window creation path in restoreTmuxSessionsFromTopology currently ignores saved topology backendSessionId, which can let a newly registered runtime overwrite the resumable id; after host.registerManagedSession(...) lookup the newly created session (e.g. host.sessions.find(s => s.id === metadata.sessionId)) and, if savedById.get(metadata.sessionId)?.backendSessionId exists, set that runtime.backendSessionId to the saved value (or pass it into the transport/registration if an API exists) so the restored runtime preserves the topology's backendSessionId; update related logic around TmuxSessionTransport, host.registerManagedSession, and host.sessions to ensure the value is applied immediately after registration.
🧹 Nitpick comments (5)
src/instance-registry.ts (1)
182-199: ⚡ Quick win
updateHeartbeatcontract is now misleading and should be tightened.The function docs/signature still imply session tracking + returned session IDs, but implementation now discards
_sessionsand always returns[]. Please align the contract to current behavior to prevent incorrect caller assumptions.Proposed contract alignment
/** - * Update heartbeat timestamp and sessions list. Also prunes dead instances. - * Returns the session IDs that were in the registry BEFORE this update - * (so the caller can detect which were claimed by comparing against expectations). + * Update heartbeat timestamp only. Also prunes dead instances. */ export async function updateHeartbeat( instanceId: string, - _sessions: InstanceSessionRef[], + _sessions: InstanceSessionRef[], cwd: string, -): Promise<string[]> { +): Promise<void> { await withLockedInstances(cwd, (instances) => { @@ }); - return []; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/instance-registry.ts` around lines 182 - 199, updateHeartbeat currently accepts an unused _sessions parameter and returns Promise<string[]> but its implementation discards sessions and always returns []; tighten the contract by removing the unused _sessions parameter from updateHeartbeat, change its return type to Promise<void>, and update the implementation to simply await withLockedInstances(...) without returning an array; also update any callers/tests that expect a string[] result and adjust imports/types accordingly (references to updateHeartbeat, withLockedInstances, pruneDeadEntries and the instanceId parameter should help locate the function to edit).package.json (1)
29-30: ⚡ Quick winBuild before restarting the OpenRig dev lane.
These scripts can restart the daemon against stale
dist/output aftersrc/**/*.tschanges, which makes runtime verification misleading. Prefix them withyarn build &&unlessscripts/dev-lanes/openrig-runtime-core.shalready guarantees that.♻️ Proposed fix
- "dev:daemon:openrig": "bash scripts/dev-lanes/openrig-runtime-core.sh daemon restart", - "dev:aimux:openrig": "bash scripts/dev-lanes/openrig-runtime-core.sh", + "dev:daemon:openrig": "yarn build && bash scripts/dev-lanes/openrig-runtime-core.sh daemon restart", + "dev:aimux:openrig": "yarn build && bash scripts/dev-lanes/openrig-runtime-core.sh",As per coding guidelines "For aimux runtime or CLI behavior changes under src/*.ts, always run yarn build to update dist/ before the daemon and metadata server see updated code".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` around lines 29 - 30, Update the two npm scripts "dev:daemon:openrig" and "dev:aimux:openrig" so they run a build before invoking scripts/dev-lanes/openrig-runtime-core.sh: ensure the command for both "dev:daemon:openrig" and "dev:aimux:openrig" is prefixed with "yarn build &&" (or an equivalent build invocation used in this repo) so that dist/ is up-to-date before restarting the daemon or running the runtime core script.src/multiplexer/service-state-snapshot.test.ts (1)
61-109: ⚡ Quick winSeed topology before asserting “no mutation.”
This temp repo starts with an empty topology, so the final
[]assertion still passes ifmergeRuntimeSnapshots()accidentally clears or rewrites topology. Pre-create a topology session and assert it survives unchanged after the merge.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/multiplexer/service-state-snapshot.test.ts` around lines 61 - 109, The test seeds an empty topology so the final assertion doesn't catch accidental topology mutations; before calling mergeRuntimeSnapshots(...) create/persist a topology session (matching the repoRoot) using the same shape as existing.sessions (e.g., a session with id "old-id" and its tmuxTarget) so the topology is non-empty, then after the merge assert that listTopologySessionStates({ statuses: ["offline"] }) still returns that original session unchanged; reference mergeRuntimeSnapshots and listTopologySessionStates to locate where to insert the topology seed and the unchanged assertion.src/instance-directory.test.ts (1)
40-54: ⚡ Quick winAssert the forwarded proxy arguments too.
Because these mocks are zero-arg functions cast to
any, this test only checks return plumbing. It still passes ifInstanceDirectorydrops or reorders"self",[], or"/tmp"when it forwards the calls.♻️ Proposed fix
expect(await directory.registerInstance("self", "/tmp")).toEqual([]); + expect(registerInstanceMock).toHaveBeenCalledWith("self", "/tmp"); await directory.unregisterInstance("self", "/tmp"); + expect(unregisterInstanceMock).toHaveBeenCalledWith("self", "/tmp"); expect(await directory.updateHeartbeat("self", [], "/tmp")).toEqual(["s1"]); + expect(updateHeartbeatMock).toHaveBeenCalledWith("self", [], "/tmp");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/instance-directory.test.ts` around lines 40 - 54, The test currently uses zero-arg mocks cast to any, so it only verifies return values but not that InstanceDirectory forwards arguments correctly; update the mocks to accept and assert the expected parameters and add calls to expect(...).toHaveBeenCalledWith(...) after invoking directory.registerInstance, directory.unregisterInstance, and directory.updateHeartbeat; specifically ensure registerInstanceMock is asserted to have been called with ("self","/tmp") (or the exact signature used by InstanceDirectory.registerInstance), unregisterInstanceMock with ("self","/tmp"), and updateHeartbeatMock with ("self", [], "/tmp") so the forwarded proxy arguments are verified.src/multiplexer/runtime-state.ts (1)
165-169: Rebuilddist/and restart the runtime before validating this path.These topology-driven session changes affect live CLI/daemon behavior, so typecheck/tests alone won't verify the packaged runtime path.
As per coding guidelines, "For aimux runtime or CLI behavior changes under src/.ts, always run yarn build to update dist/ before verifying runtime behavior" and "If a daemon or project runtime is already running after changes to src/.ts, restart or reload the relevant runtime after the build, as rebuilding alone may leave stale processes alive."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/multiplexer/runtime-state.ts` around lines 165 - 169, The change to syncSessionsFromTopology (and other src/*.ts runtime logic like restoreTmuxSessionsFromTopology, loadOfflineTopologySessions, loadOfflineServices) requires rebuilding the distributed artifacts and restarting any running aimux runtime/daemon before validating behavior; run yarn build to regenerate dist/, then stop and restart the CLI/daemon process so the updated dist/ code is loaded, and only then run your runtime checks or tests that exercise topology-driven session changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CODEX.md`:
- Line 23: Update the docs so task files created by agents exactly match the
Task interface: include the required fields id, status, assignedBy, description,
prompt, createdAt, and updatedAt (use ISO timestamps), and clarify that optional
fields remain assignedTo and tool; replace the vague "timestamps" with explicit
createdAt and updatedAt and mention id is required to avoid malformed task files
(referencing the Task interface in src/tasks.ts).
In `@docs/desktop-ui-contract.md`:
- Around line 167-173: The section stating "Desktop orchestration UI currently
exposes read-only orchestration state" conflicts with later "Action Model" write
endpoints; update the text to explicitly mark any orchestration write endpoints
in the Action Model as "legacy/historical (removed)" and add a short note under
the Desktop orchestration UI section clarifying it is strictly read-only (view
direct-message and handoff threads, view assigned tasks, browse thread history,
view workflow status) and that implementers must not wire the removed write
paths; also add a one-line callout in the Action Model subsection indicating
those write endpoints are deprecated/removed for runtime-core.
In `@docs/orchestration-prd.md`:
- Around line 305-307: Replace the machine-local file links that point to
/Users/sam/cs/aimux/... with repository-relative links so they resolve for
everyone; update the two links referencing src/orchestration-actions.ts and
src/task-workflow.ts to use repo-relative markdown paths (e.g.,
[src/orchestration-actions.ts](src/orchestration-actions.ts) and
[src/task-workflow.ts](src/task-workflow.ts)), and remove the stray orphaned "-
role:" line that appears after the list.
- Around line 625-627: Summary: The send_message semantics are
inconsistent—update the Core Verbs section to use the newer waiting/inbox model.
Find every occurrence of "send_message" and the phrase "queued if the target is
busy" (and the Core Verbs section), and change wording to state that
send_message appends the message to the target thread/inbox and places the
recipient in a waiting state (or routes it to the recipient's inbox) rather than
saying it is "queued if the target is busy"; ensure any related examples or
descriptions (e.g., in the Core Verbs section and the earlier paragraph
currently using the queued phrasing) use the same waiting/inbox terminology and
behavior consistently.
In `@README.md`:
- Around line 672-674: Remove the blank line inside the blockquote so the two
quoted lines become consecutive blockquote lines; edit the block containing
"Delegate the test writing to another agent" and "Hand off the CSS cleanup to
the codex agent" to delete the empty line between them so markdownlint MD028
(no-blanks-blockquote) is satisfied.
In `@scripts/dev-lanes/openrig-runtime-core.sh`:
- Line 9: The exec line currently runs "exec node bin/aimux-dev \"$@\"" relative
to the caller CWD which breaks when not invoked from the repo root; update the
script to resolve the repo-root-anchored path first (e.g., compute the script
directory via "$0" / dirname, derive the repository root from that, and then
exec node with the absolute/anchored path to bin/aimux-dev) so the call to exec
uses the correct bin/aimux-dev location regardless of the current working
directory.
In `@src/main.ts`:
- Around line 2457-2469: The JSON branch for the unsupported operation currently
logs { ok: false, error } and returns, yielding exit code 0; update the
opts.json branch in the same function to terminate the process with a non-zero
exit code (e.g., call process.exit(1) or throw an Error) immediately after
printing the JSON so shell callers receive failure status; locate the block that
checks opts.json and adjust it (the branch that does
console.log(JSON.stringify({ ok: false, error }, ...))) to exit non‑zero instead
of returning.
In `@src/metadata-server.ts`:
- Around line 429-436: In topologyDesktopSessionList, don't derive the API
DesktopSessionRecord.status from session.lifecycle; instead use the topology
session status returned by listTopologySessionStates (the value that can be
"running" | "idle" | "offline" | "graveyard"). Update the mapping in
topologyDesktopSessionList to assign status from the topology status field on
RuntimeTopologySessionState (preserving values like
"running"/"idle"/"graveyard") and only fall back to "offline" if that topology
status is missing; keep team mapping as team: session.team as
SessionTeamMetadata | undefined.
In `@src/multiplexer/dashboard-model.ts`:
- Around line 43-47: The teammate-selection logic still only checks
host.offlineSessions, causing resumeAgentAndDirectTeammates() to skip teammates
when the parent session is found via topology; update teammate discovery to use
the same topology-backed source as resolveOfflineSessionForAction by replacing
direct reads of host.offlineSessions with a helper that first searches
host.offlineSessions and falls back to listTopologySessionStates({ statuses:
["offline"] }), e.g., reuse resolveOfflineSessionForAction (or extract its logic
into a shared getOfflineSession(sessionId) used by
resumeAgentAndDirectTeammates() and the other occurrence mentioned) so both
parent and teammate lookups consult topology when needed.
In `@src/multiplexer/runtime-state.ts`:
- Around line 412-423: graveyardSession currently removes the session from
host.offlineSessions and topology but leaves any cached desktop/statusline
projection visible; after moveTopologySessionToGraveyard (inside
graveyardSession) also invalidate the cached dashboard/statusline state—e.g.
clear or reset host.desktopProjection and host.statuslineCache (or
host.statusline) or call whatever existing invalidate/publish helper your
runtime provides so the UI/monitoring layers stop showing the graveyarded
session; keep markLifecycleUsed and host.debug as-is.
In `@src/multiplexer/session-launch.ts`:
- Around line 25-27: The function listLaunchableTopologySessions currently
queries statuses ["running", "idle", "offline"] which allows relaunching live
sessions; change it to only query ["offline"] so default relaunch targets are
exclusively offline sessions, and keep the existing toolFilter behavior; update
any callers like resumeSessions()/restoreSessions() to rely on this function (no
additional filtering) so duplicate relaunches are prevented.
In `@src/runtime-core/topology-sessions.ts`:
- Around line 269-273: The helper moveTopologySessionToGraveyard is capturing
the session state into moved before applying the graveyard mutation, so callers
get the pre-move lifecycle; to fix, compute the session snapshot (call
topologySessionToSessionState) after you mutate existing.status = "graveyard",
existing.updatedAt = now and update topology.bindings, and then return that
post-mutation snapshot (or assign moved after the mutations) so the returned
session reflects the graveyard state; reference symbols:
moveTopologySessionToGraveyard, topologySessionToSessionState, existing,
topology, moved.
- Around line 249-254: The binding update is computed from the old
session.lifecycle rather than the new explicit status; create the updated
topology session first (you already build nextSession via
sessionToTopologySession(session, node.id, now) and status) and then derive the
binding from that updated session instead of the original session — i.e., call
sessionToBinding with the updated session (or pass the status) so
topology.bindings follows the explicit status change and will remove bindings
for sessions moved to offline/graveyard/error.
In `@src/runtime-core/topology-store.ts`:
- Around line 319-320: update() currently does a blind read-modify-write
(calling read() then write()) so concurrent processes can overwrite each other;
change update() to serialize writers by implementing either a file lock around
the read/modify/write sequence (e.g., flock on the topology file) or a CAS-style
retry: read current (read()), compute new value via mutator, attempt an atomic
replace only if the file hasn’t changed (compare mtime/checksum from read()),
and on mismatch re-read and retry until success; keep references to update(),
read(), and write() (and the rename/replace logic in write()) so you update
those methods to perform locking or CAS+retry.
In `@src/task-workflow.ts`:
- Around line 113-118: The fallback selection for reviewerRole can pick the
requesting agent's role (agentRole), causing self-reviews in requestReview();
update the selection logic to exclude agentRole and prefer explicit
review-oriented roles first: try to find a role in config.roles where key !==
agentRole and description.toLowerCase().includes("review"); if none found, then
find a role where key !== agentRole and role.canEdit is true; assign
reviewerRole to that key (or return null if still none). Ensure this logic is
used where reviewerRole is set so requestReview() never gets the author role.
- Around line 103-138: The requestReview helper currently calls the async
writeTask without awaiting it, so failures are detached; change requestReview to
be async (update its signature to return Promise<Task | null>), await
writeTask(reviewTask) before returning the reviewTask, and let any writeTask
rejection propagate (or rethrow) so callers receive the failure; also update any
callers to await requestReview accordingly.
---
Outside diff comments:
In `@src/attachment-store.ts`:
- Around line 46-57: loadAttachmentRecord currently calls readFileSync and
JSON.parse which can throw on malformed/corrupt JSON and escalate to a hard
failure; wrap the metadata read/parse in a try/catch inside loadAttachmentRecord
and return null on any error (treat as cache miss) while preserving existing
checks for parsed.contentPath and existsSync; reference the
metadataPath/getAttachmentsDir/AttachmentRecord symbols when locating the code
to modify.
In `@src/multiplexer/runtime-state.ts`:
- Around line 295-357: The recovered tmux window creation path in
restoreTmuxSessionsFromTopology currently ignores saved topology
backendSessionId, which can let a newly registered runtime overwrite the
resumable id; after host.registerManagedSession(...) lookup the newly created
session (e.g. host.sessions.find(s => s.id === metadata.sessionId)) and, if
savedById.get(metadata.sessionId)?.backendSessionId exists, set that
runtime.backendSessionId to the saved value (or pass it into the
transport/registration if an API exists) so the restored runtime preserves the
topology's backendSessionId; update related logic around TmuxSessionTransport,
host.registerManagedSession, and host.sessions to ensure the value is applied
immediately after registration.
---
Nitpick comments:
In `@package.json`:
- Around line 29-30: Update the two npm scripts "dev:daemon:openrig" and
"dev:aimux:openrig" so they run a build before invoking
scripts/dev-lanes/openrig-runtime-core.sh: ensure the command for both
"dev:daemon:openrig" and "dev:aimux:openrig" is prefixed with "yarn build &&"
(or an equivalent build invocation used in this repo) so that dist/ is
up-to-date before restarting the daemon or running the runtime core script.
In `@src/instance-directory.test.ts`:
- Around line 40-54: The test currently uses zero-arg mocks cast to any, so it
only verifies return values but not that InstanceDirectory forwards arguments
correctly; update the mocks to accept and assert the expected parameters and add
calls to expect(...).toHaveBeenCalledWith(...) after invoking
directory.registerInstance, directory.unregisterInstance, and
directory.updateHeartbeat; specifically ensure registerInstanceMock is asserted
to have been called with ("self","/tmp") (or the exact signature used by
InstanceDirectory.registerInstance), unregisterInstanceMock with
("self","/tmp"), and updateHeartbeatMock with ("self", [], "/tmp") so the
forwarded proxy arguments are verified.
In `@src/instance-registry.ts`:
- Around line 182-199: updateHeartbeat currently accepts an unused _sessions
parameter and returns Promise<string[]> but its implementation discards sessions
and always returns []; tighten the contract by removing the unused _sessions
parameter from updateHeartbeat, change its return type to Promise<void>, and
update the implementation to simply await withLockedInstances(...) without
returning an array; also update any callers/tests that expect a string[] result
and adjust imports/types accordingly (references to updateHeartbeat,
withLockedInstances, pruneDeadEntries and the instanceId parameter should help
locate the function to edit).
In `@src/multiplexer/runtime-state.ts`:
- Around line 165-169: The change to syncSessionsFromTopology (and other
src/*.ts runtime logic like restoreTmuxSessionsFromTopology,
loadOfflineTopologySessions, loadOfflineServices) requires rebuilding the
distributed artifacts and restarting any running aimux runtime/daemon before
validating behavior; run yarn build to regenerate dist/, then stop and restart
the CLI/daemon process so the updated dist/ code is loaded, and only then run
your runtime checks or tests that exercise topology-driven session changes.
In `@src/multiplexer/service-state-snapshot.test.ts`:
- Around line 61-109: The test seeds an empty topology so the final assertion
doesn't catch accidental topology mutations; before calling
mergeRuntimeSnapshots(...) create/persist a topology session (matching the
repoRoot) using the same shape as existing.sessions (e.g., a session with id
"old-id" and its tmuxTarget) so the topology is non-empty, then after the merge
assert that listTopologySessionStates({ statuses: ["offline"] }) still returns
that original session unchanged; reference mergeRuntimeSnapshots and
listTopologySessionStates to locate where to insert the topology seed and the
unchanged assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 654e98be-95ca-4660-af69-5b5b51311496
⛔ Files ignored due to path filters (2)
app/yarn.lockis excluded by!**/yarn.lock,!**/*.lockyarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (106)
CODEX.mdREADME.mdapp/app.config.jsapp/app/(main)/(tabs)/(dashboard)/agent/[sessionId]/chat.tsxapp/app/(main)/_layout.tsxapp/app/landing.tsxapp/app/shares/invite/[ownerUserId]/[token]/accept.tsxapp/components/ChatComposer.tsxapp/components/MessageBlock.tsxapp/lib/api.test.tsapp/lib/api.tsapp/lib/events.tsapp/lib/heartbeat.tsapp/lib/image-picker.d.tsapp/lib/image-picker.native.tsapp/lib/image-picker.web.tsapp/lib/parsed-transcript.test.tsapp/lib/parsed-transcript.tsapp/package.jsonapp/stores/chat.tsapp/stores/projects.tsdocs/current-architecture.mddocs/desktop-ui-contract.mddocs/global-control-plane-rfc.mddocs/latency-architecture-rfc.mddocs/multi-user-chat.mddocs/orchestration-prd.mddocs/project-host-model.mddocs/tool-integration.mdpackage.jsonscripts/dev-lanes/openrig-runtime-core.shsrc/agent-message-parts.test.tssrc/agent-message-parts.tssrc/attachment-store.tssrc/collaboration.test.tssrc/collaboration.tssrc/config.tssrc/dashboard/pending-actions.test.tssrc/dashboard/pending-actions.tssrc/debug-state.test.tssrc/debug-state.tssrc/default-plugins/gh-pr-context.test.tssrc/default-plugins/gh-pr-context.tssrc/instance-directory.test.tssrc/instance-directory.tssrc/instance-registry.test.tssrc/instance-registry.tssrc/main.tssrc/metadata-server.test.tssrc/metadata-server.tssrc/metadata-store.tssrc/multiplexer/agent-io-methods.test.tssrc/multiplexer/agent-io-methods.tssrc/multiplexer/archives.tssrc/multiplexer/dashboard-interaction.tssrc/multiplexer/dashboard-model.test.tssrc/multiplexer/dashboard-model.tssrc/multiplexer/dashboard-ops.test.tssrc/multiplexer/dashboard-ops.tssrc/multiplexer/dashboard-tail-methods.test.tssrc/multiplexer/dashboard-tail-methods.tssrc/multiplexer/index.tssrc/multiplexer/persistence-methods.test.tssrc/multiplexer/persistence-methods.tssrc/multiplexer/runtime-lifecycle-methods.test.tssrc/multiplexer/runtime-lifecycle-methods.tssrc/multiplexer/runtime-state.test.tssrc/multiplexer/runtime-state.tssrc/multiplexer/runtime-sync.tssrc/multiplexer/service-state-snapshot.test.tssrc/multiplexer/service-state-snapshot.tssrc/multiplexer/session-actions.test.tssrc/multiplexer/session-actions.tssrc/multiplexer/session-capture.tssrc/multiplexer/session-launch.test.tssrc/multiplexer/session-launch.tssrc/multiplexer/session-runtime-core.test.tssrc/multiplexer/session-runtime-core.tssrc/multiplexer/worktree-graveyard.tssrc/orchestration-dispatcher.test.tssrc/orchestration-dispatcher.tssrc/paths.tssrc/plugin-runtime.tssrc/project-events.tssrc/project-scanner.test.tssrc/project-scanner.tssrc/project-service-manifest.tssrc/runtime-core/index.test.tssrc/runtime-core/index.tssrc/runtime-core/topology-sessions.test.tssrc/runtime-core/topology-sessions.tssrc/runtime-core/topology-store.test.tssrc/runtime-core/topology-store.tssrc/session-bootstrap.test.tssrc/session-bootstrap.tssrc/session-input-operations.tssrc/session-message-history.tssrc/task-dispatcher.test.tssrc/task-dispatcher.tssrc/task-workflow.tssrc/tmux/runtime-manager.tssrc/tmux/window-open.tssrc/tool-output-watchers.test.tssrc/tool-output-watchers.tssrc/tui/screens/dashboard-renderers.tssrc/tui/screens/overlay-renderers.ts
💤 Files with no reviewable changes (24)
- src/multiplexer/dashboard-tail-methods.test.ts
- app/lib/image-picker.d.ts
- src/orchestration-dispatcher.test.ts
- src/orchestration-dispatcher.ts
- app/lib/image-picker.native.ts
- src/agent-message-parts.ts
- src/collaboration.test.ts
- src/tmux/runtime-manager.ts
- app/components/ChatComposer.tsx
- app/components/MessageBlock.tsx
- src/task-dispatcher.test.ts
- src/multiplexer/session-capture.ts
- src/agent-message-parts.test.ts
- src/multiplexer/dashboard-model.test.ts
- src/session-message-history.ts
- src/multiplexer/session-actions.ts
- src/multiplexer/session-actions.test.ts
- src/task-dispatcher.ts
- app/lib/parsed-transcript.ts
- app/package.json
- src/session-input-operations.ts
- src/tool-output-watchers.ts
- app/lib/image-picker.web.ts
- src/collaboration.ts
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/multiplexer/dashboard-interaction.ts (1)
878-889:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
requestReviewfailures insidehandleReviewRequest.If
requestReview(...)throws, this method now rejects and can surface as an unhandled rejection at call sites that intentionally fire-and-forget it. Catch and convert to a user-visible failure state here.🔧 Proposed fix
async handleReviewRequest(this: any): Promise<void> { const session = this.activeSession; if (!session) return; @@ - const reviewTask = await requestReview(session.id, role, diff, `Review ${session.command} agent's recent work`); + let reviewTask; + try { + reviewTask = await requestReview(session.id, role, diff, `Review ${session.command} agent's recent work`); + } catch (error) { + this.footerFlash = `Failed to request review: ${error instanceof Error ? error.message : String(error)}`; + this.footerFlashTicks = 3; + return; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/multiplexer/dashboard-interaction.ts` around lines 878 - 889, The handleReviewRequest method currently awaits requestReview(session.id, role, diff, ...) without error handling so any thrown error will reject this method; wrap the requestReview call in a try/catch inside handleReviewRequest (use the existing session, role, diff variables), and on catch set a user-visible failure state (e.g., update session status or call the existing UI/error reporting helper with session.id and the error message) and log the error; ensure the method still returns void and does not let the exception propagate to callers.src/main.ts (1)
811-825:⚠️ Potential issue | 🟠 Major | ⚡ Quick winSuccess path is returning a failure exit code.
Line 824 sets
process.exitCode = 1in a successful--jsonstop response (ok: true), so callers will treat success as failure.Proposed fix
if (opts.json) { console.log( JSON.stringify( { ok: true, projectRoot, projectServiceStopped: result.projectServiceStopped, tmuxSessionsKilled: result.tmuxSessionsKilled, }, null, 2, ), ); - process.exitCode = 1; return; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main.ts` around lines 811 - 825, The JSON success response currently sets process.exitCode = 1 causing callers to see failure; update the logic around opts.json in main.ts so that when the response object has ok: true (the success path used for opts.json) you either remove the process.exitCode assignment or set process.exitCode = 0 instead, and only set process.exitCode = 1 on actual failure branches (use the result.ok or result.success flag to decide); look for the opts.json handling block in the function that prints the JSON response and adjust the process.exitCode behavior accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/runtime-core/topology-store.ts`:
- Around line 331-333: The current sequence creates the lock directory with
mkdirSync(lockPath) then calls writeFileSync(join(lockPath, "owner"), ...) which
can fail and leave a stale lock directory; wrap the writeFileSync call in a
try/catch so that on any error you remove the created lockPath (using
rmSync(lockPath, { recursive: true, force: true })) before rethrowing the error,
and ensure the returned cleanup callback remains the same (rmSync(lockPath, {
recursive: true, force: true })) when write succeeds; reference the lockPath and
the owner file write in your change.
---
Outside diff comments:
In `@src/main.ts`:
- Around line 811-825: The JSON success response currently sets process.exitCode
= 1 causing callers to see failure; update the logic around opts.json in main.ts
so that when the response object has ok: true (the success path used for
opts.json) you either remove the process.exitCode assignment or set
process.exitCode = 0 instead, and only set process.exitCode = 1 on actual
failure branches (use the result.ok or result.success flag to decide); look for
the opts.json handling block in the function that prints the JSON response and
adjust the process.exitCode behavior accordingly.
In `@src/multiplexer/dashboard-interaction.ts`:
- Around line 878-889: The handleReviewRequest method currently awaits
requestReview(session.id, role, diff, ...) without error handling so any thrown
error will reject this method; wrap the requestReview call in a try/catch inside
handleReviewRequest (use the existing session, role, diff variables), and on
catch set a user-visible failure state (e.g., update session status or call the
existing UI/error reporting helper with session.id and the error message) and
log the error; ensure the method still returns void and does not let the
exception propagate to callers.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 472a0507-b8d9-4c2f-bee9-1964bdf46aa2
📒 Files selected for processing (19)
CODEX.mdREADME.mddocs/desktop-ui-contract.mddocs/orchestration-prd.mdscripts/dev-lanes/openrig-runtime-core.shsrc/main.tssrc/metadata-server.test.tssrc/metadata-server.tssrc/multiplexer/dashboard-interaction.tssrc/multiplexer/dashboard-model.tssrc/multiplexer/runtime-state.test.tssrc/multiplexer/runtime-state.tssrc/multiplexer/session-launch.tssrc/runtime-core/topology-sessions.test.tssrc/runtime-core/topology-sessions.tssrc/runtime-core/topology-store.test.tssrc/runtime-core/topology-store.tssrc/task-workflow.tssrc/workflow.test.ts
💤 Files with no reviewable changes (1)
- README.md
✅ Files skipped from review due to trivial changes (1)
- docs/desktop-ui-contract.md
This was referenced May 26, 2026
This was referenced Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Verification
Summary by CodeRabbit
Refactor
Documentation
Chores