Skip to content

docs(fork): retain same-task follow-up delivery after upstream fix - #428

Merged
lukemaj merged 1 commit into
mainfrom
chore/212-retire-child-follow-up-wake
Oct 18, 2026
Merged

lukemaj merged 1 commit into
mainfrom
chore/212-retire-child-follow-up-wake

Conversation

@lukemaj

@lukemaj lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor

What: Keep the same-task follow-up delivery adaptation; upstream fixes the notification bug with a different task lifecycle.
Why: Removing the carried non-test behavior breaks 13 behavioral tests plus the continuation-guidance test on the already-absorbed upstream base.
So what: This registry-only PR clarifies the retained purpose and watched files; the #411 absorption must keep this adaptation unless it deliberately migrates the fork's task lifecycle.

Retire check, 2026-10-18

This supports the Objective's delegated Drafters by preserving follow-up delivery without changing their task identity.

Equivalence boundary

Upstream's recordSubagentFollowup creates a fresh delegated task, node and turn item after the original result. Each task has its own pending delivery. It excludes notifications, schedules and restart-continuation intake. Its completed-follow-up regression also finishes a Monitor run and verifies no extra result transfer or task mutation.

The fork deliberately changed this during absorption, recorded in 04770c9147e8940641edb7a818f93803b371564a (wip: preserve native follow-up identity and fork result delivery). Current recordSubagentFollowup binds later requested runs to original.id, and childThreads/delegatedFollowups.ts selects and recovers results per run. The retained #212 contract keeps the first summary/result stable while showing later results as latestTerminal*. With that same-task lifecycle, upstream's normal completion planner sees an acknowledged/delivered result and will not reserve another wake. The carried rearming and stable-result handling are still required.

Removal proof

Used Node 24.15.0 and pnpm 11.10.0 through a disposable npm execution environment because this shell had Node 26 and no pnpm. Installed dependencies only in this task worktree. No versions changed.

Exact focused command, run before removal, with removal, and after restoration:

npm exec --yes --package=node@24.15.0 --package=pnpm@11.10.0 -- pnpm exec vp test run \
  apps/server/src/orchestration-v2/DelegatedCompletionDelivery.test.ts \
  apps/server/src/mcp/delegatedTaskContinuation.test.ts \
  apps/server/src/mcp/OrchestratorMcpToolkit.integration.test.ts \
  packages/provider-core/src/server/orchestrationInstructions.test.ts
  • Baseline: 4 files, 35 tests passed.
  • Removal: 3 files failed, 1 passed; 14 tests failed, 21 passed. This includes 13 behavioral failures and 1 guidance assertion.
  • The original once-per-follow-up regression reaches the second transfer but fails expected 'acknowledged' to equal 'claimed'. Previously stopped and wait-mode parent regressions fail the same way. MCP integration observes the original summary overwritten with the follow-up response. These are behavior failures, not compilation or dependency failures.
  • Removal touched only the carried non-test logic in Orchestrator.ts and the three instruction sites, following the instruction-file moves since fix(server): a delegated child's finished follow-up turn wakes its parent #213. Reverted stable-result handling, delivery rearming, forced async policy, parent-cohort reopening, and follow-up suppression of original node/card rewrites. Retained upstream per-run deduplication and unrelated later restart/Stop/Prism hooks. All tests remained byte-for-byte unchanged. This is a removal experiment inside the current fork, not a claim to have run a pristine upstream checkout or migrated the fork back to fresh tasks.
  • Restored all four non-test files byte-for-byte; git diff --quiet exited 0 before the registry edit. The delivered diff changes only the registry purpose, shared-file coverage and keywords.
  • Monitor exclusion and duplicate replay are exercised by the passing baseline and restored original regression. The failed removal run stops before that test's Monitor assertions, so it is not claimed as independent negative-wake proof.
  • Restored run: 4 files, 35 tests passed, including duplicate replay and Monitor exclusion.
  • Registry check: node scripts/fork-features.mjs check passes, 42 features. git diff --check passes.

No runtime change, server, browser, live database, Rocky work or repo-wide check. No version bump. No feature-map part is added, removed or shipped; the registry entry remains carried. No website impact: maintainer registry metadata only.

