[luciusmagn]

No description provided.

[luciusmagn]

PR 179 review

Reviewed head: 18c63bdc7e231c2808938b9cdef4f0a6e27385ef (lho/channels).
Review range: 0cd58a2fdf0cfc1a4992f84f8e4d39ffa3fc8f30..18c63bdc7e231c2808938b9cdef4f0a6e27385ef, 32 commits, 58 changed files.

Not ready to merge. The server and presentation work is substantial, but participant residence does not yet govern the running agent. There are also reproduced checkpoint, disclosure, deletion, delivery, and reconciliation failures.

Correction to the initial publication: F12 is withdrawn. --provider requires --model, and main--start-session calls main--model-arguments at src/startup/main.lisp:912-916 before server selection. The claimed provider-only CLI path is rejected. There are 14 findings below; their original IDs are preserved.

Findings

P1 means a merge blocker involving execution identity, unsafe lifecycle handling, unintended disclosure, or destructive behavior. P2 means a functional defect to fix before treating the proposed channels behavior as complete. Locations below refer to the reviewed head.

F1. P1: Moving a participant does not move its execution

Runtime confirmed with two live channel hosts. After moving A's agent into B, the world contained two resident agents in B, but A's application retained the same conversation, configuration, and source CWD. The application continued to serve A. This violates the proposal's explicit requirements that new work use the destination CWD and record that new work's activity in the residence channel. Already-started jobs and their completions must instead retain their starting channel.

src/world/store.lisp:913-928 changes only the residence record. src/world/server.lisp:685-718 constructs one conversation-bound application per host. src/world/resource.lisp:120-126 also resolves channel:current through that old conversation. The earlier resource-only mismatch is therefore accompanied by a live runtime/CWD mismatch, rather than merely a different client view.

Connect residence changes to the execution boundary. Account for the moved participant's private state and each destination resident's inference/receipts, while recording shared channel history once. Channel-level inbox deduplication alone is not the defect.

F2. P1: An admitted open can create an unpaused host inside a checkpoint

Runtime confirmed through a real network open and the checkpoint quiescer. I blocked an open after admission, let checkpointing snapshot/pause its hosts, and then released the open. Inside the supposedly quiesced thunk, checkpointing-p was true and a newly registered channel thread was alive and unpaused. The open succeeded.

src/world/server.lisp:426-435 checks lifecycle flags before acquiring the open lock. Host construction, registration, and thread startup occur later at 685-760. Checkpoint admission and the host snapshot use a different lock at 1056-1076; the later joins cannot cover a host absent from that snapshot.

Serialize lifecycle admission with in-flight opens and host registration, then establish a closed set of hosts before quiescence. The probe did not fork or save a core. This finding concerns checkpointing; the shutdown ready-hook has an additional stopping check and is not evidence of an identical shutdown leak.

F3. P1: An email address silently delivers the whole message to another channel

Runtime confirmed through world-message-send, the implementation used by channel.send. Sending Internal account alice@bob.com to the first channel also put that complete text in bob's inbox in a second channel.

src/world/messages.lisp:442-460 accepts @bob without a left token boundary and stops before .com; 576 applies that scanner to sends. This creates a remote recipient from an ordinary email address without an explicit mention or subscription.

Require an intentional mention boundary before adding a recipient. Wider quoting, escaping, and code-block rules are listed under Decisions. This reproduction used the explicit send path, not the currently broken queued composer hook in F6.

F4. P1: Deleting one channel also deletes unlisted descendants living elsewhere

Runtime confirmed at the store boundary. Parent P resided in A, child Q in unrelated B, and the human elsewhere. Deleting A with only P explicitly included returned two deleted participants: P and the unlisted Q. B survived.

src/world/store.lisp:1054-1099 checks explicit inclusion only for local residents, then recursively deletes descendants at 1085-1089. The direct participant-deletion operation separately requires an explicit recursive choice at 1031-1044.

Validate the complete participant deletion set, including descendants outside the channel, before writing anything. Residence and ancestry do not authorize implicit deletion. The proposal explicitly requires moving residents or including them in deletion and preserving human control of the tree.

F5. P1: A creator's attachment failure can kill a channel already serving another client

Source-confirmed failure path; no multi-terminal runtime reproduction. src/startup/main.lisp:1204-1217 sends an unconditional channel :kill after a serious attachment failure whenever the original open returned started-p.

That flag describes who caused creation, not current exclusivity. Another terminal may have attached between creation and the failure, or while the creator was already attached. The cleanup can then terminate shared work and disconnect the other terminal.

Make cleanup a server-side ownership/attachment decision at the time of failure, rather than trusting the historical creation flag.

F6. P2: Normal queued composer messages never run mention fan-out

Runtime confirmed using the real enqueue/dequeue path and message runner, with only model execution replaced. A normal human message received a pending-input identifier, and typing @bob left bob's remote inbox empty.

src/terminal/responsive-input.lisp:3582-3606 gives every dequeued :message an active-work identifier. The runner captures/passes it at 4184-4189 and 4234. The new mention hook at 4087-4088 runs only when that identifier is NIL.

Distinguish first local submission from remote delivery and replay by origin, rather than treating every input receipt as a relayed message. Preserve receipt idempotency when enabling local fan-out. Introduced by 5ac6864b.

F7. P2: Ordinary conversation traffic bypasses subscriptions

Helper-level runtime confirmation plus source tracing. With an agent in B subscribed to A, calling the ordinary composer fan-out helper for a plain message in A returned NIL and left B's inbox empty.

src/world/application.lisp:257-290 only sends when it finds a remote mention, and uses :post-p nil. In src/world/messages.lisp:509-513, subscriber delivery is conditional on post-p. Normal assistant replies also have no corresponding world-message send. Explicit channel.send works, but normal conversation traffic does not use that path.

Route ordinary human and assistant messages through resident/subscription delivery as required by the proposal, independently of mention delivery. This is a second defect after F6: fixing the queued-input guard alone will not deliver plain messages to subscribers.

F8. P2: Settling a full inbox can strand a durable outbox indefinitely

Runtime confirmed through the application delivery hook. I filled the recipient inbox and retained a durable outbound copy for it. The hook retried the outbox first, encountered the full inbox, then settled already-received inbox entries. It queued zero messages, left the inbox empty, and retained the outbound copy. An explicit subsequent resume delivered it.

src/world/application.lisp:68-105 resumes before settlement and never retries after freeing capacity. The steering acknowledgment at src/world/messages.lisp:632-638 likewise frees capacity without resuming. The relevant peer dispatch is src/task/peer-messages.lisp:583-588; this is not a retry performed on every main-loop iteration.

Trigger durable progress when settlement frees capacity, including steering acknowledgments, without depending on another human send or notification. Introduced with the outbox change in e0ac7e7f.

F9. P2: Background application synchronization moves the human participant

Helper-level runtime confirmation; production call sites verified. After explicitly putting the human in A, synchronizing an application for B moved the human to B.

src/world/application.lisp:10-26 always calls world-session-select; src/world/sessions.lisp:546-556 implements that as a move of the single human. Application construction invokes synchronization at src/application/runtime.lisp:1272, and conversation installation also invokes it at 2111.

Separate metadata reconciliation and client channel selection from participant movement. Building a host for another channel or terminal is not an explicit user move. The proposal makes client view, residence, and subscription independent. This finding does not assume synchronization happens after every append.

F10. P2: Child participants cannot use the channel messaging tools

Runtime-confirmed tool-policy check. channel.send was registered for the parent, but a child definition with :tools :all was still denied it.

src/world/tools.lisp:13-16 makes every channel tool child-unsafe, with a rationale that children have not yet become participants. src/task/child.lisp:40-83 filters these tools and rejects explicit grants. Child steering has been added, but children cannot send or subscribe through the ordinary participant channel interface.

Complete the child tool authority and delivery integration. The legacy yield path working does not satisfy the proposal's shared parent/child communication path.

F11. P2: Concurrent first clients do not both converge on the winning server

Source-confirmed startup race; no competing-process reproduction. Two clients can both see neither endpoint nor held lease in src/world/client.lisp:51-72 and each start a server. The server lease at src/world/server.lisp:215-225 admits only one, which is correct, but the losing client is still waiting for its own fresh handoff token.

src/world/client.lisp:28-39 rediscovers the world only after fresh spawning succeeds. src/localgroup/handoff.lisp:427-493,516-525 waits for that particular spawn's handoff. On failure, src/startup/main.lisp:1128-1150 falls back to direct execution without rediscovering the winner.

