From f1b44507bf9dad6135a3d0ea92f7b3a35a2c91bd Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:57:22 +0100 Subject: [PATCH 1/5] chore: capture the record-moving verbs that strand links to the moved record capture resolve, capture wontfix and intent plan rename a record between status folders and leave every relative link that named its old path dead, the gap spec close has too. Captured before the fix that closes it. Refs: iss-2609250846525896 Refs: iss-2609091732329046 Assisted-by: Claude:claude-opus-5-5 --- ...c-close-leave-links-to-the-moved-record-dead.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 .abcd/work/issues/open/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md diff --git a/.abcd/work/issues/open/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md b/.abcd/work/issues/open/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md new file mode 100644 index 000000000..a7fe52073 --- /dev/null +++ b/.abcd/work/issues/open/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md @@ -0,0 +1,14 @@ +--- +schema_version: 1 +id: "iss-2609250846525896" +slug: "record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead" +severity: "minor" +category: "bug" +source: "agent-finding" +found_during: "autonomous run A resumed 2026-09-25" +origin: researcher-authored +production_mode: hand-written +found_at: "internal/core/capture/workflow.go" +--- + +capture resolve, capture wontfix and intent plan move a record between status folders and leave every relative markdown link that named its old path pointing at nothing, the same gap iss-2609091732329046 records for spec close. Lane records1 of run A closed two issues that three ADRs and two draft intents linked by path, and record-lint refused seven links_resolve blockers until the links were repointed by hand. Every verb that moves a record is the one place that knows the old and the new path, so each should repoint the links through one shared primitive and report what it rewrote. From 2a6d5b637534fc50e7aa3454fada25e4519801d5 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:57:55 +0100 Subject: [PATCH 2/5] fix(record): repoint every link that named a record a verb moves A record's folder is its status, so spec close (open/ -> closed/, and planned/ -> shipped/ on the close that ships), intent plan (drafts/ -> planned/), capture resolve and capture wontfix are all renames, and each left every relative link that named the old path dead. record-lint's links_resolve then refused the tree the next command ran against, and every intent lane of run A repointed links by hand. core/relink is the one link-repoint primitive the four verbs share. It takes the moves a verb made, walks the working tree's markdown and rewrites every inline link and link-reference definition that resolved to an old path, including a moved record's own links written from the folder it left, and reports each rewrite (file, line, from, to). Three link classes, one rule: a spec already closed that names ../open/, ADRs, plans and drafts naming an intent's planned/ path, and the closing spec's bare links to siblings still in open/. A link that never resolved stays as written. The walk stays out of .git, the local tier, nested checkouts and the two append-only logs whose gates refuse an in-place edit (DECISIONS.md, DA002; reviews/, RD002). spec close derives its moves from where the records are now, so a re-run finishes a repoint an earlier attempt left. A repoint failure is a warning on stderr, not a failed verb: the move stands. The results carry `relinked` in --json and the text renders list each rewrite. The plugin pages and the intent and capture surface chapters say so. Refs: iss-2608311127491949 Refs: iss-2609091732329046 Refs: iss-2609250846525896 Assisted-by: Claude:claude-opus-5-5 --- .../brief/04-surfaces/05-intent.md | 1 + .../brief/04-surfaces/06-capture.md | 10 + commands/capture.md | 9 + commands/intent.md | 19 +- internal/README.md | 9 + internal/core/capture/capture.go | 8 + internal/core/capture/relink_test.go | 88 +++++ internal/core/capture/workflow.go | 22 ++ internal/core/intent/intent.go | 14 + internal/core/intent/lifecycle.go | 44 ++- internal/core/intent/relink_test.go | 181 ++++++++++ internal/core/relink/relink.go | 341 ++++++++++++++++++ internal/core/relink/relink_test.go | 169 +++++++++ internal/surface/cli/cli.go | 8 + internal/surface/cli/relink.go | 35 ++ internal/surface/cli/relink_cli_test.go | 82 +++++ 16 files changed, 1038 insertions(+), 2 deletions(-) create mode 100644 internal/core/capture/relink_test.go create mode 100644 internal/core/intent/relink_test.go create mode 100644 internal/core/relink/relink.go create mode 100644 internal/core/relink/relink_test.go create mode 100644 internal/surface/cli/relink.go create mode 100644 internal/surface/cli/relink_cli_test.go diff --git a/.abcd/development/brief/04-surfaces/05-intent.md b/.abcd/development/brief/04-surfaces/05-intent.md index b257c2109..8fdc0ecfd 100644 --- a/.abcd/development/brief/04-surfaces/05-intent.md +++ b/.abcd/development/brief/04-surfaces/05-intent.md @@ -451,6 +451,7 @@ The invariants below are the contract the tree is held to, and each names what h - Every intent in `drafts/` has `spec_id: null` (drafts have no plan yet). - Every intent in `planned/` has `spec_id: null` (unscheduled) or a `spc-N` id; a non-null `spec_id` points to an existing native-spec-store `-*.md` whose frontmatter `intent` field matches the intent's `id` (or contains the intent's `id` as one of a list, for bundle-member intents). - **An intent owns one or more specs, and it ships when its last spec closes.** The intent↔spec relation is 1:n (invariant 17 in [`02-constraints/03-invariants.md`](../02-constraints/03-invariants.md), per [adr-2609151513118583](../../decisions/adrs/2609151513118583-an-intent-owns-one-or-more-specs-and-it-ships-when-its-last.md)). The spec's own `intent:` field is the source of truth for the link: the intent's scalar `spec_id` names the spec it was planned with, and the set of specs realising an intent is derived from the back-links (`spec.Store.SpecsForIntent`, `lint.SpecLinkIndex.SpecsForIntent`) — no field carries a list. The bidirectional check is therefore membership, not equality: a spec naming an intent is clean when that intent's `spec_id` names *some* spec realising it (`spec_lifecycle`), so a remainder spec is not drift. Closing a spec ships the intent only when no open spec is left naming it; a remainder slug given on the close mints the follow-on spec in the same operation, and the impact is demanded at the close that ships and refused at any earlier one. The release cut's stale-intent refusal asks whether a planned intent has any OPEN spec, never whether its spec has closed — a planned intent with one closed and one open spec is the correct steady state of a partial delivery. +- **A move repoints the links that named the moved record.** An intent's and a spec's folder is its status, so planning (`drafts/ → planned/`) and closing (`open/ → closed/`, and on the close that ships `planned/ → shipped/`) are renames, and the verb that renames is the one place that knows both paths. It rewrites every relative markdown link in the tree that named an old path, from any folder — a spec already closed pointing at `../open/`, an ADR or a plan naming the intent's `planned/` path, a draft naming both, and the moved record's own links, written from the folder it left — through the one link-repoint primitive (`core/relink`) the ledger's resolve and wontfix share. A link that never resolved is left as written. The result lists each rewrite (`relinked`), so the close leaves a tree record-lint's `links_resolve` accepts, with no hand survey. The close derives its moves from where the records are now, so a re-run completes a repoint an earlier attempt left unfinished; a repoint failure is a warning, never a failed close. - **A bundle is the opposite relation and is untouched.** `kind: bundle-member` with a `bundle:` link is N:1 — several intents sharing one spec — and the bundle invariant above (all members in one phase) still holds. 1:n and N:1 are different relations, not two names for one thing; composing them into N:M is not authorised by anything in the record. An intent's own specs may sit in different phases, because the reason a second spec exists is that the work did not fit the cycle that carried the first. - Every intent in `shipped/` has `kind` set (`standalone` or `bundle-member`) and a non-null `spec_id`. (The stronger invariant — the linked spec exists and is closed, or `spec_id: null` + a `manual_ship_reason` for the no-spec case — is a later-phase gate; the shipped rule checks only that `spec_id` is non-null.) - Discipline-kind intents have `spec_id: null` always (disciplines never get a spec; this is structurally enforced). diff --git a/.abcd/development/brief/04-surfaces/06-capture.md b/.abcd/development/brief/04-surfaces/06-capture.md index b8054dbf1..2295b658c 100644 --- a/.abcd/development/brief/04-surfaces/06-capture.md +++ b/.abcd/development/brief/04-surfaces/06-capture.md @@ -163,6 +163,16 @@ stays out of the current cut. the issue to `wontfix/`. Grounds are optional here and override the recorded text only: the token stays `declined`, because a wontfix **is** that non-action. +**Both moves repoint the links that named the issue.** Resolving and marking +wontfix each rename the record out of `open/`, and in the same operation every +relative markdown link in the tree that named its old path is rewritten to the +new one — from a decision, a draft intent or a sibling issue, and the moved +issue's own links, written from `open/` — through the one link-repoint +primitive every record-moving verb shares (`core/relink`). A link that never +resolved is left as written. Each rewrite is reported (file, line, the +destination before and after), and a repoint that fails part-way is a warning, +not a failure: the transition stands. + ## 2. Which ledger a verb addresses Every verb addresses the checkout's ledger, whichever directory of the working diff --git a/commands/capture.md b/commands/capture.md index 71cb2e2d4..72cbca78c 100644 --- a/commands/capture.md +++ b/commands/capture.md @@ -282,6 +282,15 @@ whenever it is non-zero: these paths redact the note exactly as `capture` does, but their human render stays silent, so the caller learns their wording was rewritten only if you relay it. +Moving the issue repoints every relative markdown link in the tree that named +it in `open/` — an ADR, a draft intent, a sibling issue — and the moved issue's +own links, which were written from `open/`. The JSON lists each rewrite under +`relinked` (`file`, `line`, `from`, `to`) and the text render prints them; +report them, because they are files the verb changed beyond the issue. A link +that never resolved is left as written. A repoint that fails part-way leaves +the transition standing and warns on stderr; record-lint's `links_resolve` +then names each link left behind. + An id this checkout's ledger does not hold is refused. When a peer holds it — a sibling worktree or a local branch (see `/abcd:peers`) — the refusal names the peer's branch, path and folder instead of answering not found: the record diff --git a/commands/intent.md b/commands/intent.md index 271d5f79b..7b2d930e9 100644 --- a/commands/intent.md +++ b/commands/intent.md @@ -343,7 +343,10 @@ gate that will refuse the move mechanically is a recorded seed until built. This invocation IS the maintainer's sign-off act — never run it unattended or infer consent. It mints the spec stub, links both sides, stamps an identity onto every unmarked scope condition, and moves the intent - `drafts/ → planned/`. + `drafts/ → planned/`. Every relative markdown link that named the draft's + path, from any file in the tree, is repointed at `planned/` in the same + operation; the JSON lists each rewrite under `relinked` (`file`, `line`, + `from`, `to`) — report them. **`--impact` is the judgement the interview settled**, stamped here because this is the moment it is made: a draft filed without one gets it now, in the @@ -391,6 +394,20 @@ nothing refuses `--impact`, because that judgement is written only at the close that ships (adr-2609151513118583, invariant 17). Report the specs the close names as still open — they are the reason the intent did not move. +**The close repoints every link that named a record it moved.** A record's +folder is its status, so the close renames two files — the spec out of +`open/`, and on the close that ships, the intent out of `planned/` — and in the +same operation it rewrites every relative markdown link in the tree that named +either old path: a spec already closed that pointed at `../open/`, an +ADR or a plan naming the intent's `planned/` path, a draft naming both, and the +closed spec's own links, which were written from `open/`. A link that never +resolved is left as written. The JSON lists each rewrite under `relinked` +(`file`, `line`, `from`, `to`), and the text render prints them; report them, +because they are files the close changed beyond the two records. The tree the +close leaves passes record-lint's `links_resolve` with no hand repair. If the +repoint fails part-way, the close still stands and a warning on stderr says so; +re-running the same `spec close` finishes the repoint. + Run it in the **same change** that lands the intent's work — the commit or pull request that makes the acceptance criteria true — the way a captured issue is resolved in the change that fixes it. The reason is the release cut: diff --git a/internal/README.md b/internal/README.md index 432d08de9..0588f37a6 100644 --- a/internal/README.md +++ b/internal/README.md @@ -46,6 +46,15 @@ plugin surface, and a future MCP server share one engine. reader spelled twice is one the two can disagree about, which is how a bullet one writer appends becomes a bullet the other cannot find. It owns no heading's meaning: a caller supplies the pattern it is looking for. +- **`core/relink/`** — the one link-repoint primitive. A record's folder is its + status, so every lifecycle transition is a rename, and a rename strands every + relative link that named the file where it was. The verbs that move a record + — `spec close` and `intent plan` (`core/intent`), `capture resolve` and + `capture wontfix` (`core/capture`) — hand it the moves they made, and it + rewrites every markdown link in the working tree that named an old path and + reports each rewrite. A leaf on the `core/mdrecord` precedent: three record + families move, and a repoint spelled per family is one that misses a link + class the others catch. - **`core/provenance/`** — the record's disclosure vocabulary: where an item came from (`origin`) and how its text was produced (`production_mode`), plus the one parser that reads and renders them. It is a leaf for the same reason diff --git a/internal/core/capture/capture.go b/internal/core/capture/capture.go index 906bf37a0..79be7e777 100644 --- a/internal/core/capture/capture.go +++ b/internal/core/capture/capture.go @@ -20,6 +20,7 @@ import ( "github.com/intentdriven/abcd/internal/core/issueschema" "github.com/intentdriven/abcd/internal/core/recordid" + "github.com/intentdriven/abcd/internal/core/relink" ) // LedgerRelPath is the ledger root relative to the repo worktree. @@ -247,6 +248,13 @@ type TransitionResult struct { // same redactor and reports the same way. Redacted int `json:"redacted,omitempty"` Degraded string `json:"redaction_degraded,omitempty"` + // Relinked lists every relative markdown link the transition repointed + // because it named the issue's old path in open/ (iss-2609250846525896). + Relinked []relink.Rewrite `json:"relinked,omitempty"` + // RelinkError is a NON-FATAL report of a repoint that failed part-way: the + // issue has moved and the transition stands, so the surface prints it + // loudly, and record-lint's links_resolve names any link left behind. + RelinkError string `json:"relink_error,omitempty"` } // ListRequest queries one state (or "all"). diff --git a/internal/core/capture/relink_test.go b/internal/core/capture/relink_test.go new file mode 100644 index 000000000..aacea360b --- /dev/null +++ b/internal/core/capture/relink_test.go @@ -0,0 +1,88 @@ +package capture + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/core/lint" +) + +// Resolving or declining an issue moves it out of open/, and every link that +// named it there follows it — from an ADR, from a sibling issue still in open/, +// and the moved issue's own bare link to that sibling (iss-2609250846525896). +func TestTransitionRepointsLinksToTheMovedIssue(t *testing.T) { + for _, tc := range []struct { + name string + folder string + move func(repo, ir, id string) (TransitionResult, error) + }{ + {"resolve", "resolved", func(repo, ir, id string) (TransitionResult, error) { + return Resolve(ResolveRequest{Grounds: testGrounds, RepoRoot: repo, IssuesRoot: ir, ID: id, Resolution: "fixed", Impact: "fix"}) + }}, + {"wontfix", "wontfix", func(repo, ir, id string) (TransitionResult, error) { + return Wontfix(WontfixRequest{RepoRoot: repo, IssuesRoot: ir, ID: id, Reason: "declined because the cost exceeds the benefit here"}) + }}, + } { + t.Run(tc.name, func(t *testing.T) { + repo, ir := ledger(t) + mk := func(slug string) string { + res, err := Capture(CaptureRequest{RepoRoot: repo, IssuesRoot: ir, Text: "b", Severity: SeverityMinor, + Category: "bug", Source: "user-observation", FoundDuring: "t", Slug: slug}) + if err != nil { + t.Fatal(err) + } + return filepath.Base(res.Path) + } + a, b := mk("alpha"), mk("beta") + appendTo := func(rel, line string) { + f, err := os.OpenFile(filepath.Join(repo, rel), os.O_APPEND|os.O_WRONLY|os.O_CREATE, 0o644) + if err != nil { + t.Fatal(err) + } + defer f.Close() + if _, err := f.WriteString(line); err != nil { + t.Fatal(err) + } + } + if err := os.MkdirAll(filepath.Join(repo, ".abcd/development/decisions/adrs"), 0o755); err != nil { + t.Fatal(err) + } + adr := ".abcd/development/decisions/adrs/0001-x.md" + appendTo(adr, "# x\n\nFound in [alpha](../../../work/issues/open/"+a+").\n") + appendTo(LedgerRelPath+"/open/"+a, "\nSee [beta]("+b+").\n") + appendTo(LedgerRelPath+"/open/"+b, "\nSee [alpha]("+a+").\n") + + id := strings.SplitN(a, "-", 3)[0] + "-" + strings.SplitN(a, "-", 3)[1] + res, err := tc.move(repo, ir, id) + if err != nil { + t.Fatal(err) + } + + cfg := lint.Config{ + Roots: []string{".abcd/development", ".abcd/work"}, + Rules: map[string]lint.RuleConfig{"links_resolve": {Enabled: true, Severity: "blocker"}}, + } + findings, err := lint.Lint(cfg, repo) + if err != nil { + t.Fatal(err) + } + for _, f := range findings { + if f.RuleID == "links_resolve" { + t.Errorf("links_resolve after %s: %s:%d %s", tc.name, f.File, f.Line, f.Message) + } + } + if len(res.Relinked) != 3 || res.RelinkError != "" { + t.Errorf("the transition must report its three rewrites: %+v %q", res.Relinked, res.RelinkError) + } + body, err := os.ReadFile(filepath.Join(repo, adr)) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(body), "(../../../work/issues/"+tc.folder+"/"+a+")") { + t.Errorf("ADR link not repointed:\n%s", body) + } + }) + } +} diff --git a/internal/core/capture/workflow.go b/internal/core/capture/workflow.go index d7337dc41..46165aa4b 100644 --- a/internal/core/capture/workflow.go +++ b/internal/core/capture/workflow.go @@ -12,6 +12,7 @@ import ( "github.com/intentdriven/abcd/internal/core/changelog" "github.com/intentdriven/abcd/internal/core/grounds" "github.com/intentdriven/abcd/internal/core/provenance" + "github.com/intentdriven/abcd/internal/core/relink" "github.com/intentdriven/abcd/internal/fsutil" ) @@ -523,6 +524,10 @@ func transition(repoRoot, issuesRoot, issID, verb, field, note string, extra []k } result = TransitionResult{ID: issID, Path: dst, FromStatus: StateOpen, ToStatus: target, Redacted: redacted, Degraded: degraded} + // Repoint every link that named the issue in open/, still under the + // ledger lock because the links it rewrites include other issues'. A + // failure is reported, not raised: the issue has moved. + result.Relinked, result.RelinkError = repointMovedIssue(rr, src, dst) return nil }) if err != nil { @@ -534,6 +539,23 @@ func transition(repoRoot, issuesRoot, issID, verb, field, note string, extra []k return result, nil } +// repointMovedIssue repoints the links that named an issue's path before a +// transition moved it, through the one primitive every record-moving verb +// shares. A ledger outside the repository (a custom issues root) is linked from +// nowhere the repository's links can reach, so there is nothing to repoint. +func repointMovedIssue(repoRoot, src, dst string) ([]relink.Rewrite, string) { + from, err1 := filepath.Rel(repoRoot, src) + to, err2 := filepath.Rel(repoRoot, dst) + if err1 != nil || err2 != nil || !filepath.IsLocal(from) || !filepath.IsLocal(to) { + return nil, "" + } + rw, err := relink.Repoint(repoRoot, []relink.Move{{From: from, To: to}}) + if err != nil { + return rw, err.Error() + } + return rw, "" +} + // removeSourceHook, when non-nil, replaces os.Remove(src) inside // commitTransition. It is a test-only seam (nil in production, zero overhead) // used to force a deterministic non-ENOENT remove failure without relying on diff --git a/internal/core/intent/intent.go b/internal/core/intent/intent.go index cc75f3b28..45219551d 100644 --- a/internal/core/intent/intent.go +++ b/internal/core/intent/intent.go @@ -25,6 +25,7 @@ import ( "strings" "github.com/intentdriven/abcd/internal/core/recordid" + "github.com/intentdriven/abcd/internal/core/relink" "github.com/intentdriven/abcd/internal/core/spec" ) @@ -247,6 +248,10 @@ type PlanResult struct { // empty when it wrote none — because no --impact was supplied, or because the // record already carried the same value. ImpactStamped string `json:"impact_stamped"` + // Relinked and RelinkError report the repoint of links that named the + // draft's old path, as ReconcileResult's do for a close. + Relinked []relink.Rewrite `json:"relinked,omitempty"` + RelinkError string `json:"relink_error,omitempty"` } // LinkResult reports a completed Link: the updated intent and the spec it now @@ -294,6 +299,15 @@ type ReconcileResult struct { // AuditEmitError is a NON-FATAL report of a failed review emit. The review is // report-only, so the intent still ships; the surface prints this loudly. AuditEmitError string `json:"audit_emit_error,omitempty"` + // Relinked lists every relative markdown link this close repointed because + // it named the old path of a record the close moved — the spec leaving + // open/, the intent leaving planned/ (iss-2609091732329046). Empty when no + // link named either. + Relinked []relink.Rewrite `json:"relinked,omitempty"` + // RelinkError is a NON-FATAL report of a repoint that failed part-way: the + // records have moved and the close stands, so the surface prints it loudly + // and a re-run of the close completes the repoint. + RelinkError string `json:"relink_error,omitempty"` } // RemainderRequest asks a close to mint a follow-on spec for the part of the diff --git a/internal/core/intent/lifecycle.go b/internal/core/intent/lifecycle.go index bdc16da67..243d510ad 100644 --- a/internal/core/intent/lifecycle.go +++ b/internal/core/intent/lifecycle.go @@ -11,6 +11,7 @@ import ( "github.com/intentdriven/abcd/internal/core/changelog" "github.com/intentdriven/abcd/internal/core/frontmatter" "github.com/intentdriven/abcd/internal/core/recordid" + "github.com/intentdriven/abcd/internal/core/relink" "github.com/intentdriven/abcd/internal/core/spec" "github.com/intentdriven/abcd/internal/fsutil" ) @@ -291,7 +292,15 @@ func Plan(repoRoot, intentID string, opts PlanOptions) (PlanResult, error) { it.SpecID = sp.ID it.Bucket = BucketPlanned it.Path = plannedRel - return PlanResult{Intent: it, Spec: sp, ConditionsStamped: conditionsStamped, ImpactStamped: impactStamp}, nil + res := PlanResult{Intent: it, Spec: sp, ConditionsStamped: conditionsStamped, ImpactStamped: impactStamp} + // Repoint every link that named the draft's path, as a close does for the + // records it moves (iss-2609250846525896). Reported, not raised: the record + // is planned and the plan stands. + res.Relinked, err = relink.Repoint(repoRoot, []relink.Move{{From: draftRel, To: plannedRel}}) + if err != nil { + res.RelinkError = err.Error() + } + return res, nil } // draftFaceFields is the frontmatter rewrite the draft face makes in one @@ -844,6 +853,18 @@ func Reconcile(repoRoot, specID, impact string, remainder RemainderRequest) (Rec res.Spec = closed } + // 2b. Repoint every link that named either record's old path. The close is + // the one place that knows both paths, so a close that reports success + // hands on a tree record-lint accepts rather than a links_resolve refusal + // the next command meets (iss-2609091732329046). The moves are derived from + // the records' current buckets, not from what THIS invocation moved, so a + // re-run after a failure here completes the repoint. A failure is reported, + // not raised: the records have moved and the close stands. + res.Relinked, err = relink.Repoint(repoRoot, closeMoves(res)) + if err != nil { + res.RelinkError = err.Error() + } + // 3. Emit the fidelity-review OWED stub + ephemeral request over the shipped // intent. The review is REPORT-ONLY: a failure here is captured, not raised — // the intent has already shipped and must not be un-shipped by a review-emit @@ -860,6 +881,27 @@ func Reconcile(repoRoot, specID, impact string, remainder RemainderRequest) (Rec return res, nil } +// closeMoves names the renames a close stands for, derived from where the two +// records are now: a closed spec left open/, a shipped intent left planned/. +// Deriving rather than recording what this call moved is what makes the repoint +// idempotent — relink.Repoint ignores a move the tree does not show. +func closeMoves(res ReconcileResult) []relink.Move { + var moves []relink.Move + if res.Spec.Status == spec.StatusClosed { + moves = append(moves, relink.Move{ + From: filepath.Join(spec.SpecsRelDir, spec.StatusOpen, filepath.Base(res.Spec.Path)), + To: res.Spec.Path, + }) + } + if res.Intent.Bucket == BucketShipped { + moves = append(moves, relink.Move{ + From: filepath.Join(IntentsRelDir, BucketPlanned, filepath.Base(res.Intent.Path)), + To: res.Intent.Path, + }) + } + return moves +} + // otherOpenSpecs narrows a set of specs realising one intent to the OPEN ones // that are not the spec being closed — the specs that, after this close, still // hold the intent in planned/. It is the single question the 1:n lifecycle asks diff --git a/internal/core/intent/relink_test.go b/internal/core/intent/relink_test.go new file mode 100644 index 000000000..8e33c8557 --- /dev/null +++ b/internal/core/intent/relink_test.go @@ -0,0 +1,181 @@ +package intent + +import ( + "os" + "path/filepath" + "strings" + "testing" + + "github.com/intentdriven/abcd/internal/core/lint" +) + +// linksResolveFindings runs record-lint's links_resolve rule over the durable +// record, the root the committed record-lint.json declares, and returns every +// finding it raises. +func linksResolveFindings(t *testing.T, root string) []lint.Finding { + t.Helper() + cfg := lint.Config{ + Roots: []string{".abcd/development"}, + Rules: map[string]lint.RuleConfig{ + "links_resolve": {Enabled: true, Severity: "blocker"}, + }, + } + findings, err := lint.Lint(cfg, root) + if err != nil { + t.Fatal(err) + } + var out []lint.Finding + for _, f := range findings { + if f.RuleID == "links_resolve" { + out = append(out, f) + } + } + return out +} + +func readRel(t *testing.T, root, rel string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(root, rel)) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +// relinkFixture lays out the three link classes that broke by hand in the ship +// ceremony (iss-2608311127491949, iss-2609091732329046): a spec that is ALREADY +// closed linking the spec about to close through ../open/, an ADR and a plan +// linking the intent's planned/ path, and a draft linking both — plus the +// closing spec's own links, which were written from open/ and name a still-open +// sibling bare. Closing spc-2 ships itd-10, so both records move. +func relinkFixture(t *testing.T) string { + t.Helper() + root := t.TempDir() + writeFile(t, root, plannedDir+"/itd-10-alpha.md", + strings.Replace(plannedLinked("itd-10", "alpha", "spc-1"), "## Audit Notes\n", + "The work is specified in [spc-2](../../specs/open/spc-2-rest.md).\n\n## Audit Notes\n", 1)) + writeFile(t, root, plannedDir+"/itd-11-other.md", plannedLinked("itd-11", "other", "spc-3")) + writeFile(t, root, specsClosed+"/spc-1-alpha.md", + specNaming("spc-1", "alpha", "itd-10")+"\nThe rest is [spc-2](../open/spc-2-rest.md).\n") + writeFile(t, root, specsOpen+"/spc-2-rest.md", + specNaming("spc-2", "rest", "itd-10")+ + "\nRealises [itd-10](../../intents/planned/itd-10-alpha.md#acceptance-criteria).\n"+ + "Follows [spc-1](../closed/spc-1-alpha.md) and sits beside [spc-3](spc-3-other.md).\n"+ + "A link that never resolved stays as written: [gone](spc-9-gone.md).\n") + writeFile(t, root, specsOpen+"/spc-3-other.md", + specNaming("spc-3", "other", "itd-11")+"\nSibling of [spc-2](spc-2-rest.md).\n") + writeFile(t, root, ".abcd/development/decisions/adrs/0001-choice.md", + "# choice\n\nSee [itd-10](../../intents/planned/itd-10-alpha.md) and [the spec](../../specs/open/spc-2-rest.md).\n"+ + "Unmoved: [itd-11](../../intents/planned/itd-11-other.md).\n") + writeFile(t, root, ".abcd/development/plans/2026-01-01-plan.md", + "# plan\n\n- [itd-10](../intents/planned/itd-10-alpha.md)\n\n[ref]: ../intents/planned/itd-10-alpha.md\n") + writeFile(t, root, draftsDir+"/itd-12-draft.md", + "---\nid: itd-12\nslug: draft\n---\n# draft\n\nBuilds on [itd-10](../planned/itd-10-alpha.md) via [spc-2](../../specs/open/spc-2-rest.md).\n") + writeFile(t, root, ".abcd/work/CONTEXT.md", + "# context\n\nShipping [itd-10](../development/intents/planned/itd-10-alpha.md).\n") + return root +} + +// Closing a spec moves the spec (open/ -> closed/) and, on the last close, its +// intent (planned/ -> shipped/). The close leaves a tree record-lint accepts: +// every link that named either record's old path, from any folder, is repointed +// in the same operation — the already-closed sibling spec included. +func TestReconcileRepointsLinksToTheMovedRecords(t *testing.T) { + root := relinkFixture(t) + // The fixture's one deliberate dead link is the baseline: it is not the + // close's to repair, and it proves the rewrite leaves a link alone when its + // target never resolved. + if got := linksResolveFindings(t, root); len(got) != 1 { + t.Fatalf("fixture baseline: want exactly the one deliberate dead link, got %+v", got) + } + + res, err := Reconcile(root, "spc-2", "", RemainderRequest{}) + if err != nil { + t.Fatal(err) + } + if res.RelinkError != "" { + t.Fatalf("relink error: %s", res.RelinkError) + } + // The close reports what it rewrote: one entry per link, the already-closed + // sibling's among them. + if len(res.Relinked) != 13 { + t.Errorf("want 13 rewrites reported, got %d: %+v", len(res.Relinked), res.Relinked) + } + sawSibling := false + for _, rw := range res.Relinked { + if rw.File == specsClosed+"/spc-1-alpha.md" && rw.From == "../open/spc-2-rest.md" && rw.To == "spc-2-rest.md" { + sawSibling = true + } + } + if !sawSibling { + t.Errorf("the closed sibling's rewrite is not reported: %+v", res.Relinked) + } + + got := linksResolveFindings(t, root) + if len(got) != 1 || !strings.Contains(got[0].Message, "spc-9-gone.md") { + t.Fatalf("after spec close only the pre-existing dead link may remain; links_resolve found %+v", got) + } + + for rel, want := range map[string][]string{ + specsClosed + "/spc-1-alpha.md": {"[spc-2](spc-2-rest.md)"}, + specsClosed + "/spc-2-rest.md": {"(../../intents/shipped/itd-10-alpha.md#acceptance-criteria)", "(spc-1-alpha.md)", "(../open/spc-3-other.md)", "(spc-9-gone.md)"}, + specsOpen + "/spc-3-other.md": {"(../closed/spc-2-rest.md)"}, + shippedDir + "/itd-10-alpha.md": {"(../../specs/closed/spc-2-rest.md)"}, + ".abcd/development/decisions/adrs/0001-choice.md": {"(../../intents/shipped/itd-10-alpha.md)", "(../../specs/closed/spc-2-rest.md)", "(../../intents/planned/itd-11-other.md)"}, + ".abcd/development/plans/2026-01-01-plan.md": {"(../intents/shipped/itd-10-alpha.md)", "[ref]: ../intents/shipped/itd-10-alpha.md"}, + draftsDir + "/itd-12-draft.md": {"(../shipped/itd-10-alpha.md)", "(../../specs/closed/spc-2-rest.md)"}, + ".abcd/work/CONTEXT.md": {"(../development/intents/shipped/itd-10-alpha.md)"}, + } { + body := readRel(t, root, rel) + for _, w := range want { + if !strings.Contains(body, w) { + t.Errorf("%s: want %q in\n%s", rel, w, body) + } + } + } +} + +// A re-run of a completed close is a clean completion, and it repoints what a +// failed earlier attempt left behind: the moves are derived from where the +// records are now. +func TestReconcileRerunRepointsWhatAnEarlierCloseLeft(t *testing.T) { + root := relinkFixture(t) + if _, err := Reconcile(root, "spc-2", "", RemainderRequest{}); err != nil { + t.Fatal(err) + } + // An edit after the close reintroduces a link to the old path. + writeFile(t, root, ".abcd/development/plans/late.md", "[late](../specs/open/spc-2-rest.md)\n") + res, err := Reconcile(root, "spc-2", "", RemainderRequest{}) + if err != nil { + t.Fatal(err) + } + if len(res.Relinked) != 1 || res.Relinked[0].To != "../specs/closed/spc-2-rest.md" { + t.Fatalf("the re-run must repoint the stale link: %+v", res.Relinked) + } +} + +// Planning moves the draft drafts/ -> planned/, and every link that named the +// draft's path follows it (iss-2609250846525896). +func TestPlanRepointsLinksToTheDraft(t *testing.T) { + root := t.TempDir() + writeFile(t, root, draftsDir+"/itd-10-alpha.md", draftWithAC("itd-10", "alpha")) + writeFile(t, root, ".abcd/development/decisions/adrs/0001-choice.md", + "# choice\n\nSee [itd-10](../../intents/drafts/itd-10-alpha.md).\n") + writeFile(t, root, draftsDir+"/itd-11-beta.md", + "---\nid: itd-11\nslug: beta\n---\n# beta\n\nAfter [itd-10](itd-10-alpha.md).\n") + + res, err := Plan(root, "itd-10", PlanOptions{}) + if err != nil { + t.Fatal(err) + } + if got := linksResolveFindings(t, root); len(got) != 0 { + t.Fatalf("after plan, links_resolve found %+v", got) + } + if !strings.Contains(readRel(t, root, ".abcd/development/decisions/adrs/0001-choice.md"), "(../../intents/planned/itd-10-alpha.md)") || + !strings.Contains(readRel(t, root, draftsDir+"/itd-11-beta.md"), "(../planned/itd-10-alpha.md)") { + t.Fatal("links to the draft were not repointed at planned/") + } + if len(res.Relinked) != 2 || res.RelinkError != "" { + t.Fatalf("plan must report the two rewrites: %+v %q", res.Relinked, res.RelinkError) + } +} diff --git a/internal/core/relink/relink.go b/internal/core/relink/relink.go new file mode 100644 index 000000000..d227f2ec7 --- /dev/null +++ b/internal/core/relink/relink.go @@ -0,0 +1,341 @@ +// Package relink is the one link-repoint primitive: after a verb MOVES a record +// between status folders, it rewrites every relative markdown link in the +// working tree that named the record's old path so that it names the new one, +// and reports each rewrite. +// +// A record's status is its folder (open/ → closed/, planned/ → shipped/, +// open/ → resolved/), so every lifecycle transition is a rename, and a rename +// strands every relative link that named the file where it was. The verb that +// moves the record is the one place that knows both paths, so it is the place +// the links are repointed — in the same operation, not by a hand survey the next +// gate run discovers was incomplete (iss-2608311127491949, +// iss-2609091732329046). Three classes of link are repointed by the one rule: +// +// - a link FROM any other file TO a moved record, from any folder — a spec +// already closed that names ../open/, an ADR or a plan +// naming an intent's planned/ path, a draft naming both; +// - a link FROM a moved record to a file that did not move, which was written +// from the old folder and resolves against the new one differently — a +// closing spec's bare link to a sibling still in open/; +// - a link from a moved record to another record moved in the same operation. +// +// A link is rewritten only when it resolved before the move, so a link that +// never pointed anywhere stays exactly as written: repairing that is not the +// move's business, and a rewrite would disguise it. Links inside fenced code +// and HTML comments are rewritten too — a destination that resolved to the +// moved record is a reference to it wherever it is written, and record-lint's +// links_resolve judges a commented link as much as a live one. +// +// The walk covers the whole working tree's markdown — the durable record, the +// shared working tier and the user-facing docs alike — because a link to a +// record is broken wherever it is written. It never enters .git, the local +// tier (.abcd/.work.local, which is per-checkout scratch), a node_modules +// directory, or a nested checkout (any directory holding its own .git), whose +// files belong to another working tree. Nor does it write into the two +// append-only logs the repository's gates refuse an in-place edit of — the +// decision log (DA002) and the reviews folder (RD002): a link there is history +// as written, and rewriting it would trade a dead link no lint root reads for a +// refused change. It reads and writes through an os.Root, follows no symlink, +// and skips any file past a byte cap. +// +// The package is transport-free: it writes no output, and reports what it did as +// data for the front door to render. +package relink + +import ( + "errors" + "fmt" + "io/fs" + "os" + "path" + "path/filepath" + "regexp" + "sort" + "strings" + + "github.com/intentdriven/abcd/internal/fsutil" +) + +// appendOnly names the committed logs no rewrite may touch: the decision log, +// whose gate refuses a removed line below its header (DA002 in +// scripts/check-decisions-append.sh), and the reviews folder, whose gate +// refuses any change to a file after it is created (RD002 in +// scripts/check-reviews.sh). +var appendOnly = map[string]bool{ + ".abcd/work/DECISIONS.md": true, + ".abcd/work/reviews": true, +} + +// maxFileBytes caps a markdown file the walk will read and rewrite. The record +// stores cap their own files far below it; a larger file is not a record. +const maxFileBytes = 4 << 20 + +var ( + // inlineRe is an inline link or image, `[text](dest)`: the SAME shape + // record-lint's links_resolve checks (internal/core/lint linkRe), so every + // link the gate would refuse after a move is one this rewrite sees. + inlineRe = regexp.MustCompile(`\[[^\]]*\]\(([^)]+)\)`) + // refDefRe is a link-reference definition, `[label]: dest`, up to three + // spaces of indent (CommonMark). The gate does not judge these, but one that + // named the moved record is exactly as broken as an inline link. + refDefRe = regexp.MustCompile(`^ {0,3}\[[^\]]+\]:[ \t]+(\S+)`) + // schemeRe marks a destination with a URL scheme (https:, mailto:), which is + // never a repository path. + schemeRe = regexp.MustCompile(`^[a-zA-Z][a-zA-Z0-9+.\-]*:`) +) + +// Move is one record rename, as repo-relative slash paths. +type Move struct { + From string `json:"from"` + To string `json:"to"` +} + +// Rewrite is one link destination this package changed. +type Rewrite struct { + // File is the repo-relative slash path of the file holding the link, at + // its location after the move. + File string `json:"file"` + // Line is the 1-based line the link is on. + Line int `json:"line"` + // From is the destination as it was written, and To as it now reads; both + // carry any #anchor or ?query the link had. + From string `json:"from"` + To string `json:"to"` +} + +// Repoint rewrites every relative markdown link under repoRoot that the given +// moves left pointing at an old path, and returns the rewrites in file then +// line order. It runs AFTER the moves: a move whose destination is absent, or +// whose source is still present, did not happen as described and is ignored, +// which is also what makes a second call over the same moves a no-op — a verb +// may re-run it on a retry without double-rewriting anything. +// +// A move naming a path that is not a clean repo-relative slash path is refused +// before anything is read. An error part-way through the walk returns the +// rewrites already written alongside it, so a caller can report both. +func Repoint(repoRoot string, moves []Move) ([]Rewrite, error) { + root, err := os.OpenRoot(repoRoot) + if err != nil { + return nil, fmt.Errorf("relink: opening the repository root: %w", err) + } + defer root.Close() + + movedTo, oldPath, err := activeMoves(root, moves) + if err != nil { + return nil, err + } + if len(movedTo) == 0 { + return nil, nil + } + // A file that names none of the moved records' filenames cannot hold a + // link to one, so its lines are never parsed. The moved records themselves + // are always parsed: their own relative links shifted with them. + var needles []string + for from := range movedTo { + needles = append(needles, path.Base(from)) + } + + files, err := markdownFiles(root) + if err != nil { + return nil, err + } + var out []Rewrite + for _, rel := range files { + data, err := fsutil.ReadGuardedInRoot(root, rel, maxFileBytes) + if errors.Is(err, fsutil.ErrNotRegular) || errors.Is(err, fsutil.ErrTooBig) { + continue + } + if err != nil { + return out, fmt.Errorf("relink: reading %s: %w", rel, err) + } + was, moved := oldPath[rel] + if !moved { + if !containsAny(data, needles) { + continue + } + was = rel + } + updated, rw := rewriteFile(root, rel, was, string(data), movedTo) + if len(rw) == 0 { + continue + } + if err := fsutil.WriteFileAtomicPreserveModeInRoot(root, rel, []byte(updated)); err != nil { + return out, fmt.Errorf("relink: writing %s: %w", rel, err) + } + out = append(out, rw...) + } + return out, nil +} + +// activeMoves validates the moves and keeps the ones the tree shows happened: +// the destination is present and the source is gone. It returns the forward map +// (old path → new) and its inverse (new path → old). +func activeMoves(root *os.Root, moves []Move) (map[string]string, map[string]string, error) { + movedTo := map[string]string{} + oldPath := map[string]string{} + for _, m := range moves { + from, to := filepath.ToSlash(m.From), filepath.ToSlash(m.To) + if !fsutil.ValidRelPath(from) || !fsutil.ValidRelPath(to) { + return nil, nil, fmt.Errorf("relink: move %q -> %q must name clean repo-relative paths", m.From, m.To) + } + if from == to { + continue + } + if _, err := root.Lstat(from); err == nil { + continue + } + if fi, err := root.Lstat(to); err != nil || !fi.Mode().IsRegular() { + continue + } + movedTo[from] = to + oldPath[to] = from + } + return movedTo, oldPath, nil +} + +// markdownFiles lists every markdown file in the tree, as sorted slash paths, +// outside the directories the package comment names. Symlinks are never +// followed: fs.WalkDir reports a symlinked directory as a non-directory entry, +// and only regular files are kept. +func markdownFiles(root *os.Root) ([]string, error) { + var files []string + err := fs.WalkDir(root.FS(), ".", func(p string, d fs.DirEntry, err error) error { + if err != nil { + return fmt.Errorf("relink: walking %s: %w", p, err) + } + if d.IsDir() { + if p == "." { + return nil + } + if skipDir(root, p, d.Name()) { + return fs.SkipDir + } + return nil + } + if d.Type().IsRegular() && strings.EqualFold(path.Ext(p), ".md") && !appendOnly[p] { + files = append(files, p) + } + return nil + }) + if err != nil { + return nil, err + } + sort.Strings(files) + return files, nil +} + +// skipDir reports whether the walk stays out of a directory: git's own, the +// local tier, an append-only log, a dependency tree, or a nested checkout. +func skipDir(root *os.Root, p, name string) bool { + if name == ".git" || name == "node_modules" || p == ".abcd/.work.local" || appendOnly[p] { + return true + } + _, err := root.Lstat(p + "/.git") + return err == nil +} + +func containsAny(data []byte, needles []string) bool { + s := string(data) + for _, n := range needles { + if strings.Contains(s, n) { + return true + } + } + return false +} + +// rewriteFile repoints the links in one file. rel is where the file is now and +// was is where it was before the moves (the same path unless it moved itself): +// a link is resolved against the folder it was written from, mapped through the +// moves, and re-relativised against the folder the file is in now. +func rewriteFile(root *os.Root, rel, was, content string, movedTo map[string]string) (string, []Rewrite) { + lines := strings.Split(content, "\n") + var out []Rewrite + for i, line := range lines { + var spans [][2]int + for _, m := range inlineRe.FindAllStringSubmatchIndex(line, -1) { + spans = append(spans, [2]int{m[2], m[3]}) + } + if m := refDefRe.FindStringSubmatchIndex(line); m != nil { + spans = append(spans, [2]int{m[2], m[3]}) + } + if len(spans) == 0 { + continue + } + sort.Slice(spans, func(a, b int) bool { return spans[a][0] < spans[b][0] }) + var b strings.Builder + last := 0 + changed := false + for _, sp := range spans { + if sp[0] < last { + continue // overlapping spans: the earlier match owns these bytes + } + raw := line[sp[0]:sp[1]] + repl, from, to, ok := repointDest(root, rel, was, raw, movedTo) + if !ok { + continue + } + b.WriteString(line[last:sp[0]]) + b.WriteString(repl) + last = sp[1] + changed = true + out = append(out, Rewrite{File: rel, Line: i + 1, From: from, To: to}) + } + if changed { + b.WriteString(line[last:]) + lines[i] = b.String() + } + } + return strings.Join(lines, "\n"), out +} + +// repointDest returns the rewritten destination text for one link, the +// destination before and after (anchor included, whitespace and title not), +// and whether anything changed. +func repointDest(root *os.Root, rel, was, raw string, movedTo map[string]string) (string, string, string, bool) { + trimmed := strings.TrimLeft(raw, " \t") + lead := raw[:len(raw)-len(trimmed)] + dest, tail := trimmed, "" + if k := strings.IndexAny(trimmed, " \t"); k >= 0 { + dest, tail = trimmed[:k], trimmed[k:] + } + if dest == "" || strings.HasPrefix(dest, "#") || strings.HasPrefix(dest, "/") || + strings.HasPrefix(dest, "<") || schemeRe.MatchString(dest) { + return "", "", "", false + } + p, suffix := dest, "" + if k := strings.IndexAny(dest, "#?"); k >= 0 { + p, suffix = dest[:k], dest[k:] + } + if p == "" { + return "", "", "", false + } + resolved := path.Join(path.Dir(was), p) + if resolved == ".." || strings.HasPrefix(resolved, "../") { + return "", "", "", false + } + target, recordMoved := movedTo[resolved] + if !recordMoved { + if was == rel { + return "", "", "", false + } + // Only the linking file moved. A link that resolved to nothing from + // where it was written is left exactly as it is. + if _, err := root.Stat(resolved); err != nil { + return "", "", "", false + } + target = resolved + } + np, err := filepath.Rel(filepath.FromSlash(path.Dir(rel)), filepath.FromSlash(target)) + if err != nil { + return "", "", "", false + } + np = filepath.ToSlash(np) + if strings.HasPrefix(p, "./") && !strings.HasPrefix(np, "../") { + np = "./" + np + } + if np == p { + return "", "", "", false + } + return lead + np + suffix + tail, p + suffix, np + suffix, true +} diff --git a/internal/core/relink/relink_test.go b/internal/core/relink/relink_test.go new file mode 100644 index 000000000..0e79b47ff --- /dev/null +++ b/internal/core/relink/relink_test.go @@ -0,0 +1,169 @@ +package relink + +import ( + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +func write(t *testing.T, root, rel, content string) { + t.Helper() + abs := filepath.Join(root, rel) + if err := os.MkdirAll(filepath.Dir(abs), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(abs, []byte(content), 0o644); err != nil { + t.Fatal(err) + } +} + +func read(t *testing.T, root, rel string) string { + t.Helper() + b, err := os.ReadFile(filepath.Join(root, rel)) + if err != nil { + t.Fatal(err) + } + return string(b) +} + +func move(t *testing.T, root, from, to string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(filepath.Join(root, to)), 0o755); err != nil { + t.Fatal(err) + } + if err := os.Rename(filepath.Join(root, from), filepath.Join(root, to)); err != nil { + t.Fatal(err) + } +} + +const ( + openA = "specs/open/a.md" + closedA = "specs/closed/a.md" +) + +// A link to the moved file is repointed from every folder, and the moved file's +// own links, written from the folder it left, are re-relativised against the one +// it is in — while a link that never resolved, an external URL, an anchor-only +// link and an unrelated record are left exactly as written. +func TestRepointRewritesLinksToAndFromTheMovedFile(t *testing.T) { + root := t.TempDir() + write(t, root, openA, "[b](b.md) [gone](nope.md) [self](a.md#x) [web](https://example.com/a.md) [top](#top)\n") + write(t, root, "specs/open/b.md", "[a](a.md) [a again](./a.md#part)\n") + write(t, root, "specs/closed/c.md", "[a](../open/a.md)\n") + write(t, root, "docs/guide.md", "See [a](../specs/open/a.md \"title\").\n\n[a-ref]: ../specs/open/a.md\n\n```\n[fenced](../specs/open/a.md)\n```\n") + write(t, root, "docs/other.md", "[b](../specs/open/b.md) mentions a.md only in prose\n") + move(t, root, openA, closedA) + + got, err := Repoint(root, []Move{{From: openA, To: closedA}}) + if err != nil { + t.Fatal(err) + } + + want := map[string]string{ + closedA: "[b](../open/b.md) [gone](nope.md) [self](a.md#x) [web](https://example.com/a.md) [top](#top)\n", + "specs/open/b.md": "[a](../closed/a.md) [a again](../closed/a.md#part)\n", + "specs/closed/c.md": "[a](a.md)\n", + "docs/guide.md": "See [a](../specs/closed/a.md \"title\").\n\n[a-ref]: ../specs/closed/a.md\n\n```\n[fenced](../specs/closed/a.md)\n```\n", + "docs/other.md": "[b](../specs/open/b.md) mentions a.md only in prose\n", + } + for rel, w := range want { + if g := read(t, root, rel); g != w { + t.Errorf("%s:\n got %q\nwant %q", rel, g, w) + } + } + + wantRW := []Rewrite{ + {File: "docs/guide.md", Line: 1, From: "../specs/open/a.md", To: "../specs/closed/a.md"}, + {File: "docs/guide.md", Line: 3, From: "../specs/open/a.md", To: "../specs/closed/a.md"}, + {File: "docs/guide.md", Line: 6, From: "../specs/open/a.md", To: "../specs/closed/a.md"}, + {File: closedA, Line: 1, From: "b.md", To: "../open/b.md"}, + {File: "specs/closed/c.md", Line: 1, From: "../open/a.md", To: "a.md"}, + {File: "specs/open/b.md", Line: 1, From: "a.md", To: "../closed/a.md"}, + {File: "specs/open/b.md", Line: 1, From: "./a.md#part", To: "../closed/a.md#part"}, + } + if !reflect.DeepEqual(got, wantRW) { + t.Fatalf("rewrites:\n got %+v\nwant %+v", got, wantRW) + } + + // A second call over the same moves finds nothing left to do. + again, err := Repoint(root, []Move{{From: openA, To: closedA}}) + if err != nil || len(again) != 0 { + t.Fatalf("a re-run must be a no-op: %+v, %v", again, err) + } +} + +// Two records moved in one operation: a link from one to the other follows both. +func TestRepointFollowsTwoMovesAtOnce(t *testing.T) { + root := t.TempDir() + write(t, root, "specs/open/s.md", "[i](../../intents/planned/i.md)\n") + write(t, root, "intents/planned/i.md", "[s](../../specs/open/s.md)\n") + move(t, root, "specs/open/s.md", "specs/closed/s.md") + move(t, root, "intents/planned/i.md", "intents/shipped/i.md") + + if _, err := Repoint(root, []Move{ + {From: "specs/open/s.md", To: "specs/closed/s.md"}, + {From: "intents/planned/i.md", To: "intents/shipped/i.md"}, + }); err != nil { + t.Fatal(err) + } + if g := read(t, root, "specs/closed/s.md"); g != "[i](../../intents/shipped/i.md)\n" { + t.Errorf("spec: %q", g) + } + if g := read(t, root, "intents/shipped/i.md"); g != "[s](../../specs/closed/s.md)\n" { + t.Errorf("intent: %q", g) + } +} + +// The walk stays out of git's directory, the local tier and a nested checkout, +// whose files belong to another working tree, and writes nothing into the two +// append-only logs whose gates refuse an in-place edit. +func TestRepointStaysOutOfForeignTreesAndAppendOnlyLogs(t *testing.T) { + root := t.TempDir() + write(t, root, openA, "a\n") + link := "[a](../../specs/open/a.md)\n" + write(t, root, ".abcd/.work.local/x.md", link) + write(t, root, "nested/.git", "gitdir: elsewhere\n") + write(t, root, "nested/sub/x.md", link) + write(t, root, ".git/x/y.md", link) + write(t, root, ".abcd/work/DECISIONS.md", link) + write(t, root, ".abcd/work/reviews/2026-01-01-x/00-summary.md", link) + move(t, root, openA, closedA) + + got, err := Repoint(root, []Move{{From: openA, To: closedA}}) + if err != nil { + t.Fatal(err) + } + if len(got) != 0 { + t.Fatalf("no file outside the working tree's own record may be rewritten: %+v", got) + } + for _, rel := range []string{".abcd/.work.local/x.md", "nested/sub/x.md", ".git/x/y.md", + ".abcd/work/DECISIONS.md", ".abcd/work/reviews/2026-01-01-x/00-summary.md"} { + if g := read(t, root, rel); g != link { + t.Errorf("%s rewritten to %q", rel, g) + } + } +} + +// A move the tree does not show — its source still present, or its destination +// absent — is ignored, and a path that is not clean and repo-relative is +// refused before anything is read. +func TestRepointIgnoresMovesThatDidNotHappenAndRefusesUnsafePaths(t *testing.T) { + root := t.TempDir() + write(t, root, openA, "a\n") + write(t, root, "specs/open/b.md", "[a](a.md)\n") + + got, err := Repoint(root, []Move{{From: openA, To: closedA}}) + if err != nil || len(got) != 0 { + t.Fatalf("a move that did not happen must rewrite nothing: %+v, %v", got, err) + } + for _, m := range []Move{{From: "../x.md", To: closedA}, {From: openA, To: "/abs.md"}, {From: "", To: closedA}} { + if _, err := Repoint(root, []Move{m}); err == nil || !strings.Contains(err.Error(), "repo-relative") { + t.Errorf("move %+v must be refused, got %v", m, err) + } + } + if g := read(t, root, "specs/open/b.md"); g != "[a](a.md)\n" { + t.Errorf("b.md rewritten: %q", g) + } +} diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index 798b976eb..cb4b36465 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -2024,6 +2024,7 @@ func newIntentCommand(asJSON *bool) *cobra.Command { if err != nil { return &exitError{Code: 2, Msg: "abcd intent plan: " + err.Error()} } + emitRelinkError(cmd.ErrOrStderr(), "intent plan", res.RelinkError, "record-lint's links_resolve names each link left behind") return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { if res.StampOnly { // The identity step alone, over a record already planned: say what @@ -2042,6 +2043,7 @@ func newIntentCommand(asJSON *bool) *cobra.Command { if res.ImpactStamped != "" { fmt.Fprintf(w, " impact stamped: %s\n", res.ImpactStamped) } + emitRelinked(w, res.Relinked) }) }, } @@ -2550,6 +2552,7 @@ func newSpecCommand(asJSON *bool) *cobra.Command { if res.AuditEmitError != "" { fmt.Fprintf(cmd.ErrOrStderr(), "WARNING: abcd spec close — fidelity-review emit failed for %s (intent shipped anyway): %s\n", res.Intent.ID, res.AuditEmitError) } + emitRelinkError(cmd.ErrOrStderr(), "spec close", res.RelinkError, "re-run `abcd spec close "+args[0]+"` to finish the repoint") return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { fmt.Fprintf(w, "abcd spec close — %s open -> closed\n %s\n", res.Spec.ID, termsafe.Sanitize(res.Spec.Path)) if res.Remainder.ID != "" { @@ -2580,6 +2583,7 @@ func newSpecCommand(asJSON *bool) *cobra.Command { default: fmt.Fprintf(w, " intent %s already %s (no move)\n", res.Intent.ID, res.To) } + emitRelinked(w, res.Relinked) // A close is idempotent, so a re-run against an already-shipped // intent gets the SAME receipt back. Announcing "OWED" each time // reads as a fresh obligation; only the close that actually parked @@ -3531,9 +3535,11 @@ func newCaptureCommand(asJSON *bool) *cobra.Command { if err != nil { return groundsUsageError("resolve", err) } + emitRelinkError(cmd.ErrOrStderr(), "capture resolve", res.RelinkError, "record-lint's links_resolve names each link left behind") return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { fmt.Fprintf(w, "%s %s -> %s — %s%s\n", res.ID, res.FromStatus, res.ToStatus, termsafe.Sanitize(res.Path), resolvedByNote(res.ResolvedBy)) emitRedactionNote(w, res.Redacted, res.Degraded) + emitRelinked(w, res.Relinked) }) }, } @@ -3785,9 +3791,11 @@ func newCaptureCommand(asJSON *bool) *cobra.Command { if err != nil { return groundsUsageError("wontfix", err) } + emitRelinkError(cmd.ErrOrStderr(), "capture wontfix", res.RelinkError, "record-lint's links_resolve names each link left behind") return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { fmt.Fprintf(w, "%s %s -> %s — %s\n", res.ID, res.FromStatus, res.ToStatus, termsafe.Sanitize(res.Path)) emitRedactionNote(w, res.Redacted, res.Degraded) + emitRelinked(w, res.Relinked) }) }, } diff --git a/internal/surface/cli/relink.go b/internal/surface/cli/relink.go new file mode 100644 index 000000000..c6acf46bb --- /dev/null +++ b/internal/surface/cli/relink.go @@ -0,0 +1,35 @@ +package cli + +import ( + "fmt" + "io" + + "github.com/intentdriven/abcd/internal/core/relink" + "github.com/intentdriven/abcd/internal/termsafe" +) + +// emitRelinked renders the links a record-moving verb repointed at the moved +// record's new path, one per line, so the operator sees every file the move +// touched beyond the record itself. Nothing is printed when no link named it. +func emitRelinked(w io.Writer, rewrites []relink.Rewrite) { + if len(rewrites) == 0 { + return + } + noun := "links" + if len(rewrites) == 1 { + noun = "link" + } + fmt.Fprintf(w, " repointed %d %s that named the old path:\n", len(rewrites), noun) + for _, rw := range rewrites { + fmt.Fprintf(w, " %s:%d %s -> %s\n", termsafe.Sanitize(rw.File), rw.Line, termsafe.Sanitize(rw.From), termsafe.Sanitize(rw.To)) + } +} + +// emitRelinkError warns, on stderr, that a repoint failed part-way. The move +// stands, so the verb does not fail; remedy says how the rest is finished. +func emitRelinkError(w io.Writer, verb, msg, remedy string) { + if msg == "" { + return + } + fmt.Fprintf(w, "WARNING: abcd %s — the record moved, but repointing the links that named its old path failed: %s; %s\n", verb, termsafe.Sanitize(msg), remedy) +} diff --git a/internal/surface/cli/relink_cli_test.go b/internal/surface/cli/relink_cli_test.go new file mode 100644 index 000000000..8f5b0406f --- /dev/null +++ b/internal/surface/cli/relink_cli_test.go @@ -0,0 +1,82 @@ +package cli + +import ( + "encoding/json" + "os" + "path/filepath" + "strings" + "testing" +) + +// spec close names every link it repointed at the moved records, in the text +// render and in --json, so the operator sees what the close changed beyond the +// two records it moved (iss-2609091732329046). +func TestSpecCloseReportsRepointedLinks(t *testing.T) { + setup := func(t *testing.T) string { + repo := t.TempDir() + gitInitAt(t, repo) + t.Chdir(repo) + writeRepoFile(t, repo, cliPlanned+"/itd-10-alpha.md", + "---\nid: itd-10\nslug: alpha\nspec_id: spc-1\nkind: standalone\nimpact: fix\n---\n# alpha\n\n## Acceptance Criteria\n\n- ok\n") + writeRepoFile(t, repo, cliSpecsOpen+"/spc-1-alpha.md", + "---\nid: spc-1\nslug: alpha\nintent: itd-10\n---\n# alpha\n") + writeRepoFile(t, repo, ".abcd/development/plans/p.md", + "# p\n\n[itd-10](../intents/planned/itd-10-alpha.md)\n") + return repo + } + + t.Run("text", func(t *testing.T) { + setup(t) + out := string(runCLI(t, "spec", "close", "spc-1")) + if !strings.Contains(out, "repointed 1 link") || + !strings.Contains(out, ".abcd/development/plans/p.md:3 ../intents/planned/itd-10-alpha.md -> ../intents/shipped/itd-10-alpha.md") { + t.Fatalf("close text must name the repointed link:\n%s", out) + } + }) + + t.Run("json", func(t *testing.T) { + setup(t) + var got struct { + Relinked []struct { + File string `json:"file"` + Line int `json:"line"` + From string `json:"from"` + To string `json:"to"` + } `json:"relinked"` + } + out := runCLI(t, "spec", "close", "spc-1", "--json") + if err := json.Unmarshal(out, &got); err != nil { + t.Fatalf("not JSON: %v\n%s", err, out) + } + if len(got.Relinked) != 1 || got.Relinked[0].To != "../intents/shipped/itd-10-alpha.md" { + t.Fatalf("relinked = %+v\n%s", got.Relinked, out) + } + }) +} + +// capture resolve names the links it repointed at the issue's new folder. +func TestCaptureResolveReportsRepointedLinks(t *testing.T) { + repo := captureLedgerRepo(t) + capOut := runCLI(t, "capture", "an issue a decision links to", "--json") + var minted struct { + ID string `json:"id"` + Path string `json:"path"` + } + if err := json.Unmarshal(capOut, &minted); err != nil || minted.ID == "" { + t.Fatalf("capture envelope unreadable: %v\n%s", err, capOut) + } + adr := ".abcd/development/decisions/adrs/0001-x.md" + if err := os.MkdirAll(filepath.Join(repo, filepath.Dir(adr)), 0o755); err != nil { + t.Fatal(err) + } + link := "../../../work/issues/open/" + filepath.Base(minted.Path) + if err := os.WriteFile(filepath.Join(repo, adr), []byte("# x\n\n[it]("+link+")\n"), 0o644); err != nil { + t.Fatal(err) + } + + out := string(runCLI(t, "capture", "resolve", minted.ID, "fixed", "--impact", "fix", "--grounds", cliGrounds)) + want := adr + ":3 " + link + " -> ../../../work/issues/resolved/" + filepath.Base(minted.Path) + if !strings.Contains(out, "repointed 1 link") || !strings.Contains(out, want) { + t.Fatalf("resolve text must name the repointed link %q:\n%s", want, out) + } +} From 8f890c7784edcfebecab59715abf2e77b4d21671 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 09:58:19 +0100 Subject: [PATCH 3/5] =?UTF-8?q?chore:=20resolve=20the=20three=20link-repoi?= =?UTF-8?q?nt=20captures=20=E2=80=94=20verbs=20repoint=20links?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The fix makes spec close, intent plan, capture resolve and capture wontfix repoint every link that named the moved record, through core/relink. Resolving iss-2609091732329046 exercised it: the resolve repointed the one ADR line (adr-2609151513118583) that linked the record in open/. Resolves: iss-2608311127491949 Resolves: iss-2609091732329046 Resolves: iss-2609250846525896 Assisted-by: Claude:claude-opus-5-5 --- ...t-owns-one-or-more-specs-and-it-ships-when-its-last.md | 2 +- ...ceremony-s-repoint-step-names-two-link-classes-ever.md | 8 ++++++++ ...-spec-moves-its-intent-but-leaves-every-link-that-n.md | 8 ++++++++ ...han-spec-close-leave-links-to-the-moved-record-dead.md | 8 ++++++++ 4 files changed, 25 insertions(+), 1 deletion(-) rename .abcd/work/issues/{open => resolved}/iss-2608311127491949-the-ship-ceremony-s-repoint-step-names-two-link-classes-ever.md (68%) rename .abcd/work/issues/{open => resolved}/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md (74%) rename .abcd/work/issues/{open => resolved}/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md (64%) diff --git a/.abcd/development/decisions/adrs/2609151513118583-an-intent-owns-one-or-more-specs-and-it-ships-when-its-last.md b/.abcd/development/decisions/adrs/2609151513118583-an-intent-owns-one-or-more-specs-and-it-ships-when-its-last.md index d977028de..3fc24fa69 100644 --- a/.abcd/development/decisions/adrs/2609151513118583-an-intent-owns-one-or-more-specs-and-it-ships-when-its-last.md +++ b/.abcd/development/decisions/adrs/2609151513118583-an-intent-owns-one-or-more-specs-and-it-ships-when-its-last.md @@ -46,7 +46,7 @@ being a fact about delivery and becomes a claim the tool asserted on the operator's behalf. The adjacent finding is the same seam from the other side. -[iss-2609091732329046](../../../work/issues/open/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md) +[iss-2609091732329046](../../../work/issues/resolved/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md) reports that the close moves the intent and leaves every link written against the intent's old folder pointing at nothing — three closes in one sitting produced eight dead links and a red gate immediately afterwards. Both records diff --git a/.abcd/work/issues/open/iss-2608311127491949-the-ship-ceremony-s-repoint-step-names-two-link-classes-ever.md b/.abcd/work/issues/resolved/iss-2608311127491949-the-ship-ceremony-s-repoint-step-names-two-link-classes-ever.md similarity index 68% rename from .abcd/work/issues/open/iss-2608311127491949-the-ship-ceremony-s-repoint-step-names-two-link-classes-ever.md rename to .abcd/work/issues/resolved/iss-2608311127491949-the-ship-ceremony-s-repoint-step-names-two-link-classes-ever.md index 7139e3b91..3d4c25767 100644 --- a/.abcd/work/issues/open/iss-2608311127491949-the-ship-ceremony-s-repoint-step-names-two-link-classes-ever.md +++ b/.abcd/work/issues/resolved/iss-2608311127491949-the-ship-ceremony-s-repoint-step-names-two-link-classes-ever.md @@ -9,6 +9,14 @@ found_during: "itd-184 ship ceremony, cold-reading cycle 1" origin: researcher-authored production_mode: hand-written found_at: ".abcd/work/CONTEXT.md" +resolution: "The record-moving verbs (spec close, intent plan, capture resolve, capture wontfix) repoint every relative link that named the moved record's old path through one primitive, core/relink, and report each rewrite." +impact: fix +resolved_by: + commit: "2a6d5b63" --- The ship ceremony's repoint step names two link classes -- every intents/planned link to the shipping intent, and the closing spec's bare sibling links to still-open specs plus bare links to it from open specs. A third class exists and is not named: a link from an ALREADY-CLOSED spec pointing at ../open/. Closing spc-62 left spc-61, itself already closed, holding ../open/spc-62 and record-lint refused with a links_resolve BLOCKER. The survey greps the checklist implies (specs/open/ and bare siblings) do not match that shape, so it is invisible until the gate runs. Class (b) is already recorded as having broken two earlier ships; this is a third sibling of the same shape and the checklist should name all three, or the repoint should be mechanical rather than a hand survey. + +## Grounds + +- pursued: we expect a spec close, plan, resolve or wontfix to leave a tree record-lint's links_resolve accepts with no hand repair; a links_resolve blocker naming a moved record's old path after one of those verbs would show it wrong diff --git a/.abcd/work/issues/open/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md b/.abcd/work/issues/resolved/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md similarity index 74% rename from .abcd/work/issues/open/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md rename to .abcd/work/issues/resolved/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md index ec96db05d..8117da282 100644 --- a/.abcd/work/issues/open/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md +++ b/.abcd/work/issues/resolved/iss-2609091732329046-closing-a-spec-moves-its-intent-but-leaves-every-link-that-n.md @@ -9,6 +9,14 @@ found_during: "closing three specs after the sub-agent capture work" origin: researcher-authored production_mode: hand-written found_at: "internal/core/spec" +resolution: "The record-moving verbs (spec close, intent plan, capture resolve, capture wontfix) repoint every relative link that named the moved record's old path through one primitive, core/relink, and report each rewrite." +impact: fix +resolved_by: + commit: "2a6d5b63" --- Closing a spec moves its intent but leaves every link that named the intent's old folder pointing at nothing. The close verb reconciles the intent from planned to shipped, which is its job, and the spec body that was written while the intent was planned keeps its relative links to the planned folder. Those links resolve to nothing the moment the move completes, and the record gate refuses on them, so a close that reports success hands the next command a tree that will not lint. Three closes in one sitting produced eight dead links here and a red preflight immediately afterwards, with nothing in the close output hinting at it. The verb already knows both the old and the new path, so it is the one thing in the system positioned to fix or at least name them. Either rewrite links to the moved record in the same operation, or refuse the close while a link in the spec names the folder the intent is about to leave, or say at minimum which links the move has just invalidated. Silence is the worst of the three, because the failure surfaces later, in a different command, as a lint error that looks unrelated to the close that caused it. + +## Grounds + +- pursued: we expect a spec close, plan, resolve or wontfix to leave a tree record-lint's links_resolve accepts with no hand repair; a links_resolve blocker naming a moved record's old path after one of those verbs would show it wrong diff --git a/.abcd/work/issues/open/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md b/.abcd/work/issues/resolved/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md similarity index 64% rename from .abcd/work/issues/open/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md rename to .abcd/work/issues/resolved/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md index a7fe52073..e660bdf53 100644 --- a/.abcd/work/issues/open/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md +++ b/.abcd/work/issues/resolved/iss-2609250846525896-record-moving-verbs-other-than-spec-close-leave-links-to-the-moved-record-dead.md @@ -9,6 +9,14 @@ found_during: "autonomous run A resumed 2026-09-25" origin: researcher-authored production_mode: hand-written found_at: "internal/core/capture/workflow.go" +resolution: "The record-moving verbs (spec close, intent plan, capture resolve, capture wontfix) repoint every relative link that named the moved record's old path through one primitive, core/relink, and report each rewrite." +impact: fix +resolved_by: + commit: "2a6d5b63" --- capture resolve, capture wontfix and intent plan move a record between status folders and leave every relative markdown link that named its old path pointing at nothing, the same gap iss-2609091732329046 records for spec close. Lane records1 of run A closed two issues that three ADRs and two draft intents linked by path, and record-lint refused seven links_resolve blockers until the links were repointed by hand. Every verb that moves a record is the one place that knows the old and the new path, so each should repoint the links through one shared primitive and report what it rewrote. + +## Grounds + +- pursued: we expect a spec close, plan, resolve or wontfix to leave a tree record-lint's links_resolve accepts with no hand repair; a links_resolve blocker naming a moved record's old path after one of those verbs would show it wrong From a2a975640dc001fc0bf0036ae6f3ed3104081bb6 Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 10:44:46 +0100 Subject: [PATCH 4/5] fix(relink): read an already-moved record's own links where it is A re-run of `spec close` derives its moves from where the records are now, and relink re-read every moved record's own links from the folder it left. A bare link written into the closed spec after its close, such as `[the closed index](README.md)`, therefore resolved against open/ and was rewritten to `../open/README.md`, a different file. relink.Move carries MovedNow: the caller made this rename in the same operation. Only such a move re-relativises the moved file's own links; every move still repoints other files' links to the old path, so a re-run finishes that class of an interrupted repoint. Reconcile sets it from what the call moved (the spec open at entry; res.IntentMoved). Plan and the capture transition always move, so they always set it. The zero value is the conservative reading. The fixtures gain a link to an existing other file with a moved record's basename and a dead one with the same basename; both stay as written, so a basename-only matcher now fails the tests. Refs: iss-2608311127491949 Assisted-by: Claude:claude-opus-5-5 --- .../brief/04-surfaces/05-intent.md | 2 +- commands/intent.md | 6 +- internal/core/capture/workflow.go | 2 +- internal/core/intent/lifecycle.go | 31 +++++---- internal/core/intent/relink_test.go | 64 ++++++++++++++++--- internal/core/relink/relink.go | 26 +++++++- internal/core/relink/relink_test.go | 50 ++++++++++++--- 7 files changed, 146 insertions(+), 35 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/05-intent.md b/.abcd/development/brief/04-surfaces/05-intent.md index 8fdc0ecfd..1d284cdc7 100644 --- a/.abcd/development/brief/04-surfaces/05-intent.md +++ b/.abcd/development/brief/04-surfaces/05-intent.md @@ -451,7 +451,7 @@ The invariants below are the contract the tree is held to, and each names what h - Every intent in `drafts/` has `spec_id: null` (drafts have no plan yet). - Every intent in `planned/` has `spec_id: null` (unscheduled) or a `spc-N` id; a non-null `spec_id` points to an existing native-spec-store `-*.md` whose frontmatter `intent` field matches the intent's `id` (or contains the intent's `id` as one of a list, for bundle-member intents). - **An intent owns one or more specs, and it ships when its last spec closes.** The intent↔spec relation is 1:n (invariant 17 in [`02-constraints/03-invariants.md`](../02-constraints/03-invariants.md), per [adr-2609151513118583](../../decisions/adrs/2609151513118583-an-intent-owns-one-or-more-specs-and-it-ships-when-its-last.md)). The spec's own `intent:` field is the source of truth for the link: the intent's scalar `spec_id` names the spec it was planned with, and the set of specs realising an intent is derived from the back-links (`spec.Store.SpecsForIntent`, `lint.SpecLinkIndex.SpecsForIntent`) — no field carries a list. The bidirectional check is therefore membership, not equality: a spec naming an intent is clean when that intent's `spec_id` names *some* spec realising it (`spec_lifecycle`), so a remainder spec is not drift. Closing a spec ships the intent only when no open spec is left naming it; a remainder slug given on the close mints the follow-on spec in the same operation, and the impact is demanded at the close that ships and refused at any earlier one. The release cut's stale-intent refusal asks whether a planned intent has any OPEN spec, never whether its spec has closed — a planned intent with one closed and one open spec is the correct steady state of a partial delivery. -- **A move repoints the links that named the moved record.** An intent's and a spec's folder is its status, so planning (`drafts/ → planned/`) and closing (`open/ → closed/`, and on the close that ships `planned/ → shipped/`) are renames, and the verb that renames is the one place that knows both paths. It rewrites every relative markdown link in the tree that named an old path, from any folder — a spec already closed pointing at `../open/`, an ADR or a plan naming the intent's `planned/` path, a draft naming both, and the moved record's own links, written from the folder it left — through the one link-repoint primitive (`core/relink`) the ledger's resolve and wontfix share. A link that never resolved is left as written. The result lists each rewrite (`relinked`), so the close leaves a tree record-lint's `links_resolve` accepts, with no hand survey. The close derives its moves from where the records are now, so a re-run completes a repoint an earlier attempt left unfinished; a repoint failure is a warning, never a failed close. +- **A move repoints the links that named the moved record.** An intent's and a spec's folder is its status, so planning (`drafts/ → planned/`) and closing (`open/ → closed/`, and on the close that ships `planned/ → shipped/`) are renames, and the verb that renames is the one place that knows both paths. It rewrites every relative markdown link in the tree that named an old path, from any folder — a spec already closed pointing at `../open/`, an ADR or a plan naming the intent's `planned/` path, a draft naming both, and the moved record's own links, written from the folder it left — through the one link-repoint primitive (`core/relink`) the ledger's resolve and wontfix share. A link that never resolved is left as written. The result lists each rewrite (`relinked`), so the close leaves a tree record-lint's `links_resolve` accepts, with no hand survey. The close derives its moves from where the records are now, so a re-run repoints every link other files still hold to an old path; it re-reads a record's own links from the folder it left only when that same run moved the record, because a record an earlier run moved may have been edited where it is (a bare `README.md` link names the `closed/` index, not the `open/` one). A repoint failure is a warning, never a failed close. - **A bundle is the opposite relation and is untouched.** `kind: bundle-member` with a `bundle:` link is N:1 — several intents sharing one spec — and the bundle invariant above (all members in one phase) still holds. 1:n and N:1 are different relations, not two names for one thing; composing them into N:M is not authorised by anything in the record. An intent's own specs may sit in different phases, because the reason a second spec exists is that the work did not fit the cycle that carried the first. - Every intent in `shipped/` has `kind` set (`standalone` or `bundle-member`) and a non-null `spec_id`. (The stronger invariant — the linked spec exists and is closed, or `spec_id: null` + a `manual_ship_reason` for the no-spec case — is a later-phase gate; the shipped rule checks only that `spec_id` is non-null.) - Discipline-kind intents have `spec_id: null` always (disciplines never get a spec; this is structurally enforced). diff --git a/commands/intent.md b/commands/intent.md index 7b2d930e9..c7c2357fc 100644 --- a/commands/intent.md +++ b/commands/intent.md @@ -406,7 +406,11 @@ resolved is left as written. The JSON lists each rewrite under `relinked` because they are files the close changed beyond the two records. The tree the close leaves passes record-lint's `links_resolve` with no hand repair. If the repoint fails part-way, the close still stands and a warning on stderr says so; -re-running the same `spec close` finishes the repoint. +re-running the same `spec close` repoints every link other files still hold to +either old path. The re-run reads the moved records' own links from the folders +they are in, because they may have been edited there since the move, so it +never rewrites them; any of those the failed attempt left unrewritten is one +`links_resolve` names, to repair by hand. Run it in the **same change** that lands the intent's work — the commit or pull request that makes the acceptance criteria true — the way a captured diff --git a/internal/core/capture/workflow.go b/internal/core/capture/workflow.go index 46165aa4b..53ac9f71b 100644 --- a/internal/core/capture/workflow.go +++ b/internal/core/capture/workflow.go @@ -549,7 +549,7 @@ func repointMovedIssue(repoRoot, src, dst string) ([]relink.Rewrite, string) { if err1 != nil || err2 != nil || !filepath.IsLocal(from) || !filepath.IsLocal(to) { return nil, "" } - rw, err := relink.Repoint(repoRoot, []relink.Move{{From: from, To: to}}) + rw, err := relink.Repoint(repoRoot, []relink.Move{{From: from, To: to, MovedNow: true}}) if err != nil { return rw, err.Error() } diff --git a/internal/core/intent/lifecycle.go b/internal/core/intent/lifecycle.go index 243d510ad..b611503ef 100644 --- a/internal/core/intent/lifecycle.go +++ b/internal/core/intent/lifecycle.go @@ -296,7 +296,7 @@ func Plan(repoRoot, intentID string, opts PlanOptions) (PlanResult, error) { // Repoint every link that named the draft's path, as a close does for the // records it moves (iss-2609250846525896). Reported, not raised: the record // is planned and the plan stands. - res.Relinked, err = relink.Repoint(repoRoot, []relink.Move{{From: draftRel, To: plannedRel}}) + res.Relinked, err = relink.Repoint(repoRoot, []relink.Move{{From: draftRel, To: plannedRel, MovedNow: true}}) if err != nil { res.RelinkError = err.Error() } @@ -845,7 +845,8 @@ func Reconcile(repoRoot, specID, impact string, remainder RemainderRequest) (Rec // 2. Close the spec, but only if still open — a re-run on an already-closed // spec is a clean completion, not the "already closed" error spec.Close raises. - if sp.Status == spec.StatusOpen { + specMovedNow := sp.Status == spec.StatusOpen + if specMovedNow { closed, err := spec.Close(repoRoot, specID) if err != nil { return ReconcileResult{}, err @@ -858,9 +859,12 @@ func Reconcile(repoRoot, specID, impact string, remainder RemainderRequest) (Rec // hands on a tree record-lint accepts rather than a links_resolve refusal // the next command meets (iss-2609091732329046). The moves are derived from // the records' current buckets, not from what THIS invocation moved, so a - // re-run after a failure here completes the repoint. A failure is reported, - // not raised: the records have moved and the close stands. - res.Relinked, err = relink.Repoint(repoRoot, closeMoves(res)) + // re-run after a failure here completes the repoint of every other file's + // links. Only a record THIS invocation moved has its own links re-read from + // the folder it left: a record an earlier run moved may have been edited + // where it is now. A failure is reported, not raised: the records have moved + // and the close stands. + res.Relinked, err = relink.Repoint(repoRoot, closeMoves(res, specMovedNow)) if err != nil { res.RelinkError = err.Error() } @@ -884,19 +888,24 @@ func Reconcile(repoRoot, specID, impact string, remainder RemainderRequest) (Rec // closeMoves names the renames a close stands for, derived from where the two // records are now: a closed spec left open/, a shipped intent left planned/. // Deriving rather than recording what this call moved is what makes the repoint -// idempotent — relink.Repoint ignores a move the tree does not show. -func closeMoves(res ReconcileResult) []relink.Move { +// idempotent — relink.Repoint ignores a move the tree does not show. Each move +// is marked MovedNow only when this call made it (specMovedNow: the spec was +// open at entry; res.IntentMoved), so a re-run never re-reads an already-moved +// record's own links from the folder it left. +func closeMoves(res ReconcileResult, specMovedNow bool) []relink.Move { var moves []relink.Move if res.Spec.Status == spec.StatusClosed { moves = append(moves, relink.Move{ - From: filepath.Join(spec.SpecsRelDir, spec.StatusOpen, filepath.Base(res.Spec.Path)), - To: res.Spec.Path, + From: filepath.Join(spec.SpecsRelDir, spec.StatusOpen, filepath.Base(res.Spec.Path)), + To: res.Spec.Path, + MovedNow: specMovedNow, }) } if res.Intent.Bucket == BucketShipped { moves = append(moves, relink.Move{ - From: filepath.Join(IntentsRelDir, BucketPlanned, filepath.Base(res.Intent.Path)), - To: res.Intent.Path, + From: filepath.Join(IntentsRelDir, BucketPlanned, filepath.Base(res.Intent.Path)), + To: res.Intent.Path, + MovedNow: res.IntentMoved, }) } return moves diff --git a/internal/core/intent/relink_test.go b/internal/core/intent/relink_test.go index 8e33c8557..8e8ad8802 100644 --- a/internal/core/intent/relink_test.go +++ b/internal/core/intent/relink_test.go @@ -47,7 +47,9 @@ func readRel(t *testing.T, root, rel string) string { // closed linking the spec about to close through ../open/, an ADR and a plan // linking the intent's planned/ path, and a draft linking both — plus the // closing spec's own links, which were written from open/ and name a still-open -// sibling bare. Closing spc-2 ships itd-10, so both records move. +// sibling bare. Closing spc-2 ships itd-10, so both records move. Two links in +// the ADR name the intent's filename in a folder it never lived in — one to an +// existing unrelated file, one dead — and neither is the move's to rewrite. func relinkFixture(t *testing.T) string { t.Helper() root := t.TempDir() @@ -66,7 +68,9 @@ func relinkFixture(t *testing.T) string { specNaming("spc-3", "other", "itd-11")+"\nSibling of [spc-2](spc-2-rest.md).\n") writeFile(t, root, ".abcd/development/decisions/adrs/0001-choice.md", "# choice\n\nSee [itd-10](../../intents/planned/itd-10-alpha.md) and [the spec](../../specs/open/spc-2-rest.md).\n"+ - "Unmoved: [itd-11](../../intents/planned/itd-11-other.md).\n") + "Unmoved: [itd-11](../../intents/planned/itd-11-other.md).\n"+ + "Same name, other file: [x](../../archive/itd-10-alpha.md). Same name, dead: [y](../../gone/itd-10-alpha.md).\n") + writeFile(t, root, ".abcd/development/archive/itd-10-alpha.md", "# an unrelated record with the moved intent's name\n") writeFile(t, root, ".abcd/development/plans/2026-01-01-plan.md", "# plan\n\n- [itd-10](../intents/planned/itd-10-alpha.md)\n\n[ref]: ../intents/planned/itd-10-alpha.md\n") writeFile(t, root, draftsDir+"/itd-12-draft.md", @@ -82,11 +86,11 @@ func relinkFixture(t *testing.T) string { // in the same operation — the already-closed sibling spec included. func TestReconcileRepointsLinksToTheMovedRecords(t *testing.T) { root := relinkFixture(t) - // The fixture's one deliberate dead link is the baseline: it is not the - // close's to repair, and it proves the rewrite leaves a link alone when its - // target never resolved. - if got := linksResolveFindings(t, root); len(got) != 1 { - t.Fatalf("fixture baseline: want exactly the one deliberate dead link, got %+v", got) + // The fixture's two deliberate dead links are the baseline: they are not + // the close's to repair, and they prove the rewrite leaves a link alone when + // its target never resolved. + if got := linksResolveFindings(t, root); len(got) != 2 { + t.Fatalf("fixture baseline: want exactly the two deliberate dead links, got %+v", got) } res, err := Reconcile(root, "spc-2", "", RemainderRequest{}) @@ -112,8 +116,13 @@ func TestReconcileRepointsLinksToTheMovedRecords(t *testing.T) { } got := linksResolveFindings(t, root) - if len(got) != 1 || !strings.Contains(got[0].Message, "spc-9-gone.md") { - t.Fatalf("after spec close only the pre-existing dead link may remain; links_resolve found %+v", got) + if len(got) != 2 { + t.Fatalf("after spec close only the two pre-existing dead links may remain; links_resolve found %+v", got) + } + for _, f := range got { + if !strings.Contains(f.Message, "spc-9-gone.md") && !strings.Contains(f.Message, "gone/itd-10-alpha.md") { + t.Errorf("after spec close only the two pre-existing dead links may remain; links_resolve found %+v", f) + } } for rel, want := range map[string][]string{ @@ -121,7 +130,7 @@ func TestReconcileRepointsLinksToTheMovedRecords(t *testing.T) { specsClosed + "/spc-2-rest.md": {"(../../intents/shipped/itd-10-alpha.md#acceptance-criteria)", "(spc-1-alpha.md)", "(../open/spc-3-other.md)", "(spc-9-gone.md)"}, specsOpen + "/spc-3-other.md": {"(../closed/spc-2-rest.md)"}, shippedDir + "/itd-10-alpha.md": {"(../../specs/closed/spc-2-rest.md)"}, - ".abcd/development/decisions/adrs/0001-choice.md": {"(../../intents/shipped/itd-10-alpha.md)", "(../../specs/closed/spc-2-rest.md)", "(../../intents/planned/itd-11-other.md)"}, + ".abcd/development/decisions/adrs/0001-choice.md": {"(../../intents/shipped/itd-10-alpha.md)", "(../../specs/closed/spc-2-rest.md)", "(../../intents/planned/itd-11-other.md)", "[x](../../archive/itd-10-alpha.md)", "[y](../../gone/itd-10-alpha.md)"}, ".abcd/development/plans/2026-01-01-plan.md": {"(../intents/shipped/itd-10-alpha.md)", "[ref]: ../intents/shipped/itd-10-alpha.md"}, draftsDir + "/itd-12-draft.md": {"(../shipped/itd-10-alpha.md)", "(../../specs/closed/spc-2-rest.md)"}, ".abcd/work/CONTEXT.md": {"(../development/intents/shipped/itd-10-alpha.md)"}, @@ -154,6 +163,41 @@ func TestReconcileRerunRepointsWhatAnEarlierCloseLeft(t *testing.T) { } } +// A re-run reads the records it did NOT move where they are: a bare link added +// to the closed spec after its close names a file in closed/, and the re-run +// leaves it as written rather than re-reading it from the open/ folder the spec +// left — where a file of the same name also exists. +func TestReconcileRerunLeavesTheClosedRecordsOwnLinksAlone(t *testing.T) { + root := relinkFixture(t) + writeFile(t, root, specsOpen+"/README.md", "# open specs\n") + writeFile(t, root, specsClosed+"/README.md", "# closed specs\n") + writeFile(t, root, plannedDir+"/README.md", "# planned intents\n") + writeFile(t, root, shippedDir+"/README.md", "# shipped intents\n") + if _, err := Reconcile(root, "spc-2", "", RemainderRequest{}); err != nil { + t.Fatal(err) + } + specLine := "\nSee [the closed index](README.md).\n" + intentLine := "\nSee [the shipped index](README.md).\n" + spec := readRel(t, root, specsClosed+"/spc-2-rest.md") + specLine + intent := readRel(t, root, shippedDir+"/itd-10-alpha.md") + intentLine + writeFile(t, root, specsClosed+"/spc-2-rest.md", spec) + writeFile(t, root, shippedDir+"/itd-10-alpha.md", intent) + + res, err := Reconcile(root, "spc-2", "", RemainderRequest{}) + if err != nil { + t.Fatal(err) + } + if len(res.Relinked) != 0 || res.RelinkError != "" { + t.Fatalf("a re-run must rewrite nothing it did not move: %+v %q", res.Relinked, res.RelinkError) + } + if got := readRel(t, root, specsClosed+"/spc-2-rest.md"); got != spec { + t.Errorf("the closed spec's own link was rewritten:\n%s", got) + } + if got := readRel(t, root, shippedDir+"/itd-10-alpha.md"); got != intent { + t.Errorf("the shipped intent's own link was rewritten:\n%s", got) + } +} + // Planning moves the draft drafts/ -> planned/, and every link that named the // draft's path follows it (iss-2609250846525896). func TestPlanRepointsLinksToTheDraft(t *testing.T) { diff --git a/internal/core/relink/relink.go b/internal/core/relink/relink.go index d227f2ec7..475ab3cd7 100644 --- a/internal/core/relink/relink.go +++ b/internal/core/relink/relink.go @@ -19,6 +19,12 @@ // closing spec's bare link to a sibling still in open/; // - a link from a moved record to another record moved in the same operation. // +// The second and third classes read the moved record's links from the folder it +// left, which is sound only when the caller moved it in this operation +// (Move.MovedNow). A move an earlier operation made — a verb's re-run — still +// repoints the first class, but the moved record's own links are read where it +// is, because it may have been edited there since. +// // A link is rewritten only when it resolved before the move, so a link that // never pointed anywhere stays exactly as written: repairing that is not the // move's business, and a rewrite would disguise it. Links inside fenced code @@ -88,6 +94,14 @@ var ( type Move struct { From string `json:"from"` To string `json:"to"` + // MovedNow says the caller made this rename in the same operation, so the + // moved file's own relative links were written from From's folder and are + // re-relativised against To's. A move an earlier operation made (a verb's + // re-run finding the record already moved) leaves it false: the file may + // have been edited in its new folder since, so its own links are read from + // where it is, and only other files' links to From are repointed. The zero + // value is the conservative reading. + MovedNow bool `json:"moved_now,omitempty"` } // Rewrite is one link destination this package changed. @@ -108,7 +122,8 @@ type Rewrite struct { // line order. It runs AFTER the moves: a move whose destination is absent, or // whose source is still present, did not happen as described and is ignored, // which is also what makes a second call over the same moves a no-op — a verb -// may re-run it on a retry without double-rewriting anything. +// may re-run it on a retry without double-rewriting anything. A moved file's own +// links are re-relativised only for a move marked MovedNow; see Move. // // A move naming a path that is not a clean repo-relative slash path is refused // before anything is read. An error part-way through the walk returns the @@ -169,7 +184,10 @@ func Repoint(repoRoot string, moves []Move) ([]Rewrite, error) { // activeMoves validates the moves and keeps the ones the tree shows happened: // the destination is present and the source is gone. It returns the forward map -// (old path → new) and its inverse (new path → old). +// (old path → new) and the inverse (new path → old) for the moves made in this +// operation only (MovedNow): the inverse is what re-reads a moved file's own +// links from the folder it left, which is sound only for a file the caller has +// just moved. func activeMoves(root *os.Root, moves []Move) (map[string]string, map[string]string, error) { movedTo := map[string]string{} oldPath := map[string]string{} @@ -188,7 +206,9 @@ func activeMoves(root *os.Root, moves []Move) (map[string]string, map[string]str continue } movedTo[from] = to - oldPath[to] = from + if m.MovedNow { + oldPath[to] = from + } } return movedTo, oldPath, nil } diff --git a/internal/core/relink/relink_test.go b/internal/core/relink/relink_test.go index 0e79b47ff..7b7033928 100644 --- a/internal/core/relink/relink_test.go +++ b/internal/core/relink/relink_test.go @@ -51,12 +51,16 @@ func TestRepointRewritesLinksToAndFromTheMovedFile(t *testing.T) { root := t.TempDir() write(t, root, openA, "[b](b.md) [gone](nope.md) [self](a.md#x) [web](https://example.com/a.md) [top](#top)\n") write(t, root, "specs/open/b.md", "[a](a.md) [a again](./a.md#part)\n") - write(t, root, "specs/closed/c.md", "[a](../open/a.md)\n") + // Two links name a file called a.md that is not the moved record: one that + // exists in another folder and one that never resolved. Neither is the + // move's, so both stay exactly as written. + write(t, root, "specs/closed/c.md", "[a](../open/a.md) [x](../archive/a.md) [y](../gone/a.md)\n") + write(t, root, "specs/archive/a.md", "an unrelated record with the same name\n") write(t, root, "docs/guide.md", "See [a](../specs/open/a.md \"title\").\n\n[a-ref]: ../specs/open/a.md\n\n```\n[fenced](../specs/open/a.md)\n```\n") write(t, root, "docs/other.md", "[b](../specs/open/b.md) mentions a.md only in prose\n") move(t, root, openA, closedA) - got, err := Repoint(root, []Move{{From: openA, To: closedA}}) + got, err := Repoint(root, []Move{{From: openA, To: closedA, MovedNow: true}}) if err != nil { t.Fatal(err) } @@ -64,7 +68,7 @@ func TestRepointRewritesLinksToAndFromTheMovedFile(t *testing.T) { want := map[string]string{ closedA: "[b](../open/b.md) [gone](nope.md) [self](a.md#x) [web](https://example.com/a.md) [top](#top)\n", "specs/open/b.md": "[a](../closed/a.md) [a again](../closed/a.md#part)\n", - "specs/closed/c.md": "[a](a.md)\n", + "specs/closed/c.md": "[a](a.md) [x](../archive/a.md) [y](../gone/a.md)\n", "docs/guide.md": "See [a](../specs/closed/a.md \"title\").\n\n[a-ref]: ../specs/closed/a.md\n\n```\n[fenced](../specs/closed/a.md)\n```\n", "docs/other.md": "[b](../specs/open/b.md) mentions a.md only in prose\n", } @@ -88,7 +92,7 @@ func TestRepointRewritesLinksToAndFromTheMovedFile(t *testing.T) { } // A second call over the same moves finds nothing left to do. - again, err := Repoint(root, []Move{{From: openA, To: closedA}}) + again, err := Repoint(root, []Move{{From: openA, To: closedA, MovedNow: true}}) if err != nil || len(again) != 0 { t.Fatalf("a re-run must be a no-op: %+v, %v", again, err) } @@ -103,8 +107,8 @@ func TestRepointFollowsTwoMovesAtOnce(t *testing.T) { move(t, root, "intents/planned/i.md", "intents/shipped/i.md") if _, err := Repoint(root, []Move{ - {From: "specs/open/s.md", To: "specs/closed/s.md"}, - {From: "intents/planned/i.md", To: "intents/shipped/i.md"}, + {From: "specs/open/s.md", To: "specs/closed/s.md", MovedNow: true}, + {From: "intents/planned/i.md", To: "intents/shipped/i.md", MovedNow: true}, }); err != nil { t.Fatal(err) } @@ -116,6 +120,36 @@ func TestRepointFollowsTwoMovesAtOnce(t *testing.T) { } } +// A move an earlier call made (MovedNow false — a verb's re-run finding the +// record already in its new folder) still repoints every OTHER file's link to +// the old path, so a re-run finishes an interrupted repoint; but the moved +// file's own links are read from the folder it is in, because they may have +// been written there since. Re-reading them from the folder it left would +// resolve a bare link to a different file of the same name. +func TestRepointReadsAnEarlierMovesOwnLinksWhereTheFileIs(t *testing.T) { + root := t.TempDir() + write(t, root, "specs/open/README.md", "# open\n") + write(t, root, "specs/closed/README.md", "# closed\n") + own := "[index](README.md) [b](../open/b.md)\n" + write(t, root, closedA, own) + write(t, root, "specs/open/b.md", "[a](a.md)\n") + + got, err := Repoint(root, []Move{{From: openA, To: closedA}}) + if err != nil { + t.Fatal(err) + } + if g := read(t, root, closedA); g != own { + t.Errorf("the moved file's own links were re-read from the folder it left: %q", g) + } + if g := read(t, root, "specs/open/b.md"); g != "[a](../closed/a.md)\n" { + t.Errorf("a stale link to the old path was not repointed: %q", g) + } + want := []Rewrite{{File: "specs/open/b.md", Line: 1, From: "a.md", To: "../closed/a.md"}} + if !reflect.DeepEqual(got, want) { + t.Fatalf("rewrites:\n got %+v\nwant %+v", got, want) + } +} + // The walk stays out of git's directory, the local tier and a nested checkout, // whose files belong to another working tree, and writes nothing into the two // append-only logs whose gates refuse an in-place edit. @@ -131,7 +165,7 @@ func TestRepointStaysOutOfForeignTreesAndAppendOnlyLogs(t *testing.T) { write(t, root, ".abcd/work/reviews/2026-01-01-x/00-summary.md", link) move(t, root, openA, closedA) - got, err := Repoint(root, []Move{{From: openA, To: closedA}}) + got, err := Repoint(root, []Move{{From: openA, To: closedA, MovedNow: true}}) if err != nil { t.Fatal(err) } @@ -154,7 +188,7 @@ func TestRepointIgnoresMovesThatDidNotHappenAndRefusesUnsafePaths(t *testing.T) write(t, root, openA, "a\n") write(t, root, "specs/open/b.md", "[a](a.md)\n") - got, err := Repoint(root, []Move{{From: openA, To: closedA}}) + got, err := Repoint(root, []Move{{From: openA, To: closedA, MovedNow: true}}) if err != nil || len(got) != 0 { t.Fatalf("a move that did not happen must rewrite nothing: %+v, %v", got, err) } From 320ea7c1aac7b845bdfbc0ee50930e835c2c9afb Mon Sep 17 00:00:00 2001 From: REPPL <77722411+REPPL@users.noreply.github.com> Date: Fri, 25 Sep 2026 11:16:02 +0100 Subject: [PATCH 5/5] fix: say what a re-run of a failed spec close repoints Review 2 of the lane found the spec close warning, the RelinkError comment and the two docs still promising that a re-run finishes the repoint. After the MovedNow change a re-run repoints the links other files hold and leaves the moved records' own links for links_resolve, and that holds for an attempt that failed before the repoint as well as during it. Assisted-by: Claude:claude-opus-5-5 --- .abcd/development/brief/04-surfaces/05-intent.md | 2 +- commands/intent.md | 6 +++--- internal/core/intent/intent.go | 3 ++- internal/surface/cli/cli.go | 2 +- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/.abcd/development/brief/04-surfaces/05-intent.md b/.abcd/development/brief/04-surfaces/05-intent.md index 1d284cdc7..680b67340 100644 --- a/.abcd/development/brief/04-surfaces/05-intent.md +++ b/.abcd/development/brief/04-surfaces/05-intent.md @@ -451,7 +451,7 @@ The invariants below are the contract the tree is held to, and each names what h - Every intent in `drafts/` has `spec_id: null` (drafts have no plan yet). - Every intent in `planned/` has `spec_id: null` (unscheduled) or a `spc-N` id; a non-null `spec_id` points to an existing native-spec-store `-*.md` whose frontmatter `intent` field matches the intent's `id` (or contains the intent's `id` as one of a list, for bundle-member intents). - **An intent owns one or more specs, and it ships when its last spec closes.** The intent↔spec relation is 1:n (invariant 17 in [`02-constraints/03-invariants.md`](../02-constraints/03-invariants.md), per [adr-2609151513118583](../../decisions/adrs/2609151513118583-an-intent-owns-one-or-more-specs-and-it-ships-when-its-last.md)). The spec's own `intent:` field is the source of truth for the link: the intent's scalar `spec_id` names the spec it was planned with, and the set of specs realising an intent is derived from the back-links (`spec.Store.SpecsForIntent`, `lint.SpecLinkIndex.SpecsForIntent`) — no field carries a list. The bidirectional check is therefore membership, not equality: a spec naming an intent is clean when that intent's `spec_id` names *some* spec realising it (`spec_lifecycle`), so a remainder spec is not drift. Closing a spec ships the intent only when no open spec is left naming it; a remainder slug given on the close mints the follow-on spec in the same operation, and the impact is demanded at the close that ships and refused at any earlier one. The release cut's stale-intent refusal asks whether a planned intent has any OPEN spec, never whether its spec has closed — a planned intent with one closed and one open spec is the correct steady state of a partial delivery. -- **A move repoints the links that named the moved record.** An intent's and a spec's folder is its status, so planning (`drafts/ → planned/`) and closing (`open/ → closed/`, and on the close that ships `planned/ → shipped/`) are renames, and the verb that renames is the one place that knows both paths. It rewrites every relative markdown link in the tree that named an old path, from any folder — a spec already closed pointing at `../open/`, an ADR or a plan naming the intent's `planned/` path, a draft naming both, and the moved record's own links, written from the folder it left — through the one link-repoint primitive (`core/relink`) the ledger's resolve and wontfix share. A link that never resolved is left as written. The result lists each rewrite (`relinked`), so the close leaves a tree record-lint's `links_resolve` accepts, with no hand survey. The close derives its moves from where the records are now, so a re-run repoints every link other files still hold to an old path; it re-reads a record's own links from the folder it left only when that same run moved the record, because a record an earlier run moved may have been edited where it is (a bare `README.md` link names the `closed/` index, not the `open/` one). A repoint failure is a warning, never a failed close. +- **A move repoints the links that named the moved record.** An intent's and a spec's folder is its status, so planning (`drafts/ → planned/`) and closing (`open/ → closed/`, and on the close that ships `planned/ → shipped/`) are renames, and the verb that renames is the one place that knows both paths. It rewrites every relative markdown link in the tree that named an old path, from any folder — a spec already closed pointing at `../open/`, an ADR or a plan naming the intent's `planned/` path, a draft naming both, and the moved record's own links, written from the folder it left — through the one link-repoint primitive (`core/relink`) the ledger's resolve and wontfix share. A link that never resolved is left as written. The result lists each rewrite (`relinked`), so the close leaves a tree record-lint's `links_resolve` accepts, with no hand survey. The close derives its moves from where the records are now, so a re-run repoints every link other files still hold to an old path; it re-reads a record's own links from the folder it left only when that same run moved the record, because a record an earlier run moved may have been edited where it is (a bare `README.md` link names the `closed/` index, not the `open/` one). A repoint failure is a warning, never a failed close; the moved records' own links an attempt that failed before or during the repoint left unrewritten are ones `links_resolve` names, to repair by hand. - **A bundle is the opposite relation and is untouched.** `kind: bundle-member` with a `bundle:` link is N:1 — several intents sharing one spec — and the bundle invariant above (all members in one phase) still holds. 1:n and N:1 are different relations, not two names for one thing; composing them into N:M is not authorised by anything in the record. An intent's own specs may sit in different phases, because the reason a second spec exists is that the work did not fit the cycle that carried the first. - Every intent in `shipped/` has `kind` set (`standalone` or `bundle-member`) and a non-null `spec_id`. (The stronger invariant — the linked spec exists and is closed, or `spec_id: null` + a `manual_ship_reason` for the no-spec case — is a later-phase gate; the shipped rule checks only that `spec_id` is non-null.) - Discipline-kind intents have `spec_id: null` always (disciplines never get a spec; this is structurally enforced). diff --git a/commands/intent.md b/commands/intent.md index c7c2357fc..d5cabfe0f 100644 --- a/commands/intent.md +++ b/commands/intent.md @@ -405,9 +405,9 @@ resolved is left as written. The JSON lists each rewrite under `relinked` (`file`, `line`, `from`, `to`), and the text render prints them; report them, because they are files the close changed beyond the two records. The tree the close leaves passes record-lint's `links_resolve` with no hand repair. If the -repoint fails part-way, the close still stands and a warning on stderr says so; -re-running the same `spec close` repoints every link other files still hold to -either old path. The re-run reads the moved records' own links from the folders +repoint fails part-way, the close still stands and a warning on stderr says so. +After an attempt that failed before or during the repoint, re-running the same +`spec close` repoints every link other files still hold to either old path. The re-run reads the moved records' own links from the folders they are in, because they may have been edited there since the move, so it never rewrites them; any of those the failed attempt left unrewritten is one `links_resolve` names, to repair by hand. diff --git a/internal/core/intent/intent.go b/internal/core/intent/intent.go index 45219551d..9f106ab68 100644 --- a/internal/core/intent/intent.go +++ b/internal/core/intent/intent.go @@ -306,7 +306,8 @@ type ReconcileResult struct { Relinked []relink.Rewrite `json:"relinked,omitempty"` // RelinkError is a NON-FATAL report of a repoint that failed part-way: the // records have moved and the close stands, so the surface prints it loudly - // and a re-run of the close completes the repoint. + // and a re-run of the close repoints the links other files still hold. The + // moved records' own links it leaves as written, for links_resolve to name. RelinkError string `json:"relink_error,omitempty"` } diff --git a/internal/surface/cli/cli.go b/internal/surface/cli/cli.go index cb4b36465..f3b8b4852 100644 --- a/internal/surface/cli/cli.go +++ b/internal/surface/cli/cli.go @@ -2552,7 +2552,7 @@ func newSpecCommand(asJSON *bool) *cobra.Command { if res.AuditEmitError != "" { fmt.Fprintf(cmd.ErrOrStderr(), "WARNING: abcd spec close — fidelity-review emit failed for %s (intent shipped anyway): %s\n", res.Intent.ID, res.AuditEmitError) } - emitRelinkError(cmd.ErrOrStderr(), "spec close", res.RelinkError, "re-run `abcd spec close "+args[0]+"` to finish the repoint") + emitRelinkError(cmd.ErrOrStderr(), "spec close", res.RelinkError, "re-run `abcd spec close "+args[0]+"` to repoint the links other files hold; record-lint's links_resolve names each link left behind") return render(cmd.OutOrStdout(), *asJSON, res, func(w io.Writer) { fmt.Fprintf(w, "abcd spec close — %s open -> closed\n %s\n", res.Spec.ID, termsafe.Sanitize(res.Spec.Path)) if res.Remainder.ID != "" {