Skip to content

test(server): remove skipped, duplicate, and constant-restating V2 tests - #13459

Merged
juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/test-removal
Sep 24, 2026
Merged

juliusmarminge merged 1 commit into
t3code/codex-turn-mappingfrom
v2/test-removal

Conversation

@juliusmarminge

@juliusmarminge juliusmarminge commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

Some V2 server tests never run, some repeat coverage that a recorded replay fixture or a stronger sibling test already provides, and some only check a constant or a fake's call log. They cost review and maintenance time and prove nothing extra. This PR deletes them after checking each claim against the source.

Removed (one line each)

testkit/ThreadFork.integration.test.ts

  • it.effect.skip("merges a fork delta back into the source thread through context handoff"): never runs. ThreadMergeBack.integration.test.ts covers merge-back for both Codex and Claude using recorded transcripts: fork-delta summary text, the fork_delta_summary handoff, a consumed transfer with fork_delta_context, the source thread's native id preserved, handoff text kept out of the visible conversation, and sibling merges. makeExpectedForkDeltaSummary, transcriptWithMergeBackContinuation, and the helpers only they used (compactExpectedText, findCompletedAgentMessageText, the ProviderReplayEntry import) are removed too. The skipped test also asserted that a stale pending merge-back gets superseded. That assertion never ran, so no active coverage is lost (see Notes).

testkit/ClaudeReplayFixtures.integration.test.ts

  • "records simple from real Claude Code query() output": env-gated and only checks entries.length >= 3. scripts/record-claude-agent-sdk-replay-fixture.ts --scenario simple is the real recorder.
  • "replays simple as typed Claude Agent SDK query messages" and "replays multi_turn …": the helper they called just filtered emit_inbound frames out of the transcript (prompts and model were ignored). OrchestratorReplayFixtures runs simple/claudeAgent and multi_turn/claudeAgent through the full orchestrator and asserts the same assistant text plus the projection shape. The helper replayClaudeAgentSdkTranscript is now unused and is removed from ClaudeAdapterV2.testkit.ts.

EffectWorker.test.ts

  • "detaches a handed-off session only after the old turn terminalizes": asserts ["interrupt","detach","start"] on string-recording fakes, which restates the executor's andThen chain. SelectionRestart.integration.test.ts "detaches the old provider session after an active provider handoff" drives the same detach transition through the real orchestrator, session manager, and effect worker, and asserts the old session closes and the target starts once.
  • "executes durable thread title generation effects": only checks that a fake received (kind, requestId). The case is a direct field pass-through that the service signature checks at compile time. The title service's real behaviour is covered in ThreadTitleRegenerationService.test.ts and ThreadLaunchService.test.ts.

CheckpointRollbackService.test.ts

  • "wraps underlying failures with an unexpected-failure reason and cause": a mocked projection read fails, and the test asserts the mapError fallback's reason, message template, and cause. That mirrors a four-line wrapper line for line, and nothing branches on this reason.

ThreadLaunchService.test.ts

  • "does not depend on the legacy launch workflow table": drops a table and launches. It only proves that nothing reads an unreferenced table (no non-migration code mentions it).

runtimeLayer.test.ts

  • "creates and reads a thread through the production V2 composition": every other test in the same it.layer(TestLayer) block dispatches thread.create and reads a projection through the same composition, then asserts more.

Adapters/CursorAdapterV2.test.ts, Adapters/OpenCodeAdapterV2.test.ts, Adapters/ClaudeAdapterV2.test.ts

  • "advertises only capabilities exposed by the official SDK adapter" (Cursor), "advertises the identity strengths exposed by the SDK boundary" (OpenCode), "advertises Claude Agent SDK session forks" (Claude): each reads fields of an exported as const capability object and asserts the same literals.

Adapters/OpenCodeAdapterV2.test.ts

  • "fails an active turn when the event stream reaches unexpected clean EOF": "fails an active turn when the OpenCode event stream ends cleanly" makes the same assertions (session error, failed terminal, transport_error) and also checks that a later startTurn fails. The removed test also asserted threadDisposition: "broken", so that assertion moves into the surviving test.

Adapters/ClaudeAdapterV2.test.ts, Adapters/CodexAdapterV2.test.ts

  • "does not install a protocol logger when native logging is unavailable" (both files): tests a one-line if (logger === undefined) return undefined guard.

Adapters/ClaudeAdapterV2.test.ts

  • "clears the pending task when the wake notification carries no summary": the frame uses summary: null. The SDK types summary as string, and every recorded task_notification in the fixtures has a non-empty summary. I deleted it instead of changing the frame: with a realistic summary it becomes "buffers wake output and requests a single continuation run" (same frames, asserts detail === WAKE_SUMMARY) plus "terminalizes a continuation turn from a task-notification origin wake result". The adapter's defensive typeof === "string" guard stays.

provider/acp/XAiAcpExtension.test.ts

  • "encodes interrupted dialogs as xAI cancelled responses" and "builds an abandoned exit_plan_mode response that captures the plan": each checks that a zero-logic builder returns the literal it contains.

Kept after checking

  • EffectWorker.test.ts "uses durable deadlines, notifications, and a slow liveness poll": not a duplicate of FoundationPersistence.test.ts "executes a retry at its durable deadline instead of the liveness interval". That test only covers a near deadline beating the liveness poll. This one also covers a far deadline capped by the liveness poll, a notification waking the loop early, and polling with no deadline.
  • CheckpointRollbackService.test.ts "rejects a non-ready checkpoint before opening a session or restoring files": the runtimeLayer.test.ts rollback test covers the orchestrator's dispatch-time guard, which is a different check. This one covers the service's own execution-time guard, which matters because the service itself marks checkpoints stale after a rollback and an effect can run after the checkpoint changes.
  • ThreadLaunchService.test.ts "deduplicates retried launch side effects in-process": not the same as "replays a server-allocated launch". It uses a client-supplied thread id and two concurrent calls, so it is the only test that covers the concurrent receipt-replay path for client-id launches.
  • ClaudeReplayFixtures "classifies every fixture tool use" and "keeps unregistered native conversation-state transcripts reviewable": kept as asked.

Verification

  • vp test run on EffectWorker, CheckpointRollbackService, ThreadLaunchService, runtimeLayer, Cursor/OpenCode/Claude/Codex adapter tests, and XAiAcpExtension: 9 files, 443 tests passed.
  • vp test run on ThreadFork, ClaudeReplayFixtures, and ThreadMergeBack integration tests: 3 files, 12 tests passed.
  • vp test run OrchestratorReplayFixtures.integration.test.ts -t "runs (simple|multi_turn)/claudeAgent": both passed. These are the fixtures cited above as coverage.
  • vp exec tsc --noEmit -p . in apps/server: no error TS.
  • vp lint on the 12 touched files: two warnings, both already on the base branch and not caused by this diff (an unused makeClaudeAgentSdkReplayQueryRunnerLayer in the Claude testkit, and an optional-chaining warning in an untouched ThreadLaunchService test). vp fmt is clean.
  • Not run: repo-wide checks.

Notes

  • Nothing covers the orchestrator path that supersedes stale pending merge-back transfers (Orchestrator.ts ~3241 and ~4630/5786). The only test for it was the skipped one removed here. Worth a recorded fixture if that path matters.

Line delta: +3 / −983 across 12 files.

Model: Claude Opus 5.5 (Claude Code)

🤖 Generated with Claude Code


Devin Review

Drops tests that never run, repeat coverage that a replay fixture or a
stronger sibling test already provides, or only restate a constant or a
fake's call log. Moves the one assertion unique to the removed OpenCode
clean-EOF test (threadDisposition "broken") into the surviving one.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Sep 24, 2026
@macroscopeapp

macroscopeapp Bot commented Sep 24, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at f91c135

Macroscope's review found this PR approvable — This PR removes skipped and redundant server tests and unused test-harness code across test-only files; the sole assertion relocation preserves existing coverage. No production runtime behavior, product defaults, schema, or static-analysis configuration is changed.

You can add or adjust custom eligibility rules. Learn more.

@github-actions

Copy link
Copy Markdown
Contributor

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 4.9 KiB — 6.8 KiB ✅
Codex Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Codex Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Codex Live turn WebSocket decoded — 20.4 KiB — 29.3 KiB ✅
Codex Live turn messages — 2 — 8 ✅
Claude Total thread wire — 4.9 KiB — 6.8 KiB ✅
Claude Thread snapshot wire — 3.7 KiB — 4.9 KiB ✅
Claude Live turn WebSocket wire — 1.2 KiB — 2.0 KiB ✅
Claude Live turn WebSocket decoded — 20.8 KiB — 29.3 KiB ✅
Claude Live turn messages — 2 — 8 ✅

Baseline: unavailable · PR result: f91c135 · Source CI: failure

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: 106.1 KiB
  • Claude decoded thread snapshot: 106.4 KiB

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

@juliusmarminge
juliusmarminge merged commit 4ed185e into t3code/codex-turn-mapping Sep 24, 2026
23 of 25 checks passed
@juliusmarminge
juliusmarminge deleted the v2/test-removal branch September 24, 2026 19:28
juliusmarminge added a commit that referenced this pull request Sep 24, 2026
…sts (#13459)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
juliusmarminge added a commit that referenced this pull request Sep 24, 2026
…sts (#13459)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
juliusmarminge added a commit that referenced this pull request Sep 25, 2026
…sts (#13459)

Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). 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