Skip to content

feat(access,cli): the integrity checks, ow check, and source state - #8

Merged
protonspy merged 2 commits into
mainfrom
feat/integrity
Aug 1, 2026
Merged

feat(access,cli): the integrity checks, ow check, and source state#8
protonspy merged 2 commits into
mainfrom
feat/integrity

Conversation

@protonspy

@protonspy protonspy commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Closes plan tasks 7.1–7.5, 7.7 and 6.1 of plans/open-wiki.md.

The design gap the plan deferred here — and how much wider it was

Plan task 7.5 recorded a gap to settle in this group: the gate accepted a top-level codewiki/ while the skill's prose put codewiki under wiki/. Building the checks showed it was never about codewiki.

listEntityPages read only the top level of wiki/. The plan's own layout diagram files pages as wiki/projects/*.md, wiki/people/*.md, wiki/topics/*.md. So a project that followed the documented layout had no pages at all as far as the store was concerned — nothing indexed, no orphan ever reported, ow graph empty, MCP serving nothing, every [[link]] reading as broken. The gate still validated writes to those pages, which is exactly what kept the contradiction quiet: they were checked on the way in and then lost their existence.

Settled as adr:0016-a-page-is-its-slug-wherever-it-sits: a page is addressed by its slug wherever it sits under wiki/. A folder is organisation; a link is a name — which is what makes [[wikilink]] work at all, and it is the only reading that leaves both the plan's diagram and the scaffolded skill true. That matters more than usual because the skill is copied into user projects and ages there. The one rule the model needs is slug uniqueness, reported as page.duplicate-slug rather than resolved by silently picking one.

What else is here

  • 7.1–7.5 — broken wikilinks, orphans, duplicate slugs; changelog desync and uncited sources; unresolved provenance; synonyms; codewiki citations that don't resolve, run past the end of their file, or are missing from a section.
  • 7.7 — ow check, exiting 0 clean / 1 could not run / 2 found errors, matching the scc contract this repo already documents. Only errors fail: a source uploaded this morning that nothing cites yet must not turn CI red.
  • 6.1 — source state, derived from disk rather than persisted beside it.

Two judgement calls worth naming:

7.4 needs no glossary file. Every page already declares its canonical title and its aliases; a separate glossary would be a second record of one fact, and the copy is the one that goes stale. The scaffolded skill told the agent to "check the glossary" — a file that has never existed in this product — and now describes what is actually there.

6.1 derives rather than persists. "Persisted and resumable" is already what the filesystem is: manifest.json, text.md, the pages, journal.json. A state file beside those is the same two-records problem. Recorded on the plan's 6.1 line so a reader can see it was answered differently than it was asked.

From the reviews on this branch

code-review and security-review were run before this PR was opened, and between them found 17 issues — several of them regressions this change itself introduced. All are fixed:

  • Catastrophic backtracking (ReDoS) in the codewiki citation regex. / was in both the segment class and the separator, making the pattern ambiguous: ~26 slash-separated segments took 1.6 s, ~34 took minutes. That is eighty bytes of page body — which an agent writes, possibly steered by a poisoned source in raw/ — wedging ow check, CI and the future UI in a synchronous CPU spin no try/catch can interrupt. Measured at 1602 ms → 0 ms after the fix.
  • ow graph, ow search and the MCP read tools all still assumed a top-level page, so the ADR's central claim was false in three places. MCP's failure was silent: a superseded page filed under wiki/topics/ came back as type: unknown, status: active. pagePath had been written for exactly this and wired to nothing.
  • The gate began denying any page that wrote [[changelog]] or [[index]] — files that exist in every scaffolded project, and that the skill tells the agent to use — with a reason that reads as a bug.
  • checkProject created wiki/ via readIndex, so a read wrote. It is exported into the read-only surface the MCP process imports, whose whole guarantee is that read-only is what that process can do.
  • checkVocabulary blamed the wrong page when two pages claimed one alias, and told a page to stop writing its own title. Both are now reported as the conflict they are (glossary.conflict).
  • Line numbers pointed into the frontmatter; a citation on a heading line wasn't counted; a page's H1 title was reported as an uncited section; and a file ending in a newline was counted one line too long — so a citation one line past the end was accepted, by the check whose entire purpose is catching that.
  • Control characters from page content reached the terminal unescaped, letting an alias forge its own ow check: no findings line over the real report.
  • checkLinks walked the wiki once per page — 3 s over 800 pages, with the pages already in hand.

