feat(service): write plane — Propose, Merge, ListProposals (Phase 3)
The service-layer orchestration for spec-zqb: the plane where the merge
model and the proposal state machine first run under real proposals
rather than on paper. Primitives (db proposal CRUD/merge, gitx
branch/commit/merge, authn provenance, core policy matcher) already
existed and were unit-tested; this composes them.
- Propose: the write plane, identical for REST and MCP. Row-first open
(branch name derives from the serial id), branch cut, and a
provenance-stamped commit — the agent authors, the owner commits, and
the X-Agent-Session / X-Agent-Base trailers carry the rest into a
plain git log. Adds to an existing proposal via ProposalID against its
fixed base. Frontmatter/schema/id validation at propose time, mirroring
the update hook the in-process agent write bypasses. Returns
{proposal, url}.
- Auto-merge policy: a proposal whose every changed path matches the
space's .spec.yml auto_merge lands immediately with ApprovalPolicy,
best-effort — a stale or mixed-path proposal falls back to human review
rather than failing the write.
- Merge / Reject: the state machine. Merge does the already-merged
ancestry check before gitx's If-Match staleness (design: the two need
different tests), the owner-signed merge commit, and the atomic row +
document-registry flip. ErrStale / ErrAlreadyMerged / ErrForbidden map
the 409/403 boundary.
- ListProposals / GetProposal / ProposalURL: the space-scoped read the
graph Proposals port declared and left nil, plus the stable
<origin>/~owner/space/p/<id> link.
- db.ListProposalsBySpace: the per-space, per-state listing.
Tested end to end against Postgres: open, auto-merge, mixed-path
fallback, stale-base 409, human merge, reject, add-to-existing, drifted
base. Surfaces (graph wiring, spec_propose, REST api/) fan out next.
refactor: make the filter-polarity trap inexpressible
service.SpaceFilter meant "empty membership selects nothing" — a newly
created project has no members — while search.Query.Spaces was a bare
[]core.SpaceRef whose empty case meant every space. Passing a project's
members into a query therefore turned an empty project into the whole
corpus: a silent scope inversion, invisible when it happens, and passing
every test written with a non-empty project.
The filter moves to core.SpaceFilter with unexported fields, and
search.Query takes it whole. There is no slice to hand over any more, so
the inversion cannot be written. Its three states are distinct: every
space (EverythingFilter), exactly these — possibly none — (SpacesFilter),
and the zero value, which is neither. Search refuses the zero value
rather than defaulting it, because both plausible defaults are wrong for
one of the two callers that can produce one, and returns no hits for a
filter that selects no space without asking the index.
service.SpaceFilter is now an alias for the core type, so ResolveProject
and its callers keep their names. Tests that built a Query without a
scope now say core.EverythingFilter(), which is what they always meant.
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.
fix(service): separate the read-plane rev guard from the review path
The previous commit's guard broke two service tests, which were right and
the guard was too broad: rendering and diffing a proposal branch genuinely
needs to read one, so a blanket ban on ref names is not the shape.
ReadDocument keeps the strict object-name rule. ReadDocumentAtRef reaches
an arbitrary ref, deliberately as a separate awkwardly-named method rather
than a flag, so serving unreviewed content is something a caller asks for
by name and a reviewer can grep for.
A rejected rev now wraps ErrNotFound as well, preserving the existing
deliberate property that a crafted revision cannot distinguish malformed
from absent by probing.
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.
feat: projects — a saved filter over one global index, not a container
A project is a named space set; querying one filters the single global
index. The meta-project is an implicit filter at a reserved address rather
than a row: a stored +everything would need a sync job on every space
creation, and its one failure mode is silently omitting a space.
SpaceFilter distinguishes All from an empty member list, because a freshly
created project has no members and must mean selects-nothing. Collapsing
the two would make every new project silently match the whole corpus.
refactor: move the reconciler's two deletes down to the layers that own them
The reconciler reached past its layer twice, because the primitives it
needed did not exist: a raw DELETE FROM proposal — the only SQL written
outside db/ — and a go-git RemoveReference under gitx's write lock. Both
move down, with no change in behaviour.
db.Store.DeleteOpenProposal keeps the guard in the statement, as
resolveProposal does, and distinguishes the two ways it can bite: a row
that is gone is ErrNotFound, a row that has been resolved is the new
ErrProposalNotOpen, which tells the reconciler "nothing to repair" apart
from "the repair no longer applies".
gitx.Repo.DeleteProposalBranch refuses anything outside proposals/* —
the only thing between a caller bug and a deleted approved branch — takes
the per-space write lock like every other write, and treats an
already-absent branch as success: the repair is a postcondition, and the
ref may legitimately vanish between the listing that found it and the
delete.
feat: service — wiring, space lifecycle, read paths, push validation, reconciler
Phase 1 of the implementation plan: the orchestration layer's read, validate
and repair halves. The write plane (propose/merge) is Phase 3 and is absent.
- Config/New assemble a Service from the shared config.ini, reporting every
missing key in one message; TokenStore adapts db/ to authn.TokenStore,
mapping db.ErrNotFound onto authn.ErrUnknownToken so an unknown credential
is a 401 rather than a 503.
- CreateSpace writes the repository first and the row second, removing the
repository again if the insert fails; a crash between the two leaves content
that is merely unlisted rather than a phantom space.
- ReadDocument/ListDocuments/Policy/ResolveRev resolve the approved head or a
pinned revision through one code path.
- ValidatePush checks the refs rule first and unconditionally, then frontmatter
and document-id uniqueness, which --push-option=skip-validation waives. The
rejection is a structured, terminal-shaped message naming the document.
- PlanRepairs is the repair table as a pure, table-tested function; Reconcile
gathers the facts and applies them, listing stale-index spaces for Phase 2.
Two departures from the design's repair table, both to stop the reconciler
destroying live state, documented at their definitions: an open row with no
branch is left alone inside a grace window (every propose passes through that
state), and a branch still sitting on its recorded base is never treated as
merged (its tip is trivially an ancestor of the approved head).