ship: default to the server path inside a spawned workspace

[jschatz1]

Summary

• A bare kai ship inside a registered spawn now publishes via the kailab server; a plain checkout still ships via local git. --server and the new --local are explicit overrides, and passing both is refused.
• shipSpawnEntry becomes the one registry matcher shared by the session trailer, the server base, and the mode default.
• The local "nothing to ship" error names the spawn case and points at dropping --local.

Why

The local path stages only dirty files, so a spawn whose agent had already committed its work read as "nothing to ship", and the agent gave up on publishing (2026-09-10, session de960690). Spawns have no git remote by design, so local mode could never have worked for them; deciding the mode from the tree removes the flag the agent had to remember.

Test plan

• go test ./cmd/kai passes, including new TestShipUseServer_DefaultsToServerInASpawn and TestShipSpawnEntry_MatchesResolvedSymlink
• Built binary, bare kai ship --dry-run in the real spawn from session de960690 prints the server plan (4 files to kai/kai-desktop)
• --local there returns the spawn-aware error; --local --server is refused

🤖 Generated with Claude Code

[greptile-apps]

jschatz1 has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: 4ad7d228-56a4-4a83-9dc5-5f73cb8dfdaa

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[kaicontext]

Kai review

Kai Summary

Read through this one. 3 things worth your eyes before it merges, plus 1 decision to say yes to. 👇

Where I'd land: 3/5 — small fixes first.

The spawn-defaults-to-server fix is correct and well-tested, but the "one shared matcher" consolidation is only half done — shipDeltaNames and spawnEntryFor still carry their own duplicate spawn-matching loops, and the delta copy can silently re-introduce the bug on a future edit.

Additional findings

I couldn't pin these to a line in the diff:

• shipDeltaNames still uses its own inline spawn matcher instead of the shared shipSpawnEntry, so the most important copy (the one that decides what actually ships) can diverge and re-introduce the "nothing to ship" failure.
• spawnEntryFor is a byte-for-byte duplicate of shipSpawnEntry still called one line away; delete it and call shipSpawnEntry at line 113.

Decisions

Correct as written, but somebody should say yes to these:

• Making the kailab server the default publish path for every spawned workspace changes what a bare kai ship does for all such workspaces (a spawn given its own remote now routes through kailab unless --local is passed); the override exists and the tradeoff is deliberate, but the publish destination is the author's server and worth a human's confirmation.

Important files changed

File	Change
cmd/kai/ship.go	modified · +70 −8
cmd/kai/ship_mode_test.go	modified · +67 −0
cmd/kai/ship_server.go	modified · +2 −8

What I opened — 3 files, 10 turns, 46s

• cmd/kai/ship.go
• cmd/kai/ship_mode_test.go
• cmd/kai/ship_server.go

Full read-through

I have everything I need. The graph confirms there are now three copies of the spawn-matching loop: the new shipSpawnEntry (ship.go:316), the pre-existing spawnEntryFor (ship_server.go:481), and a third inline copy inside shipDeltaNames (ship_server.go:384). The diff's stated intent is "a single shared registry matcher ... for all ship decisions" — so two of those three copies are now contradictions of the intent, and one of them (shipDeltaNames) gates the actual delta computation, not just the mode default.

Let me confirm the behavior-equality concern matters: shipSpawnEntry returns *spawnpkg.Entry by taking &reg.Spawned[i], whereas the old shipDeltaNames loop iterates for _, e := range reg.Spawned (by value). That's a pointer-aliasing change for the consolidated path, but since neither mutates, it's behavior-equivalent. The real issue is the divergence risk, not aliasing.

Now the regression-test concern: the bug being fixed is "bare kai ship in a spawn defaults to local, sees nothing dirty, gives up." The test TestShipUseServer_DefaultsToServerInASpawn asserts shipUseServer(spawn, false, false) == true. If I revert the fix (delete shipSpawnEntry(cwd) != nil from shipUseServer, making it return false, nil when neither flag is set), this test fails — it would assert true against false. So the test does fail with the fix removed. That's a real regression test. Good.

Let me write the review.

---

Scope: I reviewed kaicontext/kai-cli at the merged commit (last commit d624088), reading cmd/kai/ship.go, cmd/kai/ship_server.go, and cmd/kai/ship_mode_test.go in full, and tracing the four changed symbols (shipSpawnEntry, shipUseServer, and the two tests) through the call graph. I could not read the kailab server or the spawn registry's on-disk format beyond what spawnpkg.Load/spawnpkg.Add expose here.

