Skip to content

fix: guard exited-member cancellation and reconcile lean delivery tests - #51

Merged
tt-a1i merged 4 commits into
tt-a1i:codex/lean-agent-communicationfrom
outlier27-cell:fix/issue-50-collaboration-reliability
Oct 3, 2026
Merged

tt-a1i merged 4 commits into
tt-a1i:codex/lean-agent-communicationfrom
outlier27-cell:fix/issue-50-collaboration-reliability

Conversation

@outlier27-cell

@outlier27-cell outlier27-cell commented Oct 2, 2026 •

Copy link
Copy Markdown
Contributor

Problem

Cancelling a delegated subtree after its member exits logs an expected PTY-inactive error. The lean-communication branch also has tests that still report queued dispatches or use obsolete prompt, schema, and send-prompt contracts, preventing CI from validating the current protocol.

Value

Keep real cancellation failures visible while removing the exited-worker diagnostic. Restore meaningful delivery/report/replay coverage against the current protocol, including Windows PTY behavior.

Approach

  • Suppress PtyInactiveError only after confirming that no active run remains. Durable cancellation and pending accounting are unchanged.
  • Make report fixtures actually deliver dispatches before reporting, using real Node PTY workers and persisted delivery evidence.
  • Exercise submitted-dispatch replay across an actual server restart; verify received stdin rather than terminal rendering or keyboard echo.
  • Update stale guidance, SQLite version, CLI-selection, and send-prompt expectations. Preserve Windows system executable lookup in isolated PATH fixtures.
  • Resolve the real Windows stop runner's PTY entry from the configured @lydell/node-pty dependency instead of a stale hardcoded node-pty path. Keep the real stop and stderr assertions.

Refs #50. This targets codex/lean-agent-communication; it does not complete the broad reliability investigation or change report eligibility.

Validation

Windows / Node 24.14.1:

  • corepack pnpm check, corepack pnpm build, and git diff --check passed.
  • Real CLI/HTTP/SQLite/PTY lean-communication smoke and persistence storage smoke passed.
  • A sequential combined run of all 18 affected test files passed: 190 passed, 0 failed, 0 skipped, including all 57 Dream runner cases, actual replay delivery, stopped-worker reporting, and report outbox rollback.
  • All five platform regression files passed locally on Windows under both Node 24.14.1 and Node 26.10.0 (15/15 for each). Node 26 also passed both default and scriptless installation smoke checks using the upstream CI archive.
  • Latest-head upstream CI run 37042699762 passed verify, all six Linux/macOS/Windows × Node 22.18/26 platform jobs, and result. Its Linux full suite recorded 2643 passed, 4 skipped, across 393 passing files and one skipped file. Extended full-suite matrices were not selected by the PR plan.
  • Whole-project test/web tsc --noEmit exposes pre-existing branch errors; production build compilation and the selected upstream checks passed.

One earlier Windows/Node 26 package smoke reported a SQLite disk I/O error. It did not recur locally or in the same-code upstream rerun; its cause remains unconfirmed. The final empty commit only triggers CI because GitHub requires upstream admin rights for direct reruns.

Impact

Production behavior changes only for an exited-worker cancellation diagnostic. The remaining changes reconcile tests with the branch's existing delivery protocol. No dependencies or workflow permissions change. Windows terminal rendering is not used as proof of received payloads.

Self-Review

Four independent reviews covered failure boundaries (A), architecture and cleanup (B+), test evidence (B+), and protocol alignment (A-). The stopped-worker test was strengthened to stop the actual PTY and verify that its active run disappeared. All four reviews found no remaining blocking finding. Atomicity unit coverage is complemented by the retained real HTTP/SQLite outbox tests.

The subsequent PTY-package resolution fix also received four independent reviews (A, A-, A-, A) with no blocking finding.

@outlier27-cell

Copy link
Copy Markdown
Contributor Author

CI currently fails in the full suite (18 failed files, 375 passed, 1 skipped). This is not a green-CI claim.

I checked a pristine checkout of the target branch commit 64a41f4 without this PR's diagnostic change. The atomicity, replay-recovery, and guidance suites reproduced 12 failures / 53 passes, including the missing beforeWrite expectation and queued/report lifecycle fixture mismatches seen in Actions. This confirms those failures already exist in the target branch; it does not classify all 18 CI failures.

The real collaboration scenario, persistence check, and mailbox tests passed locally for the narrow diagnostic change. The broader branch's dispatch/report fixtures and baseline test expectations still need reconciliation before claiming the collaboration branch is fully validated. Keeping this PR narrow and #50 open.

@outlier27-cell outlier27-cell changed the title fix: avoid cancellation error noise for exited delegated members fix: guard exited-member cancellation and reconcile lean delivery tests Oct 2, 2026
@outlier27-cell

Copy link
Copy Markdown
Contributor Author

