Skip to content

🤖 fix: a reawakening that resends a kept sub-agent brief stamps its own brief send id - #5581

Merged
ThomasK33 merged 3 commits into
mainfrom
fix/task-reactivation-brief-send-id
Oct 3, 2026
Merged

ThomasK33 merged 3 commits into
mainfrom
fix/task-reactivation-brief-send-id

Conversation

@ThomasK33

@ThomasK33 ThomasK33 commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

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 taskPromptSendId before 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 taskPrompt once 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, taskPrompt stayed with the launch's id, which no row held, so the next reawakening prepended the brief again. formal/task-launch models 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

  • reactivateInactiveAgentTask takes the message and a prependKeptBrief flag instead of a prompt builder. When it prepends a kept brief, it mints a brief send id and writes it as taskPromptSendId in its existing attempt-commit CAS, before the send. It writes the id only while the row still holds the brief it read.
  • createWorkspaceTurn gets an internal sendIdentities argument and passes it to the send with skipOnSendCompaction, as the launch does. On-send compaction would fold the prompt into a follow-up dispatched later without the id.
  • The identity's digest is the brief's (taskBriefSendIdentity), not the whole prompt's, so the existing lookup matches it. The id stands for the brief the row delivers.
  • dropKeptTaskPromptAlreadyInHistory keeps the brief while any backend holds a turn use 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 existing launch lease check.
  • Follow-up from the 🤖 fix: a sub-agent brief history already holds is not sent again on reawakening (U4) #5543 review: applyInterruptedTaskStatus clears taskPromptSendId together with taskPrompt. 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:

  1. 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:

    error: expect(received).not.toContain(expected)
    Expected to not contain: "Survey the repository and report back"
    Received: "Survey the repository and report back\n\nUpdated guidance from parent:\n\nAgain"
    (fail) the initial brief reaches the child once (U4) > a reawakening whose send accepted the kept brief and then failed does not send it again (#5544)
    
  2. Two variants of test 1 with another backend: one holds a turn lease 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.

  3. 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 sendIdentities passthrough, fails the target assertion. Removing skipOnSendCompaction fails its assertion.

Also run on this head:

  • All taskService*.test.ts and workspaceTurnManager*.test.ts files: 1449 pass, 0 fail.
  • formal/task-launch/check.sh, formal/task-lifecycle/check.sh and formal/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 turn lease 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

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 3, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-03T20:48:18.400199Z 674dc20 New commits
🔒 Security Review ✅ Completed 2026-10-03T20:48:09.742269Z 674dc20 New commits
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@ThomasK33
ThomasK33 force-pushed the fix/task-reactivation-brief-send-id branch from 0a0929a to 674dc20 Compare October 3, 2026 20:43

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread src/node/services/taskService.ts
@ThomasK33

Copy link
Copy Markdown
Member Author

Readiness record for head 674dc20:

  • Tests: the 🤖 Task launch: a reactivation that prepends the brief can resend it after a durable-then-Err send #5544 repro and its other-backend variants (red before each fix), the real-session Stop-rollback test, all taskService*.test.ts and workspaceTurnManager*.test.ts (1456 pass, 0 fail), make static-check, and formal/task-launch, formal/task-lifecycle, formal/delegated-turns check.sh (all exit 0 on this head).
  • Review assessments used: 8, the 6 plus the +2 extension.
    • Round 1, normal + security on 82686f6: 0 findings.
    • Final check 1: ready, with 1 finding (a brief could be lost across two backends). It was fixed in this PR.
    • Round 2, normal + security on 0a0929a: 0 findings.
    • Final check 2, on this head after the rebase onto main: ready.
    • Round 3, normal + security on this head: 1 P2 (a duplicate brief on an immediate retry, never a loss).
  • Waiver: the issue coordinator waived "latest normal review clean" for this PR only. Findings across rounds (1, 0, 1) no longer shrink, and the remaining case only resends the brief. This head is strictly better than main, which resends it on every such reactivation.
  • Deferred: 🤖 Task launch: count only other backends' leases when dropping a kept brief (#5544 follow-up) #5588 (count only other backends' leases). The fix is ready on branch fix/5544-foreign-lease-narrowing.
  • Reason for stopping: the review budget is spent and the mechanism is not converging. The coordinator chose to merge this head.

Generated with xum • Model: anthropic:claude-opus-5-5 • Thinking: high

@ThomasK33
ThomasK33 added this pull request to the merge queue Oct 3, 2026
Merged via the queue into main with commit 44b2201 Oct 3, 2026
67 of 69 checks passed
@ThomasK33
ThomasK33 deleted the fix/task-reactivation-brief-send-id branch October 3, 2026 21:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🤖 Task launch: a reactivation that prepends the brief can resend it after a durable-then-Err send

1 participant