feat(mcpsrv): spec_comment closes the agent half of the review loop (spec-by6.3.4) An agent can now read the review threads on a proposal and reply to them. It cannot open a thread or resolve one, and that is enforced by the type rather than by the handler remembering: spec_comment is written against a narrow Commenter interface naming only Threads, ReplyTo, GetProposal and ProposalDiff, so service.CommentOn and service.ResolveThread are unreachable from it however service/ later grows. An unresolved thread suppresses policy auto-merge, so an agent able to open or resolve one would hold the gate that exists to hold its own output back. Writer is now the union of Proposer and Commenter, one narrow interface per write tool, and each handler takes only its own half. Every listed thread carries its anchor state, resolved against the branch as it stands now rather than as it stood when the comment was written — often the same agent has revised it since. An agent told only "fix this paragraph", with no signal that the critique no longer describes any block, edits the wrong thing. The tool description spells out what anchored/edited/outdated mean and says plainly that replying does not close a thread, so an agent answers the critique and pushes a revision instead of replying and waiting. Replying requires the proposal as well as the thread. A thread id is a global integer and service.ReplyTo needs nothing else, so a mistyped id would post a reply onto a stranger's proposal, out of sight of the agent that wrote it; the membership check reuses the threads already read for the ACL and costs nothing. Reads stay on the uniform owner+agents gate rather than being narrowed to the proposal an agent authored. Agent identity is self-declared in X-Agent headers and all agents share one token, so an authorship check would constrain a string the caller picks — stricter on paper than the read plane it sits in, and enforcing nothing. No wiring change was needed outside this package: main.go already passes Write: svc, and *service.Service satisfies the widened Writer. spec-by6.3.4
feat: service.Archive — one accessor, one tree walk, one link graph web/ and mcpsrv/ both needed a *doc.Archive and each invented its own way to get one. web/ handed sp.Repo to doc.Scan — reaching past service/ into gitx, which the layering rule forbids — and then listed the documents a second time for their bodies, two tree walks per page view. mcpsrv/ converted []service.Document back into []gitx.Document, rebuilding hashes service/ had already stringified. Two workarounds around one missing accessor is how three agent-facing surfaces stop being identical. Service.Archive resolves the revision, walks the tree once, and returns the archive with the bodies. ArchiveFrom is the same construction over a Document set a caller already holds — the one place a hex object name is converted back to a hash, malformed ids refused rather than zeroed. doc.Page.Links and Page.WordCount were documented as "filled in by a render pass" and nothing filled them, so Archive.Backlinks always returned empty; web/ worked around that by re-rendering every document of the space on each page view. doc.Archive.LinkPass now owns the pass and the accessor runs it, so the link graph exists wherever the archive does. web/ and mcpsrv/ no longer import gitx or go-git, in tests either. doc.DirOf replaces the three copies of the same fromDir helper.
feat(mcpsrv): MCP read tools, with Host validation replacing the SDK guard Adds the Phase 2 read tools (spec_search, spec_read, spec_list) over the service layer. Two security fixes came out of building them. The read plane could serve proposal content. gitx resolves ref names, and service.resolveRev passed any string through, so rev=proposals/42 made the READ plane hand back unreviewed text — which would then flow into agent context as though approved, the single failure this service exists to prevent. ValidateReadRev now admits only the approved-head sentinel or a full 40-character object name, at the layer all three surfaces share. Abbreviations are refused too: one that is unique today can become ambiguous later, so a pinned revision would silently stop meaning one thing. The MCP SDK's DNS-rebinding guard rejects a loopback listener whose Host is not loopback, which is exactly nginx forwarding to 127.0.0.1 — it would 403 only in production, passing every local test. The SDK offers no allowlist, so the guard is disabled and replaced by a stricter check: Host must equal the configured origin, or a loopback name for development. A rebinding attack carries the attacker's name in Host and fails it. An unusable origin logs loudly rather than quietly unguarding the endpoint.