feat: standards audit infrastructure overhaul#21
Conversation
|
Important Review skippedToo many files! This PR contains 159 files, which is 9 over the limit of 150. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (159)
You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughBatched metadata and infra changes: plugin version bumps in the marketplace, migration from Biome to OXLint across plugins, new shared Vitest package, expanded workspaces and Turbo integration, CI enhancements, new dev hooks and tests added to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/sdd-webapp/package.json (1)
18-19:⚠️ Potential issue | 🟠 MajorIncomplete migration to oxlint for this package.
The PR migrates linting from Biome to oxlint across the monorepo, but this package still uses Biome in its
checkandcheck:fixscripts. All other packages/plugins in the PR have been updated to useoxlint .andoxlint --fix .respectively, and have corresponding.oxlintrc.jsonconfiguration files added.Confirm whether
packages/sdd-webappwas intentionally excluded from the oxlint migration, or if this is an oversight that should be completed for consistency.Based on learnings: The codebase previously mandated Biome v2 for linting and formatting. This PR represents a significant architectural shift away from that standard across the entire monorepo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sdd-webapp/package.json` around lines 18 - 19, The package's lint scripts still reference Biome; update the package.json scripts "check" and "check:fix" to use "oxlint ." and "oxlint --fix ." (respectively) to match the rest of the monorepo, and add a .oxlintrc.json config in packages/sdd-webapp if one is missing so behavior is consistent with other packages; modify the scripts in package.json (search for the "check" and "check:fix" entries) and ensure the repository-level or package-level oxlint configuration is present and aligned with the monorepo defaults.plugins/sdd/hooks/block-archived-edit.ts (1)
5-5:⚠️ Potential issue | 🟠 MajorSame pipeline failures as
block-unsafe-type-assertion.ts.This file has identical TypeScript errors (TS2307, TS7006) stemming from the same root cause:
@r_masseater/cc-plugin-libmodule resolution and missing type definitions for thewrapRuncallback parameter.Also applies to: 17-17
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/sdd/hooks/block-archived-edit.ts` at line 5, The TS errors come from unresolved types for the external module and an untyped callback parameter for wrapRun; fix by either restoring/adjusting the package's public types or adding local ambient types and explicit parameter typing: ensure HookLogger is correctly imported from the actual exported symbol in the package and then annotate the wrapRun callback parameter (the function passed to wrapRun in block-archived-edit.ts) with a concrete type (e.g., declare or import a HookContext/RunArgs type or use any as a short-term fix) or add a declare module "@r_masseater/cc-plugin-lib" with the required types (HookLogger and the callback signature) in a .d.ts file so TS2307/TS7006 are resolved.plugins/mutils/hooks/block-unsafe-type-assertion.ts (1)
2-2:⚠️ Potential issue | 🟠 MajorFix module resolution by building cc-plugin-lib and provide explicit type annotation for context parameter.
The CI reports two TypeScript errors:
TS2307: Cannot find module@r_masseater/cc-plugin-lib— the package'sdist/directory does not exist. Build cc-plugin-lib usingbun run buildinpackages/cc-plugin-lib/to generate declaration files.TS7006: Parametercontextimplicitly has ananytype — the callback parameter at line 43 lacks explicit type annotation. Either add explicit typing like(context: PreToolUseContext)or ensure cc-plugin-lib's type definitions are available for type inference.Also, lines 81–94 use single-tab indentation while surrounding code uses double-tab; standardize to match the rest of the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/hooks/block-unsafe-type-assertion.ts` at line 2, The import error and implicit any come from missing built types for `@r_masseater/cc-plugin-lib` and an untyped callback parameter: first run a build of cc-plugin-lib (e.g., execute "bun run build" in packages/cc-plugin-lib/) to create the dist/ and type declaration files so HookLogger and wrapRun resolve; then add an explicit type annotation to the callback parameter (e.g., change the callback signature to (context: PreToolUseContext) => ...) where the untyped parameter is used (the function passed into wrapRun) so TS7006 is fixed; finally normalize the indentation on the block around lines 81–94 to match the file's double-tab style.
🧹 Nitpick comments (7)
plugins/ui-dev/.oxlintrc.json (1)
2-2: Use the local schema path instead of a remote branch URL.Line 2 references a mutable
refs/heads/...branch URL. Official Oxlint documentation consistently recommends using the local bundled schema:./node_modules/oxlint/configuration_schema.json. This is generated and included in the oxlint npm package, providing reliable editor validation without relying on potentially unstable remote URLs.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/ui-dev/.oxlintrc.json` at line 2, The $schema value currently points to a mutable remote branch URL; replace that value with the local bundled Oxlint schema path to avoid relying on remote branches — update the "$schema" entry in .oxlintrc.json to use "./node_modules/oxlint/configuration_schema.json" so editor validation uses the packaged schema instead of the refs/heads URL.plugins/mutils/hooks/block-unsafe-type-assertion.ts (1)
80-94: Indentation inconsistency in the new WHY/FIX block.Lines 81-94 use single-tab indentation while the surrounding code in the
runcallback uses double-tab indentation. This breaks visual consistency.🔧 Fix indentation
// WHY/FIX メッセージ: フック自身が検出するパターンを直接記述しないよう文字を分割して結合 - const asU = "\x61s \x75nknown"; - const asA = "\x61s \x61ny"; - const asE = "\x61s \x7b\x7d"; - const cA = "\x3a \x61ny"; - const whyFix = [ - "", - `WHY: 型安全性を破壊するアサーション(${asU}, ${asA}, ${asE}, ${cA})は型チェッカーが検出できないバグを生み、リファクタリング耐性を失う。`, - "", - "FIX: 以下のいずれかで修正:", - "- 型ガード (is / instanceof) で narrowing", - "- satisfies で型適合チェック", - "- ジェネリクスで型パラメータ化", - "- unknown + 明示的な narrowing", - ].join("\n"); + const asU = "\x61s \x75nknown"; + const asA = "\x61s \x61ny"; + const asE = "\x61s \x7b\x7d"; + const cA = "\x3a \x61ny"; + const whyFix = [ + "", + `WHY: 型安全性を破壊するアサーション(${asU}, ${asA}, ${asE}, ${cA})は型チェッカーが検出できないバグを生み、リファクタリング耐性を失う。`, + "", + "FIX: 以下のいずれかで修正:", + "- 型ガード (is / instanceof) で narrowing", + "- satisfies で型適合チェック", + "- ジェネリクスで型パラメータ化", + "- unknown + 明示的な narrowing", + ].join("\n");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/hooks/block-unsafe-type-assertion.ts` around lines 80 - 94, The WHY/FIX message block (variables asU, asA, asE, cA and the assembled whyFix) is indented with a single tab while the surrounding run callback uses double-tab indentation; re-indent the entire block so each line matches the double-tab level used in the run callback (keep the same strings and escaping but update leading whitespace for asU, asA, asE, cA, whyFix and the surrounding comment) to restore visual consistency.packages/cc-plugin-lib/tsconfig.json (1)
16-16: Test files are excluded from type checking.Adding
src/**/*.test.tstoexcluderemoves test files fromtsgo --noEmittype checking. Since the Vitest config (per context snippet) doesn't enabletypecheck: { enabled: true }, test files won't be type-checked in CI.This is acceptable if:
- Test files are simple and unlikely to have type errors
- IDE provides real-time type feedback during development
Consider adding
vitest --typecheckto the test script or a separate CI step if test type safety becomes a concern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cc-plugin-lib/tsconfig.json` at line 16, The tsconfig.json currently excludes test files via the "exclude": ["node_modules", "dist", "src/**/*.test.ts"] entry which prevents test files from being type-checked; either remove "src/**/*.test.ts" from that exclude array so tsgo --noEmit will include tests in type checking, or keep the exclude but update the test CI/script to run Vitest with type checking (e.g., add a vitest --typecheck step or enable typecheck: { enabled: true } in the Vitest config/test script) so tests are verified in CI; locate and edit the "exclude" array in tsconfig.json or update the project's test script/Vitest config accordingly.packages/cc-plugin-lib/src/env.test.ts (1)
3-61: Consider adding direct tests forenv.home.The suite tests
xdgStateHomewhich indirectly exercisesenv.home, but direct coverage forenv.homewould verify:
- Returns
process.env.HOMEwhen set- Falls back to
homedir()whenHOMEis unset- Throws when both are unavailable (edge case)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cc-plugin-lib/src/env.test.ts` around lines 3 - 61, Add direct tests for env.home inside packages/cc-plugin-lib/src/env.test.ts by using the existing getEnv() helper: (1) set process.env.HOME to a path and assert (await getEnv()).home equals that path; (2) delete process.env.HOME and mock os.homedir() (or import homedir and stub) to return a known value and assert env.home uses the homedir result; (3) simulate both HOME undefined and homedir() returning undefined/empty and assert accessing env.home throws. Place these new tests alongside the other env tests and restore process.env/homedir stubs in afterEach to avoid cross-test pollution.packages/cc-plugin-lib/src/git.test.ts (1)
31-39: Test may be flaky if tmpdir is inside a git repository.If the system's temp directory (e.g.,
/tmp) resides within a git repository,findGitRoot(nested)will traverse upward and find that.gitinstead of returningnull. The comment acknowledges the risk but doesn't mitigate it.Consider mocking
existsSyncor asserting a weaker condition (e.g.,expect(findGitRoot(nested)).not.toBe(nested)).💡 Alternative: explicit mock or conditional assertion
test("returns null when no .git exists", () => { - // testDir has no .git, and we go up from a deeply nested path within it - // Use the testDir itself which has no .git const nested = join(testDir, "no-git-here"); mkdirSync(nested, { recursive: true }); - // findGitRoot traverses up to "/" - in a temp dir with no .git, it will eventually return null - // unless it hits a real .git above. To be safe, test the logic: - expect(findGitRoot(nested)).toBeNull(); + // findGitRoot traverses up to "/" - it returns null only if no .git exists anywhere above + const result = findGitRoot(nested); + // At minimum, verify it doesn't return the testDir (which has no .git) + expect(result).not.toBe(testDir); + expect(result).not.toBe(nested); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cc-plugin-lib/src/git.test.ts` around lines 31 - 39, The test "returns null when no .git exists" can be flaky because findGitRoot walks up to real system .git; fix it by mocking fs.existsSync within this test so any path that looks like a .git directory returns false (e.g., use jest.spyOn(fs, "existsSync").mockImplementation(p => p.endsWith(`${path.sep}.git`) ? false : realExistsSync(p))), call findGitRoot(nested), assert it returns null, and finally restore the mock (mockRestore) to avoid affecting other tests; reference symbols: findGitRoot and fs.existsSync.packages/cc-plugin-lib/src/env.ts (1)
11-13:keepCountof0would fall back to30.
Number("0") || 30evaluates to30because0is falsy. IfkeepCount=0is an intentional way to disable log rotation, use??instead:get keepCount(): number { - return Number(process.env.MASSEATER_PLUGINS_KEEP_COUNT) || 30; + const val = Number(process.env.MASSEATER_PLUGINS_KEEP_COUNT); + return Number.isNaN(val) ? 30 : val; },If
0should never be valid (deleting all logs), the current behavior is fine.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cc-plugin-lib/src/env.ts` around lines 11 - 13, The getter keepCount currently uses Number(process.env.MASSEATER_PLUGINS_KEEP_COUNT) || 30 which treats "0" as falsy and falls back to 30; change the logic in the keepCount getter to explicitly check whether the env var is set (e.g. const v = process.env.MASSEATER_PLUGINS_KEEP_COUNT) and return v !== undefined ? Number(v) : 30 so that "0" is preserved as a valid value while still defaulting to 30 when the env var is absent.package.json (1)
7-17: Add a rootknipscript for task parity.You already expose root aliases for
build/check/typecheck/test; addingknipimproves consistency and discoverability for local runs.Suggested patch
"scripts": { "build": "turbo run build", "check": "turbo run check", "typecheck": "turbo run typecheck", "test": "turbo run test", + "knip": "turbo run knip", "check:docs": "bun ./scripts/doc-engine.ts check", "docs": "bun ./scripts/doc-engine.ts generate", "check:plugin-list": "bun ./scripts/doc-engine.ts check-list", "hooks:version-check": "bun ./scripts/check-version-update.ts", "hooks:plugin-list-sync": "bun ./scripts/doc-engine.ts generate" },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 7 - 17, Add a root "knip" npm script to package.json's "scripts" object to match the existing workspace aliases (similar to "build", "check", "typecheck", "test"); specifically add a "knip": "turbo run knip" entry alongside the other scripts so local contributors can run npm/yarn/pnpm run knip at the repo root and it will invoke the workspace knip task.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 48-64: The grep checks for '"test"' and '"knip"' in the CI
workflow can match false positives (e.g., descriptions or dependency names);
update the Run tests and Run knip steps to use a precise detection of script
keys instead of a plain string match — either replace the grep pattern with a
JSON-aware regex that ensures the key appears under "scripts" (look for
'"scripts"\s*:\s*{[^}]*"test"\s*:' and '"scripts"\s*:\s*{[^}]*"knip"\s*:') or,
preferably, use jq to test .scripts.test and .scripts.knip (e.g., jq -e
'.scripts.test' package.json) so the steps only run when the actual script keys
exist.
In @.gitignore:
- Line 17: Add an ignore rule for the whole `.claude/` directory before the
existing negation so the negation `!.claude/.turbo` actually takes effect;
update the `.gitignore` so it first ignores `.claude/` and then re-includes the
specific path `!.claude/.turbo`, ensuring only `.claude/.turbo` is tracked while
the rest of `.claude/` remains ignored.
In `@AGENTS.md`:
- Line 54: Add a short descriptive entry for the package name
`@r_masseater/vitest-config` in the table in AGENTS.md so it’s not left blank;
update the row that currently reads "| **@r_masseater/vitest-config** | |" to
include a concise description like "Shared Vitest configuration for the monorepo
(presets, plugins, and common test setup)" so readers can discover what the
package provides and when to use it.
In `@plugins/context/package.json`:
- Around line 5-6: The package.json change replaced Biome with OXLint but
omitted formatting support; fix by either restoring Biome for combined
lint+format or completing the OXLint migration: add Oxfmt as a devDependency and
update the "check" and "check:fix" npm scripts (the "check" and "check:fix"
script entries) to run both linting (oxlint) and formatting checks/auto-apply
(oxfmt --check / oxfmt --apply or the equivalent oxfmt flags), and update
project docs to state the approved toolchain (Biome v2 or OXLint+Oxfmt) so
maintainers know which workflow to use.
In `@plugins/devkit/hooks/lint-on-edit.ts`:
- Around line 21-33: Validate that context.input?.tool_input?.file_path exists
and is a string before assigning to filePath (use the existing
isTypeScriptFile(filePath) after that); if missing or not a string, log via
logger.debug and return context.success({}). Wrap the Bun.spawn(["oxlint",
filePath], ...) call in a try/catch to capture startup failures (e.g., oxlint
not found), log the error details with logger.error or logger.debug and return
context.success({}) (or an appropriate failure response) instead of letting the
exception bubble; also guard use of proc (ensure proc is set) before accessing
its stdout/stderr.
In `@plugins/devkit/hooks/verify-on-stop.ts`:
- Around line 58-64: The response currently returns event: "Stop" with an
output.reason field which doesn't match the documented Stop hook pattern; update
the return to use hookSpecificOutput instead: keep event: "Stop" and replace the
output object with hookSpecificOutput that contains hookEventName: "Stop" and
additionalContext: { decision: "block", reason:
`型チェックが失敗しました。以下の型エラーを修正してください:\n\n${output}` } so callers follow the documented
response-patterns format (change the response emitted by the return/context.json
call in verify-on-stop.ts accordingly).
In `@plugins/progress-tracker/.oxlintrc.json`:
- Around line 1-3: Replace the remote HTTP schema URL in the "$schema" key with
the local bundled schema path (./node_modules/oxlint/configuration_schema.json)
in every .oxlintrc.json file; locate the "$schema" entry (e.g., in
plugins/progress-tracker/.oxlintrc.json) and update its value to the local path
so oxlint can resolve the schema correctly across all 13 .oxlintrc.json files.
In `@plugins/sdd/package.json`:
- Around line 5-6: The "check" and "check:fix" npm scripts currently run only
oxlint (linting) but must also enforce/auto-apply formatting; update the
package.json scripts named "check" and "check:fix" so that "check" runs both
linting and formatting (either by chaining oxfmt + oxlint or replacing both with
a unified tool like biome that does check+format) and ensure "check:fix" applies
fixes/formatting (e.g., run oxfmt --write then oxlint --fix, or use biome check
--write). Locate the scripts "check" and "check:fix" in package.json and change
them to invoke formatting as described to meet the SDD guideline.
---
Outside diff comments:
In `@packages/sdd-webapp/package.json`:
- Around line 18-19: The package's lint scripts still reference Biome; update
the package.json scripts "check" and "check:fix" to use "oxlint ." and "oxlint
--fix ." (respectively) to match the rest of the monorepo, and add a
.oxlintrc.json config in packages/sdd-webapp if one is missing so behavior is
consistent with other packages; modify the scripts in package.json (search for
the "check" and "check:fix" entries) and ensure the repository-level or
package-level oxlint configuration is present and aligned with the monorepo
defaults.
In `@plugins/mutils/hooks/block-unsafe-type-assertion.ts`:
- Line 2: The import error and implicit any come from missing built types for
`@r_masseater/cc-plugin-lib` and an untyped callback parameter: first run a build
of cc-plugin-lib (e.g., execute "bun run build" in packages/cc-plugin-lib/) to
create the dist/ and type declaration files so HookLogger and wrapRun resolve;
then add an explicit type annotation to the callback parameter (e.g., change the
callback signature to (context: PreToolUseContext) => ...) where the untyped
parameter is used (the function passed into wrapRun) so TS7006 is fixed; finally
normalize the indentation on the block around lines 81–94 to match the file's
double-tab style.
In `@plugins/sdd/hooks/block-archived-edit.ts`:
- Line 5: The TS errors come from unresolved types for the external module and
an untyped callback parameter for wrapRun; fix by either restoring/adjusting the
package's public types or adding local ambient types and explicit parameter
typing: ensure HookLogger is correctly imported from the actual exported symbol
in the package and then annotate the wrapRun callback parameter (the function
passed to wrapRun in block-archived-edit.ts) with a concrete type (e.g., declare
or import a HookContext/RunArgs type or use any as a short-term fix) or add a
declare module "@r_masseater/cc-plugin-lib" with the required types (HookLogger
and the callback signature) in a .d.ts file so TS2307/TS7006 are resolved.
---
Nitpick comments:
In `@package.json`:
- Around line 7-17: Add a root "knip" npm script to package.json's "scripts"
object to match the existing workspace aliases (similar to "build", "check",
"typecheck", "test"); specifically add a "knip": "turbo run knip" entry
alongside the other scripts so local contributors can run npm/yarn/pnpm run knip
at the repo root and it will invoke the workspace knip task.
In `@packages/cc-plugin-lib/src/env.test.ts`:
- Around line 3-61: Add direct tests for env.home inside
packages/cc-plugin-lib/src/env.test.ts by using the existing getEnv() helper:
(1) set process.env.HOME to a path and assert (await getEnv()).home equals that
path; (2) delete process.env.HOME and mock os.homedir() (or import homedir and
stub) to return a known value and assert env.home uses the homedir result; (3)
simulate both HOME undefined and homedir() returning undefined/empty and assert
accessing env.home throws. Place these new tests alongside the other env tests
and restore process.env/homedir stubs in afterEach to avoid cross-test
pollution.
In `@packages/cc-plugin-lib/src/env.ts`:
- Around line 11-13: The getter keepCount currently uses
Number(process.env.MASSEATER_PLUGINS_KEEP_COUNT) || 30 which treats "0" as falsy
and falls back to 30; change the logic in the keepCount getter to explicitly
check whether the env var is set (e.g. const v =
process.env.MASSEATER_PLUGINS_KEEP_COUNT) and return v !== undefined ? Number(v)
: 30 so that "0" is preserved as a valid value while still defaulting to 30 when
the env var is absent.
In `@packages/cc-plugin-lib/src/git.test.ts`:
- Around line 31-39: The test "returns null when no .git exists" can be flaky
because findGitRoot walks up to real system .git; fix it by mocking
fs.existsSync within this test so any path that looks like a .git directory
returns false (e.g., use jest.spyOn(fs, "existsSync").mockImplementation(p =>
p.endsWith(`${path.sep}.git`) ? false : realExistsSync(p))), call
findGitRoot(nested), assert it returns null, and finally restore the mock
(mockRestore) to avoid affecting other tests; reference symbols: findGitRoot and
fs.existsSync.
In `@packages/cc-plugin-lib/tsconfig.json`:
- Line 16: The tsconfig.json currently excludes test files via the "exclude":
["node_modules", "dist", "src/**/*.test.ts"] entry which prevents test files
from being type-checked; either remove "src/**/*.test.ts" from that exclude
array so tsgo --noEmit will include tests in type checking, or keep the exclude
but update the test CI/script to run Vitest with type checking (e.g., add a
vitest --typecheck step or enable typecheck: { enabled: true } in the Vitest
config/test script) so tests are verified in CI; locate and edit the "exclude"
array in tsconfig.json or update the project's test script/Vitest config
accordingly.
In `@plugins/mutils/hooks/block-unsafe-type-assertion.ts`:
- Around line 80-94: The WHY/FIX message block (variables asU, asA, asE, cA and
the assembled whyFix) is indented with a single tab while the surrounding run
callback uses double-tab indentation; re-indent the entire block so each line
matches the double-tab level used in the run callback (keep the same strings and
escaping but update leading whitespace for asU, asA, asE, cA, whyFix and the
surrounding comment) to restore visual consistency.
In `@plugins/ui-dev/.oxlintrc.json`:
- Line 2: The $schema value currently points to a mutable remote branch URL;
replace that value with the local bundled Oxlint schema path to avoid relying on
remote branches — update the "$schema" entry in .oxlintrc.json to use
"./node_modules/oxlint/configuration_schema.json" so editor validation uses the
packaged schema instead of the refs/heads URL.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 12a70cbd-a3c6-4833-ba1f-35bf4cb66238
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (77)
.claude-plugin/marketplace.json.github/workflows/ci.yml.gitignore.oxlintrc.jsonAGENTS.mdbiome.jsondocs/dev-commands.mdlefthook.ymlpackage.jsonpackages/cc-plugin-lib/.oxlintrc.jsonpackages/cc-plugin-lib/biome.jsonpackages/cc-plugin-lib/knip.jsonpackages/cc-plugin-lib/package.jsonpackages/cc-plugin-lib/src/db.test.tspackages/cc-plugin-lib/src/env.test.tspackages/cc-plugin-lib/src/env.tspackages/cc-plugin-lib/src/git.test.tspackages/cc-plugin-lib/src/index.tspackages/cc-plugin-lib/src/logger.tspackages/cc-plugin-lib/src/wrapRun.test.tspackages/cc-plugin-lib/tsconfig.jsonpackages/cc-plugin-lib/vitest.config.tspackages/sdd-webapp/package.jsonpackages/vitest-config/package.jsonpackages/vitest-config/vitest.config.tsplugins/agnix/.oxlintrc.jsonplugins/agnix/package.jsonplugins/agnix/plugin.jsonplugins/code-review/.oxlintrc.jsonplugins/code-review/package.jsonplugins/code-review/plugin.jsonplugins/code-review/tsconfig.jsonplugins/context/.oxlintrc.jsonplugins/context/package.jsonplugins/context/plugin.jsonplugins/context/tsconfig.jsonplugins/devkit/.oxlintrc.jsonplugins/devkit/AGENTS.mdplugins/devkit/hooks/hooks.jsonplugins/devkit/hooks/lint-on-edit.tsplugins/devkit/hooks/verify-on-stop.tsplugins/devkit/package.jsonplugins/devkit/plugin.jsonplugins/devkit/skills/standards/references/quality-automation.mdplugins/devkit/skills/standards/references/test.mdplugins/devkit/tsconfig.jsonplugins/eslint-lsp/.oxlintrc.jsonplugins/eslint-lsp/package.jsonplugins/eslint-lsp/plugin.jsonplugins/eslint-lsp/tsconfig.jsonplugins/github-workflow/.oxlintrc.jsonplugins/github-workflow/package.jsonplugins/github-workflow/plugin.jsonplugins/github-workflow/tsconfig.jsonplugins/mutils/.oxlintrc.jsonplugins/mutils/hooks/block-unsafe-type-assertion.tsplugins/mutils/package.jsonplugins/mutils/plugin.jsonplugins/mutils/tsconfig.jsonplugins/progress-tracker/.oxlintrc.jsonplugins/progress-tracker/package.jsonplugins/progress-tracker/plugin.jsonplugins/progress-tracker/tsconfig.jsonplugins/research/.oxlintrc.jsonplugins/research/package.jsonplugins/research/plugin.jsonplugins/research/tsconfig.jsonplugins/sdd/.oxlintrc.jsonplugins/sdd/hooks/block-archived-edit.tsplugins/sdd/package.jsonplugins/sdd/plugin.jsonplugins/sdd/tsconfig.jsonplugins/ui-dev/.oxlintrc.jsonplugins/ui-dev/package.jsonplugins/ui-dev/plugin.jsonplugins/ui-dev/tsconfig.jsonturbo.json
💤 Files with no reviewable changes (2)
- packages/cc-plugin-lib/biome.json
- biome.json
.github/workflows/ci.yml
Outdated
| - name: Run tests | ||
| working-directory: ${{ matrix.path }} | ||
| run: | | ||
| if grep -q '"test"' package.json; then | ||
| bun run test | ||
| else | ||
| echo "No test script found, skipping" | ||
| fi | ||
|
|
||
| - name: Run knip | ||
| working-directory: ${{ matrix.path }} | ||
| run: | | ||
| if grep -q '"knip"' package.json; then | ||
| bun run knip | ||
| else | ||
| echo "No knip script found, skipping" | ||
| fi |
There was a problem hiding this comment.
Grep pattern may produce false positives.
The pattern '"test"' matches any occurrence of "test" in package.json, including partial matches like "test:coverage", "vitest" in dependencies, or "test" in description fields. This could cause the step to run when no actual test script exists.
Use a more precise pattern that matches the script key:
Proposed fix for more reliable script detection
- name: Run tests
working-directory: ${{ matrix.path }}
run: |
- if grep -q '"test"' package.json; then
+ if grep -qE '"test"\s*:' package.json; then
bun run test
else
echo "No test script found, skipping"
fi
- name: Run knip
working-directory: ${{ matrix.path }}
run: |
- if grep -q '"knip"' package.json; then
+ if grep -qE '"knip"\s*:' package.json; then
bun run knip
else
echo "No knip script found, skipping"
fiAlternatively, use jq for proper JSON parsing:
if jq -e '.scripts.test' package.json > /dev/null 2>&1; then🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml around lines 48 - 64, The grep checks for '"test"'
and '"knip"' in the CI workflow can match false positives (e.g., descriptions or
dependency names); update the Run tests and Run knip steps to use a precise
detection of script keys instead of a plain string match — either replace the
grep pattern with a JSON-aware regex that ensures the key appears under
"scripts" (look for '"scripts"\s*:\s*{[^}]*"test"\s*:' and
'"scripts"\s*:\s*{[^}]*"knip"\s*:') or, preferably, use jq to test .scripts.test
and .scripts.knip (e.g., jq -e '.scripts.test' package.json) so the steps only
run when the actual script keys exist.
| .serena/ | ||
|
|
||
| !.claude/ | ||
| !.claude/.turbo |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find ignore rules involving ".claude" across repo .gitignore files
fd -H -a '.gitignore' | xargs -I{} sh -c 'echo "== {} =="; rg -n "claude|turbo" "{}"; echo'
# Show git's actual ignore decision for representative paths
git check-ignore -v .claude .claude/.turbo .claude/.turbo/cache 2>/dev/null || trueRepository: masseater/claude-code-plugin
Length of output: 229
🏁 Script executed:
cat -n .gitignoreRepository: masseater/claude-code-plugin
Length of output: 322
Add a rule to ignore the .claude/ directory before negating the specific path.
The negation pattern !.claude/.turbo alone is ineffective. Git negation patterns only un-ignore paths that are already ignored by a previous pattern. Since there is no rule ignoring .claude/ or its contents, this negation does nothing. Add:
.claude/
!.claude/.turbo
This will ignore the entire .claude/ directory and then re-include only .turbo.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore at line 17, Add an ignore rule for the whole `.claude/` directory
before the existing negation so the negation `!.claude/.turbo` actually takes
effect; update the `.gitignore` so it first ignores `.claude/` and then
re-includes the specific path `!.claude/.turbo`, ensuring only `.claude/.turbo`
is tracked while the rest of `.claude/` remains ignored.
| |-----------|------| | ||
| | **@r_masseater/cc-plugin-lib** | Common library for Claude Code plugins - logging and error handling utilities | | ||
| | **@r_masseater/sdd-webapp** | Web dashboard for SDD (Spec Driven Development) plugin | | ||
| | **@r_masseater/vitest-config** | | |
There was a problem hiding this comment.
Add a description for the new package entry.
Line 54 currently leaves @r_masseater/vitest-config undocumented in the package table, which weakens discoverability.
📝 Suggested docs fix
-| **@r_masseater/vitest-config** | |
+| **@r_masseater/vitest-config** | Shared Vitest configuration for workspace packages/plugins |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | **@r_masseater/vitest-config** | | | |
| | **@r_masseater/vitest-config** | Shared Vitest configuration for workspace packages/plugins | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` at line 54, Add a short descriptive entry for the package name
`@r_masseater/vitest-config` in the table in AGENTS.md so it’s not left blank;
update the row that currently reads "| **@r_masseater/vitest-config** | |" to
include a concise description like "Shared Vitest configuration for the monorepo
(presets, plugins, and common test setup)" so readers can discover what the
package provides and when to use it.
plugins/devkit/hooks/lint-on-edit.ts
Outdated
| const filePath = context.input.tool_input.file_path; | ||
|
|
||
| if (!isTypeScriptFile(filePath)) { | ||
| logger.debug(`Skipping: not a .ts/.tsx file: ${filePath}`); | ||
| return context.success({}); | ||
| } | ||
|
|
||
| logger.debug(`Running oxlint on ${filePath}`); | ||
|
|
||
| const proc = Bun.spawn(["oxlint", filePath], { | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify whether PostToolUse payload typing guarantees tool_input.file_path
rg -n -C4 'PostToolUse|tool_input|file_path' plugins packages
# Verify wrapRun error-handling semantics to understand failure mode on thrown exceptions
rg -n -C6 '\bwrapRun\b' plugins packagesRepository: masseater/claude-code-plugin
Length of output: 50386
Add defensive checks for file_path and handle Bun.spawn startup failures.
At line 21, direct access to context.input.tool_input.file_path can fail if the payload is malformed or the property is missing. Additionally, Bun.spawn(...) at line 30 can throw on startup failures (e.g., oxlint not found) with no recovery path.
Proposed hardening patch
run: wrapRun(logger, async (context) => {
- const filePath = context.input.tool_input.file_path;
+ const filePath = context.input?.tool_input?.file_path;
+ if (typeof filePath !== "string" || filePath.length === 0) {
+ logger.debug("Skipping: missing file_path in PostToolUse payload");
+ return context.success({});
+ }
@@
- const proc = Bun.spawn(["oxlint", filePath], {
- stdout: "pipe",
- stderr: "pipe",
- });
+ let proc: ReturnType<typeof Bun.spawn>;
+ try {
+ proc = Bun.spawn(["oxlint", filePath], {
+ stdout: "pipe",
+ stderr: "pipe",
+ });
+ } catch (error) {
+ logger.error(`Failed to start oxlint for ${filePath}: ${String(error)}`);
+ return context.success({});
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/devkit/hooks/lint-on-edit.ts` around lines 21 - 33, Validate that
context.input?.tool_input?.file_path exists and is a string before assigning to
filePath (use the existing isTypeScriptFile(filePath) after that); if missing or
not a string, log via logger.debug and return context.success({}). Wrap the
Bun.spawn(["oxlint", filePath], ...) call in a try/catch to capture startup
failures (e.g., oxlint not found), log the error details with logger.error or
logger.debug and return context.success({}) (or an appropriate failure response)
instead of letting the exception bubble; also guard use of proc (ensure proc is
set) before accessing its stdout/stderr.
| { | ||
| "$schema": "https://raw.githubusercontent.com/nicolo-ribaudo/oxc/refs/heads/nicolo/json-schema/npm/oxlint/configuration_schema.json" | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What is the official oxlint configuration schema URL for .oxlintrc.json files? Is there a stable schema published by the oxc-project organization or oxlint npm package?
💡 Result:
The official oxlint configuration schema for .oxlintrc.json files is bundled in the oxlint npm package at the relative path ./node_modules/oxlint/configuration_schema.json. This is the recommended $schema URI shown in all official oxlint documentation examples across multiple pages (oxc.rs/docs and oxc-project.github.io/docs). No stable, public HTTP URL (e.g., from JSON Schema Store or a raw GitHub link) is published by the oxc-project organization or oxlint npm package. A GitHub issue (#11654) requests adding it to JSON Schema Store but is closed as "not planned". The schema is generated in the oxc-project/oxc repository (via PRs like #3375, #4174) and included in the published oxlint npm package for editor tooling support.
Citations:
- 1: https://oxc-project.github.io/docs/guide/usage/linter/config.html
- 2: https://oxc.rs/docs/guide/usage/linter/config.html
- 3: https://oxc.rs/docs/guide/usage/linter/config
- 4: https://oxc.rs/docs/guide/usage/linter/config-file-reference
- 5: https://oxc-project.github.io/docs/guide/usage/linter/config-file-reference.html
- 6: https://oxc.rs/docs/guide/usage/linter/config-file-reference.html
- 7: https://tessl.io/registry/tessl/npm-oxlint/1.16.0/docs/configuration.md
- 8: Provide a better experience for
.oxlintrc.jsonconfiguration files oxc-project/oxc#4146 - 9: linter: .oxlintrc.json configuration file improvements oxc-project/oxc#15247
- 10: feat(linter): start adding json schema for configuration file oxc-project/oxc#3375
- 11: feat(linter): generate schemas for rules (with documentation) oxc-project/oxc#4174
- 12: linter: add oxlint config schema to JSON Schema Store oxc-project/oxc#11654
🏁 Script executed:
#!/bin/bash
# Find and examine all .oxlintrc.json files mentioned in the review
cd "$(git rev-parse --show-toplevel)"
echo "=== Searching for all .oxlintrc.json files ==="
find . -name ".oxlintrc.json" -type f | head -20
echo ""
echo "=== Examining content of each .oxlintrc.json ==="
find . -name ".oxlintrc.json" -type f -exec sh -c 'echo "--- {} ---"; cat "$1"' _ {} \;Repository: masseater/claude-code-plugin
Length of output: 3073
Replace remote schema URL with local bundled path.
All 13 .oxlintrc.json files reference an unstable remote URL to a personal GitHub branch. According to official oxlint documentation, the $schema should instead reference the bundled schema: ./node_modules/oxlint/configuration_schema.json. Remote HTTP URLs are not supported by oxlint for schema resolution.
Change from:
{
"$schema": "https://raw.githubusercontent.com/nicolo-ribaudo/oxc/refs/heads/nicolo/json-schema/npm/oxlint/configuration_schema.json"
}To:
{
"$schema": "./node_modules/oxlint/configuration_schema.json"
}Apply this fix to all 13 files: .oxlintrc.json, packages/cc-plugin-lib/.oxlintrc.json, plugins/agnix/.oxlintrc.json, plugins/code-review/.oxlintrc.json, plugins/context/.oxlintrc.json, plugins/devkit/.oxlintrc.json, plugins/eslint-lsp/.oxlintrc.json, plugins/github-workflow/.oxlintrc.json, plugins/mutils/.oxlintrc.json, plugins/progress-tracker/.oxlintrc.json, plugins/research/.oxlintrc.json, plugins/sdd/.oxlintrc.json, plugins/ui-dev/.oxlintrc.json.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/progress-tracker/.oxlintrc.json` around lines 1 - 3, Replace the
remote HTTP schema URL in the "$schema" key with the local bundled schema path
(./node_modules/oxlint/configuration_schema.json) in every .oxlintrc.json file;
locate the "$schema" entry (e.g., in plugins/progress-tracker/.oxlintrc.json)
and update its value to the local path so oxlint can resolve the schema
correctly across all 13 .oxlintrc.json files.
b42a3b7 to
cac0798
Compare
97dc2ee to
496c50e
Compare
- Remove Biome completely from all packages including sdd-webapp - Add oxfmt for formatting across all plugins and packages - Rewrite CI to use turbo (no hardcoded paths, auto-discovers workspaces) - Pin Bun version in CI to 1.3.4 - Regenerate bun.lock for workspace Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Workspace package resolution requires lockfile regeneration on CI. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
typecheck needs ^build so cc-plugin-lib dist/ exists before plugins run tsgo --noEmit. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Caution Review failedAn error occurred during the review process. Please try again later. 📝 WalkthroughWalkthroughBatched metadata and infra changes: plugin version bumps in the marketplace, migration from Biome to OXLint across plugins, new shared Vitest package, expanded workspaces and Turbo integration, CI enhancements, new dev hooks and tests added to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
- Add allowImportingTsExtensions to devkit tsconfig - Fix hookSpecificOutput → decision/reason in standards-audit.ts - Add non-null assertions for noUncheckedIndexedAccess in test files - Fix type mismatches in runner.test.ts, format.ts, grep.ts Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
14 files in .claude/ were unintentionally removed from the tree during git object corruption repair. Restored from master. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Split CI into 4 independent parallel jobs: lint, typecheck, test, build - Add test scripts to sdd-webapp (13), mutils (32), devkit (184) - Total tests: 22 → 251 across 4 packages - Add --reporter=verbose to vitest for CI visibility - Fix findGitRoot test false positive Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
bun-types→bun(正しい @types/bun 参照)、tsgo exact pinTest plan
bunx turbo run check— oxlint 全13ワークスペース passbunx turbo run typecheck— 12/13 pass (devkit は既存67エラー)bunx turbo run test— 22テスト全 passcd packages/cc-plugin-lib && bun run knip— 未使用なしcd packages/cc-plugin-lib && bun run build— dist clean🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements
Documentation
Tests