Skip to content

feat(boundaries): config-driven boundary-violations recipe#72

Merged
SutuSebastian merged 3 commits intomainfrom
feat/boundary-violations
May 5, 2026
Merged

feat(boundaries): config-driven boundary-violations recipe#72
SutuSebastian merged 3 commits intomainfrom
feat/boundary-violations

Conversation

@SutuSebastian
Copy link
Copy Markdown
Contributor

@SutuSebastian SutuSebastian commented May 5, 2026

Summary

  • New boundaries: [{name, from_glob, to_glob, action?}] config field + boundary_rules table populated by an index-time reconciler from .codemap/config.
  • Bundled boundary-violations recipe joins dependencies × boundary_rules via SQLite GLOB; --format sarif / annotations light up via the existing file_path alias.
  • Sweeps stale codemap.config.ts references to .codemap/config.{ts,js,json} across docs, agent surfaces, JSDoc, and CLI help. Historical changesets and the intentional legacy-migration mention in src/config.ts are preserved.

Test plan

  • bun run check (798 tests + golden scenarios)
  • bun src/index.ts query --recipe boundary-violations --json[] with no boundaries declared
  • bun src/index.ts query --recipes-json | jq '.[] | select(.id == \"boundary-violations\")'
  • bun test src/application/boundary-rules.test.ts src/config.test.ts

Notes

  • Schema bump 8 → 9; boundary_rules is config-derived (in dropAll) so the next index pass re-fills it. No migration required.
  • action field is optional in the user-facing config (Zod) and filled to "deny" by the resolver — keeps defineConfig({ boundaries: [{name, from_glob, to_glob}] }) ergonomic.
  • v1 only honours 'deny'; 'allow' reserves the slot for future whitelist semantics. CHECK constraint pins the column.

Summary by CodeRabbit

Release Notes

  • New Features

    • Introduced architecture-boundary rules to detect forbidden dependency edges. Configure rules in your project config with glob patterns, then run codemap query --recipe boundary-violations to identify violations. Supports SARIF and GitHub annotations output.
  • Documentation

    • Updated configuration file path references and added boundary-violations recipe documentation with setup examples.

…/config sweep

Ships the next cadence pick from the roadmap (research note § 1.5).

- New `boundaries: [{name, from_glob, to_glob, action?}]` field on the Zod
  user-config schema; `action` defaults to `"deny"`.
- New `boundary_rules` table (STRICT, WITHOUT ROWID) with CHECK constraint on
  action. Schema bump 8 → 9.
- New `reconcileBoundaryRules` helper in src/db.ts; called from
  `runCodemapIndex` after `createSchema` so the table tracks resolved config
  exactly (config = single source of truth, table = denormalised lookup).
- New runtime accessor `getBoundaryRules`.
- Bundled `boundary-violations.{sql,md}` recipe joining `dependencies` ×
  `boundary_rules` via SQLite GLOB. Recipe rows alias `from_path` to
  `file_path` so `--format sarif` / `annotations` light up automatically.
- `codemap.config.example.json` gains a sample boundary.

Lockstep + sweep:
- docs/architecture.md § Schema gains `boundary_rules` subsection.
- docs/glossary.md adds boundaries / boundary_rules / boundary-violations.
- docs/roadmap.md drops the now-shipped backlog item per Rule 2.
- README + agent rule + skill (templates/ + .agents/) document the shape.
- Stale `codemap.config.ts` references swept across the codebase to
  `.codemap/config.{ts,js,json}` per the post-D11 layout (all docs, agent
  surfaces, JSDoc, CLI help). Historical changesets and the legacy-migration
  mention in src/config.ts intentionally preserved.

Tests:
- src/application/boundary-rules.test.ts: schema, reconciler idempotency,
  CHECK constraint, recipe SQL against synthetic dependency graph.
- src/config.test.ts: default-`[]`, default action filling, unknown action
  rejection.

Verification: bun run check (798 tests + golden scenarios) passes.
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 5, 2026

🦋 Changeset detected

Latest commit: db92aa7

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@stainless-code/codemap Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Warning

Rate limit exceeded

@SutuSebastian has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 40 minutes and 24 seconds before requesting another review.

To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ce6f65d5-4343-4568-b448-0aa15db686fd

📥 Commits

Reviewing files that changed from the base of the PR and between d3407b0 and db92aa7.

📒 Files selected for processing (7)
  • docs/architecture.md
  • docs/plans/c9-plugin-layer.md
  • src/api.ts
  • src/application/boundary-rules.test.ts
  • src/application/run-index.ts
  • src/db.ts
  • templates/recipes/boundary-violations.md
📝 Walkthrough

Walkthrough

CodeMap now supports config-driven architecture boundary rules. The feature adds a boundaries configuration array to .codemap/config.ts, creates a boundary_rules database table reconciled on each index pass, and introduces a boundary-violations recipe that joins dependencies against these rules using SQLite GLOB matching to detect forbidden import edges.

Changes

Architecture Boundary Rules