Elon record

Requirements and who asked: User requested an evidence-based retire check of #213, keeping its regressions and Monitor exclusion; #212 also requires stable first-result semantics.
Deleted: The proposed wholesale retirement was rejected by removal proof; no new task model, runtime patch or test rewrite is needed.
Bottleneck: Same-task completion-delivery state, not missing upstream code: upstream's fresh-task design is already in both bases.
Checked myself: Read upstream #17696's entire diff, verified both ancestry relationships, inspected the fork's identity-preserving intake, and ran the retained tests before removal, with the edit removed, and after restoration.

Refs #212

Model and harness: GPT-6.1-Sol via Codex in T3 Code.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Independent review, relayed unchanged from the delegated reviewer (GPT-6.1-Sol via Codex):

PASS. Reviewed head 3a8e3697f14330f8a2eb17f871a19a83a8859ea3 against base dfb9235811e8d9b76f4713c383301e1a220364ce; no actionable findings.

  • Only docs/fork-features.md changes. Runtime and tests remain byte-identical to base.
  • Retention is justified: upstream creates fresh tasks; the fork reuses the original task. Stable first-result behavior is an explicit Delegated child follow-up turns wake the parent #212 acceptance criterion.
  • Added shared paths accurately cover intake, result selection and recovery while preserving existing primary ownership.
  • Inspected proof supports 35 passing baseline/restored tests and 14 removal failures: 13 behavioral, one guidance. Monitor exclusion and duplicate replay are covered by the passing regression; removal fails before its Monitor assertions.

Evidence limits: reviewed supplied logs and patch without rerunning tests. No pristine-upstream or live verification claimed. No edits or GitHub posts made.

@github-actions github-actions Bot added size:S vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. labels Oct 18, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire 5.0 KiB 5.0 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.2 KiB 1.2 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 20.9 KiB 20.9 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.0 KiB 5.0 KiB −7 B (−0.1%) 6.8 KiB ✅
Claude Thread snapshot wire 3.8 KiB 3.8 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.2 KiB 1.2 KiB −7 B (−0.6%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 21.2 KiB 21.2 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: dfb9235 · PR result: 3a8e369 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 108.5 KiB
  • Claude decoded thread snapshot: 108.8 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@lukemaj

lukemaj commented Oct 18, 2026

Copy link
Copy Markdown
Contributor Author

Review round 1 at exact head 3a8e3697f14330f8a2eb17f871a19a83a8859ea3: both required independent reviews APPROVE with no findings.

  • Grok 4.7 (medium): same-task claim verified against upstream c572cb71 (fresh task per follow-up) and the fork's original.id binding and planner gate; registry check OK (42 features); no smaller safe deletion.
  • Codex gpt-6.1-sol (high): same conclusions; did not rerun tests, so the 14-failure removal result remains author evidence.

Ready for the final train review and the user's merge decision.

🤖 Claude Opus 5.5 planner via Claude Code in T3 Code

@lukemaj
lukemaj merged commit aca0a34 into main Oct 18, 2026
27 checks passed
@lukemaj
lukemaj deleted the chore/212-retire-child-follow-up-wake branch October 18, 2026 16:40
lukemaj added a commit that referenced this pull request Oct 18, 2026
)

Refs #212

Co-Authored-By: GPT-6.1-Sol via Codex
(cherry picked from commit aca0a34)
lukemaj added a commit that referenced this pull request Oct 26, 2026
…rk contracts

Upstream's newer Stop, queue and follow-up behavior collided with retained
fork contracts and left 35 focused tests failing.

- A usage-limit continuation the user re-armed after Stop now resumes that
  thread through the retirement guard, which rechecks the arm at commit.
- A failed Codex root still settles at once, but stays reachable so Stop and
  cut-in reach its live child and background processes (fork #332).
- Tests now model the fork's same-task follow-ups (#213/#428), parked
  completions (#431), per-Stop retirement tokens and upstream's message-led
  queue release, instead of upstream's fresh-task and dispose-on-Stop shapes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant