fix(docs): apply non-public folder exclusion on runtime /docs route (#1794)#1799
Conversation
…1794) The in-app /docs route served a different surface depending on deployment mode: npm installs got the curated public bundle, but hosted-from-source instances served the entire docs/ checkout tree (release history, feature units, internal process folders), because the EXCLUDED_TOP curation lived only in the build-time bundler. FOLDER_DEFAULTS marks releases/feature_units as public, so the runtime isVisible gate kept them. Hoist the exclusion into a single shared predicate applied on all three paths: - Add NON_PUBLIC_TOP_FOLDERS + isNonPublicTopFolder() to visibility.ts. - buildDocsIndex and lookupDoc drop non-public top folders unless show-internal is set: the default (production) now matches the curated npm bundle; NEOTOMA_DOCS_SHOW_INTERNAL=true reveals the full tree for local dev. - build_bundled_docs.ts imports the shared set; the bundle additionally drops site/ as a packaging concern (npm ships .md docs only, not the marketing tree). site/ stays browsable on from-source hosts because the root-landing footer links into site/pages/en/*, so it is NOT in the shared surface set. Behavior change: on from-source hosts, /docs and /docs/<slug> for excluded folders (releases/*, feature_units/*, ...) now 404 by default, matching npm. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
review:pm 🤖 Pavo — Ateles swarm, pm lens panelist Review SummaryPR implements exactly what the pm gate signed off on #1794. All five acceptance criteria satisfied:
Scope ValidationIn scope:
Out of scope (not attempted, correctly):
Behavior AlignmentDefault production behavior now matches curated npm bundle surface on from-source instances. Local dev with PM gate sign-off: Criteria met. Ready for merge. 📎 Neotoma: issue #1794 |
|
review:ux The PR improves consistency between npm-bundled and hosted-from-source instances by applying folder-level visibility filtering uniformly. The code surface is clear — naming, error types, and the shared predicate ( [NON-BLOCKING] Behavioral change: 404 on excluded folders needs operator guidanceThis PR changes deployed behavior on hosted-from-source instances: URLs like UX concern: operators on live instances won't discover this behavior change or the opt-in without digging into the PR body or release notes. The Severity: P2 (non-blocking; this is primarily a self-service docs host, and the PR body documents the change clearly for a first reading). But consider:
[NON-BLOCKING] Error discriminator:
|
|
review:legal Compliance Checklist
Findings
📎 Neotoma: issue markmhendrickson/neotoma#1794 |
|
review:qa APPROVE SummaryThe fix hoists a per-deployment visibility inconsistency into a single shared predicate ( Bug fixed: On from-source deployments, Behavior change (intentional): From-source hosts now default to the curated public surface and require the flag to reveal the full tree, matching the npm bundle behavior. Test CoverageEdge Cases & Assertions✅ Non-public folder exclusion (production default):
✅ Flag override (show-internal):
✅ Boundary cases:
✅ Predicate invariant:
Regression Test: The Original BugThe fix corrects a logic flaw: The new test ✅ Integration surface: [NON-BLOCKING] Code coverage gap: The integration test should add explicit assertions for Shared Predicate Correctness✅ ✅ No per-type branches or hardcoded path lists introduced — the solution follows schema-agnostic design rules. Contract & CLI/MCP ParityThis is a docs-route visibility fix, not an API/MCP/CLI surface change. No openapi.yaml, contract mapping, or CLI instruction updates required. Verdict: Test coverage is adequate for the functional change. The unit tests directly assert the bug is fixed (non-public folders excluded by default, included with flag). Integration test coverage is optional but recommended (does not block merge). |
|
🤖 Vanellus — Ateles swarm, PR steward Aggregated Review Verdict — PR #1799Per-Lens SummaryPM (Pavo): ✅ APPROVE
UX (Accipiter): ✅ COMMENT (non-blocking)
Legal (Buteo): ✅ APPROVE
QA (Phoenicurus): ✅ APPROVE
CI Status
Required branch-protection checks: Blockers✅ None. All four panel lenses clear or non-blocking. Merge Readiness
Recommendation: Ready for merge. Merge is operator-gated; operator to confirm before proceeding. 📎 Neotoma: issue #1794 · PR markmhendrickson/neotoma#1799 |
|
review:pm All acceptance criteria from the pm gate sign-off on issue #1794 are met. ✅ Shared Predicate Defined and Applied Consistentlyvisibility.ts: render.ts ( index_builder.ts ( build_bundled_docs.ts (bundler): Removes hardcoded ✅ No duplication. Single source of truth applied consistently on all three paths. ✅ Hosted-from-Source Returns 404; show-internal Flag WorksDirect lookups return ✅ Behavior matches expectation: 404 by default on from-source, full tree under flag. ✅ npm Bundle Behavior Unchanged
✅ npm installs are unaffected. ✅ Test Coverage: Exclusion by Default, Inclusion Under Flagvisibility.test.ts: Unit tests for index_builder.test.ts: New tests verify render.test.ts: New tests verify direct lookup of non-public docs returns ✅ Test coverage explicit and comprehensive. ✅ No Scope CreepNo new env flags, UI changes, doc restructuring, or schema changes. Error handling reuses existing Verdict: All pm gate acceptance criteria met. Implementation is focused, well-tested, and matches the scoped intent. Ready for merge. |
|
review:ux User-facing surfaceNew runtime environment variable: New error state: direct slug lookup (e.g., Behavior change: On hosted-from-source instances, URLs to excluded folders now 404 (by default). No change for npm installs (already filtered). Interaction / flow
Discoverability & namingStrengths:
Friction point — The error kind Error & empty states✓ Non-existent files return Tests cover all three states and both code paths (flag on/off). Good coverage. AccessibilityN/A — backend-only docs-serving logic, no user-facing UI. Acceptance checklist
VerdictCOMMENT — The UX surface is clean and naming is consistent with existing patterns. The new |
|
review:legal Legal Surface Review — PR #1799Findings
Test Coverage
All pre-registered expectations satisfied. 📎 Neotoma: markmhendrickson/neotoma#1794 |
|
review:qa APPROVE SummaryThis PR fixes a deployment-mode surfacing bug in the Eval CoverageNew coverage added — all passing:
Coverage quality:
Test runs green:
DeterminismData-layer behavior is deterministic:
Functional surfaceAgent-observable effects asserted:
Breaking change:
Non-blocking observationsNone. The change is well-scoped, backward-compatible via flag, and hardening (fails closed). |
|
🤖 Vanellus — Ateles swarm, PR steward Aggregated Review Verdict — PR #1799Per-Lens SummaryPM (Pavo): ✅ APPROVE
UX (Accipiter): ✅ COMMENT (non-blocking)
Legal (Buteo): ✅ APPROVE
QA (Phoenicurus): ✅ APPROVE
CI Status
Required branch-protection checks: Blockers✅ None. All four panel lenses clear or non-blocking. Merge Readiness
Recommendation: Ready for merge. Merge is operator-gated; operator to confirm before proceeding. 📎 Neotoma: issue #1794 · PR markmhendrickson/neotoma#1799 |
Closes #1794.
Problems
The in-app
/docsroute served a different surface depending on deployment mode:dist/docs).neotoma.markmhendrickson.com) → the entiredocs/checkout tree, including release history, feature units, and internal-process folders (Releases & History 161, Development 268, ... on the RC host after docs: realign README + docs to actual functionality; bundle public docs for in-app /docs #1786 shipped the in-app browser).Root cause: the public-surface curation lived in two places that disagreed. The build-time bundler (
scripts/build_bundled_docs.ts) applied anEXCLUDED_TOPfolder set; the runtime route (buildDocsIndex,lookupDoc) applied only the per-fileisVisiblegate. BecauseFOLDER_DEFAULTSmarksreleases/feature_unitsasvisibility: public, the runtime kept them.Solutions
Hoist the exclusion into a single shared predicate, applied on all three paths:
NON_PUBLIC_TOP_FOLDERS+isNonPublicTopFolder()tosrc/services/docs/visibility.ts(single source of truth).buildDocsIndex(index_builder.ts) andlookupDoc(render.ts) drop non-public top folders unless show-internal is enabled. Default (production) now matches the curated npm bundle;NEOTOMA_DOCS_SHOW_INTERNAL=truereveals the full checkout tree for local dev.build_bundled_docs.tsimports the shared set instead of its duplicated local copy.site/is a packaging exclusion, not a surface exclusionsite/(the marketing-site pages) is intentionally not in the shared set. The root-landing footer (bundled_nav.ts) links intosite/pages/en/*(Install, API, FAQ, Privacy, Terms), and those are served on from-source hosts. The npm bundle still dropssite/(the package ships.mddocs only, not the marketing tree), so the bundler addssiteto its ownBUNDLE_EXCLUDED_TOPon top of the shared set. This keeps the bundle output byte-identical (359 docs) while preserving the landing footer on from-source hosts.Behavior change
On hosted-from-source instances,
/docsand/docs/<slug>for excluded folders (releases/*,feature_units/*,plans/*, ...) now 404 by default, matching what npm installs already do. SetNEOTOMA_DOCS_SHOW_INTERNAL=trueto browse the full tree. No change for npm installs;site/browsing on from-source hosts is unchanged.Test plan
npm run type-check— clean.src/services/docs/**,tests/integration/docs_route.test.ts,tests/unit/bundled_docs_nav.test.ts(7/7).index_builder.test.ts(non-public folders filtered by default, shown withNEOTOMA_DOCS_SHOW_INTERNAL=true),render.test.ts(excluded-folder direct lookup hidden by default),visibility.test.ts(isNonPublicTopFolder+ set/predicate sync;siteis not excluded).npm run lint(0 errors),format:check,lint:site-copy,validate:doc-deps— all pass.Note: the local pre-commit hook's full unit+integration suite was skipped via the hook-sanctioned
NEOTOMA_SKIP_TESTS=1(not--no-verify) because this worktree has unrelated failures — the inspector SPA-shell tests deterministically fail ("No bundled SPA found", Inspector not built here) and a few DB tests are parallel-SQLite-flaky (verified green in isolation:deletion.test.ts,mcp_entity_variations.test.ts). All non-test gates above still ran. CI runs the full suite on this PR.