feat: move workspace index to topo lock#487
Conversation
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
c08427f to
ed3b737
Compare
Greptile SummaryThis PR completes the Stack 1 artifact-family alignment by moving the workspace trail index out of
Confidence Score: 5/5Safe to merge — the workspace index read path is properly Zod-validated end-to-end, and the deprecated readWorkspaceLock alias preserves backward compatibility. All workspace reads flow through readTopoGraph → parseTopoGraph → topoGraphSchema.safeParse, so a corrupt or hand-edited topo.lock throws a clear actionable error rather than silently propagating bad data. The only gap is a missing test for the topo.lock exists without a workspace field warning branch, which is diagnostic text and does not affect correctness. packages/topographer/src/tests/workspace-topos.test.ts — needs a test for the untested fallback warning emitted when topo.lock exists but has no workspace field.
|
| Filename | Overview |
|---|---|
| packages/topographer/src/io.ts | Adds readWorkspaceTopoMetadata and readWorkspaceTrailIndex over the already-validated readTopoGraph path; renames readWorkspaceLock to a deprecated alias. Validation through parseTopoGraph/topoGraphSchema.safeParse correctly covers the new workspace field. |
| packages/topographer/src/types.ts | Adds workspaceTrailCollisionSchema, workspaceTopoMetadataSchema, and their inferred types; wires them into topoGraphSchema; removes workspaceTrails from lockManifestSchema. Schemas are strict and internally consistent. |
| packages/topographer/src/workspace-topos.ts | Replaces buildFromLockfile with buildFromTopoLock reading WorkspaceTopoMetadata; adds existsSync to distinguish missing topo.lock from topo.lock without workspace for the fallback warning. The topo.lock present but no workspace branch is not covered by any test. |
| packages/topographer/src/tests/workspace-topos.test.ts | Migrates all lockfile-path tests to write topo.lock; adds a new collision-ownership test. Missing coverage for the topo.lock exists but has no workspace field fallback branch. |
| packages/topographer/src/index.ts | Exports new functions and types; moves WorkspaceTrailCollision from workspace-topos.js re-export to types.js re-export; readWorkspaceLock kept as a deprecated export. |
| apps/trails/src/tests/completions-example.test.ts | Updates broken-workspace fixture from trails.lock to a full topo.lock structure with workspace.trails; structure matches the schema correctly. |
Prompt To Fix All With AI
Fix the following 1 code review issue. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 1
packages/topographer/src/__tests__/workspace-topos.test.ts:499-519
**Missing test for "topo.lock exists but has no workspace field" branch**
`buildWorkspaceTrailIndex` now uses `existsSync` to emit two distinct warnings, but only the `"No workspace topo.lock found"` path is exercised here. The second branch — where `topo.lock` is present and valid but its `workspace` field is `undefined` — produces `"Workspace topo.lock in \"...\" does not include workspace metadata; falling back to discovery."` and is never tested. Adding a test that writes a minimal `topo.lock` (no `workspace` key) and asserts the new warning string would close this gap.
Reviews (7): Last reviewed commit: "feat: move workspace index to topo lock ..." | Re-trigger Greptile
ed3b737 to
4e8c5c6
Compare
16bb0c1 to
1ef092a
Compare
3f53e58 to
2dcc22f
Compare
b03d19f to
d7e5d3f
Compare
2083937 to
49e3fd2
Compare
d7e5d3f to
0b8ee04
Compare
Merge activity
|
## Context The last Stack 1 branch moves workspace trail ownership metadata to the artifact that actually serializes resolved topology. This PR covers TRL-701. It is built on the corrected TRL-700 workspace layout: `trails.lock` stays a compact manifest, while `topo.lock` carries optional workspace metadata under `TopoGraph.workspace`. ## Changes - Moves workspace trail index metadata out of the lock manifest and into `TopoGraph.workspace` in `topo.lock`. - Adds strict schemas for workspace trail entries, workspace trail indexes, collision metadata, and workspace topo metadata. - Adds `readWorkspaceTopoMetadata()` and `readWorkspaceTrailIndex()` over `topo.lock`, while keeping `readWorkspaceLock()` as a deprecated compatibility alias. - Updates workspace index building to prefer `topo.lock` metadata, fall back to discovery when no workspace metadata exists, and distinguish missing `topo.lock` from a `topo.lock` without workspace metadata. - Preserves structured collision metadata from persisted workspace topo graphs without reloading every app. - Renames the workspace index source from `lockfile` to `topo-lock` in tests and docs. - Updates completions, Topographer README, ADR-0042, and workspace-topos tests for the new artifact ownership. - Adds a changeset for the workspace index relocation. ## Verification Local verification in this review-feedback pass: - `bun scripts/adr.ts check` - `bun test packages/topographer/src/__tests__/io.test.ts packages/warden/src/__tests__/drift.test.ts apps/trails/src/__tests__/topo-dev.test.ts apps/trails/src/__tests__/run-watch.test.ts` - `bun run typecheck` - `bun run lint` - `bun run lint:ast-grep` - `bun run format:check` - `bun run test` - `bun run build` - `bun run check` - `git diff --check` Remote CI, Greptile, and Graphite mergeability rerun on every push; use the PR check rollup for current remote status. ## Release / Risk This is the final Stack 1 artifact-family alignment: `trails.lock` remains compact, and workspace topology belongs in `topo.lock`. Consumers still using the deprecated `readWorkspaceLock()` alias should move to `readWorkspaceTrailIndex()`.
0b8ee04 to
10eae9a
Compare
49e3fd2 to
bbb1ea4
Compare

Context
The last Stack 1 branch moves workspace trail ownership metadata to the artifact that actually serializes resolved topology.
This PR covers TRL-701. It is built on the corrected TRL-700 workspace layout:
trails.lockstays a compact manifest, whiletopo.lockcarries optional workspace metadata underTopoGraph.workspace.Changes
TopoGraph.workspaceintopo.lock.readWorkspaceTopoMetadata()andreadWorkspaceTrailIndex()overtopo.lock, while keepingreadWorkspaceLock()as a deprecated compatibility alias.topo.lockmetadata, fall back to discovery when no workspace metadata exists, and distinguish missingtopo.lockfrom atopo.lockwithout workspace metadata.lockfiletotopo-lockin tests and docs.Verification
Local verification in this review-feedback pass:
bun scripts/adr.ts checkbun test packages/topographer/src/__tests__/io.test.ts packages/warden/src/__tests__/drift.test.ts apps/trails/src/__tests__/topo-dev.test.ts apps/trails/src/__tests__/run-watch.test.tsbun run typecheckbun run lintbun run lint:ast-grepbun run format:checkbun run testbun run buildbun run checkgit diff --checkRemote CI, Greptile, and Graphite mergeability rerun on every push; use the PR check rollup for current remote status.
Release / Risk
This is the final Stack 1 artifact-family alignment:
trails.lockremains compact, and workspace topology belongs intopo.lock. Consumers still using the deprecatedreadWorkspaceLock()alias should move toreadWorkspaceTrailIndex().