feat(rules): add TypeScript project gate coverage - #619
Conversation
This comment has been minimized.
This comment has been minimized.
github-actions
Bot
added
the
documentation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
de7623d to
eb9d5cf
Compare
eb9d5cf to
9719d3a
Compare
This comment has been minimized.
This comment has been minimized.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (15)
fixtures/rules/vitest-ci-path-coverage/parsed-workflow-errors/.github/workflows/bad.yml (1)
1-3: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRecord why this workflow is malformed.
The unterminated flow sequence on line 3 is intentional. It drives the parsed-workflow error path. Add a leading comment so a future reader or a YAML linter does not treat it as an accidental syntax error. The comment does not repair the parse failure, because the sequence stays unterminated.
♻️ Proposed fixture comment
+# Intentionally malformed: the unterminated flow sequence below makes YAML +# parsing fail so vitest-ci-path-coverage reports a parsed-workflow error. name: malformed jobs: test: [As per coding guidelines: "Add short comments to intentionally counterintuitive fixtures or tests to preserve the invariant they protect."
🤖 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 `@fixtures/rules/vitest-ci-path-coverage/parsed-workflow-errors/.github/workflows/bad.yml` around lines 1 - 3, Add a short leading comment to the malformed workflow fixture before the name declaration, stating that the unterminated flow sequence is intentional and exercises the parsed-workflow error path; leave the sequence itself unchanged so the YAML remains invalid.Source: Coding guidelines
crates/no-mistakes/src/check_runner/tests.rs (1)
108-124: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueAdd an invariant comment to the Playwright config fixture.
prepared.playwrightisSome(...)whenplaywright-coverageis enabled, so this test exercisesprepared.rs/fact_plan_for_consumersinstead of theNonebranch inrun_all.rs. Add a short fixture or test comment documenting that the missingplaywright.config.tsshould fail before therun_allfallback.🤖 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 `@crates/no-mistakes/src/check_runner/tests.rs` around lines 108 - 124, Add a concise comment near the invalid Playwright fixture setup or the test `run_all_contextualizes_playwright_fact_plan_preparation_failures` documenting that `playwright-coverage` makes `prepared.playwright` present, so the missing `playwright.config.ts` must fail during `prepared.rs`/`fact_plan_for_consumers` before reaching the `run_all` fallback.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/registry.rs (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winHide the exported registry macro from the public API.
#[macro_export]placesfilesystem_rules!at the crate root as a public item. The macro is an internal rule registry. Add#[doc(hidden)]so it stays out of the published API documentation while remaining callable ascrate::filesystem_rules!.♻️ Proposed change
#[macro_export] +#[doc(hidden)] macro_rules! filesystem_rules {🤖 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 `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/registry.rs` at line 1, Add #[doc(hidden)] alongside #[macro_export] on the filesystem_rules! macro definition in the registry module, keeping the macro exported and callable through crate::filesystem_rules! while hiding it from generated public API documentation.crates/no-mistakes/src/codebase/dependencies/graph/edge_workflow_topology/tests.rs (1)
10-26: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a case for a path missing from the prepared set.
The test covers only the matching-scope projection. Add an assertion for a graph path that the prepared set does not contain. That test documents the silent-drop behavior of
project_pathsand protects the invariant if the prepared scope narrows later.♻️ Suggested additional test
#[test] fn graph_paths_absent_from_prepared_workflows_are_dropped() { let root = fixture_root(); let workflow = root.join(".github/workflows/ci.yml"); // Prepared set intentionally excludes the requested workflow to pin the // projection contract: unmatched paths are dropped, not parsed. let prepared = crate::codebase::ci_workflows::ParsedWorkflowSet { documents: Vec::new() }; let projected = parsed_workflows_for_graph( &root, &[workflow], &crate::config::v2::schema::CiConfig::default(), Some(&prepared), ); assert!(projected.documents.is_empty()); }As per coding guidelines: "Add short comments to intentionally counterintuitive fixtures or tests to preserve the invariant they protect."
🤖 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 `@crates/no-mistakes/src/codebase/dependencies/graph/edge_workflow_topology/tests.rs` around lines 10 - 26, Add a second test alongside prepared_workflows_are_projected_to_the_graph_workflow_universe that passes an intentionally empty ParsedWorkflowSet to parsed_workflows_for_graph while requesting the fixture workflow path, then assert projected.documents is empty. Include a short comment explaining that the unmatched graph path is intentionally dropped rather than parsed, preserving the project_paths projection contract.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/entrypoints.rs (1)
125-132: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrepare workflows once for every rule that needs them.
vitest-ci-path-coveragereceivesworkflow_documentsand falls back to discovering workflow files, reading each file fromsnapshot/sources, and parsing YAML when it isNone. If bothvitest-ci-path-coverageandtsconfig-gate-coverageare enabled but only load workflows fortsconfig-gate-coverage, Vitest repeats the same discovery, reads, and YAML parsing in one invocation.♻️ Proposed change
- let workflows = - rule_enabled(config, crate::codebase::rules::TSCONFIG_GATE_COVERAGE).then(|| { + let workflows_needed = [ + crate::codebase::rules::TSCONFIG_GATE_COVERAGE, + crate::codebase::rules::VITEST_CI_PATH_COVERAGE, + ] + .iter() + .any(|rule_id| rule_enabled(config, rule_id)); + let workflows = workflows_needed.then(|| { crate::codebase::ci_workflows::ParsedWorkflowSet::load_from_snapshot_and_sources( &root, &config.ci, snapshot, &sources, ) });🤖 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 `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/entrypoints.rs` around lines 125 - 132, Update the workflow preparation in the entrypoint around ParsedWorkflowSet::load_from_snapshot_and_sources so workflows are loaded when either vitest-ci-path-coverage or TSCONFIG_GATE_COVERAGE is enabled. Reuse the single prepared workflow set for both rules by passing it through their workflow_documents inputs, avoiding repeated discovery, reads, and YAML parsing.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/tests.rs (1)
90-118: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRecord the invariant this test protects.
Three other tests in this file pass
workflow_documents: Noneand succeed. This test fails only because themissing-cifixture enablestsconfig-gate-coverage. Add a short comment stating that the rule requires a preparedParsedWorkflowSet, so a reader does not treatNoneas always valid.As per coding guidelines: "Add short comments to intentionally counterintuitive fixtures or tests to preserve the invariant they protect."
♻️ Proposed comment
#[test] +// Unlike the other prepared-dispatch tests, this fixture enables +// tsconfig-gate-coverage, which cannot run without a prepared +// ParsedWorkflowSet. Passing `None` must fail loudly, not silently skip. fn prepared_dispatch_rejects_tsconfig_gate_without_workflow_documents() {🤖 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 `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/tests.rs` around lines 90 - 118, Add a short comment in prepared_dispatch_rejects_tsconfig_gate_without_workflow_documents explaining that the tsconfig-gate-coverage rule requires a prepared ParsedWorkflowSet, so workflow_documents: None is intentionally rejected for this fixture despite succeeding in other tests.Source: Coding guidelines
fixtures/rules/tsconfig-gate-coverage/working-directories/.no-mistakes.yml (1)
1-18: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winName the scanner path each command exercises.
Entries 1 and 4 have the same shape:
bash -c "cd X && tsc --noEmit". Only the directory differs. A reader cannot tell what distinguishes them, or why--dir=and--projectappear in the other two entries. Add one comment per entry naming the working-directory source it covers.As per coding guidelines: "Add short comments to intentionally counterintuitive fixtures or tests to preserve the invariant they protect."
♻️ Proposed comments
checks: commands: + # Working directory comes from a `cd` inside `bash -c`. - name: typecheck-workflow-default command: [bash, -c, cd workflow-default && tsc --noEmit] fileArgs: none always: true + # Working directory comes from the runner flag `--dir=`. - name: typecheck-job-default command: [pnpm, --dir=job-default, exec, tsc, --noEmit] fileArgs: none always: true + # Project comes from an explicit `--project`, with no working-directory change. - name: typecheck-step-default command: [tsc, --noEmit, --project, step-default/tsconfig.json] fileArgs: none always: true + # Second `cd` case: proves attribution is per directory, not one shared match. - name: typecheck-cd-project command: [bash, -c, cd cd-project && tsc --noEmit] fileArgs: none always: true🤖 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 `@fixtures/rules/tsconfig-gate-coverage/working-directories/.no-mistakes.yml` around lines 1 - 18, Add a concise comment immediately above each command in the checks.commands list identifying the scanner working-directory source it exercises, including why the workflow-default, job-default, step-default, and cd-project invocation styles differ. Preserve all command definitions and options unchanged.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs (1)
176-178: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPass the vitest scan inputs as a struct instead of seven positional arguments.
Line 177 calls
check_with_files_from_snapshot_catalog_sources_and_workflowswith seven positional arguments on one line.scan.rsalready definesScanInputsfor the innerscanfunction. Exposing a prepared-input struct at this boundary matchesPreparedFilesystemRuleInputsand removes the ordering hazard betweensnapshot,vitest_catalog,sources, andworkflow_documents.🤖 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 `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs` around lines 176 - 178, Update the Vitest dispatch in the rule execution flow to construct and pass the existing prepared-input struct, following the pattern of PreparedFilesystemRuleInputs, instead of supplying seven positional arguments to check_with_files_from_snapshot_catalog_sources_and_workflows. Expose or reuse a ScanInputs-style struct at this boundary and populate it with root, config, candidates, snapshot, vitest_catalog, sources, and workflow_documents while preserving the existing scan behavior.crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rs (1)
132-139: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInclude all dispatch sibling modules in the candidate-index assertion.
The test only scans
filesystem_dispatch.rsandexecute.rs, so adding file handling or candidate preparation to another sibling module would not update this coverage. Concatenateentrypoints.rs,registry.rs,candidate_helpers.rs,candidate_index.rs,inventory.rs,preserved.rs, andrun_rule.rsas well when asserting the dispatch module surface.🤖 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 `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rs` around lines 132 - 139, Update the dispatch source assembled in the candidate-index test to include the sibling modules entrypoints.rs, registry.rs, candidate_helpers.rs, candidate_index.rs, inventory.rs, preserved.rs, and run_rule.rs alongside filesystem_dispatch.rs and execute.rs. Keep the existing assertions unchanged so they validate the complete dispatch module surface.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/application.rs (1)
4-9: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReject unknown rule options before applying exemptions.
Optionsusesserde(default), so a mistyped top-level option such asallowProjectis ignored while the validallowProjectsexemption is treated as absent. Addserde(deny_unknown_fields)to this rule config so malformed rules fail with a clear config error instead of reporting projects the author meant to exempt.🤖 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 `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/application.rs` around lines 4 - 9, Add serde’s deny_unknown_fields attribute to the Options struct so unrecognized top-level rule options, such as allowProject, produce a configuration error instead of being silently ignored. Preserve the existing defaults and camelCase mapping for valid fields, including allow_projects.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters/tests/parsed.rs (1)
27-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument the intentionally absent fixture file.
missing.ymlmust not exist. It produces theReaderror branch. A contributor could create the file and silently reduce the test to one error kind. Add a short comment that states the invariant.♻️ Proposed comment
let paths = [ root.join(".github/workflows/bad.yml"), + // Intentionally absent: exercises the Read error kind. root.join(".github/workflows/missing.yml"), ];As per coding guidelines: "Add short comments to intentionally counterintuitive fixtures or tests to preserve the invariant they protect."
🤖 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 `@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters/tests/parsed.rs` around lines 27 - 30, Add a short comment immediately before the paths fixture in the test, explicitly stating that missing.yml must remain absent because it exercises the Read error branch. Do not alter the fixture paths or test behavior.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters.rs (2)
75-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd fix guidance to the workflow load findings.
The message states what failed and where. It does not state how to fix the problem or why the check exists. Extend the text, for example: name the rule, and instruct the user to correct the YAML syntax or the file permissions so that CI path filters can be read.
As per coding guidelines: "Provide custom error messages suitable for agents, and when reporting errors explain what happened, where it happened, how to fix it, and why a check exists."
🤖 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 `@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters.rs` around lines 75 - 89, Update the workflow load finding constructed in the Err branch of the document.value match to include the rule name, actionable guidance for correcting YAML syntax or file permissions, and that CI path filters must be readable for this check. Preserve the existing read/parse-specific action and error details.Source: Coding guidelines
99-132: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the second read-and-parse pipeline.
ci_filters_from_pathsduplicates the selector prefilter, the findings accumulation, and the sort ofci_filters_from_parsed_with_sources. It also opens its own read and parse pipeline throughsources.read_pathplusextract::from_workflow. Build aParsedWorkflowSetfrom the discovered paths, then delegate toci_filters_from_parsed_with_sources.ParsedWorkflowSet::from_pathsalready exists;workflow_filters/tests/parsed.rsLine 20 uses it.♻️ Proposed direction
fn ci_filters_from_paths( root: &Path, selectors: &[WorkflowSelector], workflow_files: Vec<std::path::PathBuf>, sources: &crate::codebase::ts_source::SourceStore, ) -> (Vec<CiFilter>, Vec<RuleFinding>) { - let mut filters = Vec::new(); - ... + let parsed = ParsedWorkflowSet::from_paths(root, workflow_files.into_iter()); + ci_filters_from_parsed_with_sources(root, selectors, &parsed, sources) }This also lets
extract::from_workflowretire, since only the parsed path would remain.As per coding guidelines: "Lower layers must consume prepared facts and session-owned stores by reference and must not open independent source, read, parse, resolution, or graph-building pipelines."
🤖 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 `@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters.rs` around lines 99 - 132, Replace the duplicate read-and-parse logic in ci_filters_from_paths with ParsedWorkflowSet::from_paths using the discovered workflow_files, then delegate filtering and finding generation to ci_filters_from_parsed_with_sources while preserving the existing root, selectors, and sources inputs. Remove the local selector filtering, sources.read_path handling, extract::from_workflow calls, and sorting from this function; once no callers remain, retire extract::from_workflow.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rs (1)
145-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the error text, not only
is_err().The test accepts any error. A different failure, for example a fixture load error, keeps the test green. The sibling Vitest test asserts the message content (
vitest_ci_path_coverage/tests.rsLine 138). Match that pattern so the invalid-glob path stays covered.♻️ Proposed change
- assert!(check(&root, &config).is_err()); + let error = check(&root, &config).unwrap_err(); + assert!(error.to_string().contains("invalid glob"), "{error:#}");🤖 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 `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rs` around lines 145 - 156, Update invalid_rule_filter_is_returned_without_partial_coverage_findings to capture the error from check(&root, &config) and assert that its text matches the expected invalid-glob failure, following the message assertion pattern used by the sibling vitest_ci_path_coverage test. Keep the test focused on the configured "[" include pattern so unrelated fixture or configuration errors cannot satisfy it.crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/scan.rs (1)
30-64: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse the prepared session inputs for this rule.
VITEST_CI_PATH_COVERAGEcallscheck_with_files_from_snapshot_catalog_sources_and_workflows, but that function ignores theSourceStore/workflow_documentssupplied by the execution path and only uses them whenNoneworkflows are provided. Move theSnapshot/SourceStoreownership into the prepared inputs and keepcheck_with_filesbehind#[cfg(test)]to avoid a second incompatible snapshot/store for tests.🤖 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 `@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/scan.rs` around lines 30 - 64, Update the VITEST_CI_PATH_COVERAGE execution flow so check_with_files_from_snapshot_catalog_sources_and_workflows uses the prepared Snapshot, SourceStore, and workflow_documents rather than rebuilding or ignoring them when workflows are supplied. Move Snapshot/SourceStore ownership into the prepared inputs, and restrict check_with_files to #[cfg(test)] so tests do not construct a separate incompatible snapshot/store.Source: Coding guidelines
🤖 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 `@crates/no-mistakes/src/codebase/dependencies/graph/builder_check_facts.rs`:
- Line 101: Update the graph-build invocation containing workflow_documents:
None to pass the request-scoped prepared.workflow_documents() reference instead.
Ensure collect_remaining_edges and the plan.ci/plan.workflow_topology paths
consume this prepared workflow set without re-reading or reparsing files, and
add a fixture-backed regression test for this entrypoint.
In `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/application.rs`:
- Around line 71-88: In the validation flow containing normalized and reason,
move the reason.trim().is_empty() check before normalized.insert. Ensure entries
with empty reasons are reported and skipped without claiming a normalized path,
while valid entries continue through duplicate detection and insertion.
In
`@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan.rs`:
- Around line 40-52: In the command scanning logic, update the `tokens[0]` check
near the `static_tokens` result to use `tokens.first()` and compare the
referenced value to `"cd"`. Preserve the existing directory-change handling and
avoid direct indexing so empty token vectors are handled without panicking.
In `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow.rs`:
- Around line 65-67: Update is_project_inside_root to perform an actual
containment check using root and project, resolving the project path relative to
root and confirming it remains under that root; preserve the boolean result
expected by scan_shell_for_typechecked_projects. If the function is
intentionally only a syntax check instead, rename it to reflect that behavior
and remove the unused root parameter, then update its call site accordingly.
In `@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/tests.rs`:
- Around line 17-27: Add a test in the tests.rs helper flow that constructs a
ParsedWorkflowSet with ParsedWorkflowSet::from_paths(...) and passes it as
Some(...) in the ScanInputs workflows field, while preserving the existing
snapshot-based test setup. Assert the resulting behavior to exercise
ci_filters_from_parsed_with_sources through scan_inputs rather than only
ci_filters_from_snapshot_with_sources.
In `@crates/no-mistakes/src/config/v2/discover.rs`:
- Around line 154-161: Update the validation errors in the command loop of
discover configuration to explain that always commands run once for the whole
project and therefore cannot use fileArgs, include, or exclude selection
settings. Retain the indexed command location and state the required
configuration and rationale in each message.
---
Nitpick comments:
In `@crates/no-mistakes/src/check_runner/tests.rs`:
- Around line 108-124: Add a concise comment near the invalid Playwright fixture
setup or the test
`run_all_contextualizes_playwright_fact_plan_preparation_failures` documenting
that `playwright-coverage` makes `prepared.playwright` present, so the missing
`playwright.config.ts` must fail during `prepared.rs`/`fact_plan_for_consumers`
before reaching the `run_all` fallback.
In
`@crates/no-mistakes/src/codebase/dependencies/graph/edge_workflow_topology/tests.rs`:
- Around line 10-26: Add a second test alongside
prepared_workflows_are_projected_to_the_graph_workflow_universe that passes an
intentionally empty ParsedWorkflowSet to parsed_workflows_for_graph while
requesting the fixture workflow path, then assert projected.documents is empty.
Include a short comment explaining that the unmatched graph path is
intentionally dropped rather than parsed, preserving the project_paths
projection contract.
In
`@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rs`:
- Around line 132-139: Update the dispatch source assembled in the
candidate-index test to include the sibling modules entrypoints.rs, registry.rs,
candidate_helpers.rs, candidate_index.rs, inventory.rs, preserved.rs, and
run_rule.rs alongside filesystem_dispatch.rs and execute.rs. Keep the existing
assertions unchanged so they validate the complete dispatch module surface.
In `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/entrypoints.rs`:
- Around line 125-132: Update the workflow preparation in the entrypoint around
ParsedWorkflowSet::load_from_snapshot_and_sources so workflows are loaded when
either vitest-ci-path-coverage or TSCONFIG_GATE_COVERAGE is enabled. Reuse the
single prepared workflow set for both rules by passing it through their
workflow_documents inputs, avoiding repeated discovery, reads, and YAML parsing.
In `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs`:
- Around line 176-178: Update the Vitest dispatch in the rule execution flow to
construct and pass the existing prepared-input struct, following the pattern of
PreparedFilesystemRuleInputs, instead of supplying seven positional arguments to
check_with_files_from_snapshot_catalog_sources_and_workflows. Expose or reuse a
ScanInputs-style struct at this boundary and populate it with root, config,
candidates, snapshot, vitest_catalog, sources, and workflow_documents while
preserving the existing scan behavior.
In `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/registry.rs`:
- Line 1: Add #[doc(hidden)] alongside #[macro_export] on the filesystem_rules!
macro definition in the registry module, keeping the macro exported and callable
through crate::filesystem_rules! while hiding it from generated public API
documentation.
In `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/tests.rs`:
- Around line 90-118: Add a short comment in
prepared_dispatch_rejects_tsconfig_gate_without_workflow_documents explaining
that the tsconfig-gate-coverage rule requires a prepared ParsedWorkflowSet, so
workflow_documents: None is intentionally rejected for this fixture despite
succeeding in other tests.
In `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/application.rs`:
- Around line 4-9: Add serde’s deny_unknown_fields attribute to the Options
struct so unrecognized top-level rule options, such as allowProject, produce a
configuration error instead of being silently ignored. Preserve the existing
defaults and camelCase mapping for valid fields, including allow_projects.
In `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rs`:
- Around line 145-156: Update
invalid_rule_filter_is_returned_without_partial_coverage_findings to capture the
error from check(&root, &config) and assert that its text matches the expected
invalid-glob failure, following the message assertion pattern used by the
sibling vitest_ci_path_coverage test. Keep the test focused on the configured
"[" include pattern so unrelated fixture or configuration errors cannot satisfy
it.
In `@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/scan.rs`:
- Around line 30-64: Update the VITEST_CI_PATH_COVERAGE execution flow so
check_with_files_from_snapshot_catalog_sources_and_workflows uses the prepared
Snapshot, SourceStore, and workflow_documents rather than rebuilding or ignoring
them when workflows are supplied. Move Snapshot/SourceStore ownership into the
prepared inputs, and restrict check_with_files to #[cfg(test)] so tests do not
construct a separate incompatible snapshot/store.
In
`@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters.rs`:
- Around line 75-89: Update the workflow load finding constructed in the Err
branch of the document.value match to include the rule name, actionable guidance
for correcting YAML syntax or file permissions, and that CI path filters must be
readable for this check. Preserve the existing read/parse-specific action and
error details.
- Around line 99-132: Replace the duplicate read-and-parse logic in
ci_filters_from_paths with ParsedWorkflowSet::from_paths using the discovered
workflow_files, then delegate filtering and finding generation to
ci_filters_from_parsed_with_sources while preserving the existing root,
selectors, and sources inputs. Remove the local selector filtering,
sources.read_path handling, extract::from_workflow calls, and sorting from this
function; once no callers remain, retire extract::from_workflow.
In
`@crates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters/tests/parsed.rs`:
- Around line 27-30: Add a short comment immediately before the paths fixture in
the test, explicitly stating that missing.yml must remain absent because it
exercises the Read error branch. Do not alter the fixture paths or test
behavior.
In `@fixtures/rules/tsconfig-gate-coverage/working-directories/.no-mistakes.yml`:
- Around line 1-18: Add a concise comment immediately above each command in the
checks.commands list identifying the scanner working-directory source it
exercises, including why the workflow-default, job-default, step-default, and
cd-project invocation styles differ. Preserve all command definitions and
options unchanged.
In
`@fixtures/rules/vitest-ci-path-coverage/parsed-workflow-errors/.github/workflows/bad.yml`:
- Around line 1-3: Add a short leading comment to the malformed workflow fixture
before the name declaration, stating that the unterminated flow sequence is
intentional and exercises the parsed-workflow error path; leave the sequence
itself unchanged so the YAML remains invalid.
🪄 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: CHILL
Plan: Pro Plus
Run ID: f3958d53-700c-4eda-94b5-36be054b2d6f
⛔ Files ignored due to path filters (1)
fixtures/rules/tsconfig-gate-coverage/malformed-workflow/node_modules/ignored/tsconfig.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (95)
crates/no-mistakes/src/check_parallel.rscrates/no-mistakes/src/check_runner.rscrates/no-mistakes/src/check_runner/forbidden_plan.rscrates/no-mistakes/src/check_runner/prepared.rscrates/no-mistakes/src/check_runner/run_all.rscrates/no-mistakes/src/check_runner/tests.rscrates/no-mistakes/src/check_runner/tests/architecture.rscrates/no-mistakes/src/check_tasks/filesystem.rscrates/no-mistakes/src/codebase/ci_workflows/tests.rscrates/no-mistakes/src/codebase/ci_workflows/workflow_set.rscrates/no-mistakes/src/codebase/dependencies/graph/builder.rscrates/no-mistakes/src/codebase/dependencies/graph/builder_check_facts.rscrates/no-mistakes/src/codebase/dependencies/graph/builder_helpers.rscrates/no-mistakes/src/codebase/dependencies/graph/builder_remaining_edges.rscrates/no-mistakes/src/codebase/dependencies/graph/edge_workflow_topology.rscrates/no-mistakes/src/codebase/dependencies/graph/edge_workflow_topology/tests.rscrates/no-mistakes/src/codebase/dependencies/graph/files_config_prepared.rscrates/no-mistakes/src/codebase/dependencies/graph/tests/config_path_full_graph.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/entrypoints.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/registry.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/tests.rscrates/no-mistakes/src/codebase/rules/ids.rscrates/no-mistakes/src/codebase/rules/mod.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/application.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/findings.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/scan.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/tests.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters/extract.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters/tests.rscrates/no-mistakes/src/codebase/rules/vitest_ci_path_coverage/workflow_filters/tests/parsed.rscrates/no-mistakes/src/config/v2/discover.rscrates/no-mistakes/src/config/v2/schema/ci_checks.rscrates/no-mistakes/src/config/v2/tests.rscrates/no-mistakes/src/impacted_checks.rscrates/no-mistakes/src/impacted_checks/generate/generic.rscrates/no-mistakes/src/impacted_checks/generate/prepare.rscrates/no-mistakes/src/impacted_checks/tests.rscrates/no-mistakes/src/impacted_checks/tests/generic_checks.rscrates/no-mistakes/src/impacted_checks/tests/runner_isolation.rscrates/no-mistakes/src/napi_api/analyze_project/context/check_prepare.rscrates/no-mistakes/src/napi_api/analyze_project/context/check_run.rscrates/no-mistakes/src/napi_api/cli_parity_builders.rscrates/no-mistakes/src/napi_api/options_ci.rscrates/no-mistakes/src/napi_api/tests/ci.rscrates/no-mistakes/tests/cli_ci.rscrates/no-mistakes/tests/docs_coverage.rsdocs/cli/impacted-checks.mddocs/configuration/checks.mddocs/node-api.mddocs/rules/README.mddocs/rules/tsconfig-gate-coverage.mdfixtures/check-runner/empty/.no-mistakes.ymlfixtures/check-runner/invalid-playwright-fact-plan/.no-mistakes.ymlfixtures/ci-workflows/project-paths/.github/workflows/a.ymlfixtures/ci-workflows/project-paths/.github/workflows/b.ymlfixtures/graph/workflow-topology-prepared/.github/workflows/ci.ymlfixtures/impacted-checks/generic-only.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/allowlist-errors/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/allowlist-errors/allowed/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/allowlist-errors/collision/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/allowlist-pass/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/allowlist-pass/tooling/tsconfig.tools.jsonfixtures/rules/tsconfig-gate-coverage/malformed-workflow/.github/workflows/bad.ymlfixtures/rules/tsconfig-gate-coverage/malformed-workflow/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/malformed-workflow/app/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/missing-ci/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/missing-ci/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/missing-ci/app/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/missing-ci/tools/tsconfig.tools.jsonfixtures/rules/tsconfig-gate-coverage/missing-local/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/missing-local/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/missing-local/app/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/pass/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/pass/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/pass/app/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/pass/tools/tsconfig.tools.jsonfixtures/rules/tsconfig-gate-coverage/working-directories/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/working-directories/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/working-directories/cd-project/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/working-directories/job-default/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/working-directories/step-default/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/working-directories/workflow-default/tsconfig.jsonfixtures/rules/vitest-ci-path-coverage/parsed-workflow-errors/.github/workflows/bad.ymlpackages/no-mistakes/ci-types.d.tsskills/no-mistakes/SKILL.md
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: coderabbitai <coderabbitai@users.noreply.github.com>
Addressed all six findings with prepared-workflow reuse, validation ordering, safe token handling, clearer naming and diagnostics, and fixture-backed coverage.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: chatgpt-codex-connector <chatgpt-codex-connector@users.noreply.github.com>
Co-authored-by: chatgpt-codex-connector <chatgpt-codex-connector@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: chatgpt-codex-connector <chatgpt-codex-connector@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: coderabbitai <coderabbitai@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: chatgpt-codex-connector <chatgpt-codex-connector@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: chatgpt-codex-connector <chatgpt-codex-connector@users.noreply.github.com>
Co-authored-by: chatgpt-codex-connector <chatgpt-codex-connector@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests.rs (1)
259-273: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocument why
./node_modules/.bin/tscis accepted buttools/tscis rejected.Line 252 accepts
./node_modules/.bin/tscas a compiler invocation. Lines 259-273 reject./scripts/tsc,tools/tsc, andvendor/bin/tsc. The distinguishing invariant is not visible in the test. Add a short comment that states the accepted executable forms, so a later change to the executable allowlist does not silently invalidate this test.As per coding guidelines: "Add short comments to intentionally counterintuitive fixtures or tests to preserve the invariant they protect."
🤖 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 `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests.rs` around lines 259 - 273, Add a short explanatory comment immediately above the command loop in the test covering scan_argv_for_typechecked_projects. State that ./node_modules/.bin/tsc is an accepted compiler form, while ./scripts/tsc, tools/tsc, and vendor/bin/tsc are intentionally rejected, preserving the executable allowlist invariant.Source: Coding guidelines
crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/shell.rs (1)
68-83: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueBackslash-escaped quotes can bypass the multiline-quote rejection.
The scanner tracks quote state by character only. In
"a \" b\ntsc --noEmit"the escaped\"closes the active quote. The following newline is then treated as unquoted, and the body is accepted. The scanner can credit atscoccurrence that is only string data.Consider skipping the character after a backslash while inside a double-quoted region.
♻️ Proposed handling for escaped characters
let mut quote = None; - for character in script.chars() { + let mut characters = script.chars(); + while let Some(character) = characters.next() { + if quote != Some('\'') && character == '\\' { + // A backslash escape must not close a quoted region. + if characters.next() == Some('\n') { + return true; + } + continue; + } match quote {🤖 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 `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/shell.rs` around lines 68 - 83, Update contains_unsupported_multiline_shell_construct to treat backslash-escaped characters as literal content while inside a double-quoted region, skipping the escaped character before evaluating quote closure. Preserve single-quote behavior and ensure a newline inside the still-active double-quoted region returns true.
🤖 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 `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/no_check.rs`:
- Around line 21-30: Update non_enforcing_tsconfigs to process tracked projects
with par_iter() instead of iter(), and collect the matching project names into a
BTreeSet so parallel processing preserves deterministic output. Keep each
effective_no_check() invocation isolated with its own loading set and reuse the
shared prepared SourceStore.
In
`@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow/runtime.rs`:
- Around line 29-46: Update runs_on_can_default_to_windows and its helper
is_windows_runner_label so a bare self-hosted runs-on label is treated as an
unknown runner rather than a statically non-Windows runner, causing the
Windows-default skip path to apply. Preserve current Windows label matching and
sequence handling, while ensuring unlabeled self-hosted configurations cannot be
credited as POSIX-safe.
- Around line 93-116: Update the option-matching logic in the surrounding
runtime parser to avoid the if-let match guard syntax, unless the project
explicitly pins Rust 1.95+ through its toolchain or CI configuration. Prefer
pre-checking execution_preserving_short_option(option) before the match, then
preserve the existing failure_enforced updates and index advancement for all
options.
---
Nitpick comments:
In
`@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/shell.rs`:
- Around line 68-83: Update contains_unsupported_multiline_shell_construct to
treat backslash-escaped characters as literal content while inside a
double-quoted region, skipping the escaped character before evaluating quote
closure. Preserve single-quote behavior and ensure a newline inside the
still-active double-quoted region returns true.
In
`@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests.rs`:
- Around line 259-273: Add a short explanatory comment immediately above the
command loop in the test covering scan_argv_for_typechecked_projects. State that
./node_modules/.bin/tsc is an accepted compiler form, while ./scripts/tsc,
tools/tsc, and vendor/bin/tsc are intentionally rejected, preserving the
executable allowlist invariant.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 4bfb3f0b-5616-43ce-bf58-6ff56c5bc148
📒 Files selected for processing (41)
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/application.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/shell.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tsc_arguments.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/no_check.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests/no_check.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow/runtime.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow/runtime/tests.rscspell.config.yamldocs/rules/tsconfig-gate-coverage.mdfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/bad-array/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/bad-compiler-options/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/bad-extends/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/bad-no-check/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/base/directory/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/base/file.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/cycle/other.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/cycle/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/directory-base/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/empty/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/file-base/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/missing-base/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/package-base/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/no-check/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/no-check/base/base.jsonfixtures/rules/tsconfig-gate-coverage/no-check/base/neutral.jsonfixtures/rules/tsconfig-gate-coverage/no-check/direct/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check/inherited/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check/invalid/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check/override/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/non-enforcing-workflow/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/non-enforcing-workflow/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/non-enforcing-workflow/dynamic-runner/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/non-enforcing-workflow/implicit-windows-shell/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/non-enforcing-workflow/missing-runner/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (6)
- crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tsc_arguments.rs
- docs/rules/tsconfig-gate-coverage.md
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rs
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Co-authored-by: coderabbitai <coderabbitai@users.noreply.github.com>
Parallelized noCheck analysis, handled package extends and indeterminate self-hosted runners, rejected quoted shell separators, and documented the Rust 1.96 MSRV evidence.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage.rs (1)
51-59: 🚀 Performance & Scalability | 🟠 Major | ⚡ Quick winAvoid recomputing the tracked tsconfig set twice per invocation.
prepare_project_source_inputs(lines 109-144) already callstracked_tsconfigsto buildProjectSourceInputs, and its keys are exactly the tracked set.check_with_prepared(line 56) callstracked_tsconfigsagain on the sametracked_paths. This repeats a repository-wide filter/validate pass within one invocation.Derive
trackedfromprepared.project_source_inputs.keys()instead of recomputing it.As per coding guidelines, "Share identical repository-wide computations across independent call paths within one invocation; add a cache key only when caller inputs genuinely vary, and regression-test call counts rather than value equality."
♻️ Proposed fix
- let tracked = tracked_tsconfigs(root, prepared.tracked_paths); + let tracked = prepared + .project_source_inputs + .keys() + .cloned() + .collect::<BTreeSet<_>>(); let non_enforcing = non_enforcing_tsconfigs(root, &tracked, prepared.sources); let ci_projects = ci_typechecked_projects(prepared.workflows, &tracked, prepared.project_source_inputs);Also applies to: 109-144
🤖 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 `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage.rs` around lines 51 - 59, Update check_with_prepared to derive tracked from prepared.project_source_inputs.keys() instead of calling tracked_tsconfigs again, preserving the existing tracked set consumed by non_enforcing_tsconfigs and ci_typechecked_projects. Ensure prepare_project_source_inputs remains the single tracked-tsconfig computation for each invocation.Source: Coding guidelines
🧹 Nitpick comments (1)
crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs (1)
168-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMake the missing prepared-input error actionable.
Line 168 merges missing workflows and missing project inputs into one error. Report which input is absent. State that the failure occurred in filesystem dispatch. State that callers must prepare the missing input from the request snapshot. State that the rule uses these inputs to map CI typechecks to tracked projects.
As per coding guidelines, “Provide custom error messages suitable for agents, and when reporting errors explain what happened, where it happened, how to fix it, and why a check exists.”
Proposed error handling
- let result = workflow_documents - .zip(tsconfig_gate_project_inputs) - .map_or_else( - || { - Err(anyhow::anyhow!( - "prepared workflow documents and project inputs are required for {TSCONFIG_GATE_COVERAGE}" - )) - }, - |(workflows, project_source_inputs)| { + let result = match (workflow_documents, tsconfig_gate_project_inputs) { + (Some(workflows), Some(project_source_inputs)) => { tsconfig_gate_coverage::check_with_prepared( root, config, @@ }, ) - }); + } + (None, _) => Err(anyhow::anyhow!( + "cannot run {TSCONFIG_GATE_COVERAGE} in filesystem dispatch: \ + workflow documents were not prepared; prepare ParsedWorkflowSet from \ + the request snapshot so the rule can map CI typechecks to projects" + )), + (_, None) => Err(anyhow::anyhow!( + "cannot run {TSCONFIG_GATE_COVERAGE} in filesystem dispatch: \ + project source inputs were not prepared; derive them from the request \ + snapshot and workspace so the rule can map CI typechecks to projects" + )), + };🤖 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 `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs` around lines 168 - 188, Update the missing-input error branch in the workflow_documents/tsconfig_gate_project_inputs zip handling to identify whether workflows, project source inputs, or both are absent. Make the filesystem-dispatch message state that callers must prepare the missing input from the request snapshot and explain that these inputs map CI typechecks to tracked projects.Source: Coding guidelines
🤖 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
`@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests/review.rs`:
- Around line 5-16: Update the test cases in scan_shell_for_typechecked_projects
to credit commands where tsc --noEmit executes before or after a comment
boundary, including inline comments and literal # characters within words. Keep
only scripts whose first token is # in the rejected set, and adjust the
assertions accordingly.
---
Outside diff comments:
In `@crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage.rs`:
- Around line 51-59: Update check_with_prepared to derive tracked from
prepared.project_source_inputs.keys() instead of calling tracked_tsconfigs
again, preserving the existing tracked set consumed by non_enforcing_tsconfigs
and ci_typechecked_projects. Ensure prepare_project_source_inputs remains the
single tracked-tsconfig computation for each invocation.
---
Nitpick comments:
In `@crates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rs`:
- Around line 168-188: Update the missing-input error branch in the
workflow_documents/tsconfig_gate_project_inputs zip handling to identify whether
workflows, project source inputs, or both are absent. Make the
filesystem-dispatch message state that callers must prepare the missing input
from the request snapshot and explain that these inputs map CI typechecks to
tracked projects.
🪄 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: CHILL
Plan: Pro Plus
Run ID: 13bbf46e-a24a-4516-8849-627ffee5e542
⛔ Files ignored due to path filters (1)
fixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/package-base/node_modules/@scope/tsconfig/tsconfig.jsonis excluded by!**/node_modules/**
📒 Files selected for processing (58)
crates/no-mistakes/src/check_parallel.rscrates/no-mistakes/src/check_runner/prepared.rscrates/no-mistakes/src/check_runner/run_all.rscrates/no-mistakes/src/check_runner/tests.rscrates/no-mistakes/src/check_runner/tests/architecture.rscrates/no-mistakes/src/check_runner/tests/config_path.rscrates/no-mistakes/src/codebase/analysis_dataset.rscrates/no-mistakes/src/codebase/analysis_session/io.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/entrypoints.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/execute.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/metadata.rscrates/no-mistakes/src/codebase/rules/filesystem_dispatch/tests.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/application.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/shell.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/shell/comments.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests/review.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tsc_arguments.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/no_check.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests/fixture_policy.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests/no_check.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests/workflow.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow/runtime.rscrates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow/runtime/tests.rscrates/no-mistakes/src/codebase/ts_resolver/catalog.rscrates/no-mistakes/src/codebase/ts_resolver/catalog_membership.rscrates/no-mistakes/src/config/v2/discover.rscrates/no-mistakes/src/config/v2/discover/check_commands.rscrates/no-mistakes/src/config/v2/mod.rscrates/no-mistakes/src/config/v2/tests.rscrates/no-mistakes/src/napi_api/analyze_project/context/check_run.rsdocs/configuration/checks.mddocs/rules/tsconfig-gate-coverage.mdfixtures/rules/tsconfig-gate-coverage/auto-config-path/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/auto-config-path/.no-mistakes.yamlfixtures/rules/tsconfig-gate-coverage/auto-config-path/app/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/auto-config-suppression/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/auto-config-suppression/.no-mistakes.yamlfixtures/rules/tsconfig-gate-coverage/auto-config-suppression/app/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/base/dotted.base.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/dotted-file-base/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/no-check-edge-cases/missing-package-base/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/pass/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/pass/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/path-filter-sources-negative/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/path-filter-sources-negative/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/path-filter-sources-negative/app/src/index.tsfixtures/rules/tsconfig-gate-coverage/path-filter-sources-negative/app/tsconfig.jsonfixtures/rules/tsconfig-gate-coverage/path-filter-sources-positive/.github/workflows/ci.ymlfixtures/rules/tsconfig-gate-coverage/path-filter-sources-positive/.no-mistakes.ymlfixtures/rules/tsconfig-gate-coverage/path-filter-sources-positive/app/ignored.tsfixtures/rules/tsconfig-gate-coverage/path-filter-sources-positive/app/src/index.tsfixtures/rules/tsconfig-gate-coverage/path-filter-sources-positive/app/tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (16)
- fixtures/rules/tsconfig-gate-coverage/pass/.github/workflows/ci.yml
- crates/no-mistakes/src/codebase/rules/filesystem_dispatch/candidate_index/tests.rs
- fixtures/rules/tsconfig-gate-coverage/pass/.no-mistakes.yml
- docs/configuration/checks.md
- crates/no-mistakes/src/check_parallel.rs
- crates/no-mistakes/src/check_runner/tests/architecture.rs
- crates/no-mistakes/src/config/v2/tests.rs
- crates/no-mistakes/src/codebase/rules/filesystem_dispatch/tests.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/tests.rs
- crates/no-mistakes/src/codebase/rules/filesystem_dispatch.rs
- crates/no-mistakes/src/codebase/rules/filesystem_dispatch/entrypoints.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/no_check.rs
- crates/no-mistakes/src/check_runner/run_all.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/tests.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/command_scan/shell.rs
- crates/no-mistakes/src/codebase/rules/tsconfig_gate_coverage/workflow/runtime.rs
This comment has been minimized.
This comment has been minimized.
Updated the scanner to strip only lexical shell comments while preserving literal or escaped hash characters, with corrected regression cases.
1 participant
Summary
tsconfig-gate-coveragerepository rule, requiring every tracked TypeScript project to have both a static GitHub Actionstsc --noEmitregistration and an unconditional local checkchecks.commands[].alwaysplusimpacted-checks --generic-only/ NodegenericOnlysupport so downstream pre-push runners can execute configured repository gates without rebuilding framework analysisWhy
This upstreams the generic TypeScript project gate described in jonathanong/filaments#8933.
no-mistakesalready owns the tracked-file inventory and GitHub workflow topology, so it can enforce the policy deterministically in the same request-scoped analysis instead of requiring a downstream repository to rediscover and reparse those inputs.Impact
Downstream repositories can configure one authoritative project allowlist, declare unconditional local typecheck commands, and consume those commands through the async programmatic API. Filaments can migrate its local gate after consuming a release containing this change.
Validation
cargo test -p no-mistakes --all-features --lib --bins --tests— 5,915 passed across 60 suitescargo clippy --all-targets --all-features -- -D warningscargo run -p no-mistakes --quiet -- checkcargo fmt --all -- --checknpm run fmt:js:checknpm run lint:jsFocused coverage for the new rule remains above the repository's 99% line/function thresholds.
Shepherd Journal
facts/imports_and_symbolsbenchmark directly measurescollect_ts_facts, this PR changes neither that fact-collection path nor the benchmark, 53 peer benchmarks were unchanged, and CodSpeed warns the comparison used different runtime environments..github/workflows/ci.yml, so the stabilized if-let guard syntax is supported by the project toolchain.