add linked project support

[lancekrogers]

What changed

• add camp project add --link support for machine-local symlinked projects backed by a .camp marker
• make campaign detection, camp go, camp run, camp project run, and project git/worktree commands work from linked project directories
• teach project listing, resolution, and removal about linked git and linked non-git entries, including local ignore handling via .git/info/exclude
• add unit and integration coverage for linked add/list/run/go/remove flows

Why

The optional-submodule design now treats campaign membership as a filesystem relationship instead of a shell-managed context contract. This implementation follows that model directly: a campaign-side symlink under projects/ plus a project-side .camp marker for reverse detection.

User impact

• campaigns can attach existing local repos or directories without submodule churn
• linked git repos participate in the same run/navigation/worktree/remote flows as normal projects where that makes sense
• linked non-git directories are surfaced clearly and git-specific commands fail with explicit errors instead of ambiguous git failures

Validation

• go test ./...
• go test -tags integration ./tests/integration

[obey-agent]

Verdict: Request Changes

Overview:
This PR does a solid job threading linked-project awareness through add/list/run/go/worktree flows, and the targeted linked integration cases I ran passed. The remaining blocker is a correctness regression in the marker model: the same external repo cannot be safely linked into more than one campaign.

Key Findings:

• internal/project/link.go: AddLinked overwrites a single .camp marker in the external repo. After linking that repo into campaign B, running camp from campaign A's linked path resolves against campaign B instead. I reproduced this with camp run pwd from the first link printing the second campaign root.

What's Done Well:

• Linked vs submodule/non-git source distinctions are carried through list/resolve/run/worktree surfaces consistently.
• The new linked-project integration coverage is directionally good and the targeted linked tests I ran passed.

Staff Standard:
Not yet. I would want the linked-project marker model to stop rerouting commands into the wrong campaign before I'd be comfortable submitting this as my own work.

[obey-agent]

Verdict: Approve

Overview:
This is the stronger linked-project implementation. It removes manifest-based state, adds reverse .camp detection, and carries linked-project behavior through the command surface with solid unit and integration coverage.

Key Findings:

• No blocking issues found in the current revision.

What's Done Well:

• Keeps machine-local state out of tracked files by using .git/info/exclude.
• Distinguishes linked git and linked non-git projects cleanly in resolution and UX paths.
• Extends the feature across the command surface instead of leaving partial support behind.

Staff Standard:
Yes. I would be comfortable shipping this revision.

[obey-agent]

Verdict: Approve

Follow-up:
The linked-project marker collision is now fixed on the current head. camp project add --link rejects linking an external repo that is already attached to another live campaign instead of overwriting its .camp marker, and the branch now includes unit plus integration coverage for that cross-campaign case.

[obey-agent]

Staff-Level Review — Request Changes

Verdict: Request Changes

Overview: This is a substantial, well-scoped feature — linked projects as a first-class citizen alongside submodules is a real improvement and the filesystem-marker design (.camp JSON file + campaign-side symlink) is clean. The core architecture is right and the integration surface (detect, resolve, list, remove, run, go, worktrees) is covered thoughtfully. I can tell the implementation was thought through. That said, there are three issues in internal/project/link.go that I can't sign off on without changes: silent takeover of stale markers, a swallowed LoadCampaignConfig error, and non-atomic writes to .git/info/exclude. Individually minor, collectively they undermine the "data integrity" story the marker design depends on.

Key Findings

1. ensureLinkMarkerAvailable silently relinks over stale markers. If an existing marker points to a campaign root that no longer passes IsCampaignRoot, the function returns nil without so much as a log line. The user's marker — possibly written on another machine in a Dropbox-synced setup — gets clobbered with no audit trail. At minimum log a warning, ideally require --force to override. See inline on internal/project/link.go:176.

2. LoadCampaignConfig error swallowed in AddLinked. The marker is written with CampaignID = "" on any load failure, with no stderr signal. Worse, ctx.Err() isn't re-checked after the load, so a user who hits Ctrl-C mid-link can still have WriteMarker run, leaving a half-linked state (symlink exists, marker missing campaign_id, info/exclude not updated). See inline on internal/project/link.go:101.

3. Non-atomic os.WriteFile on .git/info/exclude. ensurePatternInFile / removePatternFromFile truncate-then-write. A crash in the gap corrupts the user's git metadata; two concurrent link invocations on the same repo will last-writer-wins. Standard fix: CreateTemp + rename. The tests will also benefit — current behavior isn't parallel-safe. See inline on internal/project/link.go:269.

Secondary Observations (no inline — acknowledge or defer)

• campaign/detect.go:detectCampaignByWalking silently drops marker read errors and treats an invalid marker.CampaignRoot as "keep walking." If a user's campaign got moved, linked projects become invisible with no diagnostic. A debug log at the skip path would help when users complain about detection.
• .camp marker has no tamper detection beyond a version integer. For a personal tool this is fine, but worth noting as a design choice rather than an oversight. A HMAC keyed off the campaign ID would be overkill; surfacing "marker version" mismatches in camp status would be enough.
• UnlinkProject's targetPath == "" branch silently leaves orphaned markers in the linked directory. Consider resolving the symlink via os.Readlink before removing it, so the function can always find the marker target on its own.
• Error wrapping consistency: ensureInfoExclude / removeInfoExclude return raw errors; most other functions in this file use camperrors.Wrap. Minor stylistic drift.

What's Done Well

• The LinkMarker struct is minimal, typed, and versioned — the JSON format is simple to debug and future-proofs extensions.
• Symlink path resolution using filepath.EvalSymlinks through normalizeCampaignRoot is the right call and matches the detection path, which means linked projects resolve consistently even when the user's cwd goes through symlinks (macOS /var → /private/var, iCloud Drive, etc.).
• pathutil.ValidateBoundary at fullPath creation is the right place to catch escape attempts, and using a shared boundary helper keeps this consistent with the sandbox package work.
• ValidateProjectName defensively guards against filepath.Base returning something odd.
• The --local vs --link mutual-exclusion check and help text make the UX clear.
• detect.go walks logical path first then resolved path, which is the right order and saves a latent bug where symlinked cwds would skip marker detection.
• Integration test tests/integration/project_linked_test.go is a good signal that this was exercised end-to-end, not just unit-mocked.

Staff Standard

This is close — the architectural shape is right and most of the integration work is well-executed. I wouldn't ship it in this state because the three inline issues are each in the "quiet data-integrity bug" category, and linked projects are explicitly about users trusting a tool to manage filesystem state across boundaries. Once the marker takeover is surfaced, the config-load error is visible, and the exclude-file writes are atomic, I'd approve this with confidence. The follow-up is small and targeted.

[obey-agent]

Verdict: Approve

Follow-up:
The current head closes the remaining linked-marker integrity gaps. .camp now stores only active_campaign_id, mismatched legacy markers are rejected instead of being silently overwritten, LoadRegistry is best-effort for ownership messaging instead of blocking the guard itself, and .git/info/exclude updates now use atomic temp-file renames. Unit and linked integration coverage were rerun on this revision.

[obey-agent]

Staff-Level Re-Review — Approve (with one small follow-up)

Verdict: Approve

Overview: All three of the inline issues I flagged in the previous review were addressed at 174c5b6 and 1a57ac3b. The fixes are more ambitious than the minimum I suggested — instead of just patching the symptoms, you reshuffled the marker schema around ActiveCampaignID as the primary identifier and added a registry lookup so stale-marker error messages can actually name the other campaign. That's the right direction.

How each prior finding landed

1. ensureLinkMarkerAvailable silently relinks over stale markers — fixed, and better than my suggestion. The function now:

   • Matches by EffectiveCampaignID() (new/legacy aware) or CampaignRoot fallback via markerMatchesCampaign.
   • Errors explicitly on legacy markers that lack an ID (linked project has an existing legacy .camp marker for another campaign; remove it before linking).
   • Uses config.LoadRegistry to resolve the conflicting campaign's current path and includes it in the error message.
   • Never silently takes over — the user gets a clear error instead of a lie.

2. LoadCampaignConfig error swallowed in AddLinked — fixed cleanly. LoadCampaignConfig is now called upfront (not lazily inside the marker construction), the error is propagated via camperrors.Wrap, cfg.ID == "" is validated explicitly, and the new ctx.Err() check after ensureLinkMarkerAvailable closes the cancellation gap I was worried about. The marker is now constructed with ActiveCampaignID set directly rather than the old pattern of "maybe-set-campaign-id-in-the-happy-path."

3. Non-atomic os.WriteFile in ensurePatternInFile/removePatternFromFile — fixed via a new writeFileAtomically helper that: preserves existing file mode via os.Stat().Mode().Perm(), creates the temp file in the same directory (so os.Rename stays atomic across filesystems), writes + Sync() + Close() + Rename(), and defers cleanup of the temp file. Both helpers now use it. Good.

Schema refactor — worth acknowledging

The move from {CampaignRoot, ProjectName, CampaignID} → {ActiveCampaignID, + legacy fields} with EffectiveCampaignID() and LinkMarkerVersion = 2 is the right long-term shape. Using campaign ID as the primary identifier is more robust than path-based matching (paths move, Dropbox/iCloud syncs differ across machines, EvalSymlinks resolution varies). The legacy field preservation keeps old markers readable. The new internal/campaign/registry_lookup.go with CAMP_REGISTRY_PATH + XDG_CONFIG_HOME is a nice touch for testability and multi-host setups.

One small follow-up (not blocking)

WriteMarker is still non-atomic. internal/campaign/link_marker.go:WriteMarker calls os.WriteFile(MarkerPath(projectDir), data, 0644) directly. The .camp marker is arguably more critical than .git/info/exclude — a corrupted marker breaks detection entirely and the user may not know why until they hit ErrNotInCampaign from a linked directory. Route this through writeFileAtomically too (or add a thin wrapper in the campaign package if you want the helper to live closer to the marker code). Small change, and it rounds out the "no critical file write is non-atomic" invariant.

What's Still Done Well

• Integration test coverage at tests/integration/project_linked_test.go was updated alongside the refactor (test +25/-2), so the new registry-lookup path is exercised end-to-end.
• link_marker_test.go got 43 new lines covering the new ID/legacy semantics.
• Error messages that name the conflicting campaign by path (resolved via registry lookup) are genuinely helpful rather than cryptic.
• The legacy-marker refusal path (remove it before linking) is explicitly conservative — better than silent takeover, even if it's slightly less ergonomic. Correct default.

Staff Standard

I'd sign off on this as my own work. The fixes went beyond what I asked for, the refactor is in the right direction, and the tests follow. The WriteMarker atomicity nit is small enough to handle as a follow-up PR. Ship it.

[obey-agent]

Verdict: Approve

Follow-up:
The remaining marker-write issue is now closed on the current head. .camp writes use the same temp-file-plus-rename pattern as the exclude-file updates, and campaign marker coverage now includes preserving existing permissions while round-tripping the new active_campaign_id schema.

[obey-agent]

Staff-Level Re-Review — Still Approved

Verdict: Approve

The atomic-write follow-up I flagged in the previous review is fixed at ae21785. WriteMarker now routes through writeFileAtomically, and the new TestWriteMarker_PreservesExistingPermissions specifically asserts that a 0600 marker stays 0600 after a rewrite — exactly the subtle permission-preservation behavior the helper depends on. Nice targeted test.

What landed

• WriteMarker (internal/campaign/link_marker.go:71) now uses the same write-temp → sync → close → rename pattern as ensurePatternInFile/removePatternFromFile in internal/project/link.go.
• New test covers the mode-preservation invariant, and the assertion on LinkMarkerVersion also confirms the round-trip through the schema upgrade path.

One small follow-up (not blocking)

writeFileAtomically is now duplicated in two packages. It lives identically in internal/campaign/link_marker.go and internal/project/link.go — 38 lines each, byte-for-byte copies. That's fine for now (both are unexported, both are self-contained), but these will drift the moment someone tweaks one and forgets the other. When you next touch either file, consider extracting it to something like internal/fsutil/atomic_write.go so there's a single source of truth. Pure cleanup, no behavior change.

Staff Standard

Still approve. The fix is correct, the test is targeted, and the "no critical file write is non-atomic" invariant now holds across the entire linked-project write surface. Ship it.

[lancekrogers]

Extracted writeFileAtomically into internal/fsutil/atomic_write.go as exported WriteFileAtomically in b5a8b23. Both internal/campaign/link_marker.go and internal/project/link.go now call the shared helper; the two identical copies are gone.

Added focused tests on the new package:

• TestWriteFileAtomically_NewFile — new-file write + default mode 0644
• TestWriteFileAtomically_PreservesExistingMode — 0600 file stays 0600 (the subtle invariant the higher-level tests already depended on)
• TestWriteFileAtomically_NoTempLeftBehindOnSuccess — no .tmp-* leftover
• TestWriteFileAtomically_UnwritableDir — surfaces the error when the parent dir doesn't exist

just test all passes (2120 unit + 145 integration). Linter is clean on all four touched files.

[obey-agent]

This only catches destination-path collisions. The same external directory can still be linked again under a different name/path in the same campaign, and the branch then gets into a broken state: both link commands succeed, but project list only shows one alias and camp project run --project <second-alias> fails with project not found in campaign. Please reject duplicate target paths within a campaign before creating the second symlink.

[obey-agent]

camp project add --link now writes only active_campaign_id, but nothing here guarantees the current campaign is present in the local registry. I reproduced camp init --no-register, then camp project add --link, then camp go p review-test --print from inside the linked repo: detection falls through to ErrNotInCampaign because resolveMarkerCampaignRoot() can only recover the root from the registry. Please either register/sync the campaign as part of linking or persist a same-machine root fallback in the marker.

[obey-agent]

Verdict: Request Changes

Overview:
The linked-project work here is directionally strong, and the branch still carries the earlier marker-integrity fixes cleanly. Re-reviewing with fresh eyes turned up two new correctness gaps in the --link flow that leave users in states the CLI cannot resolve reliably.

Key Findings:

• internal/project/link.go:109: linking into a camp init --no-register campaign writes a marker that linked-cwd detection cannot resolve, because v2 markers only store active_campaign_id and resolveMarkerCampaignRoot() can only recover the root from the local registry.
• internal/project/link.go:95: the same external directory can still be linked twice into one campaign under different names; both adds succeed, but only one alias remains discoverable and the second cannot be resolved by name.

What's Done Well:

• Linked git vs linked non-git handling is threaded through list/resolve/run/worktree surfaces consistently.
• The branch keeps the earlier marker/exclude-file integrity fixes intact while expanding integration coverage.

Staff Standard:
Not yet. I would want both broken-state cases closed before I'd be comfortable shipping this as my own work.