Replace docstring-coverage with a zero-tolerance TypeDoc gate#62
Replace docstring-coverage with a zero-tolerance TypeDoc gate#62leynos wants to merge 5 commits into
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
Summary
Validation
WalkthroughAdds a strict TypeDoc documentation gate, replaces the former coverage check, introduces CI workflow automation, and expands ODW module documentation and selected exported contracts. ChangesDocumentation and CI validation
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 19 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (19 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.3)workflows/df12-build-odw.jsFile contains syntax errors that prevent linting: Line 4037: Illegal return statement outside of a function Comment |
191cc13 to
d4d660c
Compare
There was a problem hiding this comment.
Sorry @leynos, your pull request is larger than the review limit of 150000 diff characters
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/workflows/df12-build-odw/host-review.ts (1)
484-499: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFail closed when the CodeScene probe itself fails.
runCodeSceneCheckmaps everyprobe.ok === falseto “binary not on PATH” andskipped: true, butExecStatus.okis also false for spawn, timeout, signal, and other probe faults. Skip only on a confirmed command-not-found result; surface other probe failures as gate failures so a broken host cannot silently disable an enabled check.🤖 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 `@src/workflows/df12-build-odw/host-review.ts` around lines 484 - 499, Update runCodeSceneCheck to distinguish a confirmed command-not-found probe result from other ExecStatus failures: set skipped only for the specific missing-binary condition, and treat spawn, timeout, signal, and other probe faults as clean: false with actionable detail. Preserve the existing skip behavior for environments where the CodeScene binary is genuinely absent.src/workflows/df12-build-odw/remediation.ts (2)
102-106: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not silently drop title-less proposals.
RemediationProposal.titleis optional, butif (!key) continueremoves such proposals before triage. Make the title mandatory at the boundary and reject invalid input explicitly, or preserve it in an explicit invalid-proposal result; never discard a review or audit finding without recording it.🤖 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 `@src/workflows/df12-build-odw/remediation.ts` around lines 102 - 106, Update dedupeProposals so title-less proposals are not silently discarded by the if (!key) continue branch. Enforce a mandatory title at the input boundary and explicitly reject invalid proposals, or preserve them as an explicit invalid-proposal result; ensure every review or audit finding remains recorded.
107-117: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve merged source metadata across deduplication passes.
dedupeProposalswritessourcesbut returnsRemediationProposal[], whose contract does not expose that field. Re-running the exported helper then rebuildssourcesonly fromsourceorrationale, so a proposal carrying['review:1', 'audit:2']can lose an origin and maketriageNeedsEscalationselect the medium model. Return a named deduped type and merge existingsourcesbefore adding the legacy fallback.🤖 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 `@src/workflows/df12-build-odw/remediation.ts` around lines 107 - 117, The dedupeProposals result must preserve all existing source metadata across repeated deduplication passes. Introduce and return a named deduped proposal type that exposes sources, then update the source aggregation around the source/rationale fallback to merge proposal.sources first and add the fallback only when it is not already present; keep deduplication and triage behavior unchanged.
🤖 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 @.github/workflows/ci.yml:
- Around line 11-13: Add workflow-level concurrency configuration near the top
of the CI workflow, using a group that identifies the workflow or branch and
setting cancel-in-progress to true. Keep the existing test job and its
sequential gate execution unchanged.
- Line 33: Update the CI Markdown validation setup around the markdownlint-cli2
installation to use a repository-managed, lockfile-backed dependency instead of
global npm installation. Add or reuse the project binary and committed npm
lockfile, then invoke the locked dependency without allowing transitive
resolution to occur outside version-controlled files.
In `@docs/developers-guide.md`:
- Around line 513-522: Update the documentation description near the docs-check
target to reflect the expanded configured entry-point scope rather than claiming
validation covers the entire src/workflows/df12-build-odw tree. Explicitly name
the typedoc.json exclusions for declaration files, meta.js, and internal,
private, and protected reflections, while preserving the remaining validation
behavior.
In `@src/workflows/df12-build-odw/config.ts`:
- Around line 278-286: The makeConfig documentation incorrectly describes
WorkflowConfig as immutable despite exposing mutable collections. Change the
wording in the makeConfig JSDoc from “immutable” to “fully resolved” and leave
the existing configuration behavior unchanged.
In `@src/workflows/df12-build-odw/git-evidence.ts`:
- Around line 199-205: Update the file-reading implementation documented by the
JSDoc above to eliminate the parent-symlink TOCTOU window, preferably by
resolving and opening each path component through race-resistant directory
handles while retaining final-component protection and worktree containment
checks. If that cannot be implemented safely, narrow the JSDoc guarantee so it
no longer claims untrusted content cannot redirect the read.
In `@src/workflows/df12-build-odw/recovery-decision.ts`:
- Around line 70-71: Align the exported `validation` contract and
`recoveryResumeEligibility` behavior: either update the documentation to
describe the actual missingEvidence-only downgrade rule, or restore validation’s
non-blank requirement in the eligibility logic. Ensure the documented contract
and runtime decision table express the same resume behavior.
In `@src/workflows/df12-build-odw/roadmap.ts`:
- Around line 71-76: Update expandStepRange to validate both start and end
arguments against the roadmap step-ID grammar before converting them to numbers,
returning [] for empty, negative, malformed, or otherwise invalid IDs while
preserving valid same-phase range expansion. Keep the existing cross-phase,
non-integer, and reversed-range handling and ensure the JSDoc contract remains
accurate.
In `@src/workflows/df12-build-odw/write-preflight.ts`:
- Around line 113-125: Update verifyWriteProbe so special files, including
FIFOs, are rejected before any potentially blocking fs.open call. Inspect the
path without opening it, or use non-blocking open with a bounded fail-closed
read, while preserving the documented regular-file and token-match checks.
---
Outside diff comments:
In `@src/workflows/df12-build-odw/host-review.ts`:
- Around line 484-499: Update runCodeSceneCheck to distinguish a confirmed
command-not-found probe result from other ExecStatus failures: set skipped only
for the specific missing-binary condition, and treat spawn, timeout, signal, and
other probe faults as clean: false with actionable detail. Preserve the existing
skip behavior for environments where the CodeScene binary is genuinely absent.
In `@src/workflows/df12-build-odw/remediation.ts`:
- Around line 102-106: Update dedupeProposals so title-less proposals are not
silently discarded by the if (!key) continue branch. Enforce a mandatory title
at the input boundary and explicitly reject invalid proposals, or preserve them
as an explicit invalid-proposal result; ensure every review or audit finding
remains recorded.
- Around line 107-117: The dedupeProposals result must preserve all existing
source metadata across repeated deduplication passes. Introduce and return a
named deduped proposal type that exposes sources, then update the source
aggregation around the source/rationale fallback to merge proposal.sources first
and add the fallback only when it is not already present; keep deduplication and
triage behavior unchanged.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a35c3e19-849b-4a0a-943f-b7fcfb06c95d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (27)
.github/workflows/ci.ymlAGENTS.mdMakefiledocs/architecture.mddocs/developers-guide.mdpackage.jsonscripts/docstring-coverage.mjssrc/workflows/df12-build-odw/assessment.tssrc/workflows/df12-build-odw/config.tssrc/workflows/df12-build-odw/exec.tssrc/workflows/df12-build-odw/execplan-durability.tssrc/workflows/df12-build-odw/faults.tssrc/workflows/df12-build-odw/git-evidence.tssrc/workflows/df12-build-odw/host-review.tssrc/workflows/df12-build-odw/main.tssrc/workflows/df12-build-odw/prompts.tssrc/workflows/df12-build-odw/recovery-decision.tssrc/workflows/df12-build-odw/recovery-discovery.tssrc/workflows/df12-build-odw/remediation.tssrc/workflows/df12-build-odw/roadmap.tssrc/workflows/df12-build-odw/run-task.tssrc/workflows/df12-build-odw/schemas.tssrc/workflows/df12-build-odw/types.tssrc/workflows/df12-build-odw/write-preflight.tstests/modules/docstring-coverage.test.tstypedoc.jsonworkflows/df12-build-odw.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/nixie(auto-detected)
💤 Files with no reviewable changes (2)
- tests/modules/docstring-coverage.test.ts
- scripts/docstring-coverage.mjs
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial
Cancel superseded CI runs.
Add a workflow-level concurrency group with cancel-in-progress: true; each push currently leaves obsolete make all and strict Dafny jobs running, wasting runners and allowing stale results to complete.
As per coding guidelines, keep gate execution sequential; the supplied static analysis identifies the missing concurrency limit.
🧰 Tools
🪛 zizmor (1.26.1)
[info] 12-12: workflow or action definition without a name (anonymous-definition): this job
(anonymous-definition)
🤖 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 @.github/workflows/ci.yml around lines 11 - 13, Add workflow-level
concurrency configuration near the top of the CI workflow, using a group that
identifies the workflow or branch and setting cancel-in-progress to true. Keep
the existing test job and its sequential gate execution unchanged.
Sources: Coding guidelines, Linters/SAST tools
| - name: Install Markdown and Mermaid validators | ||
| run: | | ||
| set -euo pipefail | ||
| npm install --global "markdownlint-cli2@${MARKDOWNLINT_CLI2_VERSION}" |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Make the Markdown validator installation lockfile-backed.
npm install --global pins only the direct package and resolves transitive dependencies outside a committed lockfile. Move markdownlint-cli2 into a lockfile-backed project binary, or install it from a committed npm lockfile, so the CI toolchain cannot change without a repository diff.
As per coding guidelines, keep dependency resolution reproducible through committed lockfiles; the supplied static analysis identifies this installation as ad hoc.
🧰 Tools
🪛 zizmor (1.26.1)
[warning] 33-33: ad-hoc installation of packages (adhoc-packages): installs a package outside of a lockfile
(adhoc-packages)
🤖 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 @.github/workflows/ci.yml at line 33, Update the CI Markdown validation setup
around the markdownlint-cli2 installation to use a repository-managed,
lockfile-backed dependency instead of global npm installation. Add or reuse the
project binary and committed npm lockfile, then invoke the locked dependency
without allowing transitive resolution to occur outside version-controlled
files.
Sources: Coding guidelines, Linters/SAST tools
| The `docs-check` target (run by `make all`, wrapping `bun run docs:check`) is | ||
| the zero-tolerance documentation gate: TypeDoc's `notDocumented` validation | ||
| over the whole `src/workflows/df12-build-odw/` tree, configured by | ||
| `typedoc.json`. Every module must open with a `/** … @module */` block and | ||
| every exported declaration must carry a JSDoc block; validation warnings are | ||
| errors, the run emits no documentation artefacts, and a failure prints the | ||
| qualified name and location of each undocumented declaration. JSON Schema | ||
| constants are tagged `@internal` (their `description` fields are the | ||
| per-field documentation), so TypeDoc does not recurse into the schema | ||
| literals: |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
State the TypeDoc exclusions accurately.
Replace “over the whole src/workflows/df12-build-odw/ tree” with the expanded entry-point scope and name the configured exclusions: declaration files, meta.js, and internal/private/protected reflections. The current wording overstates the enforced scope and conflicts with typedoc.json.
As per path instructions, keep documentation accurate and current.
🤖 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 `@docs/developers-guide.md` around lines 513 - 522, Update the documentation
description near the docs-check target to reflect the expanded configured
entry-point scope rather than claiming validation covers the entire
src/workflows/df12-build-odw tree. Explicitly name the typedoc.json exclusions
for declaration files, meta.js, and internal, private, and protected
reflections, while preserving the remaining validation behavior.
Source: Path instructions
| /** | ||
| * Resolve raw workflow overrides into the immutable {@link WorkflowConfig} used | ||
| * throughout the run: applies every default, clamps numeric bounds, coerces | ||
| * string-typed args, and precomputes the derived guidance strings. Pure and | ||
| * side-effect free — the `projectRoot` chdir is left to the caller — so it is | ||
| * safe to build once and share. A null or missing argument yields the full | ||
| * default configuration. Throws when `resumeMode` is not one of `assess`, | ||
| * `review`, or `continue`. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Stop advertising a mutable configuration as immutable.
WorkflowConfig exposes mutable Set<string>, Record<string, string>, and string[] properties, so makeConfig does not provide an immutable TypeScript contract. Change “immutable” to “fully resolved”, or mark the relevant fields and containers readonly and avoid mutating them.
🤖 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 `@src/workflows/df12-build-odw/config.ts` around lines 278 - 286, The
makeConfig documentation incorrectly describes WorkflowConfig as immutable
despite exposing mutable collections. Change the wording in the makeConfig JSDoc
from “immutable” to “fully resolved” and leave the existing configuration
behavior unchanged.
Source: Coding guidelines
| /** | ||
| * Read a worktree file without letting untrusted content redirect the read | ||
| * outside the checkout (see write-preflight.ts). O_NOFOLLOW rejects a symlink | ||
| * at the FINAL component; when a worktree root is given, realpath containment | ||
| * additionally rejects a symlinked PARENT directory that would resolve the | ||
| * ancestry outside the root. Callers treat any failure as an unreadable file. | ||
| */ |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Close the parent-symlink TOCTOU window or narrow the guarantee.
The new comment claims untrusted content cannot redirect the read, but the implementation checks realpath(path.dirname(filePath)) and then opens filePath; an attacker can swap a parent symlink between those operations because O_NOFOLLOW protects only the final component. Use a race-resistant directory-handle approach, or remove the absolute guarantee from the JSDoc.
🤖 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 `@src/workflows/df12-build-odw/git-evidence.ts` around lines 199 - 205, Update
the file-reading implementation documented by the JSDoc above to eliminate the
parent-symlink TOCTOU window, preferably by resolving and opening each path
component through race-resistant directory handles while retaining
final-component protection and worktree containment checks. If that cannot be
implemented safely, narrow the JSDoc guarantee so it no longer claims untrusted
content cannot redirect the read.
| /** Free-text validation evidence; a non-blank value is required to resume. */ | ||
| validation?: string |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Align the validation documentation with the decision table.
Line 71 says non-blank validation is required to resume, but recoveryResumeEligibility explicitly makes missingEvidence the sole evidence-based downgrade gate and does not reject blank validation. Update the contract text or restore the check; do not leave the exported documentation inconsistent with runtime behaviour.
🤖 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 `@src/workflows/df12-build-odw/recovery-decision.ts` around lines 70 - 71,
Align the exported `validation` contract and `recoveryResumeEligibility`
behavior: either update the documentation to describe the actual
missingEvidence-only downgrade rule, or restore validation’s non-blank
requirement in the eligibility logic. Ensure the documented contract and runtime
decision table express the same resume behavior.
| /** | ||
| * Expand a `steps X.Y-X.Z` range into its constituent ids. Returns `[]` for a | ||
| * malformed or cross-phase range (differing phase numbers, non-integer | ||
| * steps, or a start after the end) rather than throwing, so a caller parsing | ||
| * free text can simply skip the phrase. | ||
| */ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the malformed-range contract true.
The new JSDoc promises [] for malformed inputs, but the implementation only checks numeric integrality; direct calls such as expandStepRange('', '') or expandStepRange('-1.2', '-1.3') return invalid roadmap IDs. Validate both arguments against the roadmap-ID grammar before numeric conversion, or narrow the JSDoc to callers that already matched STEP_RANGE_RE.
🤖 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 `@src/workflows/df12-build-odw/roadmap.ts` around lines 71 - 76, Update
expandStepRange to validate both start and end arguments against the roadmap
step-ID grammar before converting them to numbers, returning [] for empty,
negative, malformed, or otherwise invalid IDs while preserving valid same-phase
range expansion. Keep the existing cross-phase, non-integer, and reversed-range
handling and ensure the JSDoc contract remains accurate.
| /** | ||
| * Host-side verification of one adapter's write probe. Opens the probe | ||
| * path with `O_NOFOLLOW` so the existence check and the content read | ||
| * target the same inode, closing the symlink-swap window between a | ||
| * separate stat and a path-based read; a symlink or non-regular file is | ||
| * treated as a rejected probe, not a pass. | ||
| */ | ||
| export async function verifyWriteProbe(probeFile: string, token: string): Promise<{ | ||
| /** True only when the probe file existed, was a regular file, and its content matched the expected token. */ | ||
| ok: boolean | ||
| /** Human-readable failure reason; empty when `ok` is true. */ | ||
| detail: string | ||
| }> { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
Reject special files before a blocking open.
verifyWriteProbe opens the path before calling handle.stat(). A planted FIFO can block fs.open indefinitely, so the documented “non-regular file rejected” path never runs. Inspect without a blocking open, or open non-blocking and enforce a bounded, fail-closed probe.
🤖 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 `@src/workflows/df12-build-odw/write-preflight.ts` around lines 113 - 125,
Update verifyWriteProbe so special files, including FIFOs, are rejected before
any potentially blocking fs.open call. Inspect the path without opening it, or
use non-blocking open with a bounded fail-closed read, while preserving the
documented regular-file and token-match checks.
Bring the whole `src/workflows/df12-build-odw/` tree up to the declaration-level documentation standard ahead of replacing the threshold-based docstring-coverage gate with a zero-tolerance TypeDoc gate: every exported function, interface, type alias, variable, and property now carries a JSDoc block, and modules that opened with `//` line-comment headers now open with `/** @file … */` blocks. Documentation is comment-only, with three deliberate exceptions needed so TypeDoc can attach comments to anonymous object types: three single-line return-type literals in `run-task.ts` are reflowed to multiline, and two `return { … }` shorthands are expanded, with shapes unchanged. The JSON Schema constants (`*_SCHEMA`) keep their inferred literal types (module tests pin deep paths into them) and are tagged `@internal` as machine seams — their `description` fields remain the per-field documentation. Regenerate `workflows/df12-build-odw.js` from the annotated tree.
Swap the threshold-based, allow-listed `docstring-coverage` gate for `docs-check`: TypeDoc's `notDocumented` validation over the whole `src/workflows/df12-build-odw/` tree (`entryPointStrategy: "expand"`, `emit: "none"`, warnings as errors). The gate requires 100% documentation of the selected surface, reports the qualified name of each undocumented declaration, writes no documentation artefacts, and runs through `make all` via `bun run docs:check`. - Convert every module's leading `/** @file … */` block to TypeDoc's `/** … @module */` form and require `"Module"` documentation, so the existing file-level documentation rule is preserved, not retired. - Delete `scripts/docstring-coverage.mjs`, its test, and the `DOCSTRING_MODULES`/`DOCSTRING_MIN` allow-list — the custom AST-counting gate and its percentage threshold are superseded. - Exclude `*.d.ts` (ambient ODW globals) and `meta.js` (verbatim artefact banner) from the documentation surface. - Update AGENTS.md, the developers' guide, and the architecture verification contract to name the new gate and local command. Verified: removing a JSDoc block from an exported function fails `docs:check` with the symbol's qualified name and a non-zero exit; restoring it returns the gate to green; a clean run emits no files.
Import the CI shape from `leynos/agent-template-typescript` (pinned `checkout`/`setup-bun`/`setup-uv` actions, read-only token) and extend it to this repository's aggregate gate: `bun install`, pinned `markdownlint-cli2` and `nixie-cli` installs, a pinned Dafny 4.10.0 .NET tool install, then `make all` followed by `make verify-modules-strict` so the LemmaScript/Dafny proof is a real PR gate rather than a silent local skip. The checkout fetches full history because `check-fmt` diffs back to the merge-base with `origin/main`. This also gives the new `docs-check` documentation gate a CI path through the same aggregate command used locally.
Restore the advisory residual-risk channel added on `main` while retaining the zero-tolerance TypeDoc coverage introduced by this branch. Document the upstream declarations that entered the documentation surface and regenerate the compiled ODW workflow from the resolved source tree.
|
@coderabbitai resume |
✅ Action performedReviews resumed. |
Harden roadmap ranges, write probes, CodeScene probing, and remediation proposal handling where review found live correctness gaps. Make CI dependency installation and concurrency deterministic, align the documentation contracts with the configured behaviour, and keep the generated ODW artefact synchronized with the source.
bcb1978 to
1c8c67f
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/workflows/df12-build-odw/roadmap.ts (1)
77-85: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winBound accepted range sizes before materializing ids.
Reject oversized but syntactically valid ranges before
Array.from; aRequires
range spanning millions of steps allocates millions of strings during roadmap
parsing and can stall or terminate the workflow. Define and document a maximum
range length, return[]beyond it, and add a boundary test.🤖 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 `@src/workflows/df12-build-odw/roadmap.ts` around lines 77 - 85, Update expandStepRange to define and document a maximum allowed range length, validate the calculated lastStep - firstStep + 1 before Array.from, and return [] when the length exceeds that limit. Add a boundary test covering the maximum accepted length and the first rejected oversized range.
🤖 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 @.github/workflows/ci.yml:
- Around line 39-40: Update the “Install Dafny” workflow step to append
$HOME/.dotnet/tools to $GITHUB_PATH after installing the global tool, then run
dafny --version in that step to validate the executable is available. Preserve
the existing DAFNY_VERSION installation and ensure the later
verify-modules-strict command can resolve dafny via PATH.
- Around line 29-32: Update the oven-sh/setup-bun step to specify a fixed
bun-version or bun-version-file, ensuring CI uses a deterministic Bun release
before the Install JavaScript dependencies step runs.
In `@src/workflows/df12-build-odw/execplan-durability.ts`:
- Around line 72-79: Update execplanRelPath to enforce the normalized path
against the task-scoped docs/execplans/*.md predicate before returning ok: true.
Reject paths such as docs/execplans/../../README.md with the existing detail
failure shape, and ensure run-task.ts receives ok only for valid ExecPlan paths
before performing filesystem checks or calling commitExecplanDraft.
In `@tests/modules/remediation.test.ts`:
- Around line 56-70: Create a deterministic proposal() fixture factory in the
remediation tests with stable default proposal fields, then replace the repeated
inline proposal object literals in the titleless and repeated-deduplication
tests with factory calls and overrides for title, source, and sources. Preserve
the existing test scenarios and expected aggregation behavior.
---
Outside diff comments:
In `@src/workflows/df12-build-odw/roadmap.ts`:
- Around line 77-85: Update expandStepRange to define and document a maximum
allowed range length, validate the calculated lastStep - firstStep + 1 before
Array.from, and return [] when the length exceeds that limit. Add a boundary
test covering the maximum accepted length and the first rejected oversized
range.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 89c1fd6b-fd6f-4d28-86ee-6686ed24f370
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (31)
.github/workflows/ci.ymlAGENTS.mdMakefiledocs/architecture.mddocs/developers-guide.mdpackage.jsonscripts/docstring-coverage.mjssrc/workflows/df12-build-odw/assessment.tssrc/workflows/df12-build-odw/config.tssrc/workflows/df12-build-odw/exec.tssrc/workflows/df12-build-odw/execplan-durability.tssrc/workflows/df12-build-odw/faults.tssrc/workflows/df12-build-odw/git-evidence.tssrc/workflows/df12-build-odw/host-review.tssrc/workflows/df12-build-odw/main.tssrc/workflows/df12-build-odw/prompts.tssrc/workflows/df12-build-odw/recovery-decision.tssrc/workflows/df12-build-odw/recovery-discovery.tssrc/workflows/df12-build-odw/remediation.tssrc/workflows/df12-build-odw/roadmap.tssrc/workflows/df12-build-odw/run-task.tssrc/workflows/df12-build-odw/schemas.tssrc/workflows/df12-build-odw/types.tssrc/workflows/df12-build-odw/write-preflight.tstests/modules/docstring-coverage.test.tstests/modules/host-review.test.tstests/modules/remediation.test.tstests/modules/roadmap.property.test.tstests/modules/write-preflight.test.tstypedoc.jsonworkflows/df12-build-odw.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
leynos/nixie(auto-detected)
💤 Files with no reviewable changes (2)
- tests/modules/docstring-coverage.test.ts
- scripts/docstring-coverage.mjs
| - uses: oven-sh/setup-bun@735343b667d3e6f658f44d0eca948eb6282f2b76 | ||
| - uses: astral-sh/setup-uv@c5680ec91f7b9b91406fab4ded5d45245abf7b67 | ||
| - name: Install JavaScript dependencies | ||
| run: bun install --frozen-lockfile |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== ci.yml relevant section =="
nl -ba .github/workflows/ci.yml | sed -n '1,120p'
echo
echo "== bun lock/package hints =="
rg -n --hidden -S 'bun-version|setup-bun|bun install|bun.lock|packageManager|tool-versions' .github . || trueRepository: leynos/df12-build
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== .github/workflows/ci.yml (top 120 lines) =="
sed -n '1,120p' .github/workflows/ci.yml
echo
echo "== workflow references to bun-version/setup-bun/bun install =="
rg -n -S 'bun-version|setup-bun|bun install|bun.lock|packageManager|tool-versions' .github . || trueRepository: leynos/df12-build
Length of output: 1974
🌐 Web query:
oven-sh/setup-bun bun-version default latest documentation
💡 Result:
For the oven-sh/setup-bun GitHub Action, the default behavior for the bun-version input is determined by a hierarchical check [1][2]. If no version is explicitly provided, the action will resolve the version in the following order of priority: 1. package.json: It first checks the packageManager field (e.g., "packageManager": "bun@1.0.25") [1][2]. 2. package.json: If the packageManager field is not present, it checks the engines.bun field [1][2]. 3. latest: If neither of the above fields exists or if no package.json file is found, it defaults to the latest version of Bun [1][2]. You can also override this behavior by explicitly setting the bun-version input in your workflow YAML: - uses: oven-sh/setup-bun@v2 with: bun-version: latest Additionally, the action supports a bun-version-file input (e.g., pointing to.bun-version or.tool-versions) which provides further flexibility for managing versions [1][3].
Citations:
- 1: https://github.com/oven-sh/setup-bun
- 2: feat: use packageManager from package.json as default bun version oven-sh/setup-bun#152
- 3: https://github.com/marketplace/actions/setup-bun
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package metadata =="
if [ -f package.json ]; then
sed -n '1,120p' package.json
fi
echo
echo "== version files =="
find . -maxdepth 2 \( -name '.bun-version' -o -name '.tool-versions' -o -name 'package.json' \) -print | sortRepository: leynos/df12-build
Length of output: 624
Pin Bun in CI. Set bun-version (or a bun-version-file) on oven-sh/setup-bun in .github/workflows/ci.yml:29-32; this repo has no packageManager, engines.bun, or version file, so the action resolves to the latest Bun release on every 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 @.github/workflows/ci.yml around lines 29 - 32, Update the oven-sh/setup-bun
step to specify a fixed bun-version or bun-version-file, ensuring CI uses a
deterministic Bun release before the Install JavaScript dependencies step runs.
| - name: Install Dafny | ||
| run: dotnet tool install --global dafny --version "${DAFNY_VERSION}" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## ci.yml around dafny install\n'
grep -n -A6 -B6 'dotnet tool install --global dafny' .github/workflows/ci.yml || true
printf '\n## search for dafny usage / verify-modules-strict\n'
rg -n 'verify-modules-strict|command -v dafny|dafny --version|dotnet tool install --global dafny|GITHUB_PATH|\.dotnet/tools' -S . || true
printf '\n## relevant Makefile lines if present\n'
fd -a 'Makefile|makefile|GNUmakefile' . -t f | while read -r f; do
echo "--- $f"
rg -n 'verify-modules-strict|command -v dafny|dafny' "$f" || true
doneRepository: leynos/df12-build
Length of output: 2348
Add $HOME/.dotnet/tools to PATH before the strict Dafny gate. Update the Install Dafny step in .github/workflows/ci.yml to write the global tool directory to $GITHUB_PATH and run dafny --version there; make verify-modules-strict later calls command -v dafny, and the global .NET install lands under ~/.dotnet/tools on Linux.
🤖 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 @.github/workflows/ci.yml around lines 39 - 40, Update the “Install Dafny”
workflow step to append $HOME/.dotnet/tools to $GITHUB_PATH after installing the
global tool, then run dafny --version in that step to validate the executable is
available. Preserve the existing DAFNY_VERSION installation and ensure the later
verify-modules-strict command can resolve dafny via PATH.
| export function execplanRelPath(worktree: string, planPath: unknown): { | ||
| /** True when the path resolves inside the worktree. */ | ||
| ok: boolean | ||
| /** The path relative to the worktree; empty when `ok` is false. */ | ||
| relPath: string | ||
| /** Empty when `ok`; otherwise why the path was rejected. */ | ||
| detail: string | ||
| } { |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Path Traversal (CWE-22): Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal')
Reachability
● Entry
src/workflows/df12-build-odw/main.ts:268
makeAssessment
│
▼
● Hop
src/workflows/df12-build-odw/assessment.ts:47
SalvageRecord: The assessment classification that triggered (or skipped) the salvage.
│
▼
● Sink
src/workflows/df12-build-odw/execplan-durability.ts
Enforce the ExecPlan scope before accepting ok.
execplanRelPath accepts any path inside the worktree, so docs/execplans/../../README.md normalises to README.md and returns ok: true. src/workflows/df12-build-odw/run-task.ts then passes that path into file checks and commitExecplanDraft, allowing an arbitrary in-worktree file to be treated and committed as the ExecPlan. Reuse the task-scoped docs/execplans/*.md predicate after normalisation, before any filesystem or Git operation.
🤖 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 `@src/workflows/df12-build-odw/execplan-durability.ts` around lines 72 - 79,
Update execplanRelPath to enforce the normalized path against the task-scoped
docs/execplans/*.md predicate before returning ok: true. Reject paths such as
docs/execplans/../../README.md with the existing detail failure shape, and
ensure run-task.ts receives ok only for valid ExecPlan paths before performing
filesystem checks or calling commitExecplanDraft.
| test('rejects titleless proposals explicitly', () => { | ||
| expect(() => dedupeProposals([{ title: '', rationale: 'audit:1.1.1' }])).toThrow(/non-blank string/) | ||
| }) | ||
|
|
||
| test('preserves aggregated sources across repeated deduplication', () => { | ||
| const first = dedupeProposals([ | ||
| { title: 'Fix flaky teardown', source: 'audit:1.2.3' }, | ||
| { title: 'fix flaky teardown', source: 'review:1.2.4' }, | ||
| ]) | ||
| const second = dedupeProposals([ | ||
| ...first, | ||
| { title: ' FIX FLAKY TEARDOWN ', sources: ['audit:1.2.3', 'expert:1.2.5'], source: 'review:1.2.4' }, | ||
| ]) | ||
| expect(second).toHaveLength(1) | ||
| expect(second[0].sources).toEqual(['audit:1.2.3', 'review:1.2.4', 'expert:1.2.5']) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Replace repeated proposal literals with a fixture factory.
Create a deterministic proposal() factory and use overrides for title, source,
and sources variations. This keeps the revised contract tests concise and
prevents fixture defaults from drifting.
As per coding guidelines, “Keep module tests deterministic and use fixtures and
factories instead of ad hoc object literals.”
Also applies to: 123-125
🤖 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 `@tests/modules/remediation.test.ts` around lines 56 - 70, Create a
deterministic proposal() fixture factory in the remediation tests with stable
default proposal fields, then replace the repeated inline proposal object
literals in the titleless and repeated-deduplication tests with factory calls
and overrides for title, source, and sources. Preserve the existing test
scenarios and expected aggregation behavior.
Source: Coding guidelines
1 participant
Summary
This branch replaces the threshold-based, allow-listed
docstring-coveragegate with a zero-tolerance TypeDoc documentationgate, and gives the repository a CI workflow that runs the full
commit-gate suite. TypeDoc's
notDocumentedvalidation now runs overthe whole
src/workflows/df12-build-odwmodule tree withentryPointStrategy: "expand",emit: "none", and warnings treated aserrors: every module must open with a
/** … @module */block and everyexported declaration must carry a JSDoc comment. The gate reports the
qualified name of each undocumented declaration, writes no documentation
artefacts, and fails on a single omission — replacing the previous 80%
threshold over a three-module allow-list.
The branch lands in four commits after rebasing onto
main: the first documents every exported declaration in the module tree (945 TypeDoc warnings' worth) while the old gate still runs; the second swaps the gate, converts@fileheaders to TypeDoc's@moduleform (preserving the file-level documentation rule rather than retiring it), and deletes the custom AST-counting script with itsDOCSTRING_MODULES/DOCSTRING_MINallow-list; the third adds the CI workflow, imported fromleynos/agent-template-typescriptand extended to this repository's aggregate; the fourth preservesmain's advisory residual-risk recovery contract and documents the declarations introduced upstream.Review walkthrough
for the gate's configuration: the documentation surface, the
exclusions (ambient
odw-globals.d.ts, the verbatimmeta.jsbanner), the required-kind list including
"Module", andtreatValidationWarningsAsErrors.and package.json:
docs-check(wrappingbun run docs:check) replacesdocstring-coverageinsidemake all.runs
make allthenmake verify-modules-strictwith pinned installsof
markdownlint-cli2,nixie-cli, and Dafny 4.10.0, honouring theMakefile's requirement that CI runs the strict Dafny proof rather than
the lenient skip.
src/workflows/df12-build-odw/config.ts
(the largest surface: every run-configuration knob, default, and
clamp) and
src/workflows/df12-build-odw/recovery-decision.ts
(the decision-table docs mirror the Dafny-verified twin under
verify/).shows the
*_SCHEMAtreatment: documented/** … @internal */blocksso TypeDoc does not recurse into the JSON Schema literals (module
tests pin deep literal paths, so the constants keep their inferred
types); their
descriptionfields remain the per-field documentation.docs/developers-guide.md,
and docs/architecture.md
for the updated conventions and verification contract.
Validation
make all(check-fmt, lint, typecheck, markdownlint, nixie,docs-check, module and artefact test suites, workflow-freshness,verify-modules with Dafny on PATH): exit 0 on each commit, run
sequentially via the scrutineer gate-runner. Module tests: 360
pass, 0 fail; artefact tests: 124/124; operator-script tests: 21/21;
Dafny: 4 verified, 0 errors.
makes
docs:checkexit non-zero withexec.shellQuote (CallSignature) … does not have any documentation;restoring it returns the gate to green.
docs:checkrun emits no files and no output; the regeneratedworkflows/df12-build-odw.jsartefact is committed alongside thesource (
workflow-freshnessgreen).Notes
comments to anonymous object types: three single-line return-type
literals in
src/workflows/df12-build-odw/run-task.ts
are reflowed to multiline, and two
return { … }shorthands areexpanded, with shapes unchanged.
pre-existing
spelling.ymlremains untouched even thoughmake allalso covers spelling.
References