How it was verified

  • 461 tests pass; access 95.8%, cli 96.7%, mcp 100% against the 76% floor
  • pnpm run typecheck, pnpm lint clean; scc validate 0 findings
  • Every review finding has a regression test, including a timed one for the ReDoS

Note

main is still not prettier-clean (CI runs typecheck and lint, not format:check); that churn is kept out of this diff.

🤖 Generated with Claude Code

https://claude.ai/code/session_016iMM93Wk43o44V5J2AxPgL

Summary by CodeRabbit

  • New Features
    • Added the ow check command to identify wiki integrity issues, with text or JSON output and actionable fixes.
    • Added source processing status, including transcription progress, failures, and citation tracking.
    • Wiki pages can now be discovered and accessed from nested directories, including the designated codewiki location.
  • Bug Fixes
    • Improved wikilink resolution, search, graph navigation, and page lookup for nested pages.
    • Strengthened path safety and handling of duplicate or misplaced pages.
  • Documentation
    • Documented page slugs, aliases, page locations, and codewiki conventions.

Closes plan tasks 7.1-7.5, 7.7 and 6.1.

Settles the design gap the plan deferred to group 7, and the gap was
wider than the note there described. `listEntityPages` read only the top
level of `wiki/`, while the plan's own layout files pages as
`wiki/projects/`, `wiki/people/`, `wiki/topics/`. So a project that
followed the documented layout had no pages at all as far as the store
was concerned: nothing indexed, no orphan reported, `ow graph` empty,
MCP serving nothing, every `[[link]]` reading as broken. The gate still
validated writes to them, which is what kept it quiet.

A page is now its slug wherever it sits under `wiki/` — a folder is
organisation, a link is a name — recorded as
`adr:0016-a-page-is-its-slug-wherever-it-sits`. Slug uniqueness is the
one rule that needs, and it is a finding rather than a silent choice.
Codewiki lives at `wiki/codewiki/`; a top-level `codewiki/` is no longer
gated and is reported as misplaced.

The checks (7.1-7.5) each answer a question the gate cannot, because
every one is about a relationship between things rather than a single
write. 7.4 needs no glossary file: a page's `title` is the canonical
term and its `aliases` are the synonyms, so a separate file would be a
second record of one fact. The scaffolded skill told the agent to "check
the glossary" — a file that has never existed — and now says what is
actually there.

6.1 derives state from disk rather than persisting it. The filesystem is
already persisted and resumable: manifest.json, text.md, the pages,
journal.json. A state file beside those is the same two-records problem.

`ow check` (7.7) exits 0 clean / 1 could not run / 2 found errors,
matching the `scc` contract this repo documents. Only errors fail: a
source uploaded this morning that nothing cites yet must not turn CI red.

From the two reviews on this branch:

- Catastrophic backtracking in the codewiki citation regex. `/` was in
  both the segment class and the separator, so the pattern was
  ambiguous: ~26 slash segments took 1.6s, ~34 took minutes. Eighty
  bytes of page body — which an agent writes, possibly steered by a
  poisoned source in raw/ — wedged `ow check`, CI and the UI in a
  synchronous spin no try/catch interrupts.
- `ow graph`, `ow search` and the MCP read tools all still assumed a
  page was at the top level, so the ADR's central claim was false in
  three places. MCP's was silent: a superseded page came back as
  `type: unknown, status: active`.
- The gate started denying any page that wrote `[[changelog]]` or
  `[[index]]`, with a reason that read as a bug — those files exist in
  every scaffolded project and the skill tells the agent to use them.
- `checkProject` created `wiki/` through `readIndex`, so a read wrote —
  and it is exported into the read-only surface the MCP process imports.
- `checkVocabulary` blamed the wrong page when two pages claimed one
  alias, and told a page to stop writing its own title. Both are now
  reported as the conflict they are.
- Reported lines pointed into the frontmatter; a citation on a heading
  line was not counted; a page's H1 title was reported as an uncited
  section; a file ending in a newline was counted one line too long, so
  a citation one line past the end was accepted.
- Control characters from page content reached the terminal unescaped,
  letting an alias forge `ow check: no findings`.
- `checkLinks` walked the wiki once per page; 3s over 800 pages.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iMM93Wk43o44V5J2AxPgL
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR defines nested wiki pages by globally unique slug, adds derived source-state APIs and integrity checks, exposes them through the access package, and adds the ow check CLI command. Graph, search, gate, hooks, and MCP page handling now use discovered paths.

