diff --git a/docs/site/reference/command-reference.md b/docs/site/reference/command-reference.md index 458b9b7c5..6c1857aa7 100644 --- a/docs/site/reference/command-reference.md +++ b/docs/site/reference/command-reference.md @@ -92,11 +92,11 @@ The first officer runs these against workflow state as it moves entities; you op | Command | What it does | |---------|--------------| -| `spacedock status` | Read or mutate the state: the default entity table is sorted by later workflow stage first then score descending, paginated to 25 rows by default in the human table (`--page N`, `--limit N`, `--limit 0` for all) while `--json` returns every row unless `--page`/`--limit` selects a window, omits the SOURCE column by default (`--fields source` or `--all-fields` restores it), `--next`, `--where`, `--set`, `--validate` (which also warns when a flat entity holds gate rooms, and when a retained `room-ref` no longer resolves), `--boot` (with `--identify`, the first officer's Startup identify — discovers the managed workflow(s), folds in the stage taxonomy and canonical ready-gate scheduling rows, and reports the boot sections; each row carries only `id`, `slug`, `current`, and `readiness`, while entity read and gate commands provide the complete decision record at engage; a gate stage alone is not ready; PR_STATE is a local `pr:` view, live PR state is checked at engage; local reads only, no mutation), `--read ` (a file's structured frontmatter — including the nested `stages:` taxonomy, and projectable with `--fields` — plus a heading offset/lines map, for section-scoped reads; with `--checklist` / `--ac-scan` it extracts a stage report's checklist items with line ranges and per-AC evidence citations for the first officer's gate prep; `--stage` defaults to the entity's current `status` when omitted (so a bare `--read --checklist` reads the current stage's report), and `--stage X` reads a non-current stage) | -| `spacedock gate prepare --question TEXT --artifact REVIEW.md --summary TEXT [--reference FILE ...]` | At an actionable current workflow stage (`gate: true` and nonterminal), derive and bind a recorder-ready room for folder or flat form. Where the workflow README declares `entity-form: folder`, a flat entity that holds no rooms yet is refused instead, because the room would land in a `/` companion and bind a ref that breaks if the entity later becomes `/index.md`; file it as `/index.md`. A workflow that declares no form accepts either shape, and a flat entity that already holds rooms is grandfathered under the declaration and prepares as before. At an ungated or terminal stage, preparation exits nonzero before changing the entity or review tree and names the rejected stage. Immediately after successful preparation the room contains exactly `index.json`, the canonical Briefing, with no copied sources or association. Selected files are exact local `git-root:////` objects with raw SHA-256 revisions; there is no fetch, ref requirement, or worktree fallback. Success prints `room`, `briefing`, `digest`, and `state=open`; the caller commits this prepared binding once before presentation. | +| `spacedock status` | Read or mutate the state: the default entity table is sorted by later workflow stage first then score descending, paginated to 25 rows by default in the human table (`--page N`, `--limit N`, `--limit 0` for all) while `--json` returns every row unless `--page`/`--limit` selects a window, omits the SOURCE column by default (`--fields source` or `--all-fields` restores it), `--next`, `--where`, `--set`, `--validate` (which resolves canonical `@review/...` refs for both entity forms and warns when a retained ref no longer resolves), `--boot` (with `--identify`, the first officer's Startup identify — discovers the managed workflow(s), folds in the stage taxonomy and canonical ready-gate scheduling rows, and reports the boot sections; each row carries only `id`, `slug`, `current`, and `readiness`, while entity read and gate commands provide the complete decision record at engage; a gate stage alone is not ready; PR_STATE is a local `pr:` view, live PR state is checked at engage; local reads only, no mutation), `--read ` (a file's structured frontmatter — including the nested `stages:` taxonomy, and projectable with `--fields` — plus a heading offset/lines map, for section-scoped reads; with `--checklist` / `--ac-scan` it extracts a stage report's checklist items with line ranges and per-AC evidence citations for the first officer's gate prep; `--stage` defaults to the entity's current `status` when omitted (so a bare `--read --checklist` reads the current stage's report), and `--stage X` reads a non-current stage) | +| `spacedock gate prepare --question TEXT --artifact REVIEW.md --summary TEXT [--reference FILE ...]` | At an actionable current workflow stage (`gate: true` and nonterminal), derive and bind a recorder-ready room for folder or flat form at `/review/...`, storing the form-independent `@review/...` ref. Frozen `.//review/...` and `./review/...` bindings retain their historical meaning; no migration or rewrite occurs. A declared `entity-form: folder` still refuses the first flat companion and grandfathers an existing one. At an ungated or terminal stage, preparation exits nonzero before changing the entity or review tree and names the rejected stage. Immediately after successful preparation the room contains exactly `index.json`, the canonical Briefing, with no copied sources or association. Selected files are exact local `git-root:////` objects with raw SHA-256 revisions; there is no fetch, ref requirement, or worktree fallback. Success prints `room`, `briefing`, `digest`, and `state=open`; the caller commits this prepared binding once before presentation. | | `spacedock gate withdraw --reason TEXT` | Retire the selected current-stage open prepared attempt without a Resolution, provider evidence, application, room write, or status change. Attribution is always `agent:first-officer`; the next `gate prepare` appends a successor. | | `spacedock gate record --decision approve\|revise\|hold --actor ID [--reason TEXT] [--consume]` | Record a chat decision and, for approve only, its derived one-use application. Supported chat actor IDs are `person:captain` and `agent:first-officer`. Delegated First Officer decisions require an evidence reason; the recorder does not accept or authenticate Captain-message text. A standalone close never advances status or dispatches. The current workflow stage must be an actionable gate, and the bound Briefing must use the canonical v1 stage-qualified identity and name that stage; malformed or mismatched identity fails without mutation. `--consume` is the shortest approval path: it sequences close, sync, consume, and sync in one call (usage error with `--decision revise\|hold`), each write appending a `sync=... phase=record\|consume` line. Do not run a separate state commit after a successful split-root close or consume write. Delegated First Officer decisions also require `--conn-quote` (the grant verbatim) and `--conn-source` (where it was given); citation flags are refused with `--actor person:captain`. | -| `spacedock gate record --round STAGE/CYCLE --briefing PATH/briefing.json --log PATH/briefing.review.jsonl` | For a folder-form entity (`/index.md`), publish one complete workflow-neutral correction round to the immutable derived room `review//round-` and update the current `review-round` pointer. The recorder retains canonical Briefing/log bytes and does not classify findings or write workflow body projections; `STAGE` must exist in the workflow taxonomy but may differ from current status for historical backfill. Flat entities are refused because review artifacts accumulate beside the entity. Exact replay is a no-op; divergence is refused. | +| `spacedock gate record --round STAGE/CYCLE --briefing PATH/briefing.json --log PATH/briefing.review.jsonl` | For flat or folder form, publish one complete workflow-neutral correction round to `/review//round-` and store `@review//round-`. Frozen folder `./review/...` pointers replay unchanged; no entity migration is needed. The recorder retains canonical Briefing/log bytes and does not classify findings or write workflow body projections; `STAGE` must exist in the workflow taxonomy but may differ from current status for historical backfill. Relative Artifacts must stay inside the ticket review home and cannot name the mutable entity. Exact replay is a no-op; divergence is refused. | | `spacedock gate consume ` | Spend a binding pending approval once and advance status atomically; stale approvals become superseded. A consumed nonterminal application becomes ordinary stage history. After the worker report is durable, one atomic terminal status write can complete that stage without `--force`. On an approval whose target stage is terminal, consume spends nothing and writes no status: it leaves the application `pending` and returns the route `approved-awaiting-merge` (idempotently, on repeat), and `merge guard` discovers/arms the delivery mechanism when it acts. For an ungated current-stage-to-terminal transition, finalize directly with `spacedock status --workflow-dir DIR --set SLUG status=TERMINAL completed verdict=PASSED worktree=`; do not use that route for a pending terminal-target approval, whose sole consumer remains `merge guard`. In a split-root workflow, a write (an advance or a stale-pending supersede) commits and syncs itself, appending a `sync=... phase=consume` line; a refusal or a terminal route performs no sync and emits no sync line. | | `spacedock merge guard --verdict passed\|rejected` | Run the terminal merge ceremony and, with delivery proven, finalize: the sole terminal consumer of a pending terminal-target approval — the `mod-block` is cleared in its own step, then `application.state: consumed`, the terminal status, `verdict`, and `completed` move in one locked write, and the `pr` merge sentinel is retained through archive as durable delivery proof. | | `spacedock merge guard --rework` | Delivery requires rework: write the pending terminal-target approval `pending→superseded`, route the entity through the record stage's declared `feedback-to`, and clear `pr`/`mod-block`. Refuses without a pending terminal approval, or with a missing/undefined/terminal `feedback-to`. | diff --git a/docs/specs/gate-resolution-frontmatter-contract.md b/docs/specs/gate-resolution-frontmatter-contract.md index ec6b65fac..4b218b02f 100644 --- a/docs/specs/gate-resolution-frontmatter-contract.md +++ b/docs/specs/gate-resolution-frontmatter-contract.md @@ -139,21 +139,14 @@ The room layout is the same for both entity forms: //review//briefing-/ ``` -but `room-ref` is written relative to the entity file's own directory, so folder form -binds `./review/...` while flat form binds `.//review/...`. Only the folder-form -ref is invariant under a later move of the entity. A workflow states which form it -keeps with `entity-form: folder` in its README frontmatter, and where that declaration -is present preparation refuses to create the first room beside a flat `.md`, -whose `/` companion would hold refs that break on conversion. A workflow that -declares no form accepts either shape and preparation refuses neither. Flat entities -that already hold rooms are grandfathered under the declaration, and their -slug-prefixed refs stay correct while they stay flat; converting one requires -`git mv .md /index.md` and rewriting every `room-ref: .//` to -`room-ref: ./` in the same commit, and `status --validate` reports both the -grandfathered shape and any ref that stops resolving. State commit and archive -operations continue to treat the flat -Markdown plus companion directory as one literal path-scoped unit, including tracked -deletions and rollback, without sweeping siblings. +New bindings store `room-ref: @review//briefing-`. The reserved namespace +resolves below `//review/` for both entity forms and rejects empty, +absolute, non-normalized, dot-segment, traversal, and backslash paths. Existing flat +`.//review/...`, folder `./review/...`, and other legacy refs keep their meaning +and are never rewritten. A declared `entity-form: folder` still refuses the first flat +companion and grandfathers an existing one. No migration is required; `status --validate` +uses the same resolver, and state commit/archive continue to treat flat Markdown plus +its companion as one literal path-scoped unit without sweeping siblings. Each selected source is a readable, committed, non-symlink regular file owned by the workflow's `main` or distinct `state` Git history. Its closed identity is @@ -216,11 +209,15 @@ append its authorized Cycle line before invoking the producer; the recorder pres that body byte-for-byte. The published round is the durable evidence; `gate record --round` reports every Resolution as advisory structural evidence on publication. -Round recording requires a folder-form entity at `/index.md`, so its accumulating -`review/` artifacts are scoped beside that entity. Flat entities refuse before locking -or writing; the recorder does not alter the approved derived room path to compensate. -`STAGE` must name a stage in the workflow definition, but need not equal current -`status`: explicit historical backfill remains supported. +Flat `.md` and folder `/index.md` entities publish to the same ticket review +home and store `@review//round-`. Frozen folder `./review/...` pointers +replay unchanged. The declared-folder policy applies to new flat round homes and +grandfathers an existing companion. `STAGE` must exist in the workflow definition but +may differ from current `status` for historical backfill. + +A relative round Artifact is trusted only when its resolved regular path stays inside +`//`. Paths outside that home and the mutable entity (`.md` or +`/index.md`) refuse during publication and replay; `git-root://` identities are unchanged. The room is immutable: exact whole-room replay is a whole-tree no-op; any different Briefing, log, room shape, or pointer fails closed. New-room publication rolls back if diff --git a/internal/cli/gate_one_file_room_test.go b/internal/cli/gate_one_file_room_test.go index 4f8de2cc0..e4baf19c7 100644 --- a/internal/cli/gate_one_file_room_test.go +++ b/internal/cli/gate_one_file_room_test.go @@ -46,7 +46,7 @@ func TestGateOneFileRoomJourneyPublishesAndCompletes(t *testing.T) { if binding.RequestDigest != "" { t.Fatalf("binding carries request-digest %q", binding.RequestDigest) } - if binding.RoomRef != "./task/review/validation/briefing-1" { + if binding.RoomRef != "@review/validation/briefing-1" { t.Fatalf("room-ref=%q", binding.RoomRef) } diff --git a/internal/cli/state_commit_test.go b/internal/cli/state_commit_test.go index b33ad5e36..5b54fd08c 100644 --- a/internal/cli/state_commit_test.go +++ b/internal/cli/state_commit_test.go @@ -10,6 +10,7 @@ import ( "strings" "testing" + "github.com/spacedock-dev/spacedock/internal/gates" "github.com/spacedock-dev/spacedock/internal/status" "github.com/spacedock-dev/spacedock/internal/testgit" ) @@ -260,6 +261,68 @@ func TestStateCommitFlatIncludesExactCompanionDirectoryAndTrackedDeletions(t *te } } +func TestStateCommitMakesFlatRoundDurableInFreshHost(t *testing.T) { + bare, workflowA, workflowB, stateBranch := twoHostStateWorkflow(t) + checkoutA := filepath.Join(workflowA, ".spacedock-state") + checkoutB := filepath.Join(workflowB, ".spacedock-state") + hostA := filepath.Dir(filepath.Dir(workflowA)) + const slug = "first-task" + entityA := filepath.Join(checkoutA, slug+".md") + writeEntity(t, workflowA, slug, "---\nid: first-task\nstatus: ideation\ntitle: First Task\n---\n# Flat round\n") + homeA := filepath.Join(checkoutA, slug) + if err := os.MkdirAll(homeA, 0o755); err != nil { + t.Fatal(err) + } + copyFixture := func(dst, name string) { + t.Helper() + body, err := os.ReadFile(filepath.Join("..", "gates", "testdata", "advisory-round", name)) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(dst, body, 0o644); err != nil { + t.Fatal(err) + } + } + copyFixture(filepath.Join(homeA, "candidate.patch"), "candidate.patch") + inputs := t.TempDir() + briefing, log := filepath.Join(inputs, "briefing.json"), filepath.Join(inputs, "briefing.review.jsonl") + copyFixture(briefing, "briefing.json") + copyFixture(log, "briefing.review.jsonl") + if err := gates.RecordSemantic(entityA, gates.RecordInput{Round: "ideation/1", BriefingPath: briefing, LogPath: log, WorkflowDir: workflowA}); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(checkoutA, "dirty-sibling.md"), []byte("do not commit\n"), 0o644); err != nil { + t.Fatal(err) + } + if code, _, errOut := runStateCommitCmd(t, hostA, workflowA, slug, "-m", "durable flat round"); code != 0 { + t.Fatalf("state commit exit=%d stderr=%q", code, errOut) + } + want := []string{ + "first-task.md", "first-task/candidate.patch", + "first-task/review/ideation/round-1/briefing.json", + "first-task/review/ideation/round-1/briefing.review.jsonl", + } + if got := strings.Fields(git(t, checkoutA, "show", "--name-only", "--pretty=format:", "HEAD")); strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Fatalf("flat round commit paths=%q want %q", got, want) + } + if _, ok := gitOK(t, bare, "cat-file", "-e", stateBranch+":dirty-sibling.md"); ok { + t.Fatal("dirty sibling reached the state remote") + } + git(t, checkoutB, "pull", "-q", "--rebase", "origin", stateBranch) + entityB := filepath.Join(checkoutB, slug+".md") + if _, err := gates.ValidateRoundFile(entityB, "ideation/1"); err != nil { + t.Fatalf("fresh-host round validation: %v", err) + } + roomB := filepath.Join(checkoutB, slug, "review", "ideation", "round-1") + before := git(t, checkoutB, "status", "--porcelain") + if err := gates.RecordSemantic(entityB, gates.RecordInput{Round: "ideation/1", BriefingPath: filepath.Join(roomB, "briefing.json"), LogPath: filepath.Join(roomB, "briefing.review.jsonl"), WorkflowDir: workflowB}); err != nil { + t.Fatalf("fresh-host exact replay: %v", err) + } + if after := git(t, checkoutB, "status", "--porcelain"); after != before { + t.Fatalf("exact replay changed checkout: before=%q after=%q", before, after) + } +} + func TestStateCommitFlatCompanionRenameIncludesSourceAndDestination(t *testing.T) { _, workflow, _, _ := twoHostStateWorkflow(t) checkout := filepath.Join(workflow, ".spacedock-state") diff --git a/internal/ensigncycle/claude_live_runner_test.go b/internal/ensigncycle/claude_live_runner_test.go index e5df1fc1b..945580d45 100644 --- a/internal/ensigncycle/claude_live_runner_test.go +++ b/internal/ensigncycle/claude_live_runner_test.go @@ -330,10 +330,10 @@ func runClaudeWithdrawnGateRecoveryScenario(t *testing.T, runner liveDriver, sce if readFile(t, firstBriefingPath) != firstBriefing { t.Fatalf("recovery rewrote withdrawn room bytes\nArtifacts: %s", result.artifactDir) } - secondRoom := filepath.Join(filepath.Dir(fixture.entity), filepath.FromSlash(current.Briefing.RoomRef)) + secondRoom, resolveErr := gates.ResolveRoomRef(fixture.entity, current.Briefing.RoomRef) entries, err := os.ReadDir(secondRoom) - if err != nil || len(entries) != 1 || entries[0].Name() != "index.json" || !entries[0].Type().IsRegular() { - t.Fatalf("successor room is not the emitted one-file room: entries=%v err=%v\nArtifacts: %s", entries, err, result.artifactDir) + if resolveErr != nil || err != nil || len(entries) != 1 || entries[0].Name() != "index.json" || !entries[0].Type().IsRegular() { + t.Fatalf("successor room is not the emitted one-file room: resolve=%v entries=%v err=%v\nArtifacts: %s", resolveErr, entries, err, result.artifactDir) } if current.Briefing.RequestDigest != "" { t.Fatalf("successor binding carries request-digest %q\nArtifacts: %s", current.Briefing.RequestDigest, result.artifactDir) diff --git a/internal/gates/io.go b/internal/gates/io.go index 6b381abae..36309f42c 100644 --- a/internal/gates/io.go +++ b/internal/gates/io.go @@ -208,11 +208,18 @@ func validateRetainedAuthorityExcept(entityPath, workflowDir string, doc *Docume // prepared room gets those two checks. A skip here gives the // one-file room less validation than the two-file room had, and // that inverts the point of the change. - if !preparedRoomBinding(entityPath, attempt.Briefing) { + prepared, err := preparedRoomBinding(entityPath, attempt.Briefing) + if err != nil { + return fmt.Errorf("attempt %s room-ref: %w", attempt.ID, err) + } + if !prepared { continue } if attempt.Briefing.RequestDigest != "" { - room := filepath.Join(filepath.Dir(entityPath), filepath.FromSlash(attempt.Briefing.RoomRef)) + room, err := ResolveRoomRef(entityPath, attempt.Briefing.RoomRef) + if err != nil { + return err + } requestBytes, err := os.ReadFile(filepath.Join(room, "request.json")) if err != nil { return fmt.Errorf("attempt %s retained request.json: %w", attempt.ID, err) diff --git a/internal/gates/operation.go b/internal/gates/operation.go index 203285b63..b65e8d458 100644 --- a/internal/gates/operation.go +++ b/internal/gates/operation.go @@ -104,8 +104,12 @@ func RecordSemanticSummary(entityPath string, input RecordInput) (Summary, error if filepath.Base(input.BriefingPath) != "briefing.json" || filepath.Base(input.LogPath) != "briefing.review.jsonl" { return Summary{}, fmt.Errorf("--round inputs must name briefing.json and briefing.review.jsonl") } - if filepath.Base(entityPath) != "index.md" { - return Summary{}, fmt.Errorf("gate record --round requires folder-form entity /index.md because review artifacts accumulate beside the entity; convert with `git mv .md /index.md` AND rewrite every `room-ref: .//` to `room-ref: ./` in the same commit") + workflowDir := input.WorkflowDir + if workflowDir == "" { + workflowDir = nearestWorkflowDir(filepath.Dir(entityPath)) + } + if err := refuseNewFlatCompanion(entityPath, workflowDir); err != nil { + return Summary{}, err } unlock, err := lockEntity(entityPath) if err != nil { @@ -177,10 +181,14 @@ func Withdraw(entityPath string, input WithdrawInput) (Summary, error) { if state := attemptState(attempt); state != "open" { return Summary{}, fmt.Errorf("attempt %s is frozen %s", attempt.ID, state) } - if !preparedRoomBinding(entityPath, attempt.Briefing) { + if !isPreparedRoomBinding(entityPath, attempt.Briefing) { return Summary{}, fmt.Errorf("current attempt has no prepared gate room") } - room, err := filepath.Abs(filepath.Join(filepath.Dir(entityPath), filepath.FromSlash(attempt.Briefing.RoomRef))) + room, err := ResolveRoomRef(entityPath, attempt.Briefing.RoomRef) + if err != nil { + return Summary{}, fmt.Errorf("resolve bound gate room: %w", err) + } + room, err = filepath.Abs(room) if err != nil { return Summary{}, fmt.Errorf("resolve bound gate room: %w", err) } @@ -523,7 +531,7 @@ func boundBriefingManifest(entityPath string, binding Briefing) (*briefingManife if manifest.ID != binding.ID { return nil, fmt.Errorf("bound canonical Briefing identity does not match the current binding") } - if preparedRoomBinding(entityPath, binding) { + if isPreparedRoomBinding(entityPath, binding) { if err := validatePreparedSummary(manifest); err != nil { return nil, err } @@ -538,7 +546,10 @@ func boundBriefingManifest(entityPath string, binding Briefing) (*briefingManife // reserved name first and the earlier name second. A legacy binding names the // Briefing file itself. func boundBriefingPath(entityPath string, binding Briefing) (string, error) { - retained := filepath.Join(filepath.Dir(entityPath), filepath.FromSlash(binding.RoomRef)) + retained, err := ResolveRoomRef(entityPath, binding.RoomRef) + if err != nil { + return "", err + } if binding.RequestDigest != "" { requestBytes, err := os.ReadFile(filepath.Join(retained, "request.json")) if err != nil { @@ -564,7 +575,7 @@ func boundBriefingPath(entityPath string, binding Briefing) (string, error) { } return briefingPath, nil } - if preparedRoomBinding(entityPath, binding) { + if isPreparedRoomBinding(entityPath, binding) { for _, locator := range preparedLocators { if path, err := resolveBriefingLocator(retained, locator); err == nil { return path, nil diff --git a/internal/gates/prepare.go b/internal/gates/prepare.go index dcb5843ce..13ab3d5f8 100644 --- a/internal/gates/prepare.go +++ b/internal/gates/prepare.go @@ -8,6 +8,7 @@ import ( "errors" "fmt" "os" + "path" "path/filepath" "strconv" "strings" @@ -233,7 +234,7 @@ func Prepare(entityPath string, input PrepareInput) (PrepareResult, error) { if err != nil { return PrepareResult{}, fmt.Errorf("canonicalize prepared Briefing: %w", err) } - roomRef, err := relativeRoomRef(entityPath, room) + roomRef, err := canonicalReviewRoomRef(entityPath, room) if err != nil { return PrepareResult{}, err } @@ -243,7 +244,7 @@ func Prepare(entityPath string, input PrepareInput) (PrepareResult, error) { RoomRef: roomRef, } - if previous != nil && attemptState(previous) == "open" && preparedRoomBinding(entityPath, previous.Briefing) && + if previous != nil && attemptState(previous) == "open" && isPreparedRoomBinding(entityPath, previous.Briefing) && !sameBinding(previous.Briefing, binding) { return PrepareResult{}, fmt.Errorf("open gate room binding is frozen and cannot be rebound") } @@ -298,7 +299,7 @@ func Prepare(entityPath string, input PrepareInput) (PrepareResult, error) { } func preparedEntityReplaySource(entityPath string, roots gitsource.Roots, previous *Attempt, ordinal int) (gitsource.Source, bool, error) { - if previous == nil || attemptState(previous) != "open" || !preparedRoomBinding(entityPath, previous.Briefing) { + if previous == nil || attemptState(previous) != "open" || !isPreparedRoomBinding(entityPath, previous.Briefing) { return gitsource.Source{}, false, nil } manifest, err := boundBriefingManifest(entityPath, previous.Briefing) @@ -349,7 +350,7 @@ func entityWithoutGates(data []byte) ([]byte, error) { } func preparedReplay(entityPath string, previous *Attempt, briefingID, question, summary string, sources []gitsource.Source) (PrepareResult, bool, error) { - if previous == nil || attemptState(previous) != "open" || !preparedRoomBinding(entityPath, previous.Briefing) { + if previous == nil || attemptState(previous) != "open" || !isPreparedRoomBinding(entityPath, previous.Briefing) { return PrepareResult{}, false, nil } manifest, err := boundBriefingManifest(entityPath, previous.Briefing) @@ -386,7 +387,11 @@ func preparedReplay(entityPath string, previous *Attempt, briefingID, question, return PrepareResult{}, false, nil } } - room, err := filepath.Abs(filepath.Join(filepath.Dir(entityPath), filepath.FromSlash(previous.Briefing.RoomRef))) + room, err := ResolveRoomRef(entityPath, previous.Briefing.RoomRef) + if err != nil { + return PrepareResult{}, false, fmt.Errorf("resolve prepared room: %w", err) + } + room, err = filepath.Abs(room) if err != nil { return PrepareResult{}, false, fmt.Errorf("resolve prepared room: %w", err) } @@ -483,7 +488,17 @@ func validatePreparedCandidate(roots gitsource.Roots, briefingBytes []byte) erro } func validatePreparedRoomAncestry(entityPath, room string) error { - trustedHome := filepath.Dir(entityPath) + trustedHome := reviewHome(entityPath) + if info, err := os.Lstat(trustedHome); err == nil { + if info.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("prepared room parent %s is a symlink", trustedHome) + } + if !info.IsDir() { + return fmt.Errorf("prepared room parent %s is not a directory", trustedHome) + } + } else if !os.IsNotExist(err) { + return err + } parent := filepath.Dir(room) rel, err := filepath.Rel(trustedHome, parent) if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { @@ -512,39 +527,27 @@ func validatePreparedRoomAncestry(entityPath, room string) error { return nil } -// preparedRoomBinding reports whether the binding has a prepared gate room. It -// is the shape-independent test that replaces `request-digest != ""`. Six -// runtime sites used that field to mean "this attempt has a prepared room". -// -// A request-backed binding always has a prepared room, so the test -// short-circuits. Every guard the old test gave such an attempt therefore stays -// as strict, and a room that lost its files still refuses. -// -// A request-less binding has a prepared room when its room-ref names a real -// directory that is not an archived room. -// -// An archived request-less binding fails the test three ways. An opaque -// provider ref such as `subspace-room:3k-gate-design` names no local path. A -// legacy ref names the Briefing file itself. An archived room is a directory -// that holds briefing.json. -// -// The test reads that archived name, and not the reserved name, for one reason. -// A prepared room whose Briefing is deleted must stay a prepared room. The -// archived read path gives a skip. A deleted Briefing must not take that skip, -// because gate record then closes over it. -func preparedRoomBinding(entityPath string, binding Briefing) bool { +func preparedRoomBinding(entityPath string, binding Briefing) (bool, error) { + room, err := ResolveRoomRef(entityPath, binding.RoomRef) + if err != nil { + return false, err + } if binding.RequestDigest != "" { - return true + return true, nil } if binding.RoomRef == "" { - return false + return false, nil } - room := filepath.Join(filepath.Dir(entityPath), filepath.FromSlash(binding.RoomRef)) if info, err := os.Lstat(room); err != nil || !info.IsDir() { - return false + return false, nil } - _, err := os.Lstat(filepath.Join(room, archivedBriefingLocator)) - return err != nil + _, err = os.Lstat(filepath.Join(room, archivedBriefingLocator)) + return err != nil, nil +} + +func isPreparedRoomBinding(entityPath string, binding Briefing) bool { + prepared, _ := preparedRoomBinding(entityPath, binding) + return prepared } // validatePreparedRoomEntries requires the exact file set the binding implies: @@ -750,11 +753,7 @@ func workflowDeclaresFolderForm(workflowDir string) bool { } func preparedRoomPath(entityPath, stage string, attempt int) (string, error) { - slug := entitySlug(entityPath) - home := filepath.Dir(entityPath) - if filepath.Base(entityPath) != "index.md" { - home = filepath.Join(home, slug) - } + home := reviewHome(entityPath) reviewRoot := filepath.Join(home, "review") room := filepath.Join(reviewRoot, stage, "briefing-"+strconv.Itoa(attempt)) rel, err := filepath.Rel(reviewRoot, room) @@ -764,16 +763,35 @@ func preparedRoomPath(entityPath, stage string, attempt int) (string, error) { return room, nil } -func relativeRoomRef(entityPath, room string) (string, error) { - ref, err := filepath.Rel(filepath.Dir(entityPath), room) +func reviewHome(entityPath string) string { + if filepath.Base(entityPath) == "index.md" { + return filepath.Dir(entityPath) + } + return filepath.Join(filepath.Dir(entityPath), entitySlug(entityPath)) +} + +func canonicalReviewRoomRef(entityPath, room string) (string, error) { + ref, err := filepath.Rel(filepath.Join(reviewHome(entityPath), "review"), room) if err != nil { return "", fmt.Errorf("resolve prepared room reference: %w", err) } ref = filepath.ToSlash(ref) - if !strings.HasPrefix(ref, ".") { - ref = "./" + ref + if ref == "." || ref == ".." || strings.HasPrefix(ref, "../") { + return "", fmt.Errorf("prepared room escapes the entity review directory") + } + return "@review/" + ref, nil +} + +// ResolveRoomRef resolves canonical review-home refs and preserves every legacy base. +func ResolveRoomRef(entityPath, ref string) (string, error) { + if ref == "@review" || strings.HasPrefix(ref, "@review/") { + suffix := strings.TrimPrefix(ref, "@review/") + if ref == "@review" || suffix == "" || suffix == "." || suffix == ".." || path.IsAbs(suffix) || strings.Contains(suffix, `\`) || path.Clean(suffix) != suffix { + return "", fmt.Errorf("invalid @review room-ref %q", ref) + } + return filepath.Join(reviewHome(entityPath), "review", filepath.FromSlash(suffix)), nil } - return ref, nil + return filepath.Join(filepath.Dir(entityPath), filepath.FromSlash(ref)), nil } func entitySlug(entityPath string) string { diff --git a/internal/gates/prepare_shape_test.go b/internal/gates/prepare_shape_test.go index 4d90c0b6f..af0d4c28f 100644 --- a/internal/gates/prepare_shape_test.go +++ b/internal/gates/prepare_shape_test.go @@ -1,6 +1,7 @@ package gates import ( + "bytes" "encoding/json" "fmt" "os" @@ -134,7 +135,11 @@ func TestBindingShapesClassifyAndResolve(t *testing.T) { Digest: "sha256:" + strings.Repeat("1", 64), RoomRef: tc.ref, } - if got := preparedRoomBinding(entity, binding); got != tc.prepared { + got, classifyErr := preparedRoomBinding(entity, binding) + if classifyErr != nil { + t.Fatal(classifyErr) + } + if got != tc.prepared { t.Fatalf("preparedRoomBinding(%q) = %v, want %v", tc.ref, got, tc.prepared) } path, err := boundBriefingPath(entity, binding) @@ -165,6 +170,52 @@ func TestBindingShapesClassifyAndResolve(t *testing.T) { } } +func TestMalformedCanonicalRoomRefCannotSpendGateAuthority(t *testing.T) { + workflow, _, entity, artifact, _ := prepareFixture(t, "flat") + if _, err := Prepare(entity, PrepareInput{WorkflowDir: workflow, Question: "Advance?", Artifact: artifact, Summary: "candidate"}); err != nil { + t.Fatal(err) + } + body := bytes.Replace(mustReadBytes(t, entity), []byte("@review/validation/briefing-1"), []byte("@review/a/../b"), 1) + if err := os.WriteFile(entity, body, 0o644); err != nil { + t.Fatal(err) + } + err := RecordSemantic(entity, RecordInput{Decision: "approve", Actor: "person:captain", WorkflowDir: workflow}) + if err == nil || !strings.Contains(err.Error(), "invalid @review room-ref") { + t.Fatalf("gate close accepted malformed reserved room-ref: %v", err) + } + if got := mustReadBytes(t, entity); !bytes.Equal(got, body) { + t.Fatal("malformed reserved-ref refusal changed entity bytes") + } +} + +func TestReviewRoomRefsHaveOneMeaningAcrossEntityForms(t *testing.T) { + root := t.TempDir() + flat := filepath.Join(root, "task.md") + folder := filepath.Join(root, "task", "index.md") + room := filepath.Join(root, "task", "review", "validation", "briefing-1") + for _, entity := range []string{flat, folder} { + if got, err := canonicalReviewRoomRef(entity, room); err != nil || got != "@review/validation/briefing-1" { + t.Fatalf("canonical ref for %s = %q, %v", entity, got, err) + } + if got, err := ResolveRoomRef(entity, "@review/validation/briefing-1"); err != nil || got != room { + t.Fatalf("canonical resolution for %s = %q, %v", entity, got, err) + } + } + for _, tc := range []struct{ ref, want string }{ + {"./task/review/validation/briefing-1", room}, + {"opaque-provider:task", filepath.Join(root, "opaque-provider:task")}, + } { + if got, err := ResolveRoomRef(flat, tc.ref); err != nil || got != tc.want { + t.Fatalf("legacy %q = %q, %v; want %q", tc.ref, got, err, tc.want) + } + } + for _, ref := range []string{"@review", "@review/", "@review//absolute", "@review/.", "@review/a/../b", `@review/a\b`} { + if _, err := ResolveRoomRef(flat, ref); err == nil { + t.Fatalf("malformed reserved ref %q resolved", ref) + } + } +} + // TestRoomThatLostItsBriefingStaysAPreparedRoom pins why preparedRoomBinding // tests for the archived name and not the reserved one. A room whose Briefing is // deleted must keep failing loudly. The archived read path instead gives it the @@ -218,7 +269,7 @@ func TestRetainedTwoFileRoomKeepsItsFullValidation(t *testing.T) { t.Fatal(err) } binding := doc.Records[0].Attempts[0].Briefing - if binding.RequestDigest == "" || !preparedRoomBinding(entity, binding) { + if binding.RequestDigest == "" || !isPreparedRoomBinding(entity, binding) { t.Fatalf("retained room binding = %#v", binding) } if err := validateRetainedAuthority(entity, workflow, doc); err != nil { diff --git a/internal/gates/prepare_test.go b/internal/gates/prepare_test.go index b7ec06c45..d2978c3df 100644 --- a/internal/gates/prepare_test.go +++ b/internal/gates/prepare_test.go @@ -190,13 +190,10 @@ func TestPrepareCreatesOneFileRecorderRoomForFolderAndFlatEntities(t *testing.T) if attempt.Briefing.Digest != result.Digest || attempt.Briefing.RequestDigest != "" { t.Fatalf("binding pins incomplete or request-backed: %#v", attempt.Briefing) } - if !preparedRoomBinding(entity, attempt.Briefing) { + if !isPreparedRoomBinding(entity, attempt.Briefing) { t.Fatalf("published room is not read as prepared: %#v", attempt.Briefing) } - wantRef := "./task/review/validation/briefing-1" - if form == "folder" { - wantRef = "./review/validation/briefing-1" - } + wantRef := "@review/validation/briefing-1" if attempt.Briefing.RoomRef != wantRef { t.Fatalf("room-ref=%q want %q", attempt.Briefing.RoomRef, wantRef) } @@ -1540,10 +1537,7 @@ func TestPrepareRefusesFlatEntityOnlyWhereTheWorkflowDeclaresFolderForm(t *testi if err != nil { t.Fatal(err) } - // Flat form binds a slug-prefixed ref. It is correct while the entity - // stays flat and is exactly what a later conversion must rewrite; the - // validator warning carries that instruction. - if got := doc.Records[0].Attempts[0].Briefing.RoomRef; got != "./task/review/validation/briefing-1" { + if got := doc.Records[0].Attempts[0].Briefing.RoomRef; got != "@review/validation/briefing-1" { t.Fatalf("flat room-ref=%q", got) } }) diff --git a/internal/gates/round.go b/internal/gates/round.go index 9e0b848bb..8e0b11fdb 100644 --- a/internal/gates/round.go +++ b/internal/gates/round.go @@ -18,10 +18,10 @@ type loadedRound struct { Digest string } type roundLocation struct { - stage, room, entityID string - cycle int - entity []byte - pointer RoundPointer + stage, room, reviewHome, entityID string + cycle int + entity []byte + pointer RoundPointer } func resolveRound(entityPath, spec string) (roundLocation, error) { @@ -30,7 +30,7 @@ func resolveRound(entityPath, spec string) (roundLocation, error) { if !ok || !roundStageRE.MatchString(stage) || err != nil || cycle < 1 || strconv.Itoa(cycle) != rawCycle { return roundLocation{}, fmt.Errorf("--round must be a normalized STAGE/positive-cycle") } - result := roundLocation{stage: stage, cycle: cycle} + result := roundLocation{stage: stage, cycle: cycle, reviewHome: reviewHome(entityPath)} if result.entity, err = os.ReadFile(entityPath); err != nil { return result, err } @@ -46,11 +46,18 @@ func resolveRound(entityPath, spec string) (roundLocation, error) { if result.pointer.ID != "" && result.pointer.ID != fmt.Sprintf("round:%s:%s:%d", id.Value, result.pointer.Stage, result.pointer.Cycle) { return result, fmt.Errorf("review-round identity does not match the entity") } - result.room = filepath.Join(filepath.Dir(entityPath), "review", stage, fmt.Sprintf("round-%d", cycle)) - for _, parent := range []string{filepath.Dir(result.room), filepath.Dir(filepath.Dir(result.room))} { - if info, statErr := os.Lstat(parent); statErr == nil && info.Mode()&os.ModeSymlink != 0 { - return result, fmt.Errorf("derived round room crosses symlink %s", parent) + result.room = filepath.Join(result.reviewHome, "review", stage, fmt.Sprintf("round-%d", cycle)) + if result.pointer.ID != "" && result.pointer.Stage == stage && result.pointer.Cycle == cycle { + boundRoom, resolveErr := ResolveRoomRef(entityPath, result.pointer.Briefing.RoomRef) + if resolveErr != nil { + return result, resolveErr } + if filepath.Clean(boundRoom) != filepath.Clean(result.room) { + return result, fmt.Errorf("review-round pointer room does not resolve %s", spec) + } + } + if err := validatePreparedRoomAncestry(entityPath, result.room); err != nil { + return result, err } return result, nil } @@ -89,7 +96,7 @@ func recordRoundLockedWith(entityPath string, input RecordInput, beforePublish f if _, err := applicationForDecision(entityPath, input.WorkflowDir, location.stage, "revise"); err != nil { return err } - inputRound, err := loadValidateRound(filepath.Dir(entityPath), location.room, input.BriefingPath, input.LogPath, nil) + inputRound, err := loadValidateRound(location.reviewHome, location.room, input.BriefingPath, input.LogPath, nil) if err != nil { return err } @@ -99,7 +106,10 @@ func recordRoundLockedWith(entityPath string, input RecordInput, beforePublish f } pointer := RoundPointer{ID: fmt.Sprintf("round:%s:%s:%d", location.entityID, location.stage, location.cycle), Stage: location.stage, Cycle: location.cycle, Briefing: Briefing{ID: inputRound.Manifest.ID, Digest: inputRound.Digest, - RoomRef: fmt.Sprintf("./review/%s/round-%d", location.stage, location.cycle)}} + RoomRef: fmt.Sprintf("@review/%s/round-%d", location.stage, location.cycle)}} + if location.pointer.ID == pointer.ID { + pointer.Briefing.RoomRef = location.pointer.Briefing.RoomRef + } if _, statErr := os.Lstat(location.room); location.pointer.ID == pointer.ID && os.IsNotExist(statErr) { return fmt.Errorf("round identity already has a pointer without its immutable room") } @@ -107,6 +117,9 @@ func recordRoundLockedWith(entityPath string, input RecordInput, beforePublish f beforePublish(location.room) } return publishRound(location.room, roundRoomBytes{Exists: true, Briefing: inputRound.Briefing, Log: inputRound.Log}, func(replay bool) error { + if replay && location.pointer == pointer { + return nil + } return mutateEntity(entityPath, entityExpectation{Bytes: location.entity}, func(entity []byte) ([]byte, error) { next, err := rebuildRoundEntity(entity, pointer) if err == nil && replay && !bytes.Equal(next, entity) { @@ -158,7 +171,7 @@ func ValidateRoundFile(entityPath, spec string) (RoundSummary, error) { if _, err := readRoundRoom(location.room); err != nil { return RoundSummary{}, err } - loaded, err := loadValidateRound(filepath.Dir(entityPath), location.room, filepath.Join(location.room, "briefing.json"), filepath.Join(location.room, "briefing.review.jsonl"), &location.pointer.Briefing) + loaded, err := loadValidateRound(location.reviewHome, location.room, filepath.Join(location.room, "briefing.json"), filepath.Join(location.room, "briefing.review.jsonl"), &location.pointer.Briefing) if err != nil { return RoundSummary{}, err } @@ -187,9 +200,11 @@ func readRoundPointerData(data []byte) (RoundPointer, error) { return RoundPointer{}, fmt.Errorf("entity has invalid review-round pointer") } err = node.Decode(&pointer) - wantRoom := fmt.Sprintf("./review/%s/round-%d", pointer.Stage, pointer.Cycle) + canonicalRoom := fmt.Sprintf("@review/%s/round-%d", pointer.Stage, pointer.Cycle) + legacyRoom := fmt.Sprintf("./review/%s/round-%d", pointer.Stage, pointer.Cycle) if err != nil || pointer.ID == "" || !roundStageRE.MatchString(pointer.Stage) || pointer.Cycle < 1 || - pointer.Briefing.ID == "" || !digestRE.MatchString(pointer.Briefing.Digest) || pointer.Briefing.RoomRef != wantRoom { + pointer.Briefing.ID == "" || !digestRE.MatchString(pointer.Briefing.Digest) || + (pointer.Briefing.RoomRef != canonicalRoom && pointer.Briefing.RoomRef != legacyRoom) { return RoundPointer{}, fmt.Errorf("entity has invalid review-round pointer") } return pointer, nil diff --git a/internal/gates/round_test.go b/internal/gates/round_test.go index 168d6e888..6b32e622b 100644 --- a/internal/gates/round_test.go +++ b/internal/gates/round_test.go @@ -150,38 +150,132 @@ func TestRoundAcceptsWorkflowForeignLabelsAndActors(t *testing.T) { } } -func TestRoundRequiresFolderFormWithoutCrossEntityCollision(t *testing.T) { - workflow := t.TempDir() - for _, slug := range []string{"task-a", "task-b"} { - entity := filepath.Join(workflow, slug+".md") - if err := os.WriteFile(entity, []byte("---\nid: "+slug+"\nstatus: implementation\n---\n# Task\n"), 0o644); err != nil { +func TestRoundPublishesFlatAndFolderThroughSharedReviewHome(t *testing.T) { + for _, form := range []string{"folder", "flat"} { + t.Run(form, func(t *testing.T) { + var root, entity, briefing, log string + if form == "folder" { + root, entity, briefing, log, _ = advisoryRoundFixture(t) + } else { + root, entity, briefing, log, _ = flatAdvisoryRoundFixture(t) + } + before := lifecycleBytes(t, entity) + if err := RecordSemantic(entity, inputForRound(briefing, log)); err != nil { + t.Fatal(err) + } + pointer, err := readRoundPointerData(mustReadBytes(t, entity)) + if err != nil || pointer.Briefing.RoomRef != "@review/implementation/round-1" { + t.Fatalf("pointer=%#v err=%v", pointer, err) + } + if !bytes.Equal(before, lifecycleBytes(t, entity)) { + t.Fatal("round changed status or gates") + } + if _, err := os.Stat(filepath.Join(root, "review", "implementation", "round-1", "briefing.json")); err != nil { + t.Fatal(err) + } + if _, err := ValidateRoundFile(entity, "implementation/1"); err != nil { + t.Fatal(err) + } + }) + } + + t.Run("declared folder policy refuses a new flat home and grandfathers an existing one", func(t *testing.T) { + root, entity, briefing, log, _ := flatAdvisoryRoundFixture(t) + state := filepath.Dir(root) + readme := "---\nentity-form: folder\nstages:\n states:\n - name: implementation\n---\n# Workflow\n" + if err := os.WriteFile(filepath.Join(state, "README.md"), []byte(readme), 0o644); err != nil { t.Fatal(err) } + before := treeDigest(t, state) + if err := RecordSemantic(entity, inputForRound(briefing, log)); err == nil || treeDigest(t, state) != before { + t.Fatalf("declared-folder refusal err=%v or changed bytes", err) + } + if err := os.MkdirAll(filepath.Join(root, "review", "retained"), 0o755); err != nil { + t.Fatal(err) + } + if err := RecordSemantic(entity, inputForRound(briefing, log)); err != nil { + t.Fatalf("grandfathered flat round: %v", err) + } + }) +} + +func TestRoundArtifactBoundaryIsByteCleanForFlatAndFolder(t *testing.T) { + for _, form := range []string{"folder", "flat"} { + for _, row := range []string{"outside home", "mutable entity"} { + t.Run(form+"/"+row, func(t *testing.T) { + root, entity, briefing, log := roundFixtureForForm(t, form) + target, uri := filepath.Join(filepath.Dir(root), "sibling.txt"), "../../../../sibling.txt" + if row == "mutable entity" { + target, uri = entity, "../../../index.md" + if form == "flat" { + uri = "../../../../task.md" + } + } else if err := os.WriteFile(target, []byte("sibling\n"), 0o644); err != nil { + t.Fatal(err) + } + body := bytes.Replace(mustReadBytes(t, briefing), []byte("../../../candidate.patch"), []byte(uri), 1) + body = bytes.Replace(body, []byte("sha256:8e85d4c9523a617e05b17c92390b10b2f9892152ca348433311230ac3ad98dd3"), []byte(RawDigest(mustReadBytes(t, target))), 1) + if err := os.WriteFile(briefing, body, 0o644); err != nil { + t.Fatal(err) + } + state := filepath.Dir(root) + before := treeDigest(t, state) + if err := RecordSemantic(entity, inputForRound(briefing, log)); err == nil || treeDigest(t, state) != before { + t.Fatalf("boundary refusal err=%v or changed bytes", err) + } + }) + } } - if err := os.WriteFile(filepath.Join(workflow, "unrelated"), []byte("preserve"), 0o644); err != nil { +} + +func TestRoundFrozenLegacyFolderPointerReplaysUnchanged(t *testing.T) { + _, entity, briefing, log, _ := advisoryRoundFixture(t) + input := inputForRound(briefing, log) + if err := RecordSemantic(entity, input); err != nil { t.Fatal(err) } - inputRoot := filepath.Join(workflow, "inputs") - if err := os.MkdirAll(inputRoot, 0o755); err != nil { + body := bytes.Replace(mustReadBytes(t, entity), []byte("@review/implementation/round-1"), []byte("./review/implementation/round-1"), 1) + if err := os.WriteFile(entity, body, 0o644); err != nil { t.Fatal(err) } - briefing := filepath.Join(inputRoot, "briefing.json") - log := filepath.Join(inputRoot, "briefing.review.jsonl") - copyRoundFixture(t, briefing, "briefing.json") - copyRoundFixture(t, log, "briefing.review.jsonl") - for _, slug := range []string{"task-a", "task-b"} { - before := treeDigest(t, workflow) - entity := filepath.Join(workflow, slug+".md") - err := RecordSemantic(entity, inputForRound(briefing, log)) - if err == nil || !strings.Contains(err.Error(), "folder-form entity") || treeDigest(t, workflow) != before { - t.Fatalf("%s flat refusal error=%v or changed workflow bytes", slug, err) - } - if _, statErr := os.Stat(entity + ".gates.lock"); !os.IsNotExist(statErr) { - t.Fatalf("%s flat refusal left a lock: %v", slug, statErr) - } + if _, err := ValidateRoundFile(entity, input.Round); err != nil { + t.Fatalf("legacy pointer validation: %v", err) } - if _, err := os.Stat(filepath.Join(workflow, "review")); !os.IsNotExist(err) { - t.Fatalf("flat refusals created a shared review room: %v", err) + if err := RecordSemantic(entity, input); err != nil { + t.Fatalf("legacy pointer exact replay: %v", err) + } + if got := mustReadBytes(t, entity); !bytes.Equal(got, body) { + t.Fatal("exact replay rewrote the frozen legacy room-ref") + } +} + +func TestRoundReplayRefusesSiblingAndMutableEntityArtifacts(t *testing.T) { + for _, form := range []string{"folder", "flat"} { + t.Run(form, func(t *testing.T) { + root, entity, briefing, log := roundFixtureForForm(t, form) + if err := RecordSemantic(entity, inputForRound(briefing, log)); err != nil { + t.Fatal(err) + } + candidate := filepath.Join(root, "candidate.patch") + sibling := filepath.Join(filepath.Dir(root), "sibling.patch") + copyRoundFixture(t, sibling, "candidate.patch") + for _, target := range []string{sibling, entity} { + if err := os.Remove(candidate); err != nil { + t.Fatal(err) + } + if err := os.Symlink(target, candidate); err != nil { + t.Fatal(err) + } + before := treeDigest(t, filepath.Dir(root)) + if _, err := ValidateRoundFile(entity, "implementation/1"); err == nil || treeDigest(t, filepath.Dir(root)) != before { + t.Fatalf("replay target %s was accepted or changed bytes", target) + } + if err := os.Remove(candidate); err != nil { + t.Fatal(err) + } + copyRoundFixture(t, candidate, "candidate.patch") + } + }) } } @@ -497,6 +591,28 @@ func advisoryRoundFixture(t *testing.T) (root, entity, briefing, log, feedback s return advisoryRoundFixtureAt(t, filepath.Join(t.TempDir(), "task")) } +func flatAdvisoryRoundFixture(t *testing.T) (root, entity, briefing, log, feedback string) { + state := t.TempDir() + root, folderEntity, briefing, log, feedback := advisoryRoundFixtureAt(t, filepath.Join(state, "task")) + if err := os.Rename(filepath.Join(root, "README.md"), filepath.Join(state, "README.md")); err != nil { + t.Fatal(err) + } + entity = filepath.Join(state, "task.md") + if err := os.Rename(folderEntity, entity); err != nil { + t.Fatal(err) + } + return root, entity, briefing, log, feedback +} + +func roundFixtureForForm(t *testing.T, form string) (root, entity, briefing, log string) { + if form == "flat" { + root, entity, briefing, log, _ = flatAdvisoryRoundFixture(t) + return + } + root, entity, briefing, log, _ = advisoryRoundFixture(t) + return +} + func advisoryRoundFixtureAt(t *testing.T, root string) (string, string, string, string, string) { t.Helper() if err := os.MkdirAll(filepath.Join(root, "inputs"), 0o755); err != nil { diff --git a/internal/status/hybrid_flat_rooms_warn_test.go b/internal/status/hybrid_flat_rooms_warn_test.go index a661165f8..996b7086e 100644 --- a/internal/status/hybrid_flat_rooms_warn_test.go +++ b/internal/status/hybrid_flat_rooms_warn_test.go @@ -1,5 +1,5 @@ -// ABOUTME: --validate warns on a flat entity holding gate rooms, carrying the -// ABOUTME: conversion remedy, while the plain status read path stays unaffected. +// ABOUTME: --validate resolves canonical flat review rooms without prescribing +// ABOUTME: migration, while frozen legacy refs retain their old meaning. package status import ( @@ -23,11 +23,15 @@ func hybridFlatRoomsFixture(t *testing.T) string { return "---\nid: " + slug + "\nstatus: ideation\ntitle: Task\ngates:\n version: 1\n records:\n - id: gate:" + slug + ":ideation\n stage: ideation\n attempts:\n - id: attempt:" + slug + ":ideation\n briefing: {id: briefing:" + slug + ":ideation:attempt-1:revision-1, digest: sha256:" + - strings.Repeat("1", 64) + ", room-ref: " + ref + "}\n---\n# Task\n" + strings.Repeat("1", 64) + ", room-ref: '" + ref + "'}\n---\n# Task\n" } for _, slug := range []string{"alpha", "beta"} { + ref := "./" + slug + "/review/ideation/briefing-1" + if slug == "alpha" { + ref = "@review/ideation/briefing-1" + } if err := os.WriteFile(filepath.Join(root, slug+".md"), - []byte(body(slug, "./"+slug+"/review/ideation/briefing-1")), 0o644); err != nil { + []byte(body(slug, ref)), 0o644); err != nil { t.Fatal(err) } if err := os.MkdirAll(filepath.Join(root, slug, "review", "ideation", "briefing-1"), 0o755); err != nil { @@ -45,29 +49,17 @@ func hybridFlatRoomsFixture(t *testing.T) string { return root } -// The remedy is the payload: a reader who only sees "this is a hybrid" still -// hand-converts and destroys every retained room, so the line must name the move -// AND the ref rewrite. Asserting both tokens fails if either half is dropped. -func TestValidateWarnsFlatEntityHoldingGateRoomsWithConversionRemedy(t *testing.T) { +func TestValidateResolvesCanonicalFlatRoomWithoutConversionWarning(t *testing.T) { root := hybridFlatRoomsFixture(t) out, stderr, code := runNative(t, root, pinnedEnv(t), "--workflow-dir", root, "--validate") if code != 0 || strings.TrimSpace(out) != "VALID" { t.Fatalf("--validate exit=%d stdout=%q stderr=%q", code, out, stderr) } - if got := strings.Count(stderr, "flat entity holds gate rooms"); got != 2 { - t.Fatalf("want 2 hybrid warnings, got %d: %q", got, stderr) - } - for _, want := range []string{ - "slug=alpha", "slug=beta", - "git mv alpha.md alpha/index.md", "rewrite every `room-ref: ./alpha/` to `room-ref: ./`", - } { - if !strings.Contains(stderr, want) { - t.Fatalf("hybrid warning missing %q: %q", want, stderr) + for _, unwanted := range []string{"flat entity holds gate rooms", "does not resolve", "git mv", "rewrite every"} { + if strings.Contains(stderr, unwanted) { + t.Fatalf("canonical/legacy room emitted obsolete warning %q: %q", unwanted, stderr) } } - if strings.Contains(stderr, "slug=gamma") { - t.Fatalf("folder-form entity flagged: %q", stderr) - } } // The finding must not live in findEntityFormConflicts: that runs on the read @@ -95,27 +87,25 @@ func TestHybridFindingLeavesPlainStatusReadPathUnaffected(t *testing.T) { // instead of a mid-ceremony gate failure. func TestValidateReportsRetainedRoomThatNoLongerResolves(t *testing.T) { root := hybridFlatRoomsFixture(t) - // The hand conversion, ref rewrite forgotten: alpha.md -> alpha/index.md - // leaves ./alpha/review/... resolving at alpha/alpha/review/... - body, err := os.ReadFile(filepath.Join(root, "alpha.md")) + // The hand conversion, ref rewrite forgotten: beta.md -> beta/index.md + // leaves the frozen ./beta/review/... resolving one level too deep. + body, err := os.ReadFile(filepath.Join(root, "beta.md")) if err != nil { t.Fatal(err) } - if err := os.WriteFile(filepath.Join(root, "alpha", "index.md"), body, 0o644); err != nil { + if err := os.WriteFile(filepath.Join(root, "beta", "index.md"), body, 0o644); err != nil { t.Fatal(err) } - if err := os.Remove(filepath.Join(root, "alpha.md")); err != nil { + if err := os.Remove(filepath.Join(root, "beta.md")); err != nil { t.Fatal(err) } _, stderr, code := runNative(t, root, pinnedEnv(t), "--workflow-dir", root, "--validate") if code != 0 { t.Fatalf("--validate exit=%d stderr=%q", code, stderr) } - if !strings.Contains(stderr, "retained gate room does not resolve: ./alpha/review/ideation/briefing-1") { + if !strings.Contains(stderr, "retained gate room does not resolve: ./beta/review/ideation/briefing-1") { t.Fatalf("botched conversion not reported: %q", stderr) } - // beta is untouched and its rooms resolve; gamma is folder-form and clean. - // Exactly one unresolved-ref finding, and it is alpha's. if got := strings.Count(stderr, "does not resolve"); got != 1 { t.Fatalf("want exactly 1 unresolved-ref finding, got %d: %q", got, stderr) } diff --git a/internal/status/validate.go b/internal/status/validate.go index 2daa27f4b..e7fa4d92a 100644 --- a/internal/status/validate.go +++ b/internal/status/validate.go @@ -245,29 +245,12 @@ func gateValidationDiagnostics(entities []*entity, workflowDir string) (errs, wa problem := fmt.Sprintf("unknown gate application field '%s' at %s", warning.Field, warning.Path) warns = append(warns, entityEvidenceLine("Warning", e, workflowDir, problem, e.displayID)) } - // A retained room that no longer resolves is the #739 end state: the gate - // commands fail on it mid-ceremony while every read surface still reports - // the entity as healthy. Reporting it here is what makes a hand - // conversion verifiable — the operator can confirm the rewrite landed - // instead of finding out at the next gate. + // Retained rooms use the same dual-mode resolver as gate commands, so new + // review-home refs and frozen entity-relative refs receive one diagnostic. for _, ref := range unresolvedRoomRefs(e.path) { warns = append(warns, entityEvidenceLine("Warning", e, workflowDir, "retained gate room does not resolve: "+ref, e.displayID)) } - // A flat entity that already holds prepared rooms is grandfathered by - // gate prepare: its refs are .//review/... and correct while it - // stays flat. Moving it to folder form without rewriting them in the - // same commit makes every retained room unreadable, and nothing else - // reports that — so the warning carries the whole remedy, not just the - // finding. Warn tier, and only on explicit --validate: an error here - // would exit 1 on the plain status read path. - if filepath.Base(e.path) != "index.md" { - if _, err := os.Stat(filepath.Join(filepath.Dir(e.path), e.slug, "review")); err == nil { - warns = append(warns, entityEvidenceLine("Warning", e, workflowDir, fmt.Sprintf( - "flat entity holds gate rooms in %s/review/; to convert it, `git mv %s.md %s/index.md` AND rewrite every `room-ref: ./%s/` to `room-ref: ./` in the same commit, or every retained room becomes unreadable", - e.slug, e.slug, e.slug, e.slug), e.displayID)) - } - } } return errs, warns } @@ -375,7 +358,12 @@ func unresolvedRoomRefs(entityPath string) []string { if ref == "" { continue } - if _, err := os.Stat(filepath.Join(filepath.Dir(entityPath), filepath.FromSlash(ref))); err != nil { + room, resolveErr := gates.ResolveRoomRef(entityPath, ref) + if resolveErr != nil { + missing = append(missing, ref) + continue + } + if _, err := os.Stat(room); err != nil { missing = append(missing, ref) } }