Devmain

[a-Fig]

No description provided.

[a-Fig]

Static review only, per request: I did not build the project or run tests/live sessions. I reviewed the full main...devmain diff plus adjacent code. These are the PR-introduced/integration findings I believe should be addressed before promotion.

High

1. Handoff failures become silent, non-retryable stalls (conductors/handoff/handoff.ts:223-230, 389-398). The rejection handler drops the actual error and deliberately retains lastAttemptKey; the next pass clears status before returning early for that same key. A provider/auth/network/context-limit failure therefore leaves a session at the 90% trigger with no visible error and no retry until a new block changes the key or the conductor is detached. Please persist a failure status and provide an explicit or bounded/backoff retry path.

2. Handoff does not reserve context space for its requested output (conductors/handoff/handoff.ts:35-52, 345-358). It triggers at 90% of budget, sends the entire handoff input plus wrappers/system, and always asks for up to 8,000 output tokens. With the normal budget === contextWindow initialization, a 32k model is already around 28.8k input tokens before wrappers; +8000 exceeds the window and can be rejected, feeding directly into finding 1. Derive maxOutputTokens from view.contextWindow - estimatedInput - safetyMargin, and/or trigger early enough to guarantee the output reserve.

3. Raw-wire reconciliation leaves an illegal protected fold in store state (app/src/lib/engine/store.svelte.ts:1015-1040, especially 1033; zero-delta early returns at 1543/1550). markSent({rawWire:true}) clears the birth-fold exemption after the model sees the block whole, but does not heal/refold the still-autoFolded protected block. This violates the repo's preview-is-no-more-permissive invariant. Worse, a later zero-delta context sync does not call refold(), so arming before that hook can send the stale protected fold even though its exemption is gone. Heal/refold affected blocks immediately when raw-wire reconciliation clears the exemption, and assert the block becomes live at once.

Medium

4. A failed browser discovery poll can cancel a healthy cross-session connection (app/src/lib/live/browserDiscovery.svelte.ts:69-88; disconnect in discovery.svelte.ts:123-132). Network/403/500/non-JSON failures are converted into a successful empty list. While switching A→B, connectedFallback() is still null during B's handshake; publishing [] clears selected B and calls disconnectLive(). Retain the last successful list on poll failure and do not reap a selected/connecting entry until the connection resolves.

5. Browser auth cookies collide across session ports (browserDiscovery.svelte.ts:71; extension/accordion.ts:763-768, 831-835). Cookies are host/path scoped, not port scoped, but every session writes the same accordion_token. Opening B overwrites A's cookie; A's tokenless /__accordion/sessions poll then gets 403, which also triggers finding 4. Include the current URL token on the poll when available and use a session-specific cookie name for cookie-only reloads.

6. IPv6 bind settings print unusable URLs (extension/accordion.ts:1641-1649). ACCORDION_HOST=::1 binds IPv6 but prints 127.0.0.1, while ::/other IPv6 literals are interpolated without brackets (for example http://:::8080). Preserve the actual loopback family and bracket IPv6 literals when serializing URLs; handle wildcard display hosts separately.

7. Binding a specific LAN interface breaks desktop discovery (extension/accordion.ts:145-169, registry entry 541-555; liveClient.svelte.ts:241-263). The feature explicitly accepts a specific interface such as 192.168.1.20, which excludes loopback. The registry advertises only a port and Tauri always dials 127.0.0.1, so the desktop lists a session it cannot connect to. Advertise a connect host in SessionEntry (and mirror it in Rust/client), or reject/document specific-interface binds and support only loopback/wildcard hosts.

8. Custom port/host bind failures are swallowed (extension/accordion.ts:955-960, surfaced at 1636-1651). EADDRINUSE, EADDRNOTAVAIL, etc. discard the actual error and leave /accordion reporting port starting / Browser starting... forever with no retry. Store/log the bind error and show it in /accordion; explicit-port failure should be visible rather than silently falling back.

9. Passthrough telemetry reports submitted rather than actually-applied operations (extension/accordion.ts:1400-1407, 1425-1426; filtering in app/src/lib/live/mapping.ts:317-383). The new protocol promises counts actually applied, but the ack records input array lengths before applyPlan() drops missing IDs, unsafe group members, or malformed recalls. A nonempty no-op plan is still labeled applied. Have applyPlan() return effective diagnostics/counts and emit the ack after application.

Documentation drift

• extension/README.md:43-46 still says the npm/browser experience is single-session and requires desktop for multi-session discovery, contradicting this PR.
• CLAUDE.md:55 says the WS is unauthenticated, contradicting the new off-loopback token gate.
• conductors/README.md:177-182 says the desktop launch button uses portEnv, while PR feat(conductors): portEnv launch.json convention for host-assigned ports #54 explicitly states the Tauri launcher ignores that field. Clarify that it is currently for external spawning hosts such as bellows.

I also found several issues that predate this PR. One is security-sensitive, so I have intentionally not included the exploit details in this public comment; I am reporting those separately to the maintainer rather than attributing them to this promotion.

-gpt-5.6 SOL

[a-Fig]

Second-pass adversarial follow-up. These are newly verified findings beyond my first review comment. Static analysis only; no builds or live tests were run.

High

1. passthrough is correctness-critical, so keeping it as an optional no-bump addition to protocol v7 is unsafe (app/src/lib/live/protocol.ts:35-61; liveClient.svelte.ts:391-396, 490-528; ADR 0020 lines 92-102). The protocol comment says an old v7 peer may ignore this frame because it is “purely informational,” but timeout acks are the only mechanism that repairs optimistic birth-fold bookkeeping. A pre-ack v7 extension with the new v7 GUI, or the inverse pairing, passes the strict hello check. On a timed-out fresh plan the model can receive stale/raw content while birthFolded survives, later allowing already-seen content to fold inside the protected tail. armedAck does not advertise this capability. Please bump to v8, or require a negotiated capability before birth-fold is enabled; lack of support must use conservative raw/sent bookkeeping.

2. A supported positional-ID live block can acquire a permanent birth-fold exemption even though its fold can never reach the wire. The live mapper deliberately emits m… fallbacks when response/timestamp anchors are absent (mapping.ts:64-77; this shape is exercised in extension/smoke.mjs:830-905). substOne records a protected fold in birthFolded without checking durability (store.svelte.ts:1430-1454), while computeFoldOps silently drops it (plan.ts:37-50). Armed mode then sends an empty plan, but markSent({rawWire:false}) preserves the visibly folded exemption and the empty-plan ack does not reconcile it (liveClient.svelte.ts:391-395, 503-528; extension 1413-1422). The model repeatedly receives the block whole while the UI/token accounting says it is folded, and the false “never seen whole” exemption survives conductor swaps/detach until it ages out.

   The robust fix is to clamp non-durable per-block folds when wireAttached===true before setting autoFolded/birthFolded, and make markSent preserve exemptions from the plan IDs actually emitted/applied rather than store.isFolded. Treat an all-empty plan as raw for exemption truth. Keep parsed/demo stable IDs separate rather than globally rejecting them.

Medium

3. Conductor recall is broken on parsed Pi, Demo, and Claude Code sessions (app/src/lib/engine/parse.ts:80-110, 139-185; store.svelte.ts:1315-1321, 1371-1378). Parsed blocks use stable event IDs such as <eid>:r, <eid>:0, and <eid>:u, but isDurableId recognizes only the live-wire u:/a:/r:/s: formats. recallOne therefore rejects every parsed target, and the anchor picker rejects every parsed anchor. Loading the bundled sample or a Claude transcript and selecting Recall demo folds in preview, then repeatedly clamps recall as not-recallable, contrary to ADR 0019's read-only/preview parity claim. Model durability explicitly on Block/ViewBlock, or recognize parser-stable event IDs consistently; add parsed Pi and Claude fixtures to the recall tests.

4. Recall can be injected before the block whose content it recalls (store.svelte.ts:1320-1322, 1371-1378). If a conductor birth-folds and recalls text/thinking in a fresh assistant message that also emits a pending tool call, the anchor picker correctly skips that entire message but falls back to the previous user/result. applyPlan then inserts the synthetic user recall before the assistant originally emitted that content. The frozen anchor makes the temporal inversion permanent. Require a safe anchor at or after the target message; while its tool call is unresolved, defer/clamp the recall until the result arrives and then anchor after the completed pair.

5. Port parsing accepts partial garbage despite the “invalid → 0” contract (extension/accordion.ts:164-170). parseInt("8080junk", 10) binds 8080 and parseInt("1.5", 10) binds port 1. Validate the entire trimmed value with an integer grammar plus Number.isInteger/range checking, and surface invalid explicit configuration rather than silently choosing a different port.

Low

6. The browser-side stale reaper can delete a newly refreshed session entry (extension/accordion.ts:615-625). It can read A's old heartbeat as stale, race A atomically replacing the file with a fresh heartbeat, then unlink the fresh path. The live session disappears until its next heartbeat. Avoid reaping entries whose PID is still live and revalidate before deletion; a reread alone narrows but does not eliminate the rename/unlink race.

I also found additional security-sensitive defects in the changed remote-auth/session-boundary path. I am intentionally withholding the reproducer and token-lifecycle details from a public comment until the maintainer chooses a disclosure/fix path.

-gpt-5.6 SOL