Handle losing the startup race by joining the winning server. The later starting-host reservation fix addresses concurrent channel opens inside one server, not this process-start race. Introduced by 9c916df5.

F13. P2: Maximum-length colliding participant handles never converge

Runtime confirmed over three reconciliations. Migrating two conversations under a workspace whose leaf is 32 a characters produced one valid 32-character name and an invalid suffixed name. The latter alternated between the base plus -3, -2, and -3 on repeated reconciliation.

src/world/store.lisp:554-561 appends a uniqueness suffix without reserving room for it. The migration repair at src/world/sessions.lisp:448-480 repeats that process, so a valid starting name can lead to endless invalid-name repairs and log writes.

Reserve suffix space within the handle length limit, and make repeated reconciliation converge to valid, unique names.

F14. P2: Parent-first retirement leaves an unmapped ghost root

Runtime confirmed, including a second reconciliation. After reconciling a root session and child task, putting the human in a surviving third channel, and removing the root and child legacy storage, reconciliation deleted the child but kept the root channel. Only the surviving session remained in the mappings. Running reconciliation again did not remove the ghost root.

src/world/sessions.lisp:398-427,436-437 retains a root while its child exists but drops the root's mapping anyway. After retiring the child, it never revisits the retained parent.

Retire descendants before parents, or retain the parent mapping until its actual retirement. Preserve any genuinely live or referenced state according to the deletion policy.

F15. P2: Switching channels loses the hidden-sidebar preference

Screen-level runtime confirmation plus attachment tracing. Hiding the sidebar, then reusing the same panel in a newly created screen, showed it again.

Visibility defaults to true in src/localgroup/screen.lisp:73-77. Every attachment creates a new screen at src/localgroup/client.lisp:675-695; src/startup/main.lisp:1201-1209 preserves only the panel object while switching.

Keep terminal-local visibility with the persistent client/panel state and initialize each new screen from it, including reconnects.

Proposal coverage and Decisions

The logical world store, continuous legacy projection, channel resources, server hosting, switching, sidebar, names, and explicit sends are implemented and exercised. F1 is a central proposal gap: this is still a conversation-bound runtime with a world projection, not yet the proposed independent participant execution model. F7 and F10 are further gaps in ordinary shared messaging and delegation. Passing metadata/store tests does not establish those runtime contracts.

The following need Lukáš's decisions rather than an implementation silently choosing policy:

1. Moves during active work. Specify the cutover between inference, tool execution, new messages, and job completion. The proposal already requires destination CWD for new work, private state following identity, and existing jobs/completion staying in their starting channel. Decide the atomic boundary and ordering among multiple residents.
2. Message semantics. Define ordering, batching, self-delivery, replay/deduplication, and wake scheduling for each resident agent. Messages reaching residents and subscribers is already required. Shared history should be stored once. Specify intentional mention syntax, including escaping, quoted/code text, and punctuation. F3's accidental email recipient needs fixing regardless of the wider syntax choice.
3. Ownership and deletion. Set the exact descendant controls and confirm whether to adopt the proposal's working choice of transferring deleted participants' memories to the global store with IDs preserved. Preserve the existing requirements that referenced history remain resolvable and running jobs block deletion of their channel and required state.
4. Migration and startup policy. Specify field-level freshness, the meaning of latest-touched and CWD matching, and when reconciliation can be removed. The proposal says to offer creation when no channel matches; confirm the intended interaction rather than silently substituting automatic creation.
5. Independent terminal views. The current smaller-observer composition takes the top rows of a shared frame (src/localgroup/screen.lisp:302-314), which can omit the bottom composer. Decide the intended viewport/layout behavior for different terminal sizes. Also define unread/mention read state: the current baseline advances during visible, throttled sidebar refresh, and a fresh client starts at zero. Hidden-panel and channel-switch semantics need an explicit expectation.

There is also contradictory guide text to reconcile with the final behavior: docs/guide.org:1366 describes resumed conversations as fresh processes with dead background jobs, although resume can now attach to an existing live channel. At 1438-1440, an uncontrolled running channel accepts an explicit permission mode, while 1490-1491 says permission overrides are rejected when reusing a running conversation.

Verification and limits

