Skip to content

test(server): keep server tests out of the source checkout - #601

Merged
yaacovcorcos merged 4 commits into
mainfrom
claude/replay-checkpoint-workspaces-20261019
Oct 19, 2026
Merged

yaacovcorcos merged 4 commits into
mainfrom
claude/replay-checkpoint-workspaces-20261019

Conversation

@yaacovcorcos

@yaacovcorcos yaacovcorcos commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What Changed

Server tests no longer use the Scient source checkout as a workspace.

  • Replay workspaces resolve like production. A new Scient test layer,
    orchestration-v2/scient-fork/ReplayWorkspacePolicy.testkit.ts, is the replay harness's default runtime
    policy. A local thread resolves to its worktree, else its project's root. A test thread with no project
    (which production refuses) gets its disposable replay profile. Explicit runtimePolicyOverride.cwd and
    runtimePolicyLayer still win.
  • A guard in Git itself. src/testUtils/checkoutIsolation.setup.ts sets GIT_CEILING_DIRECTORIES to the
    checkout root for every server test. Git started anywhere below the root can't find the repository. That
    covers capture, ref publication, restore, clean, worktrees and any future path, whichever process runner
    starts Git. Fixtures in the OS temp directory are unaffected.
  • Real checkpoints stay tested. The native-import projects are a small committed repository
    (checkpointWorkspace()) instead of a plain temp folder. One continuation test now checks that its run's own
    checkpoint is ready, in the project root, with its ref in that repository. A new case continues an import
    whose project folder is not a repository: the run completes there with no ready checkpoint.
  • A latent test bug surfaced. The interrupted-replacement reopen test released its project workspace with
    its first runtime. Nobody noticed while every run used the checkout. It now keeps its projects for the
    whole test, as the file's other reopen test already did.
  • PiObservedEffort drops a cwd override and comment that worked around the old behaviour.
  • Shard weights are refreshed (9a4d18e3f7) from this PR's green CI run on 33303b63b5. The old
    weights (570 entries) predated this fix and many new files; now 823. Re-splitting that run's measured times
    gives 9.2 / 19.6 / 15.7 shard-minutes with the old weights and 14.8 / 14.8 / 14.8 with the new ones
    (Codex's exact reproduction, counting shard 2's 16 untimed skipped files at the 0.6 s default). GitHub
    truncated the Test Server 2 log, so that shard comes from the run's Vitest JSON report, which matches the
    full logs of shards 1 and 3 to a mean 0.05–0.09 s per file. On the next run (9a4d18e3f7) the shard jobs
    took 20 / 20 / 21 minutes; the slowest was no faster than the first run's 21, because that run did about a
    quarter more total work. main's two recent full runs had a slowest shard of 23.5–24 test-minutes.

Why

The replay harness's default policy leaves a local thread's cwd null, and Orchestrator.ts then
checkpoints process.cwd(). In tests that is apps/server, inside the checkout. So each turn captured the
whole repository and published refs/t3/orchestration-v2/checkpoints/* into its .git.

  • Slow CI. On the last green main run, ConversationImporterContinuation.native.test.ts took 464 s (its
    tests move in ~8 s steps, one per whole-repository capture), PiNativeCloseConjunctions 127 s and
    stress/RestartRollback 41 s. test(server): isolate replay checkpoint workspaces #560's isolated run measured 29 s, 19 s and 3 s. The server shards are the
    longest jobs of every CI run.
  • Flaky CI. Merge-queue run 38846481944 and main run 39159412448 failed in a server shard because that
    capture inside the checkout exited with Git error 128.
  • Local side effects. One local run of the continuation file added 7 checkpoint refs to this checkout's
    .git, and 22 of its 27 tests timed out on Windows.

Two tests already worked around this locally, with comments saying so. This fixes it once, in the harness.

Divergence

File Change
orchestration-v2/testkit/ProviderReplayHarness.ts (upstream) Marked import and one marked block: the default policy is the Scient layer
apps/server/vite.config.ts (upstream) One marked setupFiles entry
apps/server/src/testUtils/shardWeights.json (upstream data) Regenerated

Everything else is Scient-owned. UPSTREAM.md records the seams. Upstream's harness has the same fallback,
so it's noted as worth upstreaming.

#593: it changes conversationImport.native-test-harness.ts to production's policy with a mocked
instance registry. With this PR that local override is redundant; whichever lands second drops it and keeps
#593's unrelated Layer.mergeAll additions.

Replaces #560.

Documentation Impact

Documentation impact: Updated — docs/operations/development.md (how replay tests get their workspace;
the Git guard), UPSTREAM.md.

Verification

Local, Windows 11 (Node 24.21.0, pnpm 11.10.0):

  • ReplayWorkspacePolicy.test.ts (precedence, modes kept, project-less fallback, explicit override, store
    errors) and checkoutIsolation.test.ts (Git can't find the checkout from apps/server or below; a temp
    repository and the shared Git config still work).
  • ConversationImporterContinuation.native.test.ts: all 28 pass, in 6.7 min. On main, 22 of its 27 time out
    here. With the harness seam reverted, the checkpoint check fails (scope cwd is …\apps\server), and the
    checkout's ref count stays unchanged even then: the Git guard blocked the writes.
  • ProviderSwitch.integration, ThreadLaunchService pass.
  • Server typecheck: no errors. vp fmt --check, vp lint on changed files (one existing warning),
    git diff --check.

Per-file CI times, two runs each side (main 18 and 19 October; this PR on 33303b63b5 and 9a4d18e3f7):
import continuation 464 s on main → 29 s and 34 s. Nothing else changes beyond the run-to-run spread:
replay fixtures 185 s on main, 124 s and 186 s here; ProviderSwitch 76–79 s vs 81–87 s;
NativeQueueHoldPolicy 65 s vs 68–76 s; OrchestratorMcpToolkit 22–23 s vs 24–36 s; AcpAdapterV2
87–89 s vs 73–89 s. Two runs can't settle differences that small. Second push (33303b63b5): all 12 required
checks green; rebased onto main ac705a0384 (no conflicts) with the weights commit; all 13 required checks
green on 9a4d18e3f7.

CI on the first push (afface466a): every check passed except Test Server 1, which caught the reopen-test bug
above; fixed in 33303b63b5.

Independent review, Codex gpt-6.1-sol (extra-high):

  • Design, round 1: rework. The capture-only guard missed restore, clean, ref deletion and the fork baseline
    (answered with the Git ceiling); other answers are in the proposal's table.
  • Implementation, round 2: approve with changes, no P0/P1. It confirmed the suspended policy shares the
    harness's stores, database and config. Fixed: the run's own checkpoint is checked, a non-repository case
    was added, the docs say the guard covers discovery from below the root, an unused import went, and the
    weight extractor now covers every test path and reconciles counts with each shard's summary.
  • Weights commit, round 3: approve with changes, no P0/P1. It reproduced the weights file byte for byte
    and confirmed every key matches the sequencer's path format. Its corrections, both applied above: one run
    can't show that no file slowed, and the exact re-split figures.

Implemented through the Claude Code harness (Scient) using claude-opus-5-5.

🤖 Generated with Claude Code

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

github-actions Bot commented Oct 19, 2026 •

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.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Codex Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Codex Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Codex Live turn WebSocket decoded 26.2 KiB 26.2 KiB 0 B (0.0%) 29.3 KiB ✅
Codex Live turn messages 2 2 0 (0.0%) 8 ✅
Claude Total thread wire 5.6 KiB 5.6 KiB 0 B (0.0%) 6.8 KiB ✅
Claude Thread snapshot wire 4.3 KiB 4.3 KiB 0 B (0.0%) 4.9 KiB ✅
Claude Live turn WebSocket wire 1.3 KiB 1.3 KiB 0 B (0.0%) 2.0 KiB ✅
Claude Live turn WebSocket decoded 26.6 KiB 26.6 KiB 0 B (0.0%) 29.3 KiB ✅
Claude Live turn messages 2 2 0 (0.0%) 8 ✅

Baseline: 129f4af · PR result: 8683e79 · 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.
Full compact HTTP snapshot with live cursor.

  • Codex decoded thread snapshot: 133.1 KiB
  • Claude decoded thread snapshot: 133.4 KiB

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

Yaacov and others added 3 commits October 19, 2026 17:12
The replay harness's default runtime policy left a local thread's cwd null,
and the orchestrator then checkpointed process.cwd(): apps/server, inside the
checkout. Each turn captured the whole repository and published checkpoint
refs into its .git. That made single files take minutes in CI (the import
continuation file 464 s), failed shards when the capture exited 128, and left
refs in developers' checkouts.

- Replay threads resolve their workspace as production does: worktree, else
  project root; a project-less test thread gets its disposable replay
  profile (scient-fork/ReplayWorkspacePolicy.testkit.ts, one marked seam in
  ProviderReplayHarness.ts).
- Server tests run with GIT_CEILING_DIRECTORIES at the checkout root, so Git
  cannot find the repository from below it on any path
  (testUtils/checkoutIsolation.setup.ts, one marked setupFiles entry).
- Native-import projects are a small committed repository, and a continuation
  test checks its checkpoint is ready in the project's own repository.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ened projects

- The continuation test checks the checkpoint its run recorded, not any ready
  one in the scope.
- A new case continues an import whose project folder is not a repository: the
  run completes in that folder with no ready checkpoint.
- The interrupted-replacement reopen test keeps its projects for the whole test.
  Its workspace used to be released with the first runtime, which went
  unnoticed only because every run used the checkout instead.
- Docs say what the Git ceiling covers: discovery from below the checkout root.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Weights come from CI run 39259296440 (33303b6, all checks green): the gaps
between consecutive file results in the Test Server 1 and 3 logs, as
weightedShardSequencer.ts documents. GitHub truncated the Test Server 2 log,
so that shard uses the run's Vitest JSON report: the gaps between the end
times of the files that ran. Checked against shards 1 and 3, the report gives
the same per-file times to a mean 0.05-0.09 s.

The previous weights predate this branch's checkout fix and many new files
(570 entries, now 823). With them, this run's measured times split
9.3 / 19.5 / 15.8 shard-minutes; with the refreshed weights they split
14.9 / 14.9 / 14.9. That uses the same run's times, so real runs will vary.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Keeps main's projectScope fix in the interrupted-replacement reopen test,
which this branch had also made.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@yaacovcorcos
yaacovcorcos added this pull request to the merge queue Oct 19, 2026
Merged via the queue into main with commit 7d6bd58 Oct 19, 2026
104 of 108 checks passed
@yaacovcorcos
yaacovcorcos deleted the claude/replay-checkpoint-workspaces-20261019 branch October 19, 2026 18:25
yaacovcorcos pushed a commit to nattikahana/scient-desktop that referenced this pull request Oct 20, 2026
Bring ScientFactory#353 up to current main, including the T3 alignments ScientFactory#593 and ScientFactory#607
(React Compiler migration, browser test project), ScientFactory#596, ScientFactory#601, ScientFactory#603-ScientFactory#606,
ScientFactory#608-ScientFactory#610 and ScientFactory#567.

Composition:
- Workspace writes: ScientFactory#353's mutation service keeps main's known-workspace
  boundary and Git-folder write denial. Every write path (write, create,
  binary write, delete, rename, retained replace/remove, inspect) is
  authorized first, and the mutation lock re-checks protected paths on
  each non-history resolution. Test layers provide KnownWorkspaces.
- Web: conflicted files follow main's React Compiler patterns (no manual
  memo, refs synced in layout effects, render-phase resets, effect
  events, try/finally in module helpers) with ScientFactory#353's behaviour on top,
  including ScientLatexSurface, the PDF reader, the file preview panel,
  save coordinator, workspace refresh, Pandoc tool and source PDF preview.
- Tests moved to main's browser project are ported (createRoot, async
  act); ScientFactory#353's deletions of the Markdown rename button and its seam test
  are kept.
- pnpm-lock.yaml regenerated with pnpm install.

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:XXL 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