Repository navigation
feat(engine): Tier-0 hygiene filters for --auto-chain with vault-root bridge (PAL-205-B) - #214
Merged
Merged
Conversation
…ts (#73) resolveWikilinkTarget decides which file `palee roadmap` reads and rewrites frontmatter into, so three of its behaviours were data-loss routes: - the bare target was a path candidate, so `![[assets/diagram.png]]` resolved to the PNG and the importer overwrote it with YAML frontmatter; - a resolved path escaping the vault `continue`d into the basename lookup, silently rewriting a different note that merely shares a basename; - path matching ignored the dot-namespace / `node_modules` exclusions the rest of the CLI treats as invisible, so `[[.trash/deleted-note]]` was importable. Branch 1 now offers only the `.md` candidate, throws instead of falling through on an escape, and gates the hit on a new shared `isResolvableNotePath` predicate exported from `vault-walker` — the rule lives in the layer that already owns vault visibility, so it stays next to `walkVault`'s exclusion list. `isWithinVault`'s logic is untouched; only its consequence changed. Tests: 4 new cases (non-markdown, embed, dot-namespace, escape-hijack) plus a repaired `rejects vault-escape targets`, whose old fixture had no outside file at all and so never reached the containment check. All 5 verified to fail against the pre-fix resolver. The out-of-vault fixture lives in a sibling `mkdtempSync` dir that `after()` removes.
…#73) parseRoadmapContent classified any .md with a heading and one [[...]] bullet as a wikilink roadmap, so `palee roadmap --from <ordinary note>` imported the note's outbound links as a curriculum and rewrote depends_on on every note it pointed at. A daily-log import exited 0 and inverted a learner's hand-authored prerequisite graph. The document must now declare itself: branch 4 runs only when frontmatter sets palee_roadmap: true (strict, real YAML boolean). An unmarked .md falls through to the existing "topics array" rejection and exits 2 with zero writes; a marked one with no list items gets a new specific error. This restores the pre-#204 rejection and matches how adopt already keys a note's role off palee_ owned frontmatter. The frontmatter parse is hoisted rather than re-run, so parseRoadmapContent still calls parseFrontmatter once per input, and format selection keeps exactly one home in the parser.
parseWikilinkSections accepted heading levels 1 through 6, but every section head it emits is written downstream with `depends_on: []`, and an explicit [] clears a note's existing prerequisites. So one track authored as `## Junior Track` with `### Part 1` / `### Part 2` inside it became three independent chains and silently wiped the curated depends_on on the first note of every sub-heading. The docstring said `## Track`; the regex did not implement it. Decision on what a `###` line does: it is not a heading at all here, so it is inert and its bullet lines keep extending the enclosing `##` section. That is the intent -- a sub-heading is structure inside one track, not a new chain. `### ` is also not matched by the bullet regex, verified against both bullet branches, so a sub-heading cannot leak a link into the chain either. `^##(?!#)` keeps the leading `^\s*` the old regex had, so indented `## ` still counts; `(?!#` is what stops `### Part 2` from parsing as `##` with a truncated track name. Deeper levels are excluded deliberately and the @remarks block now says so. Task-list items are rejected too: `- [ ]` / `- [x]` render as Obsidian checkboxes, which are unfinished to-dos rather than curated chain entries, and importing one rewrites that note's prerequisites. Nothing in docs/02-1 s4 or INV-48 lists them as valid roadmap syntax. The check belongs in the bullet regex, not in extractWikilinks, because deciding what counts as a chain entry is a storage-layer question -- the engine scanner stays a pure `[[...]]` parser. Tests: 3 new cases in the wikilink describe block (sub-heading grouping, non-`##` levels never minting a track, task items ignored). Each was verified to fail before the fix -- the first reported 3 sections instead of 1, the second a phantom `Sub` track, the third `todo`/`done` links that should not exist. The existing `## Track` cases are unchanged, and the four palee_roadmap gate tests from the previous commit still pass.
extractWikilinks documented that "malformed or nested [[ sequences are
skipped", but WIKILINK_GLOBAL excludes [ ] | # from the target class, so
on `[[a[[b]]` the scan simply resumes later in the string and reports
`[[b]]` as a link. The nested case was not skipped, it was reinterpreted,
and through `palee roadmap` that invented a chain edge out of a typo.
The global scan cannot see this from its own match, so the check is on
the text preceding each match: if it still holds an open `[[`, the match
is nested and gets skipped. No new regex, no parser class, and the
lastIndex reset in the finally block is untouched. Behaviour elsewhere is
unchanged -- later well-formed links after a nested one still extract
(`[[a[[b]] and [[c]]` yields just `c`), aliases and `#heading` anchors
still parse, and a trailing `[[broken` is still dropped. parseWikilink
already returned null for `[[a[[b]]`; the two functions now agree.
Tests: 1 new case asserting extractWikilinks('- [[a[[b]]') returns [],
verified to fail before the fix with [{ target: 'b' }].
`roadmap --auto-chain` ran regardless of the roadmap format, so a two-track wikilink roadmap wrote the second track's head onto the first track's tail: `resolveWikilinkRoadmap` emits `depends_on: []` for a section head meaning "this chain is complete", and `applyRoadmapAutoChain` read the same `[]` as "not chained yet". Independent learning tracks fused into one chain in the vault, on disk. The root cause is one overload at the type boundary, so the field carries the intent instead: `RoadmapTopic.chained` is set by the wikilink resolver on every topic it emits, and the auto-chain pass keys off `!topic.chained` rather than off emptiness. The call site also hoists the guard to skip the pass entirely for the wikilink format. Nothing about a user-authored YAML roadmap changes — the #137 rule (a present `[]` clears, an absent field preserves) is untouched. The `chained` key never reaches disk: the import writes frontmatter only from `resolveTopicUpdates`' explicit key set, verified by importing all three roadmap formats with `--auto-chain` and grepping the vault. Two new CLI tests: the two-track fusion regression, and a numeric-vs-lexicographic `order` discriminator pinning INV-47's "unordered topics sort last". INV-47 records the wikilink exemption in the same bullet; the stale "reproduces the same chain" remark in `resolveWikilinkRoadmap` is corrected.
Step 0 of plan 006 resolved to variant A (authorized 2026-09-22): implement the bridging that CHANGELOG, docs/03-2 and docs/02-1 already promise, rather than de-scoping the claim. The auto-chain plan was computed only over notes being adopted, so the chain restarted at the first note of every batch and incremental folder-by-folder adoption produced a forest of disconnected chains with no user-visible signal. The planner input is now the union of the notes to adopt and the notes already adopted inside the scanned scope, with the adopted ones' ids seeded from loadTopics (LoadedTopic.id) so an existing note keeps the id it has on disk and never receives a freshly minted one. Writes stay restricted to the notes being adopted: chainDependsOn is derived only for toAdopt paths, so an already-adopted note acts as a predecessor but is never rewritten. --excluded notes do not join the plan at all, so the two sets stay distinct. The dry-run previews the bridged edge, and the edge count still reports only edges actually written. Cycle checking is unchanged and still runs over the merged graph before any write, so the accepted tradeoff fails closed with exit 3 and zero writes. Test: already-adopted notes in scope are bridged over and never rewritten (byte-identical pre-adopted note, module-boundary continuation, honest edge count, dry-run preview). The existing 'already-adopted notes keep their hand-written depends_on' test's second assertion pinned the old restart behaviour this change removes, so it now asserts the bridge; its namesake assertion about the adopted note's own depends_on is unchanged.
…uards (#73) One declaration of WikilinkRoadmapSection: the duplicate in src/storage/wikilink.ts is deleted and the type is imported from src/storage/roadmap-parser.ts, the module that produces sections. It stays re-exported from wikilink.ts because test/storage-wikilink.test.ts imports the type from there and this plan may not edit that file's imports. One vault-escape predicate: isWithinVault() is now exported from src/storage/wikilink.ts and src/cli/roadmap.ts's two inline copies (the validation loop and the import loop) call it. The recommended home in src/storage/vault-walker.ts is out of this change's scope, so the helper lives in the file whose form the plan named as the most complete. vault-walker.ts is untouched: its walk() check inspects a relative path built inside the symlink branch, not a caller-supplied absolute one, and Step 2 forbids contorting a shared helper to cover a differently-shaped site; ensureVaultDirectory()'s pre-existing copy is likewise left alone. Basename helpers kept, not deleted: path.basename() is not a drop-in substitute for targetBaseName()/baseNameOf() on win32 — it also splits on '\', so [[notes\01-a]] would stop failing closed and start matching the note named 01-a, and it strips a trailing separator ('dir/' -> 'dir' where the helpers yield ''). Verified with node on this platform. The engine also stays free of a `path` import (agent.md: fs-free layer). Exit codes: the #73 numerics now use ExitCode.Usage / ExitCode.Validation. In src/cli/roadmap.ts every remaining bare literal in the handler was converted too, because leaving '= 3' or '= 2' beside a newly named constant in the same function is the inconsistency Step 3 carves out. The ten pre-existing '= 2' usage guards in src/cli/adopt.ts are outside the auto-chain blocks and are left for a dedicated sweep. Also in this commit: the resolveNoteTitle compatibility re-export is deleted (test/cli-adopt-batch.test.ts now imports the owning module), the blank-line runs left by the note-title/topic-id extraction are collapsed, the single-element pathCandidates loop in resolveWikilinkTarget is reduced to straight-line code (behaviour-preserving), and the unreachable {@link detectCyclesBounded} reference in auto-chain.ts is now plain text naming src/engine/dependency.ts.
agent.md's "roadmap --from is YAML-only" was the highest-cost line here: it denied three of the four formats the parser accepts and every future session reads it first. It now lists YAML, frontmatter, code fence, and the wikilink format with its palee_roadmap: true gate. docs/02-1: "three curriculum formats" -> four. The Wikilink Roadmap example was a user-facing example that had become actively wrong — a bare `# DevOps Roadmap` with no frontmatter, which plan 003's gate now rejects — so it starts with the `palee_roadmap: true` block and the prose states the marker is required (boolean, not the string 'true') and that an unmarked .md exits 2 with zero writes. "Each heading starts a new chain section" -> each `##` heading, with the depends_on: [] reason. Adds the --auto-chain-does-not-apply note (INV-47) and documents `order`, the load-bearing input for roadmap --auto-chain, in the example and in prose. Re-pins three stale code links whose targets moved out of src/cli/adopt.ts (resolveNoteTitle -> src/storage/note-title.ts, the topic-id generator -> src/engine/topic-id.ts) and drops the numeric anchors on the constants that stayed in adopt.ts. Exit-code tables: `palee adopt` exit 3 was documented as N/A in both docs/02-0 and docs/02-1 even though --auto-chain exits 3 on a cycle in the planned graph. docs/03-2: the auto-chain row said "bridging modules and excluded/adopted notes", which conflates two different things; it now states what plan 006 implemented — the plan spans every note in the scanned scope, an already-adopted note acts as a predecessor and is never rewritten, and filtered-out notes join no plan at all. planning/invariants.md: INV-46 gains the bridging clause plan 006 asked for and could not apply (its own scope excluded this file); INV-48 gains the `##`-only section rule from plan 004. Both stay single INV-NN bullets, and test/planning-invariant-ids.test.ts passes. Already correct, verified and left alone: CHANGELOG.md's [Unreleased] ### Changed entry for the palee_roadmap: true gate (plan 003 landed it) and INV-47's wikilink exemption (plan 005). Nothing in these docs claims Variant A makes cycles block adoption more often — that claim is false and is not propagated here.
Removed from the published surface (src/index.ts re-exports both facades): parseWikilinkSections, buildVaultNoteIndex, resolveWikilinkTarget, ResolvedWikilink, AmbiguousWikilinkError, UnresolvedWikilinkError from src/storage/index.ts; parseNumericPrefix, compareLessonOrder from src/engine/index.ts. Survey for the next reviewer — every real consumer reaches the owning module directly, not the barrel: parseWikilinkSections via test/storage-roadmap-parser.test.ts:3 and src/storage/roadmap-parser.ts:238; buildVaultNoteIndex / resolveWikilinkTarget / the two error classes via test/storage-wikilink.test.ts:6-13 (imports '../src/storage/wikilink'); parseNumericPrefix / compareLessonOrder via test/engine-auto-chain.test.ts:3-8 (imports '../src/engine/auto-chain'). The files that do import through a barrel (test/storage-barrel-census, test/challenger1-*, test/e2e/tier*, test/session-*, test/cli-mastery-fallback) name none of the eight, and a grep of README.md and docs/ for all eight returns zero hits, so nothing documented is being taken away. AmbiguousWikilinkError and UnresolvedWikilinkError come out under the rule Step 5 sets for them: nothing catches them by name across the facade — src/cli/roadmap.ts catches `err: unknown` and prints the message — so they stay module exports for test/storage-wikilink.test.ts only. Kept, with the consumer grepped: parseRoadmapContent and resolveWikilinkRoadmap (src/cli/roadmap.ts imports both from '../storage'), resolveNoteTitle (src/cli/adopt.ts:29, src/storage/wikilink.ts:27), generateTopicId (src/cli/adopt.ts:25, src/storage/wikilink.ts:24), planAutoChain (src/cli/adopt.ts:26), parseWikilink and extractWikilinks (src/storage/roadmap-parser.ts:16, test/storage-wikilink.test.ts:14), and the WikilinkRoadmapSection type (part of the kept ParsedRoadmapResult.sections). NumericPrefix stays exported as a type — Step 5 does not list it, and the barrel rule here is to delete only what the plan names. All eight symbols were introduced by this same unreleased PR, so no published 0.5.x version ever carried them; the npm surface narrows relative to nothing. typecheck, lint, build and the full 1001-test suite are unchanged.
#73) The vault-boundary and note-visibility guards ran only inside the `fs.existsSync` branch, so an unsafe target that happened to be missing fell through to the basename lookup and resolved an unrelated in-vault note — one the user never named, whose frontmatter the roadmap importer then rewrites. `[[../../outside/note]]`, `[[.trash/note]]` and `[[node_modules/note]]` all resolved `MODULES/note.md`. Validate the lexical candidate before touching the filesystem, and keep the post-`realpathSync` checks so a symlink still cannot escape.
…cided (#73) `hasUnnumbered` inspected only the deepest directory segment, so a genuinely unnumbered *ancestor* level that decided group order went unreported: `MODULES/01-a` and `OTHER/01-b` chained alphabetically with no warning. Flag a segment level only when two group directories actually differ there. Checking every segment instead would flag the canonical unnumbered `MODULES/` container from #73's own example on essentially every vault and turn the warning into noise.
…#73) The test name implied it covered the new-to-existing bridge edge, but the fixture's cycle already exists on disk, so it passes whether or not the bridge is emitted. Name what it actually guards and record that a cycle created *by* a bridge edge is unconstructible. Assertions unchanged.
The CodeRabbit docstring pre-merge check failed at 72.50% (threshold 80%) over functions touched by this diff. Document the remaining helpers: lessonRank, compareStrings and parentDirOf in the engine, and the runCLI / freshVault / frontmatterOf / dependsOn fixtures in the CLI test suites.
Wikilink targets are `/`-separated only, but `path.resolve` treats `\` as a separator on Windows, so `[[MODULES\01-foundations\01-systems]]` resolved the exact note on Windows while throwing on POSIX — verified by probe. Reject any target containing a backslash before path resolution so resolution is identical on every platform, matching the slash-only semantics `targetBaseName` already documents. Also docstrings the error constructors and the test file's `link` helper (coverage gate).
Two routes let a path-bearing target whose exact candidate is absent fall through to the basename lookup and resolve an unrelated in-vault note: - the target names an existing non-Markdown file: `[[assets/diagram.png]]` resolved `other/diagram.png.md`, because the index keys that note under `diagram.png`; - an ancestor directory is a symlink leaving the vault and the requested file is missing inside it: `vault/link -> outside` made `[[link/note]]` resolve the in-vault `note.md`. Both were reproduced by probe before the fix. Reject an existing non-note file at the target path, and realpath the deepest existing ancestor to catch escaping symlinks, before any basename fallback.
…roadmaps (#73) `1. [ ] [[todo]]` is valid Markdown/Obsidian syntax, but only the bullet branch carried the `(?!\[[ xX]\]\s)` lookahead, so the ordered branch captured `[ ] [[todo]]` and imported the to-do as a chain entry — rewriting that note's prerequisites. Guard both list-prefix forms.
Three more routes let a path-bearing target reach the basename lookup and resolve an unrelated in-vault note, which the roadmap importer then rewrites: - the exact `.md` candidate is a directory (`dirnote/note.md/` made `[[dirnote/note]]` resolve `MODULES/note.md`); - the candidate is a dangling symlink (`existsSync` is false, so it read as merely missing); - a path component is a regular file, raising ENOTDIR below it (`[[blocker.md/note]]` resolved `MODULES/note.md`). Validate the deepest existing component with `lstatSync` instead of `existsSync`, and do not read a `realpathSync` failure as a missing ancestor — that would let dangling symlinks through. Each case is reproduced by a test that first asserts the basename hijack route is live, so none can pass vacuously.
…rsing (#73) Addresses the five must-fix items from the PR #204 review round. Each is surgical; no redesign, and no other observable behavior changed. Item A1 - adopt dry-run preview fidelity: `--dry-run` printed `chainPlan.predecessorOf`, which spans every note in the scanned scope including already-adopted ones the commit never rewrites, so a mixed vault previewed edges that would never be applied. It now prints the write plan only (chain order, filtered to the notes being adopted) and lists bridged already-adopted notes under their own heading, making the preview diffable against what the commit actually does. Item A2 - cycle diagnostics: a rejected cycle in `adopt` is now reported by vault-relative path with the `palee_id` retained for cross-referencing, and names `palee validate` as the fix path. In `roadmap`, cycle errors label which hops `--auto-chain` synthesized versus which the learner authored. Item A3 - roadmap chain cycles and premature success log: the `Auto-chain: N roadmap topics chained by order.` line fired before graph validation, so a rejected chain still announced itself as fact; it is now logged only after validation passes. A synthesized edge that would close a cycle against an authored dependency is skipped with a `chain edge X -> Y skipped: would close a cycle` warning and that topic starts a new chain, so the rest of the roadmap imports. The skipped topic's own `depends_on` is left untouched rather than set to an explicit empty list, which would have cleared dependencies the note already had on disk. Item A4 - numeric prefix parsing: `parseNumericPrefix` treated an optional separator as a lesson number, so `3d-printing.md` chained as lesson 3 between `02-` and `04-` and `2024-recap.md` landed after every real module, both silently. A prefix now requires a separator or end-of-name after its digits and spans at most three digits; non-matching names fall to rank-2 alphabetical and raise the existing unnumbered warning. Item A5 - warning wording: the unresolvable-predecessor warning said "is out of scope" when the predecessor is in scope and simply has no resolvable `palee_id`; reworded to point at the real debug path. Tests: 10 new (4 engine unit, 3 adopt integration, 3 roadmap integration), including dry-run == commit plan and the reviewer's roadmap cycle reproduction. The four A4 tests fail against the pre-fix regex and pass after it. Docs and CHANGELOG are updated in this same commit. Co-Authored-By: Qoder <noreply@qoder.com>
…ge (#73) The skip path in applyRoadmapAutoChain was only covered for a skipped topic at the end of the chain. When the skip happens mid-list the topic becomes a new head and the following topic must still chain onto it, rather than being orphaned or fragmenting the remaining chain. Asserts all four resulting depends_on lists for the C -> A, B -> C, A -> [], D -> A outcome, plus the summary line counting skipped edges. Co-Authored-By: Qoder <noreply@qoder.com>
…1..B5,B7) Pure, fs-free classification of vault-relative note paths into backbone / leaf / excluded, as data + predicates Work Order C consumes after rebase. - B1 REPO_META_STEMS exact-basename blocklist (.md only) - B2 translation copies: structural translations/ segment plus a locale suffix matcher over TRANSLATION_LANG_CODES (includes the ja/pt/it codes a hand-copied list historically missed), with a generic-doc arm and a programming-language collision guard pinning guide-js.md - B3 template exclusion by basename or directory segment - B4 PHASE_DIR_SEGMENTS subtree collapse via isPhaseSubtree - B5 isContentDocName backbone gate (README-class / numeric / phase prefix) - B7 isValidPaleeId closes the truthy non-string palee_id type hole
This was referenced Sep 30, 2026
…ides it Found in review on #216: the Tier-0 bullet said an ad-hoc unnumbered sibling is attached to the chain instead of gating it, stated as though the classifier settled that for every tier. It settles the numbered tree. Under `toc`/`full` a README that enumerates such a leaf puts it ahead of the notes listed after it — `tocChainable` declines excluded, phase-subtree and translation notes only, so an ordinary leaf stays a candidate. Those edges do not gate, and the sentence now says which of the two mechanisms is responsible: the advisory `toc` label does, not the classification. Left as it stood, the bullet would have read as a second, independent guarantee that the tier does not provide.
This was referenced Sep 30, 2026
…fail loudly Three review findings on #239, all real, all on the pass itself. Direction. `tiedByName` says two filenames carry the same rank; it does not say which of them the planner would place first. `02-a` stored as depending on `02-b` is the same equal-rank pair pointed against the enumeration, nothing in the chain produces it, and relabelling it would retire a gate somebody set on purpose. Now the candidate must also satisfy `compareLessonOrderTier0(pred, own) < 0`, so the stored predecessor is the one the planner would have chosen. Stale decision. The audit and the write are separated, and the OCC fingerprint was computed from the very bytes being written, so it could not fail: an edit landing inside that window kept its new prerequisites and lost its gate. Each candidate is now confirmed against its own fresh content — still `numbered`, still one predecessor, still that predecessor — before `atomicWrite` runs. Silent failure. A write that failed for a reason other than a conflict was logged and the command exited 0, so a permission or disk error read as a completed migration with the notes still gated. Failures now exit 5; a conflict still exits 4; a note that simply no longer matches the pattern is skipped, counted and leaves the exit code alone. Proven by reverting each guard: dropping the direction check, dropping the tie re-check, ignoring --dry-run and swallowing a write error each fail a named test. The write-time recheck's predicate is tested directly, because no single-process run can edit a note between this module's scan and its write — which also means its negative branch is proven as a unit, not reached through the CLI.
…d to end Review finding on #240: the extractor's tests show sentences are ignored, but nothing showed what `adopt --auto-chain` writes for a note named only in prose. A scan re-added downstream in the adoption path would have kept every unit assertion green while the false gate came back. So the case runs the CLI over the exact shape that used to lock a learner out — one real, uniquely resolvable note named in a sentence and linked nowhere — and asserts the report claims no declared edge, the note keeps the numbered predecessor, and the prose-named note is not among its dependencies. Proven to bite the way the finding asks: injecting a naive prose scanner back into the extractor fails the new CLI test as well as the two extractor tests, and removing it restores all nine.
…g-scope docs(changelog): scope the leaf non-gating claim to the tree that decides it
fix(engine): stop reading prerequisites out of sentences
…way as the vault Three review findings on this pass, all three pointing at the same mistake: a decision computed in one place and defended in another. The write-time check compared a hand-parsed `palee_id` against a loader-derived id. The loader trims both `palee_id` and every `depends_on` entry (`src/storage/loader.ts:240`), so a note stored as `palee_id: " T-a "` was the same topic to one side of the comparison and a different one to the other: an unchanged note was declared drifted and passed over. Fixed by removing the second reader rather than matching its trimming by hand — the check now reloads the predecessor through `loadTopics`, so both sides of the equality come from the same parser. The pass no longer carries an audit list into its writes. A run that is going to write reads the vault for that purpose, derives one set, and prints exactly that: "the predecessor is not the one first seen" stopped being a state the code can reach, and a note re-pointed at another earlier same-rank sibling is judged as the edge it now is. Report-only runs keep using the scan in hand, so the extra read stays out of the way of anything counting file reads — which is what a first cut of this change broke in `migrate --fix`'s TOCTOU test. Neither move made the renamed-predecessor case safe on its own, so each write still re-confirms the pair: the dependent's fingerprint, and that the recorded id still lives at the recorded path. Drift, conflict or an active lock exits 4 with a re-run instruction; a non-conflict write failure exits 5. Four of the five guards fail a named test when reverted — including reverting this commit's frontmatter-reading change, which is what shows the loader call doing the work. The fifth, the dependent's own fingerprint compare, has no test that can fail: no single-process run can be scheduled into the gap between the check and the promotion. That is the check-then-write class #217 documents, not a gap closed here.
CHANGELOG.md was the only conflict: main's normalized Added/Fixed/Changed headings against this branch's PAL-205 groupings. Folded the four headings into main's three and kept every bullet from both sides. Two assertions in test/cli-roadmap-edge-integrity.test.ts fail on the merged tree, and both fail identically on origin/main's own tip (proved by running that file against a clean checkout of main): #218's lstat guard refuses a directory in a note's path with "Skipped ... is not a regular file" before the write loop reaches it, while #220's test still asserts "Failed T-2". Pre-existing, not a resolution artifact. Fixed in the next commit.
Two assertions here have failed on origin/main's own tip since #218 landed: the fixtures stand a directory in a note's path, which the `lstat` guard now refuses with `Skipped <id>: <path> is not a regular file` before the write is attempted, while the tests still asserted `Failed <id>` from the write path. Proven on main directly, not inferred: that file run against a clean checkout of main's tip fails the same two assertions. Only the reported wording changes. The scenarios still hold — the note does not land, its dependant's edge is still named as dangling, and the command still exits 1 — and the write-error catch these tests no longer reach is still asserted by test/cli-exit-codes.test.ts:527 and test/challenger1-stress.test.ts:354, so nothing is left unobserved.
This was referenced Sep 30, 2026
feat: support hierarchical auto-chaining (--auto-chain) and wikilink dependency graph resolution
#73
Closed
Closed
CodeRabbit's finding is correct: `reportStoredTies` derives its candidates from the scan the caller already holds, while this function's remarks, its `@param scanned` line, the caller's comment and the CHANGELOG entry all described a second read taken at write time. The behaviour is the intended one, so the claims go rather than the control flow. A second whole-vault scan at the top of the pass would sit one statement after the caller's, closing a window microseconds wide. The gap that matters is between deriving a tie and promoting its own write, and that one is already closed per candidate by the re-check of the note's fingerprint and the predecessor's identity. Re-scanning would not let a drifted candidate be relabelled; it would only decide which drift the re-check sees. So a note that moved mid-pass stays gated, is counted, and the command exits 4 with the re-run instruction the entry documents. Counting was the half that was genuinely missing: a note whose stored predecessor no longer resolves was dropped in silence by the scan. It is now reported with the rest, pointing at `palee validate`, and left alone -- with the pair gone there is no order to derive from it. Two tests, each proven by reverting its deciding predicate: dropping the same-directory guard fails `a same-rank name in another directory...`, and dropping the count fails `a note whose predecessor no longer resolves...`. Both restore to green.
…means CI's one failure on Linux: `a case-fold matching several real files is ambiguous and skipped` wrote `dup/README.md` and `dup/readme.md`, linked `dup/README.md`, and expected the link to be dropped as ambiguous. That is NTFS-only reasoning — on a case-sensitive filesystem the link names a file that exists, and `lookupNote` (src/storage/toc.ts:60) answers an exact path before it folds at all. The test asked the fold to outrank the file on disk. The file is `skip: win32` for the opposite half of the same fact, so the whole suite here was green while CI was not: 1270 tests, 1 skipped. Both halves of the rule are now asserted on the one constructible fixture: the exact spelling resolves onto `dup/README.md`, and `dup/readme.MD` — which matches neither file — is the ambiguous skip. `isResolvableNotePath` folds the extension check (`src/storage/vault-walker.ts:28`), so the mixed-case spelling reaches the lookup rather than being reported `missing`.
…document four helpers CodeRabbit's Linked Issues check asked for exactly this: a test that moves the dependent note after the scan and before the write, then requires the conflict to be reported, the note left relabelled-not, and the conflict status returned. It is reachable in-process, because the three reads of that note are distinct: the loader's (src/storage/loader.ts:229), the write loop's re-check, and `atomicWrite`'s own OCC read -- so the second one *is* the window, and the existing write-error test already established the harness for patching fs here. Neutralising the fingerprint re-check fails this test alone (15 pass, 1 fail: "drift under the pass is a conflict, not a success") and restores to 16/16, so the guard it pins is the last one on this pass that nothing could falsify. Also documents the four functions the diff touched that had no docstring (63.64% against an 80% threshold): `emptyRuleCounts`, `tier0LessonRank`, `flushRun` and the test's `padded` note builder.
CodeRabbit on #239 is right: with more than five qualifying notes, \--relabel-ties --dry-run\ printed five paths and an omitted count, so the user could not inspect every note before approving the writes. The dry run is the preview of exactly what the pass would touch, so it now lists all candidates; the audit-only report keeps its five-path limit.
Docstring coverage on #239 is 71.43% against an 80% threshold: the four touched test helpers runCLI, freshVault, frontmatterOf and readyIds carry no docstring. Each gets one describing what it runs, builds or asserts.
…ties feat(migrate): relabel stored ties so pre-#234 vaults unlock
Kuldeep2822k
added a commit
that referenced
this pull request
Oct 12, 2026
… ADR-0010 Two corrections from review: - The tradeoff bullet quoted the *old* docstring claim that guide-js.md 'stays a content doc / on the backbone' as if it were current. #214 corrected those docstrings: the collision arm only stops a note being read as a translation, and guide-js.md is a leaf either way — an unnumbered non-content sibling. The no-op conclusion stands, the stated mechanism was inverted. - The first mermaid box merged the translations/ (excluded) and empty-stem (leaf) outcomes into one label; name both classes explicitly since the text distinguishes them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
09cf886onfeat/73-tier0-hygiene.origin/mainis merged in at5075087, so this branch now reads clean against the default branch: mergeable, no conflicts.Closeskeyword on a merge into the default branch - which is why their issues are all still open. The two that are fully satisfied close here.npm run check(typecheck + lint) clean. Two stale assertions intest/cli-roadmap-edge-integrity.test.tsare fixed in09cf886; both have been failing onorigin/main's own tip since fix(roadmap): stop importing through a symlinked note path #218 landed, which is recorded in that commit.Evidence (independent scoring, SD-Reviewer-1)
Closes
Both fully verified against this tip; per-criterion evidence is in each issue's verification-round comment.
Closes #222
Closes #233
Closes #231
The PR appears safe to merge based on the reviewed changes.
Summary
The PR adds Tier-0 note classification and dependency planning for
adopt --auto-chain, plus roadmap chaining and wikilink resolution. Since the previous review, the wikilink extractor was changed to distinguish odd and even runs of preceding backslashes.Reviews (3) · Last reviewed commit: "fix(engine): judge an escaped wikilink b..."