From d75ee1cca5550c76871cc3b674afa0298305a1fd Mon Sep 17 00:00:00 2001 From: Kevin Nguyen Date: Sat, 1 Aug 2026 19:50:31 +0700 Subject: [PATCH] CB-507: regression tests for worktree cwd resolution MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cases in WorktreeSessionManagerTest, covering the gap that let the NPE ship (313 tests, was 311). 1. worktreeAcquireWithNoRequestedOrCallerCwdStillResolvesANonNullRepoRoot — the null/null case a plain REST spawn produces. 2. worktreeAcquireHonoursTheProfileConfiguredCwd — the quieter second bug on the same line, where a pinned per-profile cwd: was ignored entirely. Both assert on the cwd RECORDED by FakeWorktrees rather than expecting a throw. That is deliberate: FakeWorktrees.repoRoot only records its argument and returns a canned root, so a null passes through the fake harmlessly while the real GitWorktrees runs `git -C null` and NPEs. The fake being more permissive than the real seam is exactly why 311 tests stayed green over a broken feature — asserting "an exception was raised" would be untestable here and would give false confidence. Verified as genuine regressions, not tautologies: with the pre-CB-507 expression restored both fail, with the messages they were written to give (expected: not , and expected but was ). Restored after. Drafted by an opencode-free worker over the bridge in an isolated worktree (branch worker/cb-507-regression-test-11591f-4). Its test 1 was correct as written. Test 2 was wrong and went red: it passed "/pinned/dir" as the 4th constructor argument, which is configDir, not cwd (the 11th, after mcpUrl), so cwd stayed null and the chain fell through to the daemon cwd. Corrected on integration, along with removing two unused locals and adding the rationale comments. --- .../session/WorktreeSessionManagerTest.java | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java index 774dc6d..8f8f581 100644 --- a/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java +++ b/bridged/src/test/java/dev/ltms/bridged/session/WorktreeSessionManagerTest.java @@ -166,4 +166,58 @@ class WorktreeSessionManagerTest { assertEquals(2, sessions.roster().size()); } + /** + * CB-507 regression. A plain REST spawn supplies neither a requested nor a caller cwd + * ({@code BridgedApp} hardcodes {@code callerCwd = null}), and the worktree branch used to + * resolve the repo root from just those two — yielding {@code null}, which the real + * {@code GitWorktrees} turns into {@code git -C null} and an NPE out of {@code ProcessBuilder} + * (HTTP 500). + * + *

Note this asserts on the recorded cwd rather than expecting a throw: + * {@link FakeWorktrees#repoRoot} only records its argument and returns a canned root, so a + * null flows through the fake harmlessly. That permissiveness is precisely why the whole + * suite stayed green while the feature was broken in production — so the assertion has to be + * "a usable cwd was passed down", not "an exception was raised". + */ + @Test + void worktreeAcquireWithNoRequestedOrCallerCwdStillResolvesANonNullRepoRoot() { + FakeHerdr herdr = new FakeHerdr(); + FakeWorktrees worktrees = new FakeWorktrees(); + SessionManager sessions = new SessionManager(workerService(herdr), worktrees); + + sessions.acquire("ltms-local", null, null, null, new WorktreeRequest("cb-507", null)); + + assertFalse(worktrees.repoRootCalls().isEmpty(), + "repoRoot should have been called to resolve the repo root"); + String cwd = worktrees.repoRootCalls().getFirst().cwd(); + assertNotNull(cwd, "a null cwd here becomes `git -C null` and NPEs in the real GitWorktrees"); + assertFalse(cwd.isBlank(), "a blank cwd is as unusable as a null one"); + } + + /** + * The same line carried a second, quieter bug: it never consulted the profile's configured + * {@code cwd:}, so a worktree spawn silently ignored a pinned per-profile working directory. + * Routing through {@code effectiveCwd} honours it. + */ + @Test + void worktreeAcquireHonoursTheProfileConfiguredCwd() { + FakeHerdr herdr = new FakeHerdr(); + FakeWorktrees worktrees = new FakeWorktrees().withRepoRoot("/repo").withPrefix("/wt"); + // Argument order matters: configDir is the 4th parameter, cwd the 11th (after mcpUrl). + BridgedConfig.Worker cfg = new BridgedConfig.Worker( + "ltms-local", "http://gx00.gw:8000", "coder", null, "BRIDGED_WORKER_TOKEN", + List.of("ccs", "ltms-local"), "tab", "bridged-workers", + "worker: {profile} #{n}", null, "/pinned/dir", null); + ClaudeCodeLauncher launcher = new ClaudeCodeLauncher( + new AgentControl(herdr), new WorkspaceControl(herdr), + new SubscriptionGuard(Set.of("gx00.gw")), Map.of(cfg.profile(), cfg), cfg.profile(), _ -> null); + SessionManager sessions = new SessionManager(launcher, worktrees); + + sessions.acquire("ltms-local", null, null, null, new WorktreeRequest("cb-507b", null)); + + assertEquals(1, worktrees.repoRootCalls().size()); + assertEquals("/pinned/dir", worktrees.repoRootCalls().getFirst().cwd(), + "the profile's configured cwd must reach repoRoot, not be ignored"); + } + }