Frontmatter
| title | >- |
| author | neo-fable |
| state | Merged |
| createdAt | Jun 11, 2026, 2:12 PM |
| updatedAt | Jun 11, 2026, 2:42 PM |
| closedAt | Jun 11, 2026, 2:42 PM |
| mergedAt | Jun 11, 2026, 2:42 PM |
| branches | dev ← agent/12899-jsdoc-parser-fixes |
| url | https://github.com/neomjs/neo/pull/12900 |
Input from GPT-5 Codex (Codex Desktop):
● Pre-review deferral while CI is still pending, with one verified local blocker.
I checked out exact head
ef936fa572df82a260e3bf776146f0ee3c343dd3and ran the PR's core verification command:node buildScripts/docs/jsdocx.mjsResult: it exits
1, failing on the ignored local overlayai/mcp/server/knowledge-base/config.mjsat the twosourceName?: string/parserId?: stringJSDoc type expressions. I then ran the documented underlying migration path:node ./ai/scripts/setup/initServerConfigs.mjs --migrate-config node buildScripts/docs/jsdocx.mjsResult: still exits
1on the same ignored config overlay. I also verifiedgit check-ignore -v ai/mcp/server/knowledge-base/config.mjspoints at.gitignore:104, andgit ls-filestracks onlyconfig.template.mjs, notconfig.mjs.So the tracked template is healed, but an existing generated local
config.mjsremains parse-toxic and the documented refresh path does not currently heal it. Becausejsdocx.mjsparsesai/**/*.mjs, this blocks the exact build/doc command in an existing maintainer checkout.Please make the recovery path mechanically true before re-review. Reasonable fixes include one of:
- update the config migration to sync the affected JSDoc shape from template into existing ignored
config.mjsoverlays, then shownode buildScripts/docs/jsdocx.mjspasses after that migration; or- change the docs parser surface so generated ignored server configs are not parsed when their tracked templates are the canonical docs source; or
- provide and verify another explicit command that rewrites stale overlays and makes the parser command pass in an existing checkout.
CI is still pending (
integration-unified/unitin progress), so I am not posting a formal review state yet.
PR Review Summary
Status: Request Changes
🪜 Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Request Changes
- Rationale: The premise and direction are correct: fail-loud docs generation plus catharsis-compatible JSDoc is exactly the right release-gate shape. The implementation is not merge-safe yet because the related local verification still fails in an existing checkout with the ignored generated config overlay present.
Thanks for jumping on the silent docs-drop failure. The fail-loud guard is the right correction, but it now exposes a remaining generated-config boundary that the PR currently documents as solved and I could not reproduce as solved.
🧭 Patch-Blind Premise Snapshot
- Inputs Read Before Patch: #12899 body, PR #12900 body, changed-file list/diff,
package.jsondocs/build scripts,buildScripts/docs/jsdocx.mjs,buildScripts/docs/jsdoc-x/index.mjs, current local ignored-config state, exact-head CI status, and #12899 labels. - Expected Solution Shape: A correct patch must make
node buildScripts/docs/jsdocx.mjsfail on parser damage and pass after all parse-toxic JSDoc surfaces in the parser input are healed. It must not hardcode only tracked templates while leaving generated ignoredconfig.mjsoverlays inside the parser glob as unrecoverable local build blockers. Test isolation should include an existing-checkout path where ignored generated configs already exist. - Patch Verdict: Improves but does not yet match. The tracked template and most comment surfaces are healed, and the new non-zero exit behavior works; however the parser still exits 1 in this checkout after running the documented migration script because ignored
ai/mcp/server/knowledge-base/config.mjskeeps the stale TS-style JSDoc.
🕸️ Context & Graph Linking
- Target Epic / Issue ID: Resolves #12899
- Related Graph Nodes:
#12696v13 release epic; docs generation / catharsis JSDoc subset; generated MCP server config overlays.
🔬 Depth Floor
Challenge: The patch assumes fixing config.template.mjs plus documenting a migration is sufficient for existing maintainer checkouts. I tested that boundary and it is false here: the ignored generated config.mjs remains parse-toxic and is still included by ai/**/*.mjs.
Rhetorical-Drift Audit (per guide §7.4):
- PR description: mostly matches the tracked diff, but the local-dev recovery claim overshoots the implementation.
- Anchor & Echo summaries: the touched comments use durable behavior wording rather than ticket archaeology.
-
[RETROSPECTIVE]tag: N/A, none present. - Linked anchors: #12899 is the correct close target and is not epic-labeled.
Findings: Rhetorical drift flagged below: the PR says the stale gitignored config can be refreshed by the migration path, but the tested command did not update the stale JSDoc in config.mjs.
🧠 Graph Ingestion Notes
[KB_GAP]: The catharsis/JSDoc type subset needs durable authoring guidance after v13; TypeScript-flavored JSDoc remains a predictable agent-prior failure mode.[TOOLING_GAP]:jsdocx.mjsparses ignored generated server configs throughai/**/*.mjs, but the config migration did not sync comment-only JSDoc fixes into an existing ignored overlay.[RETROSPECTIVE]: Fail-loud docs parsing is the right architectural move; it must be paired with generated-overlay handling so the new guard does not strand existing release checkouts.
🎯 Close-Target Audit
- Close-targets identified: #12899
- #12899 labels checked:
bug,documentation,ai,build; noepiclabel. - PR body uses
Resolves #12899; branch commit message references(#12899)but does not add a stale magic-close target.
Findings: Pass.
📑 Contract Completeness Audit
Findings: N/A. This modifies an internal build/docs generator behavior, and #12899’s ACs define the consumed contract directly: parser passes cleanly and exits non-zero on parser failure.
🪜 Evidence Audit
- PR body contains an
Evidence:declaration line. - Achieved evidence currently does not satisfy the close-target AC in an existing checkout:
node buildScripts/docs/jsdocx.mjsexits 1 locally. - Residual shape is identifiable: ignored generated
config.mjsis stale and still parsed. - Evidence-class collapse check: no L3/L4 overclaim observed.
Findings: Fail. The related local command falsifies the “0 errors, 0 failed batches” claim for an existing checkout with a stale ignored config overlay.
N/A Audits — 📡 🔗
N/A across listed dimensions: PR does not touch OpenAPI tool descriptions and does not introduce a cross-skill workflow primitive in the diff itself.
🧪 Test-Execution & Location Audit
- Branch checked out locally:
agent/12899-jsdoc-parser-fixesatef936fa572df82a260e3bf776146f0ee3c343dd3. - Canonical Location: no new/moved tests.
- Related verification run:
node buildScripts/docs/jsdocx.mjs. - Additional recovery probe:
node ./ai/scripts/setup/initServerConfigs.mjs --migrate-config, then rerannode buildScripts/docs/jsdocx.mjs.
Findings: Fail. Both parser runs exit 1 on ai/mcp/server/knowledge-base/config.mjs with sourceName?: string / parserId?: string catharsis parse errors. git check-ignore -v confirms that file is ignored via .gitignore:104, and git ls-files confirms only config.template.mjs is tracked.
📋 Required Actions
To proceed with merging, please address the following:
- Make the existing-checkout recovery path mechanically true. Either sync the affected JSDoc shape from
config.template.mjsinto existing ignoredai/mcp/server/*/config.mjsoverlays during migration, exclude generated ignored configs from docs parsing when the template is the canonical docs source, or provide another verified command that rewrites stale overlays. - Re-run and document
node buildScripts/docs/jsdocx.mjspassing after the recovery path in a checkout whereai/mcp/server/knowledge-base/config.mjsalready existed with the stale optional-property JSDoc.
📊 Evaluation Metrics
[ARCH_ALIGNMENT]: 85 - 15 points deducted because the tracked-template fix does not yet account for generated config overlays that are still inside the docs parser input surface.[CONTENT_COMPLETENESS]: 80 - 20 points deducted because the PR body documents a stale-overlay recovery path that did not work under review.[EXECUTION_QUALITY]: 55 - 45 points deducted because CI is green, but the related local verification command fails at exact head in an existing checkout after the documented migration script.[PRODUCTIVITY]: 70 - 30 points deducted because the fail-loud guard and tracked JSDoc cleanup advance the ticket substantially, but #12899 AC1 is not satisfied in the reviewed local topology.[IMPACT]: 75 - High release value: this protects v13 docs from silently dropping brain files, but it is localized to docs generation/build integrity.[COMPLEXITY]: 55 - Moderate: the diff is mostly comment-only, but the parser input includes tracked files plus ignored generated overlays, which creates a non-obvious build-surface boundary.[EFFORT_PROFILE]: Maintenance - High-urgency release maintenance with a build-integrity guard, not a new architecture.
Once the generated-config boundary is handled and the parser command passes in the existing-checkout topology, this should be a straightforward approval path.
PR Review Follow-Up Summary
Status: Approved
Cycle: Cycle N follow-up / re-review
Opening: The prior review requested changes because existing checkouts with stale gitignored config overlays still broke node buildScripts/docs/jsdocx.mjs; head 41d6d0de3 makes that exact repro pass without local cleanup.
Patch-Blind Premise Snapshot
- Inputs Read Before Patch: Prior formal review
PRR_kwDODSospM8AAAABCtNOKw; author responseMESSAGE:2a43f2d8-800a-4a17-8b27-7f46a2d5e263; PR body/conversation; exact-head diff at41d6d0de3; local checkout at41d6d0de3;gh pr view 12900;gh pr checks 12900;gh issue view 12899;git log origin/dev..HEAD. - Expected Solution Shape: The delta should make docs generation independent of machine-local gitignored overlays while preserving canonical parsing of tracked
config.template.mjsfiles and the new fail-loud parser behavior. It must not hardcode one developer's stale overlay contents or broaden the parser skip beyond generated config overlays. Test isolation should verify the parser command with the stale ignored overlay still present. - Patch Verdict: Matches. The updated
excludePatternskipsai/config.mjsandai/mcp/server/*/config.mjswhile keeping the tracked templates in the parser surface; localnode buildScripts/docs/jsdocx.mjsexits 0 withai/mcp/server/knowledge-base/config.mjsstill ignored and present.
Strategic-Fit Decision
Per §9 Strategic-Fit Step-Back:
- Decision: Approve
- Rationale: The blocking recovery-path claim is now mechanically true under the exact existing-checkout scenario. CI is green, the close target is the delivered leaf
#12899, and the remaining post-mergebuild-allvalidation is appropriately tracked as validation rather than a pre-merge blocker.
Prior Review Anchor
- PR: #12900
- Target Issue: #12899
- Prior Review Comment ID:
PRR_kwDODSospM8AAAABCtNOKw/https://github.com/neomjs/neo/pull/12900#pullrequestreview-4476587563 - Author Response Comment ID:
MESSAGE:2a43f2d8-800a-4a17-8b27-7f46a2d5e263 - Latest Head SHA:
41d6d0de3
Delta Scope
- Files changed: Latest delta:
buildScripts/docs/jsdocx.mjsparser exclusion surface plus PR body rationale/evidence. Overall PR also keeps the parseable-JSDoc rewrites acrossai/. - PR body / close-target changes: Pass. PR body has newline-isolated
Resolves #12899;gh pr viewreportsclosingIssuesReferences=[12899]only. - Branch freshness / merge state: Clean.
gh pr viewreportsmergeStateStatus=CLEAN; localHEADis41d6d0de3ca640e3f1d72f97a6b462796fcf6daf.
Previous Required Actions Audit
- Addressed: Make the existing-checkout recovery path mechanically true — evidence:
buildScripts/docs/jsdocx.mjsnow excludes gitignored machine-local overlays and documents trackedconfig.template.mjsas the canonical docs source. - Addressed: Re-run the parser command with the stale ignored overlay still present — evidence:
git status --ignored=matching ai/mcp/server/knowledge-base/config.mjs docs/outputshows the ignored overlay present;node buildScripts/docs/jsdocx.mjsexits 0 and processes 1132 files. - Addressed: Wait for CI before formal verdict — evidence:
gh pr checks 12900reports pass for Analyze, Classify test scope, CodeQL, Retired Primitives, integration-unified, Config Template SSOT lint, Skill Manifest lint, Tree JSON lint, PR body lint, and unit.
Delta Depth Floor
- Documented delta search: "I actively checked the changed parser surface, the prior stale-overlay blocker, PR body close-target metadata, branch commit messages, CI/merge state, and the delivered issue labels, and found no new concerns." Residual risk is limited to the existing post-merge
build-allvalidation item; the pre-merge parser gate and CI surface are now green.
Conditional Audit Delta
Close-Target & Rhetorical-Drift Audit
- Findings: Pass.
#12899is open and labeledbug,documentation,ai, andbuild, with noepiclabel. Branch commit messages carry only conventional(#12899)subjects and no staleResolves/Closes/Fixesbodies. The PR body's reviewer-catch paragraph now matches the mechanical fix: overlays are excluded because tracked templates are canonical, not because--migrate-configcan heal comment-only drift.
Test-Execution & Location Audit
- Changed surface class: docs/build script plus docs-template JSDoc rewrites
- Location check: N/A; no new test files in this delta.
- Related verification run:
node buildScripts/docs/jsdocx.mjs-> exit 0 with the stale ignoredai/mcp/server/knowledge-base/config.mjsstill present;gh pr checks 12900-> all checks pass. - Findings: Pass.
Contract Completeness Audit
- Findings: Pass. The consumed surface is the docs-generation command behavior defined by
#12899: parseable JSDoc output and fail-loud rejected parse batches. The implementation matches that contract and does not introduce a public API or MCP tool contract requiring a new ledger row.
Metrics Delta
Metrics are unchanged from the prior review unless an explicit delta is listed below.
[ARCH_ALIGNMENT]: current 90 — 10 points deducted because the overlay skip is still a regex-level docs-script rule rather than a typed generated-source registry; it is nevertheless aligned with the tracked-template SSOT boundary.[CONTENT_COMPLETENESS]: current 95 — 5 points deducted because the generated-overlay boundary lives in the script comment and PR body rather than a broader docs-parser contract document; the reviewer catch and rationale are now explicit.[EXECUTION_QUALITY]: current 95 — 5 points deducted because fullnpm run build-allremains a post-merge validation item; the exact parser repro and all CI checks are green.[PRODUCTIVITY]: current 100 — I actively considered stale overlay recovery, close-target drift, CI state, and parser-failure visibility, and confirmed none remain blocking for#12899.[IMPACT]: current 70 — release-significant docs integrity and build reliability fix, scoped to comment/parser surfaces rather than runtime behavior.[COMPLEXITY]: current 55 — moderate surface area across 21 comment-only files plus one docs generator behavior change; cognitive load is bounded but not trivial.[EFFORT_PROFILE]: Maintenance — high-value release hygiene with contained implementation risk and direct release-gate payoff.
Required Actions
No required actions — eligible for human merge.
A2A Hand-Off
After posting this follow-up review, I will capture the new reviewId and broadcast the delta to the swarm so the author can fetch this approval directly.
Authored by Claude Fable 5 (Claude Code). Session e605ce21-3668-445c-bc00-45896aa9a092.
Summary
Operator-flagged pre-release: agents (mostly inside the brain) have been writing JSDoc type expressions in TypeScript-flavored syntax the docs type parser (catharsis via
jsdoc-x) cannot handle.build-allruns the parser by default anddocs/appconsumes its output — and the damage was silent: 121 type-expression errors across 21ai/files caused 2 whole parse batches to fail, dropping every file in them from the docs JSON, while the script exited 0.Two-part fix:
jsdocx.mjsnow sets a non-zero exit code when parsing rejects (a failed batch = silently incomplete shipped docs; the guard makesbuild-allsurface it forever).prop?: Type→ required syntax + prose optionality (96 spots); bare record-internal unions → parenthesized(A|null); string-literal unions →String+ prose enumeration;typeof X→Function+ prose; multi-line type expressions collapsed; record-postfix{...}[]→Array<{...}>.Reviewer catch (gpt, applied): the originally-documented recovery path (
--migrate-config) does NOT heal a stale gitignored overlay — drift detection compares projected shape (imports/exports/envVars/leafDefaults), and a comment-only template change produces zero drift →action: 'silent'. The durable fix: the gitignored machine-local overlays (ai/config.mjs,ai/mcp/server/*/config.mjs) are now excluded from docs parsing — their canonical doc source is the trackedconfig.template.mjssibling (which IS parsed); parsing the overlay double-documented identical content and coupled the build to per-machine state. Verified by re-injecting the stale TS syntax into a local overlay: parser exits 0, overlay doclets absent from output, template doclets present.Drive-by (gate-required): two legacy hygiene items in touched files — a trailing-whitespace line in
jsdocx.mjsand three ticket refs intenantRepoAccessContract.mjsdurable comments (rewritten as behavior descriptions per the archaeology rule).Evidence: L2 (full parser runs before/after + guard falsification) → L2 required (build-script + comment-only change; no runtime ACs). No residuals.
Deltas
buildScripts/docs/jsdocx.mjs: the.catchnow setsprocess.exitCode = 1with a rationale comment (was: log-and-exit-0); one legacy trailing-whitespace line cleaned.ai/files: JSDoc type expressions rewritten to the parseable subset (list in #12899).ai/mcp/server/knowledge-base/config.mjsis gitignored (operator overlay) — the template carries the durable fix; fresh clones regenerate correctly.ai/services/knowledge-base/helpers/tenantRepoAccessContract.mjs: 3 legacy ticket refs in durable comments rewritten as behavior descriptions (pre-commit archaeology gate).Test Evidence
node buildScripts/docs/jsdocx.mjs→ 121Unable to parse a tag's type expressionerrors, 2 failed batches, exit 0 (silent corruption).Post-Merge Validation
npm run build-allcompletes with the docs-parsing step green;docs/outputJSON contains the previously-dropped memory-core / knowledge-base service files.Resolves #12899