CLI series: review findings we declined or deferred

This is a list of the Codex review findings on the CLI feedback PRs (C1 #88, C2 #94, C3 #95, and the PRs after them) that were declined or left for later rather than fixed in their PR. Each entry has a class and a one-line reason, and the linked comment has the full trace. New entries are added as later rounds decline more.

Classes: crash/race needs a crash, a dropped connection or a race at an exact step; load shows up only under a burst or a stalled component; hostile input needs malformed or adversarial input; contract is a choice about which correct answer to give; planned is already scheduled in a later PR of this series.

C1 #88: safe retries

Keyed create checks the workspace and provider before recognizing a resend. crash/race. A resend after the workspace or provider vanished gets a true, actionable error, and the same resend gets duplicate: true once they're back.
run --new resend with a changed prompt after create committed and submit was never sent. crash/race. No turn exists yet, so the key stood for one piece of work only; fixing it would tie the bot record to its first message.
Deletion tombstone keeps only the key, so a changed request answers bot_deleted, not idempotency_conflict. contract. Both answers refuse; bot_deleted is the more useful one, and the docs say so.
A keyed fork with its own gate can't be resent once its source is deleted. planned. It goes away in C5, which removes --approval/--approve from fork.

C2 #94: errors as JSON

~~Concurrent failed auto-starts can read each other's error from the shared daemon log.~~ Fixed in Report each failed daemon start's own error #100.

C3 #95: one turn view

Interrupt racing a turn's own end could answer stale_turn. crash/race. It doesn't reproduce: the job keeps its cancellation receiver alive through commit_finish, so cancel() returns false only after the end has committed.
Finished pending-read tasks stay in the JoinSet until the next pending wait. load. One finished task record per read of the last burst, freed by the next pending wait or shutdown; bounded by the peak burst. Fix: reap on completion, for example with a small reaper task.
Shutdown waits on pending reads before draining socket outputs. load. Needs a reader job stalled for the whole five-second window. Fix: poll the socket drains alongside the reads.

One turn view for wait, interrupt and turns (#95)

#118 (C6, per-turn token cap): one finding was declined.

Reserve the in-flight call before refreshing its cache
- Class: cap overshoot in an edge case.
- What happens: a reply that streams longer than the keep-warm interval (240 s) sends refreshes that bill against the cap before the call's own usage is known.
- Why declined: the overshoot is bounded by those refreshes within one call, and the cap's contract is already that one call may overshoot.
- Fix if wanted: suppress refreshes while a capped call streams, or reserve an estimate of its usage.

Generated by Claude Code

#241: trigger checked rows, fire handle, `--replace`

A process killed between --replace's plist swap and its row write leaves the new generation without the kept row. crash/race. It needs a kill in the microseconds between two fsynced writes inside one add --replace, and re-adding the trigger recovers it. A failed write, as opposed to a kill, now restores the old plist (7fc7d6f). Fix if wanted: a journal entry, or a two-phase rename across the LaunchAgents and triggers folders.

Generated by Claude Code

#241 (continued)

A replayed --commit ask settles with the HEAD from before its send. crash/race. It needs a kill or a replacement between the saved request and the settle, plus a later wake in that window. The worst case is one extra turn for a commit that was already delivered. Fix if wanted: save seen with the request, scoped to its repository, and restore it on replay.

Generated by Claude Code

#261 (Sonny: auto-review status, denials, /approve)

Hostile input / design: bind a /approve retry structurally (a digest or call-id grant) instead of an approval message in prose. Declined: Codex's own /approve is prose, and we follow Codex. The full payload is now shown before the second Enter (38fb9cb), and the approval quotes the exact arguments. Sonny: show auto-review in progress and its denials; /approve retries one #261 (comment)

Generated by Claude Code

#242 (pictures)

Deferred to a follow-up PR (owner's pick): request-wide picture limits. A long Anthropic conversation can build up more than 100 pictures, or more than 32 MB, across turns, and every submission still passes its own 20-picture check. The follow-up keeps the newest pictures within both limits and sends older ones as their text line. Pictures in messages and from read, counted at their token cost; Sonny's shell editing keys #242 (comment)

Generated by Claude Code

#245 (Sonny: ask puts a decision with reasons where it was asked)

Declined, owner's call (George picked "Merge as is"): open decisions older than the 200 replayed history items aren't restored after a Sonny restart. The per-bot asks file from Sonny: keep open questions older than the replayed history #226 was removed on purpose. Such a question isn't on screen, and the person's reply still reaches the lead. Only the number shortcut and the You asked: prefix are lost. Sonny: ask puts a decision with reasons where it was asked #245 (comment)

Generated by Claude Code

#276 (Codex security review): the cap checks the folders in a sandboxed bot's create, fork or submit, and then dispatch resolves those paths a second time. A bot that swaps a link between the two can store a folder outside its own. This is declined as a race. The fix would be to check the folders after dispatch has resolved them: move the session's caller into Service and check there. A link swapped in after the bot is created is already refused, because each confined call checks its roots against their real paths (2b8dae0). Between that check and bubblewrap's mount there is a narrower race, which is declined as well.

Generated by Claude Code

From #254 (apply_patch in the sandbox): the daemon checks a sandboxed bot's patch removals (*** Delete File, and the source of a move) against the bot's roots, but runs them outside the sandbox. A concurrent sandboxed command that swaps an in-root parent directory for a symlink between the check and the remove_file could delete a file outside the roots. The fix would be a removal mode in the sandbox helper, beside --agent-run-as-write, or descriptor-relative removal that rejects symlinks. #254 (comment)

Generated by Claude Code

#291: calls in a row run together

A batch's results queue in an unbounded channel while the store queue is full. load. This needs the shared store queue to stay full. Even then the channel holds at most one capped result per call in the model's response. Committed acknowledgements are now dropped as they arrive (008da79). Possible fix: spill waiting results to their artifact rows, or cap how many calls a batch admits while the queue is full.

Generated by Claude Code

A batch builds a sandboxed bot's policy once for all of its commands (Run a row of read and command calls together, up to four at a time #291). crash/race, hostile input. This needs the bot itself to swap a root for a symlink between the batch's check and a later spawn in the same batch. Fix: build the policy per shell call in run_parallel, as the lone path does.

Generated by Claude Code

#296 (custom roles: `extends` / `skills`)

Index edge, deferred. A workspace X.md with extends: X shadows a home X.md with extends: lead. The Profiles index then still lists X as a peer role, because it only follows extends across files it indexes, and the shadowed home file is skipped. Starting a peer in that role gives it lead text. The index only advertises roles and nothing breaks; the fix means reading shadowed heads for self-extensions. thread

Generated by Claude Code

Navigation Menu

Repository navigation

CLI series: review findings we declined or deferred #96

C1 #88: safe retries

C2 #94: errors as JSON

C3 #95: one turn view

#241: trigger checked rows, fire handle, `--replace`

#241 (continued)

#261 (Sonny: auto-review status, denials, /approve)

#242 (pictures)

#245 (Sonny: ask puts a decision with reasons where it was asked)

#291: calls in a row run together

#296 (custom roles: `extends` / `skills`)

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

CLI series: review findings we declined or deferred #96

Description

C1 #88: safe retries

C2 #94: errors as JSON

C3 #95: one turn view

Activity

lydakis commented on Oct 10, 2026

lydakis commented on Oct 28, 2026

#241: trigger checked rows, fire handle, --replace

lydakis commented on Oct 28, 2026

#241 (continued)

lydakis commented on Oct 28, 2026

#261 (Sonny: auto-review status, denials, /approve)

lydakis commented on Oct 28, 2026

#242 (pictures)

lydakis commented on Oct 28, 2026

#245 (Sonny: ask puts a decision with reasons where it was asked)

lydakis commented on Oct 28, 2026

lydakis commented on Oct 28, 2026

lydakis commented on Oct 28, 2026

#291: calls in a row run together

lydakis commented on Oct 28, 2026

lydakis commented on Oct 28, 2026

#296 (custom roles: extends / skills)

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

#241: trigger checked rows, fire handle, `--replace`

#296 (custom roles: `extends` / `skills`)