What it does / overall take: A bare kai ship inside a registered spawn now defaults to the kailab server publish path; --server/--local are explicit overrides and are mutually exclusive; a single new shipSpawnEntry matcher is meant to drive the mode default, the session trailer, and the server base so they can't disagree. The fix is sound and the regression test genuinely fails on the old behavior. The one real problem is that the consolidation the change claims is only half done: two duplicate spawn-matching loops still ship alongside the new shared one, and one of them gates the actual delta computation — so the very disagreement the change was written to prevent can still occur.

Concerns:

• cmd/kai/ship_server.go:380-391 (shipDeltaNames) — the delta path still carries its own inline spawn matcher (reg.Spawned loop, EvalSymlinks on both sides, e.Path == cwd || p == resolved), which is now a third copy of the logic in shipSpawnEntry. This is the function that decides what files actually get published, and it is not the shared matcher. The change's stated intent — "a single shared registry matcher ... for all ship decisions ... so they cannot disagree" — is therefore only true of the mode default, the session trailer, and the server base; the delta itself uses a separate, hand-rolled check. If someone later edits the matching condition (e.g. to also accept a case-insensitive path, or to handle a relocated spawn), they will fix shipSpawnEntry and spawnEntryFor but shipDeltaNames will silently keep the old rule, and a spawn can then be detected for mode-selection purposes while reading as a plain checkout for delta purposes — shipping only dirty-vs-HEAD instead of baseline-vs-tree and re-introducing the exact "nothing to ship" failure this change was written to close. Fix it by replacing the inline loop at line 380-391 with isSpawn := shipSpawnEntry(cwd) != nil. This is the highest-traffic of the three copies (it runs on every server ship), so leaving it divergent is the worst of the three to leave.

• cmd/kai/ship_server.go:474-489 (spawnEntryFor) — this is the second pre-existing copy, byte-for-byte identical to shipSpawnEntry in everything but the name. runShipServer calls it at line 113 to get the entry whose BaseGitSHA feeds shipContentAgainst. It is now dead-equivalent code: the same caller file already calls shipSpawnEntry (via shipServerBase at line 324) three lines of logic away. Two copies of "is this cwd a spawn, return its entry" in the same file, after a change whose whole premise is "one matcher," is a maintenance trap — they already must agree, and they will drift. Replace spawnEntryFor(cwd) at line 113 with shipSpawnEntry(cwd) and delete the function. (Within kaicontext/kai-cli the only caller of spawnEntryFor is that one line, confirmed via the call graph.)

• cmd/kai/ship.go:117-120 — minor, but worth a glance. When useServer is true because of the spawn default (not the flag), the code prints the "spawned workspace" notice and sets shipServer = true, then calls runShipServer. The shipServer = true assignment is dead: runShipServer and everything it calls (resolveShipServerTarget, shipDeltaNames, shipServerBase, shipServerCall) never reads the shipServer package var — I confirmed via the call graph that shipServer is read only inside runShip itself. So the assignment conveys nothing and could be removed; the Fprintln is the only thing with an effect. Not a defect, just noise that suggests a state dependency that doesn't exist.

Test quality: TestShipUseServer_DefaultsToServerInASpawn asserts shipUseServer(spawn, false, false) == true. Reverting the fix (dropping shipSpawnEntry(cwd) != nil from the no-flag branch so it returns false, nil) makes this case assert true against false and fail — so the test does fail with the fix removed, which is the bar. The --server && --local rejection is also exercised. TestShipSpawnEntry_MatchesResolvedSymlink covers the symlink-resolve direction that the old resolveShipSession already handled, and which the new shared matcher preserves. Good.

Decisions:

• The change makes the server path the default for every spawned workspace, where previously the agent (or user) had to pass --server. A spawn that someone has given a real git remote and intends to push to themselves will now silently route through the kailab server on a bare kai ship, requiring --local to recover the old behavior. The --local override exists and the stderr notice names it, so this is a deliberate, reversible tradeoff — but it changes what a bare command does for a whole class of workspaces, and the publish destination is a server the author owns. Worth a human's "yes, that's what we want for every spawn" before it rides to every user.

Merge readiness: The core fix is correct and the regression test holds, but the consolidation is half-finished in a way that can silently re-introduce the bug on a future edit of the matcher — the delta path, the most important of the three, still has its own copy. That's a small, local fix (replace two loops with calls to shipSpawnEntry), but it should be done before merge so the change delivers what it advertises. Small fixes first.

_{+139 −16 · 3 files · reaches 5 · the full analysis
💬 Reply to any of my comments and I'll answer, or say @kaicontext anywhere on this PR — a question, or "take another look at the retry logic".}

[kaicontext]

the shipServer = true assignment in the default-server branch is dead (no callee reads the shipServer var); harmless noise.