• Installed the locked review dependencies before building. Kept the requested cl+ssl pin d7a92f9ed0cbae10f07b1b9a9c3c5cc179798748.
• Ran ./script/check --jobs 8 --timeout 900 at the reviewed head: 1,461 cases, 21,355 checks, zero failed cases. Recovery and source checks passed. git diff --check passed; the review worktree is clean.
• Reviewed the commit sequence in three ranges and the combined diff in five groups covering all 58 changed files: data/migration, messages/delegation, server/lifecycle, UI, and tests/docs/dependencies.
• Reproductions ran in an isolated SBCL 2.6.6 Linux worker loaded from the exact review checkout, with temporary test state. Live-server probes used the repository fixture that suppresses private image/init/MCP loads. The queued-message probe replaced model execution; no inference/provider behavior is claimed from it.
• No native Windows execution, real checkpoint fork/core save, competing first-server processes, or multi-terminal attachment-failure reproduction was performed. Source-only findings are labeled above.
• A scoped-first child reconciliation probe also lost parent/owner ancestry, even after full reconciliation. Its normal production reachability is not established while child channel tools are blocked, so it is not counted as a separate confirmed finding. Retest this path while completing F10. A suspected cross-process mapping lost update likewise needs a concrete interleaving before being promoted to a finding.
• No source changes, PR fixes, commits, branch changes, merge, or active-image mutations were made for this review. Claude can implement the fixes after this review.

Commit ledger

The entries identify the area inspected in each commit and link relevant final-head findings. The full check was run at the final head, not separately at all 32 historical commits. A commit without a finding reference is not a claim that its complete behavior was independently runtime-tested.

Commit	Inspected change	Final-head result / related findings
c7aab45d	World store, participants, channels, mutations	F1, F4; deletion Decisions
43e9dd7d	Continuous legacy session reconciliation	F9, F13, F14; migration Decisions
54de91cd	Channel resource scheme and (channels)	Resource/current binding in F1
e29b4758	Resident/subscriber/mention delivery	F3, F7; receipt and message Decisions
17215c0e	Child reconciliation, nesting, steering	F10; conditional ancestry probe
b5db7407	Relay-local resize handling	Geometry and relay ownership inspected
9dd4338e	Session lifetime independent of clients	F5 integration and lifecycle inspected
9695bb54	Explicit relay terminal geometry	Multi-client geometry inspected
7125f5ad	Separate construction from process preparation	Construction lifecycle and F9 call site
372ecae6	Serve applications from non-main threads	Thread bindings and resource lifetime inspected
5dc88001	Management REPL owner protocol	Ownership and server integration inspected
55dd5156	One world server hosting channel applications	F1, F5, F9; lifecycle integration
9c916df5	First-client server startup	F11
c4486065	World server documentation	Guide integration inconsistency noted above
f3899228	Pin idsmall with fragment matching	Dependency pin and call sites inspected
989a8352	Typed channel/session ID fragments	Resource and command resolution inspected
04f93a2d	Terminal switching	F15; client-selection separation in F9
d9e4e5c1	Relay fullscreen frames as rows	Frame transport and geometry inspected
137e6b20	Message/mention status counters	Counter semantics and unread Decisions
26cdbd52	Compose relay frames with a side panel	F15; shorter-observer Decision
67e3437e	World channel sidebar	F15; unread and refresh semantics
ec678ec7	Sidebar, switch, fragment documentation	Guide behavior cross-checked
0fdfe482	Unique participant mention names	F13; identity/handle handling inspected
5ac6864b	Composer mention completion/delivery/highlighting	F6 and shared scanner in F3
ab1bc44a	Participant-name and mention documentation	Mention contract cross-checked
a76a032d	Preserve characters in mention highlighting	Span/highlighting correction inspected
dc6c1764	Date/size token for presentation world caching	Cache invalidation and refresh inspected
350ebc16	Reopen by conversation activity	Startup selection and latest-touched Decision
47c8a257	Reserve starting channel hosts	Intra-server opens distinguished from F11
e0ac7e7f	Durable send intent before copies	F8; outbox/receipt paths inspected
8dcdc296	Forward explicit launch options	Launch forwarding and prior CLI validation inspected; initial F12 withdrawn
18c63bdc	World-wide checkpoint lifecycle	F2