🤖 fix: a reawakening that resends a kept sub-agent brief stamps its own brief send id - #5581
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
0a0929a to
674dc20
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 674dc2014a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
Readiness record for head 674dc20:
Generated with |
Summary
A sub-agent's initial brief is no longer sent twice when a reawakening's send makes its row durable and then returns Err. The reactivation that prepends a kept brief now gives that brief a fresh send id: it stores the id as
taskPromptSendIdbefore the send and stamps it on the row that accepts the prompt. The next reawakening's lookup (isTaskBriefInHistory, from #5543) then recognizes that row and does not prepend the brief again.Fixes #5544
Background
#5543 (U4) made the launch's brief send carry an id, so a reawakening drops a kept
taskPromptonce a history row carries that id. The reawakening's own send that prepends the kept brief carried no id. When it made its rows durable and returned Err,taskPromptstayed with the launch's id, which no row held, so the next reawakening prepended the brief again.formal/task-launchmodels the reawakening (React) as one atomic step, so the model did not show this. With this change, the model's abstraction (a history copy of the brief is recognized) holds for the reawakening's row too.Implementation
reactivateInactiveAgentTasktakes the message and aprependKeptBriefflag instead of a prompt builder. When it prepends a kept brief, it mints a brief send id and writes it astaskPromptSendIdin its existing attempt-commit CAS, before the send. It writes the id only while the row still holds the brief it read.createWorkspaceTurngets an internalsendIdentitiesargument and passes it to the send withskipOnSendCompaction, as the launch does. On-send compaction would fold the prompt into a follow-up dispatched later without the id.taskBriefSendIdentity), not the whole prompt's, so the existing lookup matches it. The id stands for the brief the row delivers.dropKeptTaskPromptAlreadyInHistorykeeps the brief while any backend holds aturnuse lease on the task. The new id makes a reawakening's row count as proof, and that send can run on another backend, where a Stop can still roll the row back. The order is: the lookup sees the row, then no turn lease is held, then a second lookup still sees the row. A session publishes its turn lease before it writes the row and releases it after the send settles, so a row that passes all three checks is permanent. A turn that starts after the config read carries a newer id, and the drop's CAS refuses it. This mirrors the existinglaunchlease check.applyInterruptedTaskStatusclearstaskPromptSendIdtogether withtaskPrompt. The id has no effect without the brief, so this has no test.The bash-monitor wake reactivation does not prepend the brief, and it is unchanged.
Validation
Test-first, in
taskService.taskLaunchFormalRepro.test.ts:The launch's send fails before any row. The first reawakening's send appends its row (with the send's ids, as AgentSession does) and returns Err. The second reawakening must not prepend the brief. Before the fix it failed at the target assertion:
Two variants of test 1 with another backend: one holds a
turnlease during the second reawakening, and in the other that backend rolls the row back and ends right after the first lookup. In both, the brief must be sent again rather than lost. Before the lease check, both failed: the second reawakening's prompt lacked the brief. Removing the second lookup fails the roll-back variant.Follow-up from the 🤖 fix: a sub-agent brief history already holds is not sent again on reawakening (U4) #5543 review: a real-session test. A real AgentSession over the real HistoryService runs the launch's send. The user's real Stop lands after the brief's row is on disk, so the session rolls the row back ("Send refused: the caller's admission became stale before the turn was accepted."). The reawakening then puts the brief in history exactly once. A mutant that never prepends the kept brief fails it.
Mutation checks on test 1: removing the id write in the CAS, or the
sendIdentitiespassthrough, fails the target assertion. RemovingskipOnSendCompactionfails its assertion.Also run on this head:
taskService*.test.tsandworkspaceTurnManager*.test.tsfiles: 1449 pass, 0 fail.formal/task-launch/check.sh,formal/task-lifecycle/check.shandformal/delegated-turns/check.sh: all match their EXPECT tables. No model or EXPECT table changes.Review record
Round 1 (normal + security on 82686f6): no findings.
The independent readiness check found that the new reawakening id lets another backend's in-flight send count as proof (a brief could be lost across two backends). This PR added that surface, so it is fixed here (the
turnlease check above). The check verified the queued-target path by reading the code: a sealed queue entry keeps its ids (messageQueue.ts). No test covers it.Round 3 (normal + security on 674dc20, after the rebase): 1 P2. The turn-lease check also counts this backend's own lease, whose release is not awaited, so an immediate retry can resend the brief (a duplicate, never a loss). Deferred by the issue coordinator, who waived the clean-review condition for this PR. The fix is tracked in 🤖 Task launch: count only other backends' leases when dropping a kept brief (#5544 follow-up) #5588, ready on branch
fix/5544-foreign-lease-narrowing.Risks
Low to medium. The change touches only the reawakening of an inactive child that still holds its initial brief: the prompt text is unchanged, and its row now carries one more send id. A failed lookup still resends the brief rather than losing it, as before.
Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high