Layer / File(s) Summary
Configuration Schema
src/config.ts, src/config.test.ts
Added optional boundaries array (Zod-validated) with name, from_glob, to_glob, and action ("deny" or "allow"; defaults to "deny"). ResolvedCodemapConfig gains boundaries property. Tests verify defaulting and validation behavior.
Database Schema & Reconciliation
src/db.ts
Schema version bumped 8→9. New boundary_rules table with columns name (PK), from_glob, to_glob, action (CHECK constraint). Exported reconcileBoundaryRules(db, rules) clears and repopulates the table; dropAll() now drops boundary_rules.
Runtime Access & Index Integration
src/runtime.ts, src/application/run-index.ts
New getBoundaryRules() accessor exposes config boundaries. runCodemapIndex calls reconcileBoundaryRules() at the start of each index pass (after FTS5 handling), before branching into full/files/incremental modes.
Query Recipe & SQL
templates/recipes/boundary-violations.{sql,md}
New SQL template joins dependencies to boundary_rules on action = 'deny' using GLOB predicates for from_glob and to_glob, returning path pairs and rule name. Markdown doc covers config shape, CLI invocation, SARIF/annotations output, and v1 scope/exclusions.
Boundary Rules Tests
src/application/boundary-rules.test.ts
Test suite (150+ lines) verifies: table starts empty, reconciliation inserts rows, reconciliation is idempotent, action CHECK constraint, and boundary-violations query correctly joins and filters on GLOB match.
Configuration Path Standardization
src/api.ts, src/cli/bootstrap-codemap.ts, src/cli/cmd-mcp.ts, src/application/audit-engine.ts, docs/*, templates/*, .agents/*, README.md
Updated config file path references throughout from codemap.config.ts/.json to .codemap/config.ts/.json; also updated roadmap to remove "Boundary violations" backlog item (now implemented) and corrected config path references in FTS5 and audit documentation.
Example Configuration & Documentation
codemap.config.example.json, docs/architecture.md, docs/glossary.md, docs/roadmap.md, .agents/rules/codemap.md, .agents/skills/codemap/SKILL.md, .changeset/boundary-violations.md
Updated example config with a boundaries entry; documented boundary-rules schema, reconciliation behavior, SARIF formatting, and recipe usage in architecture and glossary; removed boundary-violations from backlog; updated CLI reference and skill documentation.

Sequence Diagram

sequenceDiagram
    participant User
    participant CLI
    participant ConfigResolver
    participant IndexEngine
    participant Database
    participant QueryEngine

    User->>CLI: codemap index
    CLI->>ConfigResolver: resolveCodemapConfig()
    ConfigResolver->>ConfigResolver: validate & default boundaries (action="deny")
    ConfigResolver-->>CLI: resolved config with boundaries[]

    CLI->>IndexEngine: runCodemapIndex(config)
    IndexEngine->>Database: reconcileBoundaryRules(db, config.boundaries)
    Database->>Database: DROP & INSERT into boundary_rules
    Database-->>IndexEngine: rules reconciled

    IndexEngine->>IndexEngine: index files & dependencies
    IndexEngine-->>CLI: indexing complete

    User->>CLI: codemap query --recipe boundary-violations --format sarif
    CLI->>QueryEngine: execute boundary-violations.sql
    QueryEngine->>Database: SELECT from dependencies JOIN boundary_rules ON GLOB(from_glob, from_path) AND GLOB(to_glob, to_path)
    Database-->>QueryEngine: matching violation rows
    QueryEngine->>QueryEngine: format as SARIF (from_path as location, rule_name in output)
    QueryEngine-->>CLI: SARIF report
    CLI-->>User: boundary violations output
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • stainless-code/codemap#23: Modifies src/application/run-index.ts and src/db.ts similarly; both PRs adjust the index engine and database schema/handling.
  • stainless-code/codemap#15: Touches the same core files (src/application/run-index.ts, src/db.ts); related changes to indexing and database infrastructure.

Suggested labels

enhancement, documentation

Poem

🐰 Boundaries drawn with careful GLOB,
Config rules that block and throb,
Dependencies checked at every pass,
Violations found—violations amassed!
A recipe to keep code neat,
Architecture now complete!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 46.15% 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
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'feat(boundaries): config-driven boundary-violations recipe' clearly and concisely summarizes the main change: a new feature adding boundary-driven violation detection via configuration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/boundary-violations

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 and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
docs/architecture.md (1)

184-184: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Schema version not updated to 9.

This line still reads **8** but the PR bumps SCHEMA_VERSION to 9. The boundary_rules section was added just below, but the version header was missed. As per coding guidelines, documentation files must be kept up-to-date with code changes.

📝 Proposed fix
-Current schema version: **8** — see [Schema Versioning](`#schema-versioning`) for details.
+Current schema version: **9** — see [Schema Versioning](`#schema-versioning`) for details.
🤖 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/architecture.md` at line 184, The docs show the schema version as **8**
but the code bump changed SCHEMA_VERSION to 9; update the displayed version
number in the architecture.md header from 8 to 9 and ensure the new
boundary_rules section is referenced in the Schema Versioning description so the
docs match the SCHEMA_VERSION constant and recent schema changes.
🧹 Nitpick comments (1)
src/db.ts (1)

364-375: ⚡ Quick win

Make reconcileBoundaryRules atomic.

If an insert fails after Line 368 (for example, duplicate name), boundary_rules can be left partially rebuilt. Wrapping this in a savepoint keeps the table all-or-nothing.

💡 Proposed change
 export function reconcileBoundaryRules(
   db: CodemapDatabase,
   rules: ReadonlyArray<BoundaryRuleRow>,
 ) {
-  db.run("DELETE FROM boundary_rules");
-  if (rules.length === 0) return;
-  for (const rule of rules) {
-    db.run(
-      "INSERT INTO boundary_rules (name, from_glob, to_glob, action) VALUES (?, ?, ?, ?)",
-      [rule.name, rule.from_glob, rule.to_glob, rule.action],
-    );
-  }
+  db.run("SAVEPOINT reconcile_boundary_rules");
+  try {
+    db.run("DELETE FROM boundary_rules");
+    for (const rule of rules) {
+      db.run(
+        "INSERT INTO boundary_rules (name, from_glob, to_glob, action) VALUES (?, ?, ?, ?)",
+        [rule.name, rule.from_glob, rule.to_glob, rule.action],
+      );
+    }
+    db.run("RELEASE SAVEPOINT reconcile_boundary_rules");
+  } catch (error) {
+    db.run("ROLLBACK TO SAVEPOINT reconcile_boundary_rules");
+    db.run("RELEASE SAVEPOINT reconcile_boundary_rules");
+    throw 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 `@src/db.ts` around lines 364 - 375, reconcileBoundaryRules currently deletes
all rows and then inserts new ones without transactional protection; wrap the
delete-and-insert sequence in a transaction/savepoint so the update is atomic:
begin a transaction (or SAVEPOINT), execute the DELETE and all INSERTs inside a
try block, and on success COMMIT (or RELEASE SAVEPOINT); on any error ROLLBACK
(or ROLLBACK TO SAVEPOINT) and rethrow or return the error. Update the function
reconcileBoundaryRules to use
db.run("BEGIN")/db.run("COMMIT")/db.run("ROLLBACK") or equivalent transaction
API around the existing db.run calls so the table is not left partially rebuilt.
🤖 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 `@docs/plans/c9-plugin-layer.md`:
- Line 30: Update the doc sentence that currently references
`.codemap/config.ts` and `plugins: [...]` to use broader, consistent terminology
that reflects supported config variants; replace the explicit
`.codemap/config.ts` mention with a canonical form such as
`.codemap/config.{ts,js,json}` or "`.codemap/config.*`" (and keep the `plugins:
[...]` example) so the documentation matches the new config surface and uses
consistent wording across docs.

In `@src/api.ts`:
- Around line 34-35: Update the public API JSDoc for the configFile property in
src/api.ts to explicitly list .js alongside .ts and .json (e.g.,
`.codemap/config.{ts,js,json}`) so callers know JS config files are supported;
modify the comment block that documents the configFile and config properties
(look for the JSDoc referencing "configFile" and "config") to include .js in the
supported formats and keep the existing phrasing about CLI `--config`.

In `@src/application/run-index.ts`:
- Around line 117-119: The call to reconcileBoundaryRules(db,
getBoundaryRules()) runs before indexFiles(...) which (via indexFiles ->
dropAll(...) -> db.dropTable('boundary_rules')) wipes the reconciled table; move
the reconcileBoundaryRules call to after the full rebuild completes by invoking
reconcileBoundaryRules(db, getBoundaryRules()) only after indexFiles(db, files,
true, ...) returns so the reconciled boundary_rules data is not dropped by
dropAll; update the call site in run-index.ts accordingly.

In `@templates/recipes/boundary-violations.md`:
- Line 45: Update the line in templates/recipes/boundary-violations.md that
claims "src/ui/* matches one level only": replace it with a clarification that
SQLite GLOB's `*` crosses `/` boundaries so `src/ui/*` will match files at any
depth (e.g. `src/ui/components/Button.tsx`), and add the recommended pattern to
match a single path segment such as `src/ui/[^/]*`; also remove the incorrect
parenthetical about "no `**` recursion" or reword it to state that GLOB has no
special `**` semantics and depth must be controlled with explicit character
classes or segment patterns.

---

Outside diff comments:
In `@docs/architecture.md`:
- Line 184: The docs show the schema version as **8** but the code bump changed
SCHEMA_VERSION to 9; update the displayed version number in the architecture.md
header from 8 to 9 and ensure the new boundary_rules section is referenced in
the Schema Versioning description so the docs match the SCHEMA_VERSION constant
and recent schema changes.

---

Nitpick comments:
In `@src/db.ts`:
- Around line 364-375: reconcileBoundaryRules currently deletes all rows and
then inserts new ones without transactional protection; wrap the
delete-and-insert sequence in a transaction/savepoint so the update is atomic:
begin a transaction (or SAVEPOINT), execute the DELETE and all INSERTs inside a
try block, and on success COMMIT (or RELEASE SAVEPOINT); on any error ROLLBACK
(or ROLLBACK TO SAVEPOINT) and rethrow or return the error. Update the function
reconcileBoundaryRules to use
db.run("BEGIN")/db.run("COMMIT")/db.run("ROLLBACK") or equivalent transaction
API around the existing db.run calls so the table is not left partially rebuilt.
🪄 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: defaults

Review profile: CHILL

Plan: Pro

Run ID: 12311da2-33fb-4bb9-af68-44275652c1a7

📥 Commits

Reviewing files that changed from the base of the PR and between edee493 and d3407b0.

📒 Files selected for processing (25)
  • .agents/rules/codemap.md
  • .agents/skills/codemap/SKILL.md
  • .changeset/boundary-violations.md
  • README.md
  • codemap.config.example.json
  • docs/architecture.md
  • docs/glossary.md
  • docs/plans/c9-plugin-layer.md
  • docs/roadmap.md
  • src/api.ts
  • src/application/audit-engine.ts
  • src/application/boundary-rules.test.ts
  • src/application/run-index.ts
  • src/cli/bootstrap-codemap.ts
  • src/cli/cmd-mcp.ts
  • src/config.test.ts
  • src/config.ts
  • src/db.ts
  • src/runtime.ts
  • templates/agents/rules/codemap.md
  • templates/agents/skills/codemap/SKILL.md
  • templates/recipes/boundary-violations.md
  • templates/recipes/boundary-violations.sql
  • templates/recipes/text-in-deprecated-functions.md
  • templates/recipes/text-in-deprecated-functions.sql

Comment thread docs/plans/c9-plugin-layer.md Outdated
Comment thread src/api.ts Outdated
Comment thread src/application/run-index.ts Outdated
Comment thread templates/recipes/boundary-violations.md Outdated
Verified each CodeRabbit thread; applied real bugs (T3, T4) and tightened
config-path wording (T1, T2).

- T3 (MAJOR bug): full-rebuild path called `reconcileBoundaryRules` BEFORE
  `indexFiles`, but `indexFiles` runs `dropAll` internally on full rebuild,
  wiping the just-reconciled `boundary_rules`. Moved the reconciler into a
  `try/finally` around the existing branching so rules are written AFTER
  every code path returns. Regression test (`survives a full rebuild
  (reconciler runs after dropAll)`) confirms the fix.
- T4: SQLite `GLOB *` is NOT filesystem-aware — it matches `/` like any
  other character. The recipe `.md` claim that `src/ui/*` "matches one
  level only" was wrong. Rewrote the GLOB section: documented that `*`
  crosses `/`, gave the `[^/]*` recipe for single-segment matches, and
  clarified there is no `**` in SQLite.
- T1: `docs/plans/c9-plugin-layer.md` Q2 broadened from `.codemap/config.ts`
  to `.codemap/config.{ts,js,json}` to match the supported variants.
- T2: `src/api.ts` `configFile` JSDoc broadened from `.ts | .json` to
  `.codemap/config.{ts,js,json}` so JS callers aren't misled.
Two more fact-checked CodeRabbit points from PR #72:

- T6: `reconcileBoundaryRules` was DELETE-then-INSERT without transactional
  protection. Zod doesn't dedupe `name`, but the schema's `name TEXT PRIMARY
  KEY` does — a duplicate would throw mid-loop, leaving the table partially
  populated (DELETE already ran). Wrapped the body in a SAVEPOINT so the
  prior good state is preserved on any insert failure. SAVEPOINT works
  inside or outside an open transaction, so callers don't need to
  coordinate. New regression test `rolls back on duplicate name —
  preserves prior good state (atomic)` confirms the prior row survives.
- T5: `docs/architecture.md` § Schema header still read **8**. Bumped to
  **9** to match `SCHEMA_VERSION` (the `boundary_rules` row was added in
  the same PR but the version line was missed).
@SutuSebastian
Copy link
Copy Markdown
Contributor Author

@coderabbitai both fact-checked and applied in db92aa7:

  • T5 (outside-diff: docs/architecture.md schema version): ✅ confirmed — SCHEMA_VERSION = 9 in src/db.ts, doc lagged at 8. Bumped.
  • T6 (nitpick: reconcileBoundaryRules atomicity): ✅ confirmed real risk — Zod doesn't dedupe name but the schema enforces name TEXT PRIMARY KEY, so a config typo (duplicate name) would throw mid-loop after DELETE already ran, leaving the table partially populated. Wrapped the body in SAVEPOINT reconcile_boundary_rules with ROLLBACK TO on error. Works inside or outside an open transaction, so callers don't need to coordinate. New regression test rolls back on duplicate name — preserves prior good state (atomic) confirms the prior row survives a failed reconcile.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 5, 2026

Tip

For best results, initiate chat on the files or code changes.

@SutuSebastian: Both fixes look excellent — thank you for the thorough follow-through!

  • T5: Good catch on the doc lag — keeping docs/architecture.md in sync with SCHEMA_VERSION avoids confusion when readers cross-reference the schema table docs against the source.
  • T6: The SAVEPOINT approach is the right call here. Using a named savepoint (reconcile_boundary_rules) means the atomicity guarantee holds regardless of whether the caller is already inside a transaction, which is a robust design. The regression test covering the "duplicate name → rollback → prior state preserved" scenario is exactly the kind of test that would catch a future refactor accidentally removing the savepoint.

Both changes are well-executed. 🎉


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@SutuSebastian SutuSebastian merged commit 2c3045d into main May 5, 2026
10 checks passed
@SutuSebastian SutuSebastian deleted the feat/boundary-violations branch May 5, 2026 07:57
SutuSebastian added a commit that referenced this pull request May 5, 2026
…te (#73)

* docs(plans): GitHub Marketplace Action plan + LSP repo-structure update

New plan `docs/plans/github-marketplace-action.md` for a composite
`stainless-code/codemap@v1` Marketplace Action. Resolves all 10 open Qs
from a grilling pass (Q-A premise → Q10 Marketplace metadata):

- Q-A scope: ship full v1.0 (5 slices including PR-comment writer for
  GHAS-less private repos + bot-context seeding); inline-review comments
  deferred to v1.x with concrete demand trigger.
- Q-B publishing: same-repo (`stainless-code/codemap`); split-repo's
  drift risk between `action.yml` and CLI flags is asymmetric.
- Q5 default: `audit --base ${{ github.base_ref }} --ci` on
  `pull_request` events; no-op + log on other events.
- Q1 input surface: ship the ideal day-one (~16 declarative inputs,
  `mode` + structured); `command:` is the universal escape hatch.
- Q2 + Q3: delegate to `package-manager-detector` (antfu/userquin) +
  `resolveCommand`'s `'execute-local'` / `'execute'` intents.
- Q7: floating major `@v1` + changesets-driven release; Action version
  stream is independent of CLI version stream (CLI at 0.4.0).
- Q4 PR-comment: (b) summary in v1.0 (opt-in); (c) inline reviews
  deferred. Closes the SARIF-vs-conversation surfaces gap (private repos
  without GHAS, aggregate audit deltas, bot-context seeding).
- Q6 ripple: `mode: aggregate` shipping day-one promotes multi-recipe
  SARIF rule.id de-dup to a v1.0 blocker.

Two genuine new CLI surfaces ship alongside the Action (caught by
fact-check 2026-05): `audit --format sarif` (today only emits `--json`)
and the `--ci` aggregate flag on `query` + `audit`. Sliced as 1a/1b.

LSP plan (`docs/plans/lsp-diagnostic-push.md`) updates:

- Repo-structure tradeoffs: third surface added — docs site (Fumadocs +
  `@fumadocs/local-md`, or Fumapress). Verified against cloned
  fuma-nama/fumadocs HEAD: multi-framework (Next.js / Astro /
  Tanstack Start / Waku / React Router); markdown-only site does NOT
  trigger monorepo conversion.
- Trigger #5 added: docs site needs to import codemap source (live
  recipe runner / WASM demo / interactive schema explorer).
- Q4 stale-claim fixed: `boundary-violations` parenthetical "(assuming
  § 1.5 ships first)" removed — boundary-violations shipped PR #72.

Roadmap backlog: new "GitHub Marketplace Action" entry; existing
audit-verdict entry annotated with "Action shipping is the most likely
accelerant for the verdict-trigger fire".

No code changes; plan-only PR. Implementation lands in subsequent PRs.

* docs(plans): address PR #73 review — fix deprecated set-output + MD040

CodeRabbit fact-checked both findings against current code; both real.

- T1 (Major): replaced deprecated `::set-output` (deprecated 2022-10 per
  GitHub changelog) with the `GITHUB_OUTPUT` environment file approach
  in the package-manager-detector wrapper sketch (Q2 resolution). Used
  `appendFileSync(process.env.GITHUB_OUTPUT, ...)` per current GitHub
  Actions docs.
- T2 (Minor, MD040): added `text` language tag to the Q3 resolution
  pseudo-code block so markdownlint-cli2 stops flagging the unlanguaged
  fence.

* docs(plans): clarify query/audit asymmetry + Q5 target-state qualifier

CodeRabbit's deeper fact-check (via GitHub API after sandbox clone failed
twice) confirmed all earlier claims accurate, but flagged two clarity
gaps in the new-CLI-surfaces framing:

- L.6 didn't surface that `--ci` is asymmetric across verbs: on `query`
  it's a new alias for already-shipped `--format sarif` (PR #43); on
  `audit`, both `--format sarif` and `--ci` are net-new. Added an
  explicit "Asymmetry worth surfacing:" line with the per-verb breakdown.
- Q5's resolution presented `audit --base ... --ci` as the default
  Action command without a temporal qualifier — consumers reading just
  Q5 would assume it's runnable today. Added "Note: this command is the
  target state — both flags ship in Slice 1 alongside the Action; the
  default isn't runnable on the current CLI."

Both clarifications also help future PR reviewers spot whether
implementation slices match the documented surface set.
SutuSebastian added a commit that referenced this pull request May 5, 2026
`non-goals-reassessment-2026-05.md` § 1 Shipped table + § 7 Lifted-to
table contradicted each other on § 1.5 — § 1 didn't list it (lifted
table was last edited before PR #72), § 7 still said "(pending)".
Boundary-violations shipped 2026-05 as PR #72; both tables now reflect
that, plus the "Pending picks" enumeration drops § 1.5.

Per Rule 8, research notes get closed/lifted but factual state may
still be corrected when the codebase moves under them. This is
hygiene, not extension of the analysis.
SutuSebastian added a commit that referenced this pull request May 5, 2026
* feat(audit): --format sarif on `codemap audit` (Slice 1a of #73 plan)

First half of Slice 1 from the GitHub Marketplace Action plan. `audit`
today only emits `--json`; this adds SARIF 2.1.0 output directly, no
JSON→SARIF transform step needed in CI workflows.

CLI:

- New `--format <text|json|sarif>` flag (default `text`).
- `--json` stays as backward-compat shortcut for `--format json`.
- `--json` + `--format <other>` rejected as a contradiction with a
  helpful error message.
- `--summary` is a no-op with `--format sarif` (SARIF results are
  per-row, not counts) and surfaces a stderr warning.
- `runAuditCmd` signature updated: `json: boolean` → `format:
  AuditOutputFormat` (`"text" | "json" | "sarif"`).

SARIF shape:

- One rule per delta key (`codemap.audit.files-added`,
  `codemap.audit.dependencies-added`,
  `codemap.audit.deprecated-added`).
- One result per `added` row (severity = `warning`; audit deltas are
  more actionable than per-recipe `note`).
- Locations auto-detected via the existing
  `file_path`/`path`/`to_path`/`from_path` priority list (same as
  `query --format sarif`); `line_start`/`line_end` populate the SARIF
  `region`.
- `removed` rows intentionally excluded — SARIF surfaces findings to
  act on, not cleanups.
- Location-only rows (e.g. files-added has only `path`) get a "new
  files: src/foo.ts" message, not the generic "(no message)" fallback.

Tests:

- 4 new SARIF-formatter unit tests (rules/results shape; empty deltas;
  missing location column; line range).
- 4 new audit-CLI parser tests (--format sarif/json/=json, unknown
  format value, --json + --format contradiction, --json + --format
  json reconcilable).
- All existing audit / output-formatters tests updated for the
  `json: bool` → `format: AuditOutputFormat` field rename.

Lockstep updates:

- `.agents/rules/codemap.md` + `templates/agents/rules/codemap.md`
  audit-vs-git-ref row gains `--format sarif` mention.
- New changeset (.changeset/audit-format-sarif.md, minor).

Slice 1b (`--ci` aggregate flag on `query` + `audit`) lands in the
follow-up PR.

* feat(cli): --ci aggregate flag on `query` + `audit` (Slice 1b of #73 plan)

Second half of Slice 1. `--ci` is the GitHub-Action-shaped CI flag:
- Aliases `--format sarif`
- Sets process.exitCode = 1 when ≥1 finding/addition surfaced
- Suppresses the no-locatable-rows stderr warning (CI templates would
  surface it as red noise; the row-set itself is the gating signal)

Lands on both `query` and `audit` (the two finding-producing verbs).
Same parser / resolver semantics on both:
- Mutually exclusive with `--json` (different format aliases)
- Mutually exclusive with `--format <other>` (contradicts the alias)
- `--ci --format sarif` redundant but accepted (consumers may set both
  for clarity in CI templates)

Wiring:
- `parseQueryRest` / `parseAuditRest` gain `--ci` token + `ci: boolean`
  in the run-shape union.
- `runQueryCmd` / `runAuditCmd` gain `ci?: boolean` opt; threaded
  through to `printFormattedQuery` (query) and the post-render
  exit-code branch (audit).
- `query`: exit 1 if `rows.length > 0` after SARIF emit.
- `audit`: exit 1 if any delta has `added.length > 0` after SARIF emit.

Tests:
- 4 new `cmd-query` parser tests (--ci alias; --ci+--json reject;
  --ci+--format json reject; --ci+--format sarif accept).
- 4 new `cmd-audit` parser tests (same matrix).
- All existing toEqual tests updated for the `ci: false` field default.

Smoke verified end-to-end:
- `query --ci` with results → SARIF stdout, exit 1.
- `audit --baseline X --ci` against identical baseline → 0 additions,
  exit 0. Diff with adds → exit 1.
- Contradiction tests (`--ci --json`) emit clear error + exit 1.

* feat(action): action.yml + scripts/detect-pm.mjs (Slice 2 of #73 plan)

Composite GitHub Action wrapping codemap CLI for the Marketplace.
~16 declarative inputs per Q1 resolution; package-manager detection +
codemap CLI invocation resolution via package-manager-detector
(antfu/userquin, MIT, 0 transitive deps, 23 kB).

action.yml shape:

- Skip-on-non-PR-events for the headline α default (audit --base
  ${{ github.base_ref }} --ci). Other events (push, schedule, …)
  no-op + log "no PR context, skipping" + exit 0 unless an explicit
  command: input is passed.
- 16 declarative inputs across 3 categories:
  - WHERE TO RUN: working-directory, package-manager (override),
    version (CLI pin), state-dir
  - WHAT TO RUN: mode (audit | recipe | aggregate | command), recipe,
    params, baseline, audit-base, changed-since, group-by, command
    (escape hatch)
  - WHAT TO DO WITH OUTPUT: format (default sarif), output-path,
    upload-sarif, pr-comment (Slice 3 stub for v1.0), fail-on, token
- Validation precedence: command > mode > defaults; mode='aggregate'
  rejected (reserved for v1.x post-Q6 SARIF rule.id de-dup work).
- 4 outputs: agent / exec / install_method (debug breadcrumbs) +
  output-file (echoes inputs.output-path).
- Composite steps: gate → setup-node → detect-pm → validate → run →
  upload-sarif (if Code Scanning enabled) → pr-comment-stub.

scripts/detect-pm.mjs:

- Wraps `package-manager-detector`'s `detect()` + `resolveCommand()`.
- Implements the Q3 invocation logic:
  - VERSION env var set → 'execute' intent (dlx-pinned)
  - codemap in devDependencies → 'execute-local'
  - else → 'execute' intent (dlx-latest)
- Outputs to $GITHUB_OUTPUT per current Actions convention (set-output
  deprecated 2022-10).
- Validates PACKAGE_MANAGER override against known agents.
- 8 unit tests covering: pnpm/bun/npm autodetect, no-lockfile fallback,
  execute-local for project-installed, dlx-pinned override, manual PM
  override, unknown PM rejection, packageManager-field priority over
  lockfile.

New runtime dep: package-manager-detector@1.6.0 (MIT, antfu/userquin,
0 transitive deps).

* feat(cli): codemap pr-comment + action.yml integration (Slice 3 of #73 plan)

Slice 3 — markdown PR-summary writer for the cases SARIF→Code-Scanning
doesn't cover well: private repos without GHAS, repos that haven't
enabled Code Scanning, aggregate audit deltas without a single
file:line anchor, and bot-context seeding (review bots read PR
conversation, not workflow artifacts).

Architecture (engine + CLI separation, mirrors show / impact / audit):

- src/application/pr-comment-engine.ts — pure transport-agnostic
  renderer. Auto-detects input shape (audit envelope vs SARIF doc) +
  renders markdown grouped by delta key (audit) or rule id (SARIF).
  Lists >50 entries collapse to `… and N more`. Removed rows surface
  in their own collapsed section (audit only).
- src/cli/cmd-pr-comment.ts — CLI wrapper. Reads JSON from a file or
  stdin (`-`). `--shape audit|sarif` overrides autodetection;
  `--json` emits structured envelope `{ markdown, findings_count,
  kind }` for action.yml steps.
- src/cli/main.ts + src/cli/bootstrap.ts wire the new `pr-comment`
  verb (whitelist + dispatch).

action.yml integration (Slice 2 stub replaced):

- pr-comment toggle now actually invokes `codemap pr-comment` against
  the SARIF / JSON output file produced by the run step, then posts
  via `gh pr comment <PR> -F -`. Same binary that produced the output
  renders the comment — version stream stays coherent.

Tests:

- 12 new pr-comment-engine unit tests (input shape detection,
  no-drift / no-findings ✅ rendering, audit summary line + per-delta
  sections, SARIF rule grouping, location formatting, >50 collapse).
- Smoke verified: real audit envelope produces markdown with file:
  links + delta sections; SARIF doc groups findings by rule id.

Lockstep updates (per docs/README.md Rule 10):

- .agents/rules/codemap.md + templates/agents/rules/codemap.md
  gain rows for `--ci` aggregate flag (Slice 1b) and `pr-comment`
  renderer (Slice 3).

* ci(action): dogfood action-smoke job + bundle changeset (Slice 4 of #73 plan)

Adds an `action-smoke` job to .github/workflows/ci.yml that runs
`uses: ./` from this repo on every PR. Validates:

- action.yml YAML syntax + composite-step flow
- gate step (skip-on-non-PR; command-set-overrides-skip)
- setup-node + npm install of package-manager-detector
- scripts/detect-pm.mjs execution + GITHUB_OUTPUT writes
- detect-pm command resolution (`<pm> dlx codemap@latest --version`)

Smoke uses `command: --version` to avoid the real-audit dependency
chain (audit baselines etc.) — codemap audit logic is covered by the
unit-test suite. The action-smoke validates the wrapper, not the
underlying CLI.

Non-blocking (`continue-on-error: true`) until v1.0.0 of codemap is
published — today the action pulls codemap@latest from npm (0.4.0),
which works for `--version` but doesn't validate v1.x flags. Promote
to a hard gate when v1.0.0 ships.

Bundles the v1.0 changeset:
- .changeset/marketplace-action.md describes the full Slice 1b-4
  surface (--ci flag, action.yml, pr-comment, dogfood) as one minor
  release. Slice 5 (Marketplace publish + listing metadata) is
  post-merge once a v1.0.0 tag exists.

* chore: slim new comments + sweep docs staleness

Per .agents/rules/concise-comments.md — re-read every comment authored
in this PR and slimmed the ones the code itself already conveys. Net:
+68 / -163 lines of comments / docstrings; same information density,
fewer words.

Cuts/slims (in order of file):
- output-formatters.ts: AuditSarifDelta + formatAuditSarif JSDoc; the
  message-fallback inline comment.
- cmd-audit.ts: AUDIT_OUTPUT_FORMATS doc-block; jsonShortcut /
  ci / Reconcile / --ci-exit / emitAuditError comments.
- cmd-query.ts: --ci variable + resolveFormat doc; printFormattedQuery's
  ci option doc; "no-locatable-rows warning" inline; runQueryCmd's
  ci option JSDoc; parseQueryRest's ci field JSDoc.
- pr-comment-engine.ts: file header; RenderedComment JSDoc; "kind"
  field doc; detectCommentInputShape doc; renderAuditComment +
  renderSarifComment JSDocs; "Header summary line" / "Group by ruleId"
  / "Prefer the most-specific identity columns first." / "Unknown
  shape — fall through to JSON." inline comments.
- cmd-pr-comment.ts: PrCommentOpts.shape doc; printPrCommentCmdHelp
  JSDoc; runPrCommentCmd JSDoc; "detected here is …" inline;
  readStdinSync gotcha doc.
- scripts/detect-pm.mjs: file header; "Step 1/2/3:" labels; "Local /
  non-Actions invocation —" comment; codemapInDevDependencies JSDoc.
- action.yml: "Build args based on inputs" inline.

Docs staleness sweep — fact-checked against the codebase:
- docs/architecture.md § "Audit wiring" listed audit's flags but
  pre-dated --format sarif + --ci. Added both + the formatAuditSarif
  shape note.
- docs/architecture.md § "Output formatters" missed formatAuditSarif.
  Added (one rule per delta key, severity warning, removed-rows
  excluded, location-only fallback).
- docs/architecture.md § "Query wiring" missed --ci. Added.
- docs/architecture.md added a new "PR-comment wiring" section
  (mirrors the cmd ↔ engine seam pattern).
- docs/glossary.md added `--ci` (under C) and `pr-comment` (under P)
  entries per Rule 9.
- README.md CLI examples updated with `audit --format sarif`,
  `audit --ci`, `query --ci`, and the `pr-comment` pipe-to-`gh pr
  comment` idiom.

Pre-existing comments (preserve-comments rule) untouched.

* fix(action): empty defaults for github-context inputs (composite actions reject expressions in input defaults)

CI failure on PR #74's Action smoke job:

  TemplateValidationException: Unrecognized named-value: 'github'.
  Located at position 1 within expression: github.base_ref / github.token

GitHub composite actions do NOT allow ${{ github.* }} expressions in
input defaults. Only `runs:` step expressions can reference the github
context. Two inputs were affected:

- `audit-base`: now defaults to "". The existing run step already does
  `BASE="${AUDIT_BASE:-$BASE_REF}"` where `BASE_REF: ${{ github.base_ref }}`
  is set as an env var (legal in step env blocks), so empty-input → PR
  base_ref behavior is preserved.
- `token`: now defaults to "". Two call sites (`upload-sarif` step's
  `token:` arg + `pr-comment` step's `GH_TOKEN`) now use
  `${{ inputs.token != '' && inputs.token || github.token }}` to fall
  back to `github.token` when unset.

Both inputs' descriptions updated to document the empty-falls-back
behavior so consumers know what to expect.

* fix: address PR #74 CodeRabbit review (6 findings, all real)

Fact-checked each finding against the codebase; all valid. Two were
critical correctness bugs (T2, T3, T5) that would have shipped silently
broken behavior.

T2 (Major) — `scripts/detect-pm.mjs` wrong package-name keys

`codemapInDevDependencies` checked `manifest?.dependencies?.codemap` but
the published package is `@stainless-code/codemap` — the project-installed
branch was dead code for all real consumers; `dlx-pinned` / `dlx-latest`
also pulled the unscoped `codemap` package (a different npm namespace
entirely). Fixed:

- Lookup checks both scoped (`@stainless-code/codemap`) and bare
  (`codemap`) keys — the latter for monorepos that alias via
  `"codemap": "workspace:*"`.
- `'execute'` (dlx) commandArgs now use the scoped published name so
  npm/pnpm/yarn/bun resolve the right registry entry.
- `'execute-local'` keeps `["codemap"]` because that's the bin alias
  per `package.json#bin`, regardless of the scoped name.
- Tests updated: scoped-dev-dep / bare-dev-dep / scoped-dlx-pinned
  cases. Old tests that asserted dlx-with-unscoped-`codemap@<v>` were
  themselves testing a bug.

T3 (Critical) — no-op `expect()` in formatAuditSarif test

`expect(run.results.every(...))` without a chained matcher creates and
discards the expectation. If formatAuditSarif reverted to severity
`note`, the test would still pass. Added `.toBe(true)`.

T5 (Major) — `require()` in ESM module

`readStdinSync` in `cmd-pr-comment.ts` used `require("node:fs").readSync`
but the file loads via `await import()` (ESM); `require` is undefined at
runtime. `codemap pr-comment -` would have crashed for every user. My
unit tests passed file paths, not stdin — caught nothing. Imported
`readSync` from `node:fs` at the top, used directly.

T4 (Major) — pr-comment SARIF renderer dropped runs[1+]

`renderSarifComment` only read `doc.runs?.[0]?.results`. Valid SARIF
allows multiple runs (merged / multi-tool docs). Now flattens via
`(doc.runs ?? []).flatMap(run => run.results ?? [])`. New test
`aggregates results across multi-run SARIF docs (not just runs[0])`.

T1 (Minor) — `mode: command` without `command:` input falls through

The validate step accepted `mode=command` but didn't guard against
empty `command:`. Run step's if/elif only handled `audit` + `recipe`,
so `$EXEC` would invoke with empty `$ARGS`. Added an explicit guard
mirroring the `mode=recipe` pattern.

T6 (Minor) — `--ci` doc said "no-findings warning"; actual is "no-locatable-rows"

Both `.agents/rules/codemap.md` and `templates/agents/rules/codemap.md`
described the suppressed warning incorrectly. Aligned wording with the
implementation in `printFormattedQuery`.

* docs(research): mark § 1.5 boundary-violations as shipped (PR #72)

`non-goals-reassessment-2026-05.md` § 1 Shipped table + § 7 Lifted-to
table contradicted each other on § 1.5 — § 1 didn't list it (lifted
table was last edited before PR #72), § 7 still said "(pending)".
Boundary-violations shipped 2026-05 as PR #72; both tables now reflect
that, plus the "Pending picks" enumeration drops § 1.5.

Per Rule 8, research notes get closed/lifted but factual state may
still be corrected when the codebase moves under them. This is
hygiene, not extension of the analysis.

* docs(plan): consolidate Slice 5 runbook for later pickup

Slice 5 (publish + Marketplace listing) is post-merge work — anyone
should be able to pick it up without triangulating across the plan's
Q7 release-workflow + Q10 listing-metadata + Slice-5 one-liner.

Added a "Slice 5 runbook (post-merge)" subsection that:

- Surfaces the CLI / Action version stream decoupling explicitly:
  Action ships at v1 against CLI 0.5.0 (the version PR #74's
  changeset bumps to). CLI v1.0.0 is not required.
- Lists 7 sequenced executable steps from "merge PR #74" through to
  "delete this plan per Rule 3" (the canonical close-out per
  docs-governance).
- Calls out which docs already hold the durable design decisions
  (architecture.md / glossary.md / agent rules / MARKETPLACE.md) so
  the deletion in step 7 is safe.
- Documents subsequent-release cadence (changesets-driven; force-push
  the floating v<major> tag) and the major-bump policy.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant