Skip to content

feat: add required entrypoint reachability rule - #629

Merged
jonathanong merged 12 commits into
mainfrom
feat/required-entrypoint-reachability
Aug 9, 2026
Merged

feat: add required entrypoint reachability rule#629
jonathanong merged 12 commits into
mainfrom
feat/required-entrypoint-reachability

Conversation

@jonathanong

@jonathanong jonathanong commented Aug 8, 2026

Copy link
Copy Markdown
Owner

Summary

  • add a reusable required-entrypoint-reachability repository rule
  • validate source globs, literal entrypoints, runtime import reachability, exclusions, and optional depth bounds
  • share the canonical dependency graph across CLI and N-API aggregate runners

Motivation

Applications with explicit worker or scheduler registries can silently leave source modules unregistered. Existing rules cannot require every file in a configured source set to be runtime-reachable from one or more entrypoint files.

Validation

  • focused rule and graph-runner tests: 14 passed
  • prepared graph architecture test: passed
  • repository self-check: passed
  • docs coverage: passed
  • cargo fmt --check: passed
  • cargo clippy --workspace --all-targets --all-features -- -D warnings: passed
  • Codecov: all modified and coverable lines covered
  • optimized candidate against Filaments' full check: passed in 18.2 seconds with the existing 60-second timeout
  • full Rust suite and coverage: passed in CI

Notes

The rule follows runtime value edges only: static imports, dynamic imports, CommonJS require, workspace imports, and named/star re-exports. Type-only imports do not satisfy reachability.

Related issues

Closes jonathanong/filaments#9186

@coderabbitai

coderabbitai Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@jonathanong, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 39 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2321d54f-5ef8-4eda-a7e7-008556e48943

📥 Commits

Reviewing files that changed from the base of the PR and between 24acef8 and e718a3a.

📒 Files selected for processing (1)
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/tests/scoped.test.ts
📝 Walkthrough

Summary by CodeRabbit

  • New Features

    • Added the required-entrypoint-reachability rule to identify source files not reachable from configured runtime entrypoints.
    • Supports static and dynamic imports, re-exports, workspace dependencies, require.resolve, path patterns, depth limits, exclusions, and file-level suppressions.
    • Improved graph-based analysis for type-only imports, workspace relationships, and filesystem-skip scenarios.
  • Documentation

    • Added configuration guidance, violation details, remediation advice, examples, and documented limitations for the new rule.

Walkthrough

The change adds the required-entrypoint-reachability rule, expands dependency-graph edge handling, and replaces forbidden-dependency-specific graph preparation with canonical graph-plan preparation across runners and N-API contexts.

Changes

Graph reachability rule

Layer / File(s) Summary
Implement reachability validation
crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability.rs, crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability/tests.rs, test-cases/rules/required-entrypoint-reachability/*, docs/rules/*
Adds configuration validation, runtime dependency traversal, depth limits, suppression handling, deterministic findings, fixture coverage, rule registration, and documentation.

Dependency graph edge semantics

Layer / File(s) Summary
Add graph edge classifications
crates/no-mistakes/src/codebase/dependencies/*, docs/graph-edges.md
Adds RequireResolve and WorkspaceTypeImport, updates graph construction, filtering, sorting, impact traversal, serialization, acceptance tests, and edge documentation.

Canonical graph execution

Layer / File(s) Summary
Generalize graph planning
crates/no-mistakes/src/check_runner/*, crates/no-mistakes/src/codebase/rules/run.rs, crates/no-mistakes/src/codebase/rules/run/prepared.rs, crates/no-mistakes/src/codebase/rules/run/standalone.rs
Uses canonical_graph_plan for graph preparation, shared facts, Playwright integration, graph-file discovery, and standalone execution.
Centralize graph-rule execution
crates/no-mistakes/src/codebase/rules/run/prepared/execution/*
Runs forbidden-dependency and required-entrypoint-reachability checks through shared graph-rule orchestration and propagates graph-related errors.
Align N-API preparation and scope
crates/no-mistakes/src/napi_api/analyze_project/context/*
Derives graph state, graph facts, graph-file scope, and traversal-graph use from canonical graph requirements.

Validation and documentation

Layer / File(s) Summary
Validate graph scope and integration
crates/no-mistakes/src/check_runner/tests/*, crates/no-mistakes/src/codebase/rules/tests*, crates/no-mistakes/src/napi_api/analyze_project/tests.rs, test-cases/check-runner/*, test-cases/codebase-analysis/*
Adds regression coverage for filesystem skips, graph-rule errors, standalone discovery, project-analysis parity, and invalid configuration handling.
Document the rule
docs/rules/README.md, docs/rules/required-entrypoint-reachability.md, crates/no-mistakes/tests/docs_coverage.rs
Adds the rule index entry, configuration and behavior documentation, examples, suppression guidance, and documentation-coverage assertions.

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

Possibly related PRs

Suggested labels: rust, tests

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the primary change: adding the required entrypoint reachability rule.
Description check ✅ Passed The description explains the rule, implementation scope, motivation, validation, and related issue.
Linked Issues check ✅ Passed The changes implement the linked issue requirements for runtime reachability, graph reuse, diagnostics, tests, documentation, and CLI/N-API parity.
Out of Scope Changes check ✅ Passed The dependency-graph, runner, fixture, test, and documentation changes support the linked rule objectives; excluded Filaments changes are not present.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Aug 8, 2026
@sourcery-ai

sourcery-ai Bot commented Aug 8, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds a new repository rule required-entrypoint-reachability, wires it into the shared canonical dependency graph plan used by CLI and N-API runners, centralizes graph-backed rule execution, and updates tests/docs and configuration plumbing accordingly.

Sequence diagram for required-entrypoint-reachability rule execution

sequenceDiagram
    participant Exec as graph_rules::graph_rule_findings
    participant Reach as required_entrypoint_reachability
    participant Config as NoMistakesConfig
    participant Graph as DepGraph
    participant Facts as CheckFactMap

    Exec->>Config: rule_enabled(REQUIRED_ENTRYPOINT_REACHABILITY)
    alt rule enabled
        Exec->>Reach: check_with_graph_and_inferred(root, config, Facts.graph_file_universe(), Graph, inferred_roots)
        Reach->>Reach: file_universe = normalize_path(files)
        Reach->>Reach: rule_applications(RULE_ID)
        Reach->>Reach: RulePathFilter::new_with_inferred(...)
        Reach->>Reach: matching_files(sourceGlobs, scoped_files, target_roots)
        Reach->>Reach: resolve_entrypoint(entrypoints)
        Reach->>Graph: deps_of_in_file_universe(roots, max_depth, runtime_edge_kinds, file_universe)
        Graph-->>Reach: reachable nodes
        Reach->>Reach: build RuleFinding for unreachable sources
        Reach-->>Exec: findings
    end
    Exec-->>Exec: aggregate findings
    Exec-->>Exec: return findings
Loading

File-Level Changes

Change Details Files
Introduce the required-entrypoint-reachability repository rule that validates runtime reachability of source files from configured entrypoints using the dependency graph.
  • Define rule ID, options schema (sourceGlobs, entrypoints, optional maxDepth), and configuration-level graph_plan hook
  • Implement rule execution over a DepGraph, including resolving entrypoints, matching sourceGlobs within rule filters, and computing runtime-reachable files using import/require edge kinds
  • Emit findings for unreachable sources and configuration errors (missing entrypoints, invalid glob patterns, zero-match sourceGlobs)
  • Provide focused unit tests that exercise reachability behavior, configuration validation, suppression, and aggregate runner integration
  • Add fixture TS project and .no-mistakes configs for reachability scenarios and suppression cases
crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability.rs
crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability/tests.rs
crates/no-mistakes/src/codebase/rules/ids.rs
crates/no-mistakes/src/codebase/rules/mod.rs
test-cases/rules/required-entrypoint-reachability/fixture/**
Share a canonical graph build plan across forbidden-dependencies and required-entrypoint-reachability and reuse it in CLI and N-API aggregate runners.
  • Expose canonical_graph_plan from codebase rules run module and make it include imports/workspace plus any configured reachability or forbidden-dependencies plans
  • Replace runner-specific forbidden_graph_plan wiring with canonical_graph_plan in check_runner::run_all, standalone runner, and N-API SharedCheckContext
  • Use canonical_graph_plan to decide whether shared facts and full graph files are needed for checks and playwright fact planning
  • Update architecture tests to assert use of the new graph_plan module and its prepare_graph_config/ts_fact_plan calls
crates/no-mistakes/src/codebase/rules/run/prepared.rs
crates/no-mistakes/src/codebase/rules/run.rs
crates/no-mistakes/src/check_runner/run_all.rs
crates/no-mistakes/src/codebase/rules/run/standalone.rs
crates/no-mistakes/src/napi_api/analyze_project/context/check_prepare.rs
crates/no-mistakes/src/check_runner/tests/architecture.rs
crates/no-mistakes/src/check_runner.rs
Centralize graph-backed rule execution logic into a dedicated graph_rules helper and update control flow for graph fact validation and error propagation.
  • Add execution::graph_rules module with graph_rule_findings that runs forbidden-dependencies and required-entrypoint-reachability against the prepared dependency graph and shared facts
  • Replace inline forbidden-dependencies execution in prepared::execution::run with a call to graph_rule_findings, passing inferred roots and shared graph file universe
  • Adjust the shared-facts coverage check message to be generic for configured codebase rules rather than forbidden-dependencies-only
  • Introduce tests that assert graph_rule_findings propagates missing-facts and invalid-filter errors for both forbidden-dependencies and reachability rules
crates/no-mistakes/src/codebase/rules/run/prepared/execution.rs
crates/no-mistakes/src/codebase/rules/run/prepared/execution/graph_rules.rs
crates/no-mistakes/src/codebase/rules/run/prepared/execution/graph_rules/tests.rs
Clean up configuration helpers and enablement flags to reflect the new graph-rule family and ensure docs coverage for the new rule.
  • Remove forbidden_dependencies_configured helper and replace usage with graph_rules_enabled flag that checks presence of a canonical graph_plan
  • Update N-API SharedCheckContext and check_run to use graph_rules_enabled for early-exit conditions and graph-file needs computation
  • Register the new rule ID in docs coverage tests and rules README, with rule description and example configuration
  • Add tests and fixtures that validate invalid include glob behavior for both forbidden-dependencies and required-entrypoint-reachability at the aggregate runner level
crates/no-mistakes/src/check_tasks.rs
crates/no-mistakes/src/napi_api/analyze_project/context/check_prepare.rs
crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs
crates/no-mistakes/tests/docs_coverage.rs
docs/rules/README.md
docs/rules/required-entrypoint-reachability.md
crates/no-mistakes/src/codebase/rules/tests.rs
test-cases/codebase-analysis/forbidden-dependencies-basic/fixture/invalid-include.no-mistakes.yml
test-cases/rules/required-entrypoint-reachability/fixture/invalid-include.no-mistakes.yml

Assessment against linked issues

Issue Objective Addressed Explanation
https://github.com/jonathanong/filaments/issues/9186 Implement a reusable required-entrypoint-reachability rule in no-mistakes that enforces runtime reachability from configured entrypoints for all files matching sourceGlobs, with optional maxDepth, using the canonical dependency graph and runtime edges only, and integrate it into CLI and N-API runners.
https://github.com/jonathanong/filaments/issues/9186 Add comprehensive, fixture-backed tests for required-entrypoint-reachability covering runtime edge kinds vs type-only edges, maxDepth behavior, repeated rule applications, deterministic diagnostics, invalid/common filters, and file suppression, and ensure aggregate runners propagate relevant errors.
https://github.com/jonathanong/filaments/issues/9186 Document the required-entrypoint-reachability rule (configuration, counterexamples, fixes, suppression behavior) and ensure it is included in rule listings and that the upstream PR references and closes the Filaments tracking issue.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@codecov

codecov Bot commented Aug 8, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.24%. Comparing base (9ff8ab1) to head (e718a3a).

Additional details and impacted files
@@           Coverage Diff            @@
##             main     #629    +/-   ##
========================================
  Coverage   99.24%   99.24%            
========================================
  Files        1277     1279     +2     
  Lines      100537   100819   +282     
========================================
+ Hits        99778   100061   +283     
+ Misses        759      758     -1     
Flag Coverage Δ
javascript 99.67% <ø> (ø)
rust 99.23% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed-hq

codspeed-hq Bot commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 54 untouched benchmarks
⏩ 4 skipped benchmarks1


Comparing feat/required-entrypoint-reachability (e718a3a) with main (9ff8ab1)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@jonathanong
jonathanong marked this pull request as ready for review August 8, 2026 07:54

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey - I've found 1 issue, and left some high level feedback:

  • In required_entrypoint_reachability::config_finding, the file is always reported as .no-mistakes.yml; consider wiring through the actual config path so findings point to the correct configuration source, especially for non-default config filenames.
  • canonical_graph_plan is now pub and re-exported from both run and rules::mod with slightly different visibility annotations; it may be clearer to expose a single canonical public export to avoid confusion over which path callers should use.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In `required_entrypoint_reachability::config_finding`, the file is always reported as `.no-mistakes.yml`; consider wiring through the actual config path so findings point to the correct configuration source, especially for non-default config filenames.
- `canonical_graph_plan` is now `pub` and re-exported from both `run` and `rules::mod` with slightly different visibility annotations; it may be clearer to expose a single canonical public export to avoid confusion over which path callers should use.

## Individual Comments

### Comment 1
<location path="crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability/tests.rs" line_range="58-67" />
<code_context>
+    check_with_graph(&root, &config, &files, &graph)
+}
+
+#[test]
+fn accepts_static_dynamic_require_named_and_star_reexports() {
+    let findings = run(vec![application(
+        r#"
+sourceGlobs:
+  - sources/static.ts
+  - sources/dynamic.ts
+  - sources/required.ts
+  - sources/named.ts
+  - sources/star.ts
+entrypoints: [entrypoints/api.ts]
+"#,
+    )]);
+
+    assert!(findings.is_empty(), "unexpected findings: {findings:?}");
+}
+
</code_context>
<issue_to_address>
**suggestion (testing):** Add a negative test for absolute entrypoints that fall outside the repository root.

`accepts_an_absolute_entrypoint_inside_the_repository` covers the happy path for absolute entrypoints. Please add a complementary test where the config uses an absolute entrypoint outside `root`, confirming that `resolve_entrypoint` flags the configuration as invalid rather than treating the entrypoint as reachable. This will better exercise the boundary conditions of entrypoint resolution.

Suggested implementation:

```rust
#[test]
fn accepts_static_dynamic_require_named_and_star_reexports() {
    let findings = run(vec![application(
        r#"
sourceGlobs:
  - sources/static.ts
  - sources/dynamic.ts
  - sources/required.ts
  - sources/named.ts
  - sources/star.ts
entrypoints: [entrypoints/api.ts]
"#,
    )]);

    assert!(findings.is_empty(), "unexpected findings: {findings:?}");
}

#[test]
fn rejects_an_absolute_entrypoint_outside_the_repository() {
    let findings = run(vec![application(
        r#"
sourceGlobs:
  - sources/static.ts
entrypoints:
  # Absolute path that does not reside under the repository root.
  # The resolver should treat this as an invalid configuration rather than a reachable entrypoint.
  - /outside-root/api.ts
"#,
    )]);

    // The configuration should be rejected because the absolute entrypoint falls
    // outside the repository root; `resolve_entrypoint` must not treat it as reachable.
    //
    // Adjust the predicate below to match the project's way of expressing
    // "invalid configuration" findings (e.g. by kind, code, or message).
    assert!(
        findings.iter().any(|finding| finding.is_invalid_configuration()),
        "expected invalid configuration for entrypoint outside repository root, got: {findings:?}",
    );
}

```

To fully implement this negative test according to your codebase conventions, you will likely need to:

1. Replace `finding.is_invalid_configuration()` with the actual way you detect configuration-validation failures, for example:
   - Matching a `RuleFindingKind::ConfigurationError` or similar enum variant, or
   - Checking a `code()`/`category()` field, or
   - Matching the diagnostic message produced by `resolve_entrypoint` when an absolute path lies outside `root`.

2. If your existing tests for invalid configs use a helper (e.g. `assert_invalid_config(findings, "...")`), update the assertion to reuse that helper so the test is consistent with other tests.

3. If your configuration parser does not treat a bare `/outside-root/api.ts` as an absolute path on all platforms, construct an explicit absolute path guaranteed to be outside `root` using `PathBuf` and interpolate it into the YAML string, mirroring whatever pattern is used in the existing `accepts_an_absolute_entrypoint_inside_the_repository` test.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 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/check_runner/run_all.rs`:
- Around line 111-113: Update the graph-file selection predicates in
crates/no-mistakes/src/check_runner/run_all.rs:111-113 and
crates/no-mistakes/src/napi_api/analyze_project/context/check_prepare.rs:142-145
so canonical_graph_plan does not implicitly request the full graph; use a
separate explicit full-graph predicate while keeping dynamic-import-only
configurations on views.filesystem. Add fixture-backed parity tests covering
both runners and both scoped and full-graph configurations.

In
`@crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability/tests.rs`:
- Around line 58-73: Add a fixture-based workspace-package import case to
accepts_static_dynamic_require_named_and_star_reexports, using the existing test
fixture conventions to create a WorkspaceImport graph edge and make the
entrypoint reach a source through it. Extend the assertion to verify that this
source produces no findings, preserving the existing coverage for static,
dynamic, require, and re-export reachability.

In `@docs/rules/required-entrypoint-reachability.md`:
- Around line 22-25: Update the entrypoints documentation to state that literal
repository-relative paths and absolute paths within the repository are accepted,
matching resolve_entrypoint and its rule test; do not change the implementation.
🪄 Autofix

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: ae51f984-851d-414c-9f22-a593a0f8283d

📥 Commits

Reviewing files that changed from the base of the PR and between 9ff8ab1 and fd34128.

📒 Files selected for processing (37)
  • crates/no-mistakes/src/check_runner.rs
  • crates/no-mistakes/src/check_runner/graph_plan.rs
  • crates/no-mistakes/src/check_runner/run_all.rs
  • crates/no-mistakes/src/check_runner/tests/architecture.rs
  • crates/no-mistakes/src/check_tasks.rs
  • crates/no-mistakes/src/codebase/rules/ids.rs
  • crates/no-mistakes/src/codebase/rules/mod.rs
  • crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability.rs
  • crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability/tests.rs
  • crates/no-mistakes/src/codebase/rules/run.rs
  • crates/no-mistakes/src/codebase/rules/run/prepared.rs
  • crates/no-mistakes/src/codebase/rules/run/prepared/execution.rs
  • crates/no-mistakes/src/codebase/rules/run/prepared/execution/graph_rules.rs
  • crates/no-mistakes/src/codebase/rules/run/prepared/execution/graph_rules/tests.rs
  • crates/no-mistakes/src/codebase/rules/run/standalone.rs
  • crates/no-mistakes/src/codebase/rules/tests.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/check_prepare.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs
  • crates/no-mistakes/tests/docs_coverage.rs
  • docs/rules/README.md
  • docs/rules/required-entrypoint-reachability.md
  • test-cases/codebase-analysis/forbidden-dependencies-basic/fixture/invalid-include.no-mistakes.yml
  • test-cases/rules/required-entrypoint-reachability/fixture/barrels/named.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/barrels/star.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/api.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/worker.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/invalid-include.no-mistakes.yml
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/dynamic.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/named.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/required.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/star.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/static.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/suppressed.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/type-only.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/unreachable.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/suppression.no-mistakes.yml
  • test-cases/rules/required-entrypoint-reachability/fixture/tsconfig.json
💤 Files with no reviewable changes (1)
  • crates/no-mistakes/src/check_tasks.rs

Comment thread crates/no-mistakes/src/check_runner/run_all.rs Outdated
Comment thread docs/rules/required-entrypoint-reachability.md Outdated

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fd34128afd

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Co-Authored-By: OpenAI Codex <codex@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0e01c55864

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability.rs Outdated
Comment thread crates/no-mistakes/src/check_runner/run_all.rs Outdated
Co-Authored-By: OpenAI Codex <codex@openai.com>
@jonathanong

Copy link
Copy Markdown
Owner Author

@codex review

Co-Authored-By: OpenAI Codex <codex@openai.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4226e936fb

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread crates/no-mistakes/src/codebase/rules/run/standalone.rs
jonathanong and others added 2 commits August 8, 2026 02:50
Co-Authored-By: OpenAI Codex <codex@openai.com>
Co-Authored-By: OpenAI Codex <codex@openai.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (4)
crates/no-mistakes/src/codebase/rules/run/standalone/tests.rs (1)

21-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use a repository fixture for this graph-scope case.

Replace the temporary directory setup with a committed fixture. Keep the skipped-directory layout in that fixture.

As per coding guidelines, use fixture-based tests and continuously add fixtures for discovered cases.

🤖 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/run/standalone/tests.rs` around lines
21 - 42, Update graph_files_for to use a committed repository fixture instead of
creating a tempfile and writing src/api.ts and generated/worker.ts at runtime.
Preserve the fixture’s skipped-directory layout, then build the
VisiblePathSnapshot and discovery inputs from that fixture so
standalone_graph_files continues testing the same graph scope.

Source: Coding guidelines

test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/.no-mistakes.yml (1)

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

Document the skipped-path invariants in these fixtures.

These changes intentionally overlap reachability inputs with filesystem-skipped paths. Add one short comment at each site so future edits preserve the intended test behavior.

  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/.no-mistakes.yml#L13-L13: explain that sourceGlobs intentionally targets a skipped file.
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/tests/scoped.test.ts#L2-L2: explain that the dynamic import intentionally targets a skipped file.
  • test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/.no-mistakes.yml#L8-L8: explain that sourceGlobs intentionally targets skipped sources.
  • test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/entrypoints/runtime.ts#L1-L1: explain that the side-effect import intentionally targets a skipped source.
🤖 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
`@test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/.no-mistakes.yml`
at line 13, Add one brief comment at each specified site documenting that the
configured sourceGlobs or import intentionally targets a filesystem-skipped
file/source:
test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/.no-mistakes.yml:13-13,
test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/tests/scoped.test.ts:2-2,
test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/.no-mistakes.yml:8-8,
and
test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/entrypoints/runtime.ts:1-1.
Preserve the existing fixture behavior and add no other changes.

Source: Coding guidelines

crates/no-mistakes/src/codebase/dependencies/graph/tests/edge_kind_semantics.rs (1)

94-118: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a comment for the require.resolve asset edge.

Explain that require.resolve() creates an inspectable dependency edge without loading the non-code asset. The assertion message only appears after failure.

As per coding guidelines, add short comments to intentionally counterintuitive 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/tests/edge_kind_semantics.rs`
around lines 94 - 118, Add a short explanatory comment immediately before the
RequireResolve asset-import assertion in the test. Reference the require.resolve
case and state that it creates an inspectable dependency edge without loading
the non-code asset; leave the assertions and behavior unchanged.

Source: Coding guidelines

docs/graph-edges.md (1)

23-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Document the runtime-reachability caveat.

State that require.resolve() only resolves a target and does not load it. State that workspace-type-import is type-only. Add a short counterexample that confirms runtime-reachability consumers must exclude both edge kinds.

As per coding guidelines, document each added graph edge kind with its direction, filter mapping, examples, and caveats.

🤖 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/graph-edges.md` around lines 23 - 27, Update the graph-edge
documentation entries for require-resolve and workspace-type-import to
explicitly state that require.resolve() resolves without loading its target and
workspace-type-import is type-only. Add a concise counterexample clarifying that
runtime-reachability consumers must exclude both edge kinds, while preserving
each edge’s direction, filter mapping, examples, and existing caveats.

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
`@test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/tests/scoped.test.ts`:
- Around line 1-2: Update the fixture test containing “filesystem skips remain
outside dynamic-import reachability” to explicitly import the Vitest test API
from “vitest” before invoking test, preserving the existing test body.

---

Nitpick comments:
In
`@crates/no-mistakes/src/codebase/dependencies/graph/tests/edge_kind_semantics.rs`:
- Around line 94-118: Add a short explanatory comment immediately before the
RequireResolve asset-import assertion in the test. Reference the require.resolve
case and state that it creates an inspectable dependency edge without loading
the non-code asset; leave the assertions and behavior unchanged.

In `@crates/no-mistakes/src/codebase/rules/run/standalone/tests.rs`:
- Around line 21-42: Update graph_files_for to use a committed repository
fixture instead of creating a tempfile and writing src/api.ts and
generated/worker.ts at runtime. Preserve the fixture’s skipped-directory layout,
then build the VisiblePathSnapshot and discovery inputs from that fixture so
standalone_graph_files continues testing the same graph scope.

In `@docs/graph-edges.md`:
- Around line 23-27: Update the graph-edge documentation entries for
require-resolve and workspace-type-import to explicitly state that
require.resolve() resolves without loading its target and workspace-type-import
is type-only. Add a concise counterexample clarifying that runtime-reachability
consumers must exclude both edge kinds, while preserving each edge’s direction,
filter mapping, examples, and existing caveats.

In
`@test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/.no-mistakes.yml`:
- Line 13: Add one brief comment at each specified site documenting that the
configured sourceGlobs or import intentionally targets a filesystem-skipped
file/source:
test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/.no-mistakes.yml:13-13,
test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/tests/scoped.test.ts:2-2,
test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/.no-mistakes.yml:8-8,
and
test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/entrypoints/runtime.ts:1-1.
Preserve the existing fixture behavior and add no other changes.
🪄 Autofix

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: 10a02299-bc5b-4874-8378-df8a2e36160f

📥 Commits

Reviewing files that changed from the base of the PR and between fd34128 and 24acef8.

📒 Files selected for processing (74)
  • crates/no-mistakes/src/check_runner/run_all.rs
  • crates/no-mistakes/src/check_runner/tests.rs
  • crates/no-mistakes/src/check_runner/tests/graph_scope.rs
  • crates/no-mistakes/src/codebase/check_facts/map.rs
  • crates/no-mistakes/src/codebase/dependencies/args_relationships_filter.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/build_plan.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/edge_import_reachability.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/edge_imports.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/edge_symbols_exports.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/edge_symbols_helpers.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/edge_symbols_reexport_namespaces.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/edge_symbols_scoped_imports.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/edge_symbols_star_reexports.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/lazy_import_neighbors.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/tests/edge_kind_semantics.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/tests/extra_symbol_helpers.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/tests/mod.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/tests/module_cases.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/tests/route_import.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/types_edges.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/types_edges_sort.rs
  • crates/no-mistakes/src/codebase/dependencies/graph/types_edges_sort/tests.rs
  • crates/no-mistakes/src/codebase/dependencies/output/tests/edge_kinds.rs
  • crates/no-mistakes/src/codebase/dependencies/tests/args_relationships.rs
  • crates/no-mistakes/src/codebase/rules/mod.rs
  • crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability.rs
  • crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability/tests.rs
  • crates/no-mistakes/src/codebase/rules/run.rs
  • crates/no-mistakes/src/codebase/rules/run/prepared.rs
  • crates/no-mistakes/src/codebase/rules/run/standalone.rs
  • crates/no-mistakes/src/codebase/rules/run/standalone/tests.rs
  • crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/config/prepared_tests.rs
  • crates/no-mistakes/src/codebase/rules/test_no_unmocked_dynamic_imports/with_facts.rs
  • crates/no-mistakes/src/codebase/rules/tests.rs
  • crates/no-mistakes/src/codebase/rules/tests/extended.rs
  • crates/no-mistakes/src/codebase/symbols/impact.rs
  • crates/no-mistakes/src/codebase/symbols/impact_collect_targets.rs
  • crates/no-mistakes/src/codebase/symbols/impact_collect_targets_tests.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/check_prepare.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/check_run.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/scope_materialize.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/scope_prepare.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/scope_project_reports.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/scope_types.rs
  • crates/no-mistakes/src/napi_api/analyze_project/tests.rs
  • crates/no-mistakes/src/tests/impact.rs
  • crates/no-mistakes/src/tests/plan_bfs.rs
  • crates/no-mistakes/tests/cli_codebase_acceptance/graph.rs
  • crates/no-mistakes/tests/cli_codebase_acceptance/workspace.rs
  • docs/graph-edges.md
  • docs/rules/required-entrypoint-reachability.md
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/.no-mistakes.yml
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/skipped/setup.ts
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/skipped/target.ts
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/skipped/vitest.config.ts
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/tests/scoped.test.ts
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/tsconfig.json
  • test-cases/check-runner/dynamic-import-respects-filesystem-skip/fixture/vitest.config.ts
  • test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/.no-mistakes.yml
  • test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/entrypoints/runtime.ts
  • test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/sources/registered.ts
  • test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/sources/unreachable.ts
  • test-cases/check-runner/required-reachability-ignores-filesystem-skip/fixture/tsconfig.json
  • test-cases/codebase-analysis/cross-boundary-monorepo/fixture/apps/backend/api/workspace-resolve.cjs
  • test-cases/codebase-analysis/import-forms/fixture/require-resolve.js
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/api.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/require-resolve-asset.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/require-resolve-workspace.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/require-resolve.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/type-workspace.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/package.json
  • test-cases/rules/required-entrypoint-reachability/fixture/packages/runtime/index.ts
  • test-cases/rules/required-entrypoint-reachability/fixture/packages/runtime/package.json
  • test-cases/rules/required-entrypoint-reachability/fixture/sources/config.json
🚧 Files skipped from review as they are similar to previous changes (7)
  • test-cases/rules/required-entrypoint-reachability/fixture/entrypoints/api.ts
  • crates/no-mistakes/src/check_runner/run_all.rs
  • crates/no-mistakes/src/codebase/rules/mod.rs
  • docs/rules/required-entrypoint-reachability.md
  • crates/no-mistakes/src/codebase/rules/required_entrypoint_reachability.rs
  • crates/no-mistakes/src/codebase/rules/run.rs
  • crates/no-mistakes/src/napi_api/analyze_project/context/check_prepare.rs

Co-Authored-By: OpenAI Codex <codex@openai.com>
@jonathanong
jonathanong merged commit 4a68588 into main Aug 9, 2026
21 checks passed
@jonathanong
jonathanong deleted the feat/required-entrypoint-reachability branch August 9, 2026 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

configuration documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant