Repository navigation
docs(workspace): specify the Engine seam for the work-tree diff - #617
Merged
Merged
Conversation
Problem: the spec describes GET /workspace/changes as a git diff, so a second version-control engine has no contract to meet. Design: the spec names the Engine and Repo interfaces that internal/workspace consumes, the ordered engine list that only Changes walks, and the detection marker that each engine owns. Repo carries no storage path. The git behavior moves into a git engine bullet. Branch becomes an optional label. Semantic change: none for clients. head, base.sha, and not_a_git_repo stay deprecated. Verification: go test -race ./internal/gates/.
There was a problem hiding this comment.
🟡 Changes recommended
The spec claims an unimplemented engine seam and does not define its concrete interface contract.
2 open findings
What changed in this PR
Generalizes workspace-change architecture from git-specific behavior to a consumer-owned VCS engine seam.
Changes:
- Defines conceptual
EngineandReporesponsibilities. - Separates generic workspace behavior from git-specific behavior.
- Preserves existing response fields and errors.
| File | Description |
|---|---|
docs/architecture.md |
Specifies the engine-neutral workspace diff architecture. |
🧠 Review effort: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Problem: the spec names the Engine operations without signatures or a no-match signal, and it says a read never changes the repository, which an engine that snapshots its working copy cannot meet. Design: the spec shows the Engine, Repo, Head, and Base method sets, states that Open returns ok false with a nil error for a directory that is not its kind and that a non-nil error ends the request, and states that only a missing base wraps ErrNoBase. The read rule becomes engine-neutral: a read may snapshot the working copy, adds no operation when nothing changed, and writes no index, reference, or object a user could see. Two sentences are reworded. Semantic change: none for clients. Verification: go test ./internal/gates/.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The spec describes
GET /workspace/changesas a git diff, so a second version-control engine has no contract to meet. The plan is VCS step 6,vcs-engine-plan.mdsection 5, PR 1.Design
Spec: docs/architecture.md
Internal packages(theinternal/workspacerow),Runtime(WorkspaceChanges), andworkspace.The spec names the
EngineandRepointerfaces thatinternal/workspaceconsumes.Repocarries no storage path, so an engine can keep its data in any form.workspace.Changesowns the scope check, the path containment, and the 28 s budget, and it is the only code that walks one ordered engine list. Each engine owns its detection marker, and the order does not assume that two engines share a marker. No code branches on an engine name. The git behavior (private index, private object directory, empty tree, merge base) moves into onegitengine bullet.branchbecomes an optional label that the engine fills. The spec gives the exact method sets ofEngineandRepo, the no-match signal ofOpen(ok false with a nil error means "not this engine"; a non-nil error ends the request), and the engine-neutral read rule: a read may snapshot the working copy, adds no operation when nothing changed, and writes no index, reference, or object that a user could see. The structure-gate row and the contract table are described in the PRs that build them. The stack #617 to #622 merges back to back, so main matches the spec at the end.Semantic change
None for clients.
head,base.sha, andnot_a_git_repostay deprecated and are not removed. This PR changes only the spec.Verification
go test -race ./internal/gates/passes (112 s for the package; the CI Gates job runs onlyTestRepository).CI job times
No job grew by more than 5 s. The change adds no test.