Repository navigation
Conversation
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works and I found nothing blocking. I copied the PR's tests onto the base, where both "recovers the parent wake" cases fail: the parent delivers nothing after the reboot. On this branch they pass, including when repeated. The nearby subagent, execution, job and migration tests pass, and the core tsc build is clean.
The marker is set in the same transaction that removes the outbox row, so a crash can't lose the wake. Suspended sessions are left out of the startup wakes, so budget accounting still runs first. Marks are also cleared when an execution completes and on user stop, so a finished or exhausted session isn't woken again on the next boot. Running user stop inside a database transaction is safe because coordinator.interrupt only forks the fiber interruption.
Public API: unchanged. The only addition is an internal SQLite column, session_inbox.wake_on_restart, whose migration defaults it to false.
Related: #36349
Before: the server restarts, but the parent stays idleflowchart TD
A["Child finishes its work"] --> B["Server saves the child's report in the parent's inbox"]
B --> C["Server removes the completed background job record"]
C -->|Server keeps running| D["Server wakes the parent"]
D --> E["Parent processes the report"]
C -->|Server restarts before the wake call| X["❌ Old process exits with the wake still owed"]
X --> F["New server scans for pending jobs and interrupted sessions"]
F --> G["The job is gone. The parent was idle and has no execution claim."]
G --> H["Server is running. Parent stays idle. Report stays in the inbox."]
style X fill:#ffe3e3,stroke:#c62828,stroke-width:3px,color:#701515
With this PR: startup can see which parent needs a wakeflowchart TD
A["Child finishes its work"] --> B["Server saves the child's report in the parent's inbox"]
B --> C["One SQLite transaction: mark the inbox row wake_on_restart and remove the completed job record"]
C -->|Server keeps running| D["Server wakes the parent"]
C -->|Same restart before the wake call| R["New server finds the marked inbox row"]
R --> W["After accounting for resume attempts, recovery wakes the eligible parent"]
W --> E["Parent promotes the original report into its conversation"]
D --> E
E --> F["Promotion consumes the inbox row and its wake marker"]
style R fill:#e4f5e8,stroke:#287040,stroke-width:2px,color:#143820
The regression cases cover a restart at acknowledgment during normal delivery and a second restart during recovery. In both cases the parent receives the original notification ID exactly once; an unrelated idle session stays parked. |
User decisions: - Make parent notifications durable across restart for live completion and startup replay. - Preserve cancellation, idempotency, conflict handling and restart limits. - Cover both crash paths with isolated core regression tests.
9e0abe2 to
640c535
Compare
There was a problem hiding this comment.
The new push is a rebase onto newer dev, and the fix itself is unchanged. The only difference in the PR's diff is one context line: the SubagentMetadata.notify check that #63413 removed upstream. On the new base, both "recovers the parent wake" cases still fail. On this branch they pass, along with the nearby subagent, execution, job and migration tests. The core tsc build is clean, and the new migration is still the latest one.
Public API: unchanged. Nothing new to raise.
Issue for this PR
Follow-up to #63209.
Type of change
What does this PR do?
A crash after acknowledging a child report can leave the parent idle with one pending notice, an empty outbox, and no execution claim. This also occurs if a second crash interrupts startup replay before its deferred wake runs.
Acknowledgement now marks the pending inbox row in the same SQLite transaction that removes the outbox row. Startup can then wake the parent after restart-budget accounting. Promotion or inbox cancellation consumes the marker with the row. Existing rows default to unmarked: their metadata alone cannot distinguish a lost wake from an intentional admit-only notice. No feature flag is needed for this fix.
How did you verify your code works?
The regression cases take a SQLite snapshot immediately after acknowledgement during normal completion and during replay, then boot a fresh core graph from it. Both cases promoted zero notices on
6b8c796. On this branch each promoted the original notification ID once and left an unrelated admit-only session parked.From
packages/core:bun run script/test.ts test/subagent-idle-e2e.test.ts -t 'recovers the parent wake' --rerun-each 10Both regressions passed 10 consecutive runs. The subagent, job, and migration files passed (78 tests), as did 24 focused execution tests including user stop, conflicts, and budget exhaustion. The migration check and core TypeScript check also passed.
Screenshots / recordings
Core only.
Checklist
Added by the opencode bot (AI-generated).
Related: #36349