Changes

Wiki addressing and placement

Layer / File(s) Summary
Recursive page addressing and placement
docs/adr/..., packages/access/src/store/*, packages/access/src/wikilinks.ts, packages/access/src/gate/gate.ts, packages/access/src/skills.ts, packages/access/tests/store-*, packages/access/tests/gate-decision.spec.ts
Pages are discovered recursively by slug. Wikilinks resolve against discovered slugs. wiki/codewiki/ is the canonical codewiki location.
Derived source state
packages/access/src/sources/*, packages/access/tests/sources-state.spec.ts, plans/open-wiki.md
Source state is derived from manifests, files, journals, citations, and transcription evidence.
Integrity finding model and project checks
packages/access/src/check/*, packages/access/tests/check.spec.ts
The project check loads pages once and reports sorted findings for links, records, provenance, vocabulary, codewiki citations, and schema issues.
CLI and consumer integration
packages/cli/src/*, packages/cli/tests/*, packages/mcp/src/tools.ts, packages/mcp/tests/mcp.spec.ts
ow check supports text or JSON output and distinct exit codes. Graph, search, hooks, and MCP use discovered page paths.
Read-only API exports
packages/access/src/index.ts, packages/access/src/read.ts
The access package exports page inventory, source-state, integrity-check, report, and finding APIs.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant AccessChecks
  participant WikiStore
  participant Filesystem
  CLI->>AccessChecks: run check project
  AccessChecks->>WikiStore: load discovered pages
  WikiStore->>Filesystem: read nested wiki files
  Filesystem-->>WikiStore: return page contents
  WikiStore-->>AccessChecks: return loaded pages
  AccessChecks-->>CLI: return findings and exit status
Loading

Possibly related PRs

  • protonspy/open-wiki#6: Extends the same wiki store, wikilink, gate, CLI, MCP, and page-path behavior with recursive nested-page support.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 45.10% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main changes: integrity checks, the ow check command, and derived source state.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/integrity

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (4)
packages/mcp/src/tools.ts (1)

107-125: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win

Cache the page list during MCP index construction

readPageFrontmatter resolves each slug through lookupPagePath, which performs a full listPages walk. Therefore, indexStructure performs one directory walk per page and becomes O(n²). Reuse one precomputed page list, matching the known-set pattern in checkLinks.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mcp/src/tools.ts` around lines 107 - 125, Update indexStructure to
precompute the page list once and pass or reuse it when resolving each slug
through readPageFrontmatter and pagePath, avoiding repeated
lookupPagePath/listPages walks. Follow the existing known-set caching pattern in
checkLinks while preserving page path confinement and frontmatter behavior.
packages/mcp/tests/mcp.spec.ts (1)

225-227: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

The ternary has identical branches.

Both arms evaluate spec.startsWith(a), so the a.endsWith(":") test decides nothing. Either drop the condition or implement the distinction the condition implies, for example an exact match for a plain package name and a prefix match only for a node:-style scheme.

♻️ Proposed cleanup
-    return ALLOWED.some(
-      (a) => spec === a || (a.endsWith(":") ? spec.startsWith(a) : spec.startsWith(a)),
-    );
+    return ALLOWED.some((a) => spec === a || spec.startsWith(a));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/mcp/tests/mcp.spec.ts` around lines 225 - 227, Update the ALLOWED
matching logic in the visible .some callback to remove the redundant ternary and
implement the intended distinction: keep exact matching for plain package names,
while allowing prefix matching only for entries ending in “:”, such as node:
schemes.
packages/cli/tests/hooks.spec.ts (1)

40-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add the negative case for a top-level codewiki/.

packages/cli/src/hooks.ts Lines 104-107 removed codewiki/ from the shell-write targets so the hook and the gate agree. No test covers that removal, so a later re-add of the pattern passes.

💚 Proposed test
   it("flags cp/mv/tee/sed -i into wiki/, codewiki included", () => {
     expect(detectShellWrite(`cp /tmp/x wiki/fenix.md`, "/p")).toBe("wiki/fenix.md");
     expect(detectShellWrite(`mv /tmp/x wiki/codewiki/dispatch.md`, "/p")).toBe(
       "wiki/codewiki/dispatch.md",
     );
+  });
+
+  it("ignores a top-level codewiki/, which the gate allows (adr:0016)", () => {
+    expect(detectShellWrite(`echo hi > codewiki/x.md`, "/p")).toBeNull();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/tests/hooks.spec.ts` around lines 40 - 44, Add a negative
assertion to the existing “flags cp/mv/tee/sed -i into wiki/, codewiki included”
test around detectShellWrite, verifying that a write targeting top-level
codewiki/ returns no flagged path. Keep the existing wiki/ and nested
wiki/codewiki/ positive cases unchanged.
packages/cli/src/commands/check.ts (1)

25-33: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Reject unknown arguments.

parseCheckArgs ignores any argument it does not recognise. A typo such as ow check --jsonn runs the text renderer and still exits 0 on a clean wiki. A CI job that parses stdout as JSON then fails on output it cannot explain.

Return a parse error for unrecognised arguments, and let main.ts report it through fail.

♻️ Proposed refactor
 export function parseCheckArgs(args: string[]): CheckOptions {
+  const known = new Set(["--json", "--errors-only"]);
+  const unknown = args.filter((a) => !known.has(a));
+  if (unknown.length > 0) {
+    throw new Error(`ow check does not take ${unknown.join(", ")}`);
+  }
   return {
     json: args.includes("--json"),

Note that main.ts Lines 91-103 already catch a throw here and return CHECK_FAILED_TO_RUN, which is the correct code for "the check could not run".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/cli/src/commands/check.ts` around lines 25 - 33, Update
parseCheckArgs to validate every argument against the supported --json and
--errors-only flags, throwing a parse error for any unrecognised argument.
Preserve the existing defaults and return shape for valid arguments, allowing
main.ts to catch the throw and report it through fail with CHECK_FAILED_TO_RUN.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/access/src/check/checks.ts`:
- Line 209: Apply safe() to the raw interpolated issue values in checkProvenance
and checkSchema: sanitize issue.reason in the provenance message at
packages/access/src/check/checks.ts:209-209, and sanitize both issue.field and
issue.reason at packages/access/src/check/checks.ts:536-536. Extend the
assertion in packages/access/tests/check.spec.ts:504-507 to include provenance
and schema findings, ensuring all finding messages enforce the scrubbing
invariant.
- Around line 412-443: Update the citation scan before iterating in the
page-body check to mask fenced code blocks, replacing non-newline characters
with spaces so line positions remain unchanged. Run CODEWIKI_CITATION matching
against the masked content instead of page.body, while preserving the existing
resolution and finding logic.
- Around line 505-520: Replace the recursive readdirSync call in the stray-page
check with an explicit walk matching listPages, and skip any entry where
entry.isSymbolicLink() is true before descending or counting files. Preserve
recursive discovery of regular .md files under stray while preventing symlinked
directories from contributing pages.

In `@packages/access/src/sources/state.ts`:
- Around line 116-123: Update listSourceStates to handle readManifest failures
independently for each source returned by listSources, including malformed JSON
and concurrent deletion, so one invalid manifest does not abort listing valid
states. Skip or otherwise safely exclude the failing source while preserving
sorting and the existing citations behavior for successfully loaded sources; do
not modify the unrelated ow check path.

In `@packages/access/src/store/index.ts`:
- Around line 60-90: Update the extension filter in listPages to compare a
case-folded entry.name against ".md", matching gatedPageRel’s case-insensitive
behavior; leave slug extraction and page path handling unchanged so files such
as fenix.MD are discovered.

In `@packages/access/tests/sources-state.spec.ts`:
- Around line 118-120: Add a test case alongside the existing “refuses an id
that escapes raw/” test using a single-parent traversal such as “../something”,
and assert that sourceState(root, ...) throws. Keep the existing project-root
escape coverage intact while ensuring the test specifically validates rejection
of paths outside raw/ but still within projectRoot.

---

Nitpick comments:
In `@packages/cli/src/commands/check.ts`:
- Around line 25-33: Update parseCheckArgs to validate every argument against
the supported --json and --errors-only flags, throwing a parse error for any
unrecognised argument. Preserve the existing defaults and return shape for valid
arguments, allowing main.ts to catch the throw and report it through fail with
CHECK_FAILED_TO_RUN.

In `@packages/cli/tests/hooks.spec.ts`:
- Around line 40-44: Add a negative assertion to the existing “flags
cp/mv/tee/sed -i into wiki/, codewiki included” test around detectShellWrite,
verifying that a write targeting top-level codewiki/ returns no flagged path.
Keep the existing wiki/ and nested wiki/codewiki/ positive cases unchanged.

In `@packages/mcp/src/tools.ts`:
- Around line 107-125: Update indexStructure to precompute the page list once
and pass or reuse it when resolving each slug through readPageFrontmatter and
pagePath, avoiding repeated lookupPagePath/listPages walks. Follow the existing
known-set caching pattern in checkLinks while preserving page path confinement
and frontmatter behavior.

In `@packages/mcp/tests/mcp.spec.ts`:
- Around line 225-227: Update the ALLOWED matching logic in the visible .some
callback to remove the redundant ternary and implement the intended distinction:
keep exact matching for plain package names, while allowing prefix matching only
for entries ending in “:”, such as node: schemes.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 91d15756-83f9-4d30-886f-98aa4798c085

📥 Commits

Reviewing files that changed from the base of the PR and between 1f28659 and f7dd413.

📒 Files selected for processing (28)
  • docs/adr/0016-a-page-is-its-slug-wherever-it-sits.md
  • packages/access/src/check/checks.ts
  • packages/access/src/check/findings.ts
  • packages/access/src/gate/gate.ts
  • packages/access/src/index.ts
  • packages/access/src/read.ts
  • packages/access/src/skills.ts
  • packages/access/src/sources/state.ts
  • packages/access/src/store/index-write.ts
  • packages/access/src/store/index.ts
  • packages/access/src/store/page.ts
  • packages/access/src/store/wikilinks.ts
  • packages/access/tests/check.spec.ts
  • packages/access/tests/gate-decision.spec.ts
  • packages/access/tests/sources-state.spec.ts
  • packages/access/tests/store-index.spec.ts
  • packages/access/tests/store-wikilinks.spec.ts
  • packages/cli/src/commands/check.ts
  • packages/cli/src/commands/graph.ts
  • packages/cli/src/commands/search.ts
  • packages/cli/src/hooks.ts
  • packages/cli/src/main.ts
  • packages/cli/tests/check.spec.ts
  • packages/cli/tests/hooks.spec.ts
  • packages/cli/tests/queries.spec.ts
  • packages/mcp/src/tools.ts
  • packages/mcp/tests/mcp.spec.ts
  • plans/open-wiki.md

Comment thread packages/access/src/check/checks.ts Outdated
Comment thread packages/access/src/check/checks.ts
Comment thread packages/access/src/check/checks.ts
Comment thread packages/access/src/sources/state.ts
Comment thread packages/access/src/store/index.ts
Comment thread packages/access/tests/sources-state.spec.ts Outdated
- `listPages` folds case on `.md`, as the gate does. `gatedPageRel`
  lowercases before testing the extension, so it validates and accepts
  `wiki/fenix.MD` — and matching case-sensitively here meant that page
  was accepted by the gate and then invisible to the index, the orphan
  check, `ow graph` and MCP. That is the failure this addressing model
  exists to end, reappearing one level down.
- A citation inside a fenced code block is an example, not a citation.
  A codewiki page documenting the citation form — which the skill's own
  prose does — failed `ow check` for its own sample.
- The stray-codewiki walk no longer follows symlinked directories out of
  the project; `recursive: true` does.
- `listSourceStates` keeps going when one manifest will not parse or a
  source vanishes mid-listing. A sources screen showing nothing because
  of one bad directory is worse than one showing the other nineteen.
- `listSources` reads dirents rather than stat'ing each entry, so a
  dangling symlink under `raw/` no longer throws ENOENT and aborts the
  whole run — reachable now that `checkRecords` calls it.
- `sourceState` confines against `raw/`, not merely the project: an id
  like `../wiki` stays inside the project and is still not a source. The
  test that claimed to cover this only exercised an id that left the
  project entirely, which proves less.
- `safe()` on the remaining interpolated values, in `checkProvenance`
  and `checkSchema`.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016iMM93Wk43o44V5J2AxPgL

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
packages/access/tests/sources-state.spec.ts (1)

149-157: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Add a dangling-symlink regression test.

listSources now uses Dirent.isDirectory() to avoid statSync failures for dangling entries. Add a dangling symlink beside a valid source and assert that listSourceStates(root) keeps the valid source and does not throw.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/access/tests/sources-state.spec.ts` around lines 149 - 157, Add a
regression test alongside the existing malformed-manifest case that creates a
valid source and a dangling symlink in the root directory, then call
listSourceStates(root) and assert it returns the valid source ID without
throwing. Use the existing filesystem helpers and preserve the expected
valid-source listing behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/access/src/sources/manifest.ts`:
- Around line 92-100: Update the source-manifest discovery logic around the
manifest listing loop to read and parse each existing manifest.json, rather than
only checking existsSync. When parsing fails, add a finding identifying the
malformed manifest and its source directory; retain valid manifest IDs and
continue checking other entries without aborting.

---

Nitpick comments:
In `@packages/access/tests/sources-state.spec.ts`:
- Around line 149-157: Add a regression test alongside the existing
malformed-manifest case that creates a valid source and a dangling symlink in
the root directory, then call listSourceStates(root) and assert it returns the
valid source ID without throwing. Use the existing filesystem helpers and
preserve the expected valid-source listing behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ae406955-cc0f-407d-bfb6-102e416f2871

📥 Commits

Reviewing files that changed from the base of the PR and between f7dd413 and 2961037.

📒 Files selected for processing (6)
  • packages/access/src/check/checks.ts
  • packages/access/src/sources/manifest.ts
  • packages/access/src/sources/state.ts
  • packages/access/src/store/index.ts
  • packages/access/tests/check.spec.ts
  • packages/access/tests/sources-state.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/access/src/sources/state.ts
  • packages/access/src/store/index.ts
  • packages/access/src/check/checks.ts

Comment on lines +92 to +100
for (const entry of readdirSync(raw, { withFileTypes: true })) {
if (entry.name === INBOX) continue;
// `withFileTypes` describes the entry itself, so a dangling symlink is
// reported rather than stat'd. `statSync` on one throws ENOENT, which used
// to abort the whole listing — and `ow check` with it.
if (!entry.isDirectory()) continue;
const dir = join(raw, entry.name);
if (!existsSync(join(dir, "manifest.json"))) continue;
ids.push(entry);
ids.push(entry.name);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n -C 12 \
  'listSources\(projectRoot\)|readManifest|JSON\.parse|catch|exitCode|process\.exit' \
  packages/access/src/check/checks.ts \
  packages/access/src/sources \
  packages/access/tests \
  packages/cli/src \
  packages/cli/tests

Repository: protonspy/open-wiki

Length of output: 50376


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- check implementation ---'
sed -n '1,235p' packages/access/src/check/checks.ts
printf '%s\n' '--- check result and CLI boundary ---'
sed -n '560,635p' packages/access/src/check/checks.ts
rg -n -C 14 'runCheck|checkProject|findings|sources:|exit|check' packages/cli/src packages/access/src packages/cli/tests packages/access/tests \
  | head -n 500
printf '%s\n' '--- malformed-manifest coverage ---'
rg -n -C 8 'malformed|invalid|manifest\.json|listSourceStates|source\.uncited|checkProvenance' \
  packages/access/tests packages/cli/tests

Repository: protonspy/open-wiki

Length of output: 50377


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path

checks = Path("packages/access/src/check/checks.ts").read_text()
manifest = Path("packages/access/src/sources/manifest.ts").read_text()
provenance = Path("packages/access/src/store/provenance.ts").read_text()

def section(text, start, end):
    a = text.index(start)
    b = text.index(end, a)
    return text[a:b]

records = section(checks, "export function checkRecords", "export function checkProvenance")
project = section(checks, "export function checkProject", "}")
source_exists = section(manifest, "export function sourceExists", "/**")
resolve = section(provenance, "export function resolveProvenance", "}")
print("checkRecords calls listSources:", "listSources(projectRoot)" in records)
print("checkRecords calls readManifest:", "readManifest(" in records)
print("checkProject calls listSources:", "listSources(projectRoot)" in project)
print("sourceExists calls readManifest:", "readManifest(" in source_exists)
print("resolveProvenance calls sourceExists:", "sourceExists(" in resolve)
print("resolveProvenance calls readManifest:", "readManifest(" in resolve)
PY

printf '%s\n' '--- provenance resolver ---'
rg -n -C 18 'function resolveProvenance|sourceExists|readManifest|JSON\.parse' \
  packages/access/src/store/provenance.ts packages/access/src/sources/manifest.ts

Repository: protonspy/open-wiki

Length of output: 9272


Validate source manifests in ow check.

ow check only checks that manifest.json exists. It does not parse the manifest or report malformed JSON. Add a finding for each malformed manifest.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/access/src/sources/manifest.ts` around lines 92 - 100, Update the
source-manifest discovery logic around the manifest listing loop to read and
parse each existing manifest.json, rather than only checking existsSync. When
parsing fails, add a finding identifying the malformed manifest and its source
directory; retain valid manifest IDs and continue checking other entries without
aborting.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant