fix(server): detect nested Git workspaces for checkpoints - #9842

💭 I want to check the changed non-test files—CheckpointReactor.ts, ProviderRuntimeIngestion.ts, and the deleted git/Utils.ts—and see whether isGitRepository exists in the CheckpointStore service.

1. browse_code — apps/server/src/checkpointing/**/*.ts

2. browse_code — apps/server/src/vcs/**/*.ts

💭 I'm not sure isGitRepository actually exists on CheckpointStore since the diff only shows usages, so I need to check CheckpointStore.ts directly to confirm.

3. browse_code — apps/server/src/checkpointing/CheckpointStore.ts

💭 I'm checking whether isGitRepository was introduced in this PR or already existed in CheckpointStore.ts before the merge base — since the diff doesn't touch that file, it must have already been present. Now I'm looking at how the changed consumer files, CheckpointReactor.ts and ProviderRuntimeIngestion.ts, use it. In ProviderRuntimeIngestion.ts, I see the CheckpointStore namespace imported and its service pulled in via generator, with isGitRepository called inside the yield — but this adds CheckpointStoreError to the error channel, so I want to confirm that's properly typed and handled. I also need to verify whether production layer wiring actually provides CheckpointStore now, since the test file adds it explicitly — I should check the layer definitions around where ProviderRuntimeIngestion sets up its dependencies.

4. browse_code — In apps/server/src/orchestration/Layers/ProviderRuntimeIngestion.ts, show the layer definition(s) at the bottom of the file and how dependencies are provided. Also show lines 2000-2060 around the checkpointStore.isGitRepository usage, including error handling of the surrounding Effect (any catch/orElse for CheckpointStoreError). Quote code.

💭 I'm weighing whether making isGitRepository fallible is actually a convention violation or just a correctness concern worth flagging separately, since the error is properly typed in Effect per the stated conventions. It seems more like a behavior change to note than a strict violation of the Effect service rules being checked. Now I want to verify whether production composition actually provides CheckpointStore.layer for ProviderRuntimeIngestion, and double-check the Effect.fn.Return annotation change and error handling in CheckpointReactor.ts.

5. browse_code — apps/server/src/orchestration/Layers/CheckpointReactor.ts

6. browse_code — apps/server/src/orchestration/Layers/CheckpointReactor.ts

💭 CheckpointReactor checks out convention-wise with its namespace import, service via env, and typed error. Now I want to look at line 751 in CheckpointReactor.

7. browse_code — apps/server/src/orchestration/Layers/CheckpointReactor.ts

💭 Since git/Utils.ts is being deleted, I should double-check there are no remaining consumers still importing isGitRepository anywhere else in the codebase, since the convention requires fully removing old files rather than leaving compatibility shims.

8. browse_code — Search the whole repo for any remaining references to git/Utils.ts, isGitRepository imported from a local module (not CheckpointStore), and any other directory src/git/ files. List every file and line referencing isGitRepository.

💭 I need to verify server.ts wiring actually provides CheckpointStore to ProviderRuntimeIngestionLive now that the requirement changed, since a missing dependency would cause a type error CI might miss. I should also check whether other consumers like integration harnesses or MCP need similar updates, per the convention of updating every consumer.

9. browse_code — Which files reference ProviderRuntimeIngestionLive or ProviderRuntimeIngestionService layer wiring? For each, show whether CheckpointStore.layer (apps/server/src/checkpointing/CheckpointStore.ts) is provided in the same layer composition. Include server.ts and any test/integration harness.

💭 Everything looks properly wired, and the change follows conventions with namespace imports, environment-based service resolution, and typed errors. One thing I want to check: in ProviderRuntimeIngestion, the sync check can now fail with a CheckpointStoreError that aborts event processing and gets swallowed by a generic warning log, so I need to verify this still satisfies the convention around keeping failures typed rather than bypassed. That regression concern touches pre-existing catchCause logic, not the changed lines, so it's likely out of scope for this check's convention list — I'm only supposed to flag violations introduced by the actual diff. Tests were added for the behavior change, deleted diagnostic comments look fine, and now I'm scanning ProviderRuntimeIngestion's imports to make sure nothing there violates the bare "effect" named-import convention. I'm satisfied this is just semantic behavior, not a convention issue, and confirm nothing else changed in the diff. My conclusion is "All clear" — I'll need to format the complete_check response with a title and summary both satisfying minimum length while conveying that verdict.