mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-11 13:28:57 +00:00
xarmian
69a6ebfe69
* fix(links,store): make the rename cascade escape-aware in both directions (BUG-2805) Item titles containing `]`, `|` or `\` did not round-trip through a rename. Repro through the real API on TASK-2826 (Rook) with hex receipts; this fixes both directions plus a third half that repro did not name. Direction 1 — MATCHING. The rewriter compared the RAW bracket body against the unescaped title, so a link stored as `[[Weird \] Title]]` never matched. The link was left naming the OLD title while the reparse flipped its index row to broken: content and index disagreeing, with the staleness visible to the user. Matching now runs on the UNESCAPED body, full-body first and then the split-on-unescaped-pipe form — the same preference order the parser and the renderer use, which is what keeps literal-pipe titles like "A|B" working. Direction 2 — EMISSION. The rewriter emitted the new title by plain concatenation. A title containing `]` produced a bracket the grammar cannot parse, so the reparse found no link and DELETED the index row — permanent damage from an ordinary rename, since a later rename has no row left to cascade. A title containing `\]` produced valid syntax for a DIFFERENT title. Emission now escapes, byte-for-byte the same rule as the editor's escapeWikiBody (markdown.ts:748). Third half, not in the filing or the repro — the close-scan. The rewriter found its closing `]]` with strings.Index, which is not the grammar: in `[[A\]]]` it stops at the `]` belonging to the `\]` escape. That was latent only while the rewriter never emitted escapes; this fix makes escaped bodies routine, so each rename would have corrupted what the last one wrote. scanBracketBody now implements the parser's body production exactly. Allocation discipline carried forward from BUG-2804 R5: escaping happens ONCE per cascade via a TitleEscaper, not per source and not per bracket. A cascade renames one item to one title, but the projection and rewrite run once per source and the scan bound admits very many sources when their titles are short — escaping inside those calls would multiply an unbounded title by an unbounded source count, which is the R5 defect in a new costume. The projection path measures with escapedWikiBodyLen and builds nothing. The frozen V0 oracle is now scoped rather than blanket: it stays authoritative for bodies with no escape characters, and every deliberate departure is listed by name with its reason in TestRewriteBracketAt_IntentionalDivergencesFromV0. One departure is outside BUG-2805's scope and called out there: an empty body `[[]]` is not a link under the parser's `+` production, so the scanner refuses it where V0 would have MINTED a link from a non-link on a drifted offset. Two accidental compatibilities the repro identified as non-reproducing directions are pinned so the fix cannot trade two silent successes for two silent failures: the legacy full-body-with-pipe fallback, and unescapeWikiBody's leniency toward a stray backslash. Instruments: a 4000-iteration property that every emitted title parses back to itself; a scanner/parser parity property; end-to-end cascade tests for both directions including a PERMANENCE check, since a fix that merely delayed the index-row destruction by one rename would otherwise pass. Gates: gofmt, vet, go build ./... clean; go test ./... PASS on SQLite and Postgres; go test -race PASS on internal/links and internal/store; zero failures, zero data races. * test(links): lock the scanner's delimiter edges + correct a stale renderer citation (BUG-2805) Codex R1 named scanBracketBody's edges as a review priority. Probing them against the parser as oracle found no disagreement — all ten shapes agree, including trailing backslash at EOF, `]]]` runs, an escape consuming the would-be close, and the two where BOTH refuse (`[[]]`, and a bare `]` mid-body). Locked in one assertion per shape so a failure names WHICH edge broke rather than printing a random string. Separately, and more substantive: extract.go carried a stale cross-file claim. It said renderMarkdown at markdown.ts:300 uses a simpler `[^\]]+` regex and therefore REJECTS escaped bodies, making escaped links index-only and never clickable. That is no longer true. markdown.ts:300 is inside a doc comment now, and BOTH wiki-link regexes in that file — renderMarkdown at :326 and wikiLinksToMarkdown at :625 — use this package's exact escape-aware production; the renderer comment at :323-325 names BUG-1744 as the change that aligned them. The staleness is load-bearing for BUG-2805 in the direction that makes the bug WORSE: the stale link a rename left behind was a working, clickable link rather than an invisible one. It had also been quoted forward into TASK-2826's repro as a live constraint, which is how a stale comment turns into a shared wrong premise. Verified by reading both regexes rather than by re-citing the comment. * fix(links): refuse empty-title emission; agree with the parser on backslash-LF (BUG-2805, codex R2) Codex R2, triaged per finding with a receipt. The orchestrator did not spot-check these, so each was confirmed or refuted here first. P1 — CONFIRMED as a test weakness, REFUTED as irrecoverable. The added test counted index rows, so it would have passed with the row present and target_item_id cleared. Measured: after a rename to "" the content survives, the row survives, target_item_id goes NULL, and renaming back RESTORES it. So the broken state is correct rather than damage — no title-form link can resolve to an item with no title, and NULL is exactly what a renderer would resolve — and it is recoverable precisely because the CONTENT was never destroyed. The test now asserts the full post-state plus the recovery leg, which is what distinguishes "broken but honest" from "irrecoverable". Separately and genuinely destructive, found while probing the new TitleEscaper's zero value: an empty new title emitted `[[]]`, which is not a link under the parser's `+` production, so the reparse DELETED the row. Reachable two ways — a zero-value escaper, and UpdateItem accepting a rename to "" because the empty-title guard lives only in handleCreateItem. The rewriter now refuses to emit an empty title segment in both the build and the projection path, kept in lockstep. The door-level validation gap is BUG-2833. P2 backslash-LF — CONFIRMED and introduced by this diff. Go's regexp `.` does not match a newline, so `\\.` cannot consume one and the parser rejects the body; scanBracketBody treated backslash-ANY as a pair and accepted it. Now excludes LF only, which is what the Go parser does. Measured both ways before and after. Establishing that surfaced a PRE-EXISTING divergence, filed as BUG-2834: the Go and JS wiki-link regexes are byte-identical source text, but `.` excludes only LF in Go and all four line terminators in JS (measured with node), so Go indexes a backslash-CR link the renderer will not render. P2 qualified-vs-literal — PRE-EXISTING. Byte-identical output between the frozen V0 oracle and current for both slug variants, so this diff neither introduced nor changed it. Matches BUG-2830's mechanism, already filed. P2 cross-workspace parity — PRE-EXISTING. Two receipts: extract.go's diff against main is comment-only, and no line of this diff touches cross-workspace parsing. Gates: gofmt, vet, go build ./... clean; go test ./... PASS on SQLite and Postgres; go test -race PASS on internal/links and internal/store; zero failures, zero data races.