Follow-up to the earlier red-CI note: commit 21c4d2f now repairs the branch's stale dispatch/report/replay fixtures and expectations in addition to the narrow cancellation diagnostic fix.

All 18 previously failing test files passed together on Windows / Node 24.14.1: 190 passed, 0 failed, 0 skipped. Report fixtures wait for real PTY delivery, restart replay checks actual worker stdin, and the stopped-worker report case stops and verifies the real run. Static checks and production build also passed; four independent reviews found no remaining blocker in this scoped change.

The new upstream CI run is 37038342539. It is still running; I am not claiming full-suite or cross-platform success yet. #50 remains open for the broader reliability work.

@outlier27-cell

Copy link
Copy Markdown
Contributor Author

The latest head is 764f6c8 (code changes through c3c98d3; the final commit is an empty CI trigger). CI run 37042699762 is now successful: verify, all six Linux/macOS/Windows × Node 22.18/26 platform jobs, and result passed.

The dispatch/report/replay fixture failures are repaired, and the real Windows PTY stop test now resolves the configured @lydell/node-pty dependency. Local Windows verification passed 190/190 affected tests, plus 15/15 platform regressions under both Node 24 and Node 26. Node 26 default and scriptless package smoke checks also passed using the upstream archive.

One prior Windows/Node 26 package smoke produced a SQLite disk I/O error. The same-code rerun and local reproduction attempts passed; I cannot establish its cause from that single failure. No assertion was removed or weakened to obtain the green result. Extended full-suite platform jobs were not selected by this PR's CI plan.

No remaining blocking finding was identified by the four independent reviews. This PR is ready for maintainer review within its stated scope; #50 remains open for broader collaboration reliability work.

@tt-a1i
tt-a1i merged commit 831870d into tt-a1i:codex/lean-agent-communication Oct 3, 2026
10 checks passed
@tt-a1i

tt-a1i commented Oct 3, 2026

Copy link
Copy Markdown
Owner

Thanks for the real CLI/HTTP/SQLite/PTY coverage and cross-platform validation. I see the Windows affected-suite results, the restart replay test that records actual stdin, and the successful latest-head CI. Those establish useful correctness evidence.

Before we decide whether to bring the entire lean-communication branch into main, could you clarify the actual before/after validation?

  1. For this PR's cancellation diagnostic, do you have a real-PTY reproduction showing that cancellation is already durable, the member has exited, and the old code emits the diagnostic while the new code suppresses it? Please share the command and observed logs/state, including a control where a live member's delivery failure remains visible.
  2. The added getActiveRunByAgentId call scans the run registry and can synchronize persisted run state. Was its incremental cost measured with many members/run-history records, or a burst of subtree cancellations? If so, please share the scale and before/after timing, including any database writes. It does not add a model turn, but it is still server work.
  3. Separately, for the broader lean-communication branch, has there been a controlled real-agent comparison using the same task/team/model/budget, measuring model/tool turns, relay traffic, elapsed time, tokens and completion quality? If not, please explicitly distinguish protocol correctness from an unmeasured efficiency benefit. A smoke test or shorter guidance alone does not demonstrate a lower total collaboration cost.

This is a follow-up evidence request on the already-merged PR, not a claim that the existing real integration tests are missing. The main-branch integration is currently paused.

@outlier27-cell

Copy link
Copy Markdown
Contributor Author

Thanks for distinguishing the merged protocol fix from integration of the broader branch. Here is the scope of the evidence I can substantiate:

  1. I do not yet have the exact old/new real-PTY diagnostic comparison with both an exited-member case and a live-member failure control. I reran corepack pnpm exec vitest run tests/server/workflow-worker-pty-exit.test.ts --no-file-parallelism --maxWorkers=1 --reporter=verbose on baseline 64a41f4 and the PR branch: both passed, but neither emitted the delegated-cancellation diagnostic. This proves the existing workflow-exit cancellation regression still passes; it does NOT establish the requested diagnostic before/after comparison, and I am not presenting it as such.
  2. No measured incremental-cost or database-write comparison is available for the added lookup. Source inspection shows the lookup materializes the registry, filters/sorts matching runs, and may call persisted-run synchronization. That is server work; the absence of a model turn is not evidence that its cost is zero or negligible. No scale/timing claim is made.
  3. No controlled real-agent A/B comparison with fixed task/team/model/budget has been performed. The existing HTTP/SQLite/PTY, restart, Windows and CI results establish protocol correctness within their tested scope. They do not demonstrate fewer total model/tool turns, tokens, relay traffic, elapsed time, or unchanged completion quality for the broader lean-communication branch.

Accordingly, I withdraw any implication that these integration tests establish an efficiency improvement or that the broader branch is ready for main based on that claim. Main integration remains paused; the requested diagnostic comparison and performance/real-agent evidence remain unvalidated. This comment adds clarification, not a claim that those experiments have been completed.

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.

2 participants