Skip to content

Preserve Session completion replies after background notifications - #1416

Merged
JacobStephens2 merged 3 commits into
mainfrom
issue-1405
Oct 27, 2026
Merged

JacobStephens2 merged 3 commits into
mainfrom
issue-1405

Conversation

@JacobStephens2

@JacobStephens2 JacobStephens2 commented Oct 27, 2026 •

Copy link
Copy Markdown
Owner

Summary

Keep a Session's completion-bearing reply when a later background-task notification produces a reply without a completion line. Claude and Grok share this selection, so every existing completion-line reader receives the same preserved reply. Later completion lines still win.

root assistant/result reply
  remember the whole reply when its last line is a completion line
completed turn
  return the remembered reply, falling back to the latest result

Closes #1405.

Evidence

  • Before: At main (06f08b4c), cargo test --bin thirdshift harness::interpretation_tests::completion_line_survives_background_notification -- --exact failed with exit 101: both Grok and Claude returned the background-notification reply instead of PR ready.\nBug reproduced: fixed.
    After: The same command passed (1 test). The retained test also passes in the final interpretation suite.
  • Before: The Spec review's exact command, cargo test --bin thirdshift harness::interpretation_tests::assistant_completion_line_survives_a_late_result, failed with exit 101 on the initial fix: the earlier completion line appeared only in an assistant message, while the late reply supplied the single result.
    After: The same command passed (1 test), and its regression test remains in the change.
  • Confirmed cause: Result events overwrote final_message; earlier assistant text was available to trace recording but was ignored by reply selection. The shared interpreter now remembers completion-bearing root assistant and result replies. Payload validation, terminal outcomes, and child-message filtering remain with their existing owners.
  • cargo test --bin thirdshift harness::interpretation: 89 passed, 1 ignored.
  • cargo test --test bug_run: 14 passed, 1 ignored. Both Claude and Grok replay the single-result notification pattern for reproduced/not-reproduced endings; missing completion lines retain the exact existing error.
  • cargo fmt --check and git diff --check: passed.
  • scripts/affected-tests main: passed, including all 50 site tests and the full Rust suite. Nextest was unavailable, so the selector used cargo test; /usr/bin/time -p measured the Rust run at 1,373.54 seconds wall time, including compilation.

Test seams

  • Harness Interpretation: raw Claude/Grok records through condense/decode and finish to Ended.final_message and require_final_message.
  • Completion-line readers: the Ticket run command through Delivery diagnosis, and interpreted replies through Security review parsing.

Review coverage

Standards: 0 findings. Spec: 1 finding, reproduced and fixed; its regression test was kept. Both reviewers inspected every changed file, including the follow-up fix. Unread changed files: none. Generated-output exclusions: none.

Unaddressed findings

Standards

None.

Spec

None.

Security

None.

Security review outcome

No unaddressed introduced findings.

Merge Danger

Door: two-way

Revert the code change to restore prior reply selection.

Blast Radius: Sessions

Changes which reply completion-line readers receive from Claude and Grok when background notifications cause extra replies.

Built with codex codex-cli 0.164.0 · gpt-6.1-sol · xhigh

A child result that ends on a completion line was kept when the root session's later reply had none, so that delegated text could replace the root security review. Root results still supply the handoff.
@JacobStephens2
JacobStephens2 merged commit edde830 into main Oct 27, 2026
18 checks passed
@JacobStephens2
JacobStephens2 deleted the issue-1405 branch October 27, 2026 00:53
@JacobStephens2 JacobStephens2 mentioned this pull request Oct 27, 2026
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.

A background-task notification after the completion line fails a finished Run

1 participant