chore(ci): refactor agents for codex compatibility#76
Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request consolidates Claude-specific hooks under a unified ChangesHook Migration and Consolidation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.gh.lumenfield.work/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 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 @.agents/hooks/notification.sh:
- Around line 1-4: The hook currently calls the macOS-only binary osascript in
.agents/hooks/notification.sh with set -euo pipefail, causing failures on
Linux/CI; update the script to guard execution by detecting the platform (e.g.,
check uname or OSTYPE for "Darwin") and verify osascript exists (command -v
osascript) before running the osascript command so on non-macOS the hook becomes
a safe no-op and does not exit non‑zero; keep the shebang and existing strict
flags but only invoke osascript inside that guarded branch.
In @.agents/hooks/post-tool-use-edit-write.sh:
- Around line 61-82: Run ESLint only once and derive the human-readable output
from the JSON rather than invoking eslint twice; replace the second invocation
that sets readable with a conversion of json_output (the variable set by the
pnpm exec eslint --format json call) into a readable format (e.g., using jq to
pretty-print messages or an eslint formatter library to transform the JSON into
classic CLI output), then reuse that readable value for both echoing and error
handling (references: json_output, readable, the pnpm exec eslint invocation).
In @.agents/hooks/pre-tool-use-edit-write.sh:
- Around line 26-48: The current guard only matches root filenames via BASENAME,
so directory entries like ".husky" and "config" aren’t catching edits under
those folders; update the loop that iterates PROTECTED_FILES to also treat
entries that represent directories as path prefixes by checking REL_PATH (or
FILE_PATH relative to PROJECT_ROOT) for a leading "PROTECTED/" segment in
addition to the existing BASENAME equality check (i.e. if PROTECTED is a
directory, block when REL_PATH starts with PROTECTED/); keep the existing
behavior for root files and use the existing symbols PROTECTED_FILES, PROTECTED,
BASENAME, DIRNAME, REL_PATH, FILE_PATH, and PROJECT_ROOT to locate and implement
this additional path-prefix check.
In @.agents/hooks/stop.sh:
- Around line 7-13: The IS_CODEX_STOP detection uses an overly broad jq
predicate that ORs has("turn_id"), has("stop_hook_active") and
has("last_assistant_message"); update the jq expression used to set
IS_CODEX_STOP to only check for the Codex-specific discriminant turn_id (i.e.,
replace the full OR condition with has("turn_id") only) so emit_success still
suppresses output for true Codex Stop events; change the jq invocation that
reads from INPUT (the line setting IS_CODEX_STOP) and leave emit_success and
other logic unchanged.
In @.codex/config.toml:
- Around line 1-6: The root-level notify = [...] block is not a supported Codex
hook and will be ignored; remove the notify key and its array or replace it with
a short TODO comment indicating notification hooks are unsupported yet. Ensure
any intended runtime hooks are defined using the supported schema (inline
[[hooks.EventName.hooks]] blocks with keys type/command/timeout/statusMessage or
the hooks.json file) and if you keep a placeholder, clearly mark it as
non-functional for future implementation.
In @.codex/hooks.json:
- Around line 9-58: The hook commands currently use root="$(git rev-parse
--show-toplevel)" and short-circuit with && if git fails; update each hook
command string that references root="$(git rev-parse --show-toplevel)" (e.g.,
the session-start.sh, pre-tool-use-bash.sh, pre-tool-use-edit-write.sh,
post-tool-use-edit-write.sh, stop.sh command entries) to compute a resilient
AGENTS_PROJECT_DIR/root using a fallback that silences git errors and falls back
to pwd, for example: root="${AGENTS_PROJECT_DIR:-${CLAUDE_PROJECT_DIR:-$(git
rev-parse --show-toplevel 2>/dev/null || pwd)}}" ; AGENTS_PROJECT_DIR="$root"
"$root/.agents/hooks/your-hook.sh" (replace your-hook.sh with the specific
script). Ensure you apply this change to all five command strings under
SessionStart, PreToolUse, PostToolUse and Stop.
In `@package.json`:
- Line 20: Replace the npx-based preinstall with a pinned devDependency: add
"only-allow" at a specific version into package.json's devDependencies (so it's
installed and reproducible) and change the "preinstall" script value from "npx
only-allow pnpm" to simply "only-allow pnpm" so the local binary from
node_modules is used; target the package.json entries "preinstall",
"devDependencies", and the "only-allow" package when making this change.
In `@projects/internals/tools/src/context/index.ts`:
- Line 25: The string "Elements provides a CLI and MCP server (`nve`) to help
you create, setup, and validate projects." uses "setup" incorrectly; change the
verb to "set up" so it reads "help you create, set up, and validate projects."
Locate the exact string in context/index.ts (the exported description text or
constant containing that sentence) and update the wording to follow the Google
developer documentation prose guidelines.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: a78af46c-41c6-48ae-84af-9268fd73722b
📒 Files selected for processing (32)
.agents/README.md.agents/hooks/lib/project-root.sh.agents/hooks/notification.sh.agents/hooks/post-tool-use-edit-write.sh.agents/hooks/pre-tool-use-bash.sh.agents/hooks/pre-tool-use-edit-write.sh.agents/hooks/session-start.sh.agents/hooks/stop.sh.agents/skills/accessibility/SKILL.md.agents/skills/api-design/SKILL.md.agents/skills/build-system/SKILL.md.agents/skills/code-review/SKILL.md.agents/skills/component-creation/SKILL.md.agents/skills/documentation/SKILL.md.agents/skills/examples/SKILL.md.agents/skills/pattern-creation/SKILL.md.agents/skills/testing/SKILL.md.agents/skills/troubleshooting/SKILL.md.agents/skills/typescript/SKILL.md.claude/hooks/format-prettier.sh.claude/hooks/guard-critical-files.sh.claude/hooks/guard-package-manager.sh.claude/hooks/lint-commitlint.sh.claude/hooks/lint-eslint.sh.claude/hooks/lint-stylelint.sh.claude/hooks/lint-vale.sh.claude/hooks/worktree-create.sh.claude/settings.json.codex/config.toml.codex/hooks.jsonpackage.jsonprojects/internals/tools/src/context/index.ts
💤 Files with no reviewable changes (8)
- .claude/hooks/format-prettier.sh
- .claude/hooks/lint-eslint.sh
- .claude/hooks/lint-stylelint.sh
- .claude/hooks/guard-package-manager.sh
- .claude/hooks/worktree-create.sh
- .claude/hooks/lint-commitlint.sh
- .claude/hooks/guard-critical-files.sh
- .claude/hooks/lint-vale.sh
Comment on lines
+61
to
+82
| json_output=$(cd "$project_dir" && pnpm exec eslint -c ./eslint.config.js --no-warn-ignored --cache --cache-location .eslintcache/ --format json "$rel_path" 2>/dev/null) || true | ||
|
|
||
| hard_errors=$(echo "$json_output" | jq -r --arg soft "$soft_rules" ' | ||
| [.[].messages[] | select(.severity == 2) | select(.ruleId | test($soft) | not)] | length | ||
| ') 2>/dev/null || hard_errors="0" | ||
|
|
||
| total_errors=$(echo "$json_output" | jq -r ' | ||
| [.[].messages[] | select(.severity == 2)] | length | ||
| ') 2>/dev/null || total_errors="0" | ||
|
|
||
| if [[ "$total_errors" == "0" ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| readable=$(cd "$project_dir" && pnpm exec eslint -c ./eslint.config.js --no-warn-ignored --color --cache --cache-location .eslintcache/ "$rel_path" 2>&1) || true | ||
|
|
||
| if [[ "$hard_errors" != "0" ]]; then | ||
| echo "$readable" >&2 | ||
| mark_failed | ||
| else | ||
| echo "$readable" >&2 | ||
| fi |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
ESLint is invoked twice per file with errors — consider capturing the readable output in a single run.
ESLint runs first in --format json (for programmatic error classification) and again in the default readable format (for display). With --cache, the second run is fast, but it still adds fork + analysis overhead per edited file. An alternative is to post-process the cached JSON output into readable text via jq or eslint-formatter-*, eliminating the second process.
🤖 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 @.agents/hooks/post-tool-use-edit-write.sh around lines 61 - 82, Run ESLint
only once and derive the human-readable output from the JSON rather than
invoking eslint twice; replace the second invocation that sets readable with a
conversion of json_output (the variable set by the pnpm exec eslint --format
json call) into a readable format (e.g., using jq to pretty-print messages or an
eslint formatter library to transform the JSON into classic CLI output), then
reuse that readable value for both echoing and error handling (references:
json_output, readable, the pnpm exec eslint invocation).
coryrylan
force-pushed
the
topic-codex-fixes
branch
3 times, most recently
from
8e73cb0 to
57d5be4
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
projects/internals/tools/src/context/index.ts (1)
88-88:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse "set up" (two words) as a verb.
"setup" is a noun/adjective; the verb form is "set up". Since this description is surfaced in agent prompts, it should be grammatically correct.
✏️ Proposed fix
- description: 'Instructions for ensuring the Elements Design System is setup correctly', + description: 'Instructions for ensuring the Elements Design System is set up correctly',🤖 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 `@projects/internals/tools/src/context/index.ts` at line 88, Update the description string for the context object so the verb is "set up" (two words) instead of "setup"; locate the object with the description property in projects/internals/tools/src/context/index.ts (the description field shown in the diff) and change the phrasing to use "set up" to ensure correct verb usage in agent prompts..agents/hooks/pre-tool-use-bash.sh (1)
7-11:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail-closed behavior is required for command parsing errors.
At Line 7,
|| truesuppresses parse failures; then Line 10 treats emptyCOMMANDas safe and exits. A malformed/changed hook payload would bypass destructive git blocking entirely.🔧 Suggested fix
-COMMAND=$(hook_command_from_input "$INPUT" || true) +if ! COMMAND=$(hook_command_from_input "$INPUT"); then + echo "BLOCKED: Unable to parse bash tool input safely." >&2 + exit 2 +fi🤖 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 @.agents/hooks/pre-tool-use-bash.sh around lines 7 - 11, The current parsing silences failures by using "|| true" when calling hook_command_from_input, letting malformed input fall through; change the logic so hook_command_from_input errors are not suppressed: capture its exit status and if it fails, log or emit a clear error and exit non‑zero (e.g., exit 1) instead of continuing; then only allow early exit 0 when the command was successfully parsed and confirmed to start with "git" (keep the existing grep check on COMMAND), otherwise abort with non‑zero to enforce fail‑closed behavior (refer to hook_command_from_input and the COMMAND variable).
♻️ Duplicate comments (6)
projects/internals/tools/src/context/index.ts (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse "set up" (two words) as a verb.
"setup" is a noun/adjective; the verb form is "set up". Since this string is surfaced directly in agent prompts, the phrasing should be grammatically correct.
✏️ Proposed fix
-Elements provides a CLI and MCP server (`nve`) to help you create, setup, and validate projects. +Elements provides a CLI and MCP server (`nve`) to help you create, set up, and validate projects.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@projects/internals/tools/src/context/index.ts` at line 25, The string literal "Elements provides a CLI and MCP server (`nve`) to help you create, setup, and validate projects." uses the noun "setup" where the verb phrase "set up" is required; locate that string (e.g., in the exported context/description in index.ts) and replace "setup" with "set up" so it reads "...to help you create, set up, and validate projects." Ensure the updated text is used wherever the same prompt/string is surfaced..codex/config.toml (1)
1-6:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRemove the non-functional
notifyblock from Codex config.This block is explicitly marked unsupported and won’t execute, so it adds dead configuration and ambiguity.
Codex config.toml hook schema: is a root-level `notify` key supported, or should hooks only be configured via `hooks.*` entries / hooks.json?🤖 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 @.codex/config.toml around lines 1 - 6, Remove the dead "notify" config block (the top-level notify key and its array) from the Codex configuration so unsupported hooks are not present; locate the notify key and its array entries in the .codex configuration (the notify = [...] block shown in the diff) and delete the entire block, ensuring hooks are configured only via the supported hooks schema (e.g., hooks.* or hooks.json) elsewhere..agents/hooks/stop.sh (1)
7-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winNarrow Codex Stop detection to
turn_idonly.The current predicate is overly broad; it can classify non-Codex payloads as Codex Stop and suppress expected success output.
🔧 Suggested fix
-IS_CODEX_STOP=$(jq -r 'if (.hook_event_name == "Stop" and (has("turn_id") or has("stop_hook_active") or has("last_assistant_message"))) then "1" else "0" end' <<<"$INPUT" 2>/dev/null || echo "0") +IS_CODEX_STOP=$(jq -r 'if (.hook_event_name == "Stop" and has("turn_id")) then "1" else "0" end' <<<"$INPUT" 2>/dev/null || echo "0")🤖 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 @.agents/hooks/stop.sh at line 7, The IS_CODEX_STOP detection currently treats any Stop event with turn_id OR stop_hook_active OR last_assistant_message as Codex Stop; narrow it to only consider presence of turn_id: update the jq predicate used to set IS_CODEX_STOP so it returns "1" only when .hook_event_name == "Stop" and has("turn_id"), and return "0" otherwise, keeping usage of INPUT and preserving the 2>/dev/null fallback behavior and echo "0" fallback..codex/hooks.json (1)
9-9:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
git rev-parsestill lacks2>/dev/null || pwdfallback — hook commands silently skip when git is unavailable.The previous review flagged this across all five hook command strings, and it was marked as addressed, but the current code retains the original pattern. If
git rev-parse --show-toplevelexits non-zero (e.g., in a Codex sandbox that strips git tooling), the&&short-circuits and the hook script is never invoked at all — theproject-root.shlibrary's fallback chain never gets a chance to run.🛡️ Proposed fix (apply to all five command strings)
-"command": "root=\"$(git rev-parse --show-toplevel)\" && AGENTS_PROJECT_DIR=\"$root\" \"$root/.agents/hooks/session-start.sh\"", +"command": "root=\"$(git rev-parse --show-toplevel 2>/dev/null || pwd)\" && AGENTS_PROJECT_DIR=\"$root\" \"$root/.agents/hooks/session-start.sh\"",Also applies to: 22-22, 33-33, 46-46, 58-58
🤖 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 @.codex/hooks.json at line 9, The hook command strings use root="$(git rev-parse --show-toplevel)" && ... which short-circuits when git is unavailable; change each command to assign root using the git command with a fallback (git rev-parse --show-toplevel 2>/dev/null || pwd) so the subsequent call to the hook script (the existing session-start.sh, session-end.sh, etc. invocations in .codex/hooks.json) always runs; update all five command entries that currently use root="$(git rev-parse --show-toplevel)" to the fallback form so project-root.sh’s fallback chain can execute..agents/hooks/pre-tool-use-edit-write.sh (1)
41-49:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDirectory entries
.huskyandconfiginPROTECTED_FILESstill don't protect their contents.This was flagged in the previous review and remains unresolved. Editing a file like
.husky/pre-commityieldsBASENAME=pre-commit,DIRNAME=.husky— neither matches.husky, so the guard never fires.🤖 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 @.agents/hooks/pre-tool-use-edit-write.sh around lines 41 - 49, The current protection check in the for-loop over PROTECTED_FILES only compares BASENAME and exact FILE_PATH, so directory entries like ".husky" or "config" don't match files inside them; update the conditional in the loop that checks PROTECTED (the block using BASENAME, DIRNAME, FILE_PATH, PROJECT_ROOT) to also detect when PROTECTED is a directory by testing if DIRNAME == "$PROTECTED" or if FILE_PATH is under that directory (e.g. FILE_PATH starts with "$PROJECT_ROOT/$PROTECTED/" ), and if so treat it as protected and exit 2 with the same error messages..agents/hooks/post-tool-use-edit-write.sh (1)
61-82: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winESLint is still invoked twice per file — the JSON output on Line 61 is sufficient to derive the human-readable form.
This was flagged in the previous review and remains unresolved. The second invocation (Line 75) adds a fork + analysis round-trip for every edited file that has errors. The JSON already contains
message,line,column,ruleId, andseverity; piping$json_outputthroughjqoreslint-formatter-codeframeeliminates the redundant process.🤖 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 @.agents/hooks/post-tool-use-edit-write.sh around lines 61 - 82, The script currently runs pnpm exec eslint twice (producing json_output then readable); instead, derive the human-readable report from the existing json_output (and soft_rules/hard_errors logic) to avoid the second fork: parse $json_output with jq or pipe it to an eslint formatter like eslint-formatter-codeframe to produce the same colored/text output, assign that to the readable variable (replacing the second pnpm exec call), and preserve the current branching that echoes readable to stderr and calls mark_failed when hard_errors != "0".
🤖 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 @.codex/hooks.json:
- Line 5: The SessionStart hook's matcher currently only matches "startup" so
resume events are skipped; update the matcher in .codex/hooks.json for the
SessionStart hook (the "matcher" property for SessionStart / session-start.sh)
to include both startup and resume (for example "startup|resume" or an
equivalent regex) so session-start.sh runs on both new sessions and resumed
sessions; ensure the regex covers whole-match behavior if needed (e.g.,
"^startup|resume$" or the documented "startup|resume") and keep the rest of the
hook configuration unchanged.
In `@package.json`:
- Around line 10-16: The devEngines.packageManager.onFail value "download" is
invalid for Corepack and won't auto-download pnpm 11.0.8; update
devEngines.packageManager.onFail to "error" so Corepack throws on a package
manager mismatch (preserving the intended enforcement behavior) — locate the
devEngines.packageManager.onFail entry in package.json and replace "download"
with "error".
In `@pnpm-workspace.yaml`:
- Around line 119-129: The project’s pnpm v11 will ignore the non-auth entries
in .npmrc (engine-strict and node-options), so add them into pnpm-workspace.yaml
instead: create a top-level config mapping and set engine-strict: true and
node-options: "--max-old-space-size=8192" (or alternatively put them in
~/.config/pnpm/config.yaml); leave registry and scoped registry entries in
.npmrc. Locate pnpm-workspace.yaml (it already contains allowBuilds) and add the
new config keys there using the exact names engine-strict and node-options.
---
Outside diff comments:
In @.agents/hooks/pre-tool-use-bash.sh:
- Around line 7-11: The current parsing silences failures by using "|| true"
when calling hook_command_from_input, letting malformed input fall through;
change the logic so hook_command_from_input errors are not suppressed: capture
its exit status and if it fails, log or emit a clear error and exit non‑zero
(e.g., exit 1) instead of continuing; then only allow early exit 0 when the
command was successfully parsed and confirmed to start with "git" (keep the
existing grep check on COMMAND), otherwise abort with non‑zero to enforce
fail‑closed behavior (refer to hook_command_from_input and the COMMAND
variable).
In `@projects/internals/tools/src/context/index.ts`:
- Line 88: Update the description string for the context object so the verb is
"set up" (two words) instead of "setup"; locate the object with the description
property in projects/internals/tools/src/context/index.ts (the description field
shown in the diff) and change the phrasing to use "set up" to ensure correct
verb usage in agent prompts.
---
Duplicate comments:
In @.agents/hooks/post-tool-use-edit-write.sh:
- Around line 61-82: The script currently runs pnpm exec eslint twice (producing
json_output then readable); instead, derive the human-readable report from the
existing json_output (and soft_rules/hard_errors logic) to avoid the second
fork: parse $json_output with jq or pipe it to an eslint formatter like
eslint-formatter-codeframe to produce the same colored/text output, assign that
to the readable variable (replacing the second pnpm exec call), and preserve the
current branching that echoes readable to stderr and calls mark_failed when
hard_errors != "0".
In @.agents/hooks/pre-tool-use-edit-write.sh:
- Around line 41-49: The current protection check in the for-loop over
PROTECTED_FILES only compares BASENAME and exact FILE_PATH, so directory entries
like ".husky" or "config" don't match files inside them; update the conditional
in the loop that checks PROTECTED (the block using BASENAME, DIRNAME, FILE_PATH,
PROJECT_ROOT) to also detect when PROTECTED is a directory by testing if DIRNAME
== "$PROTECTED" or if FILE_PATH is under that directory (e.g. FILE_PATH starts
with "$PROJECT_ROOT/$PROTECTED/" ), and if so treat it as protected and exit 2
with the same error messages.
In @.agents/hooks/stop.sh:
- Line 7: The IS_CODEX_STOP detection currently treats any Stop event with
turn_id OR stop_hook_active OR last_assistant_message as Codex Stop; narrow it
to only consider presence of turn_id: update the jq predicate used to set
IS_CODEX_STOP so it returns "1" only when .hook_event_name == "Stop" and
has("turn_id"), and return "0" otherwise, keeping usage of INPUT and preserving
the 2>/dev/null fallback behavior and echo "0" fallback.
In @.codex/config.toml:
- Around line 1-6: Remove the dead "notify" config block (the top-level notify
key and its array) from the Codex configuration so unsupported hooks are not
present; locate the notify key and its array entries in the .codex configuration
(the notify = [...] block shown in the diff) and delete the entire block,
ensuring hooks are configured only via the supported hooks schema (e.g., hooks.*
or hooks.json) elsewhere.
In @.codex/hooks.json:
- Line 9: The hook command strings use root="$(git rev-parse --show-toplevel)"
&& ... which short-circuits when git is unavailable; change each command to
assign root using the git command with a fallback (git rev-parse --show-toplevel
2>/dev/null || pwd) so the subsequent call to the hook script (the existing
session-start.sh, session-end.sh, etc. invocations in .codex/hooks.json) always
runs; update all five command entries that currently use root="$(git rev-parse
--show-toplevel)" to the fallback form so project-root.sh’s fallback chain can
execute.
In `@projects/internals/tools/src/context/index.ts`:
- Line 25: The string literal "Elements provides a CLI and MCP server (`nve`) to
help you create, setup, and validate projects." uses the noun "setup" where the
verb phrase "set up" is required; locate that string (e.g., in the exported
context/description in index.ts) and replace "setup" with "set up" so it reads
"...to help you create, set up, and validate projects." Ensure the updated text
is used wherever the same prompt/string is surfaced.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c0758caf-2633-4332-9486-ac5b5e608b9b
📒 Files selected for processing (39)
.agents/README.md.agents/hooks/lib/project-root.sh.agents/hooks/notification.sh.agents/hooks/post-tool-use-edit-write.sh.agents/hooks/pre-tool-use-bash.sh.agents/hooks/pre-tool-use-edit-write.sh.agents/hooks/session-start.sh.agents/hooks/stop.sh.agents/skills/accessibility/SKILL.md.agents/skills/api-design/SKILL.md.agents/skills/build-system/SKILL.md.agents/skills/code-review/SKILL.md.agents/skills/component-creation/SKILL.md.agents/skills/documentation/SKILL.md.agents/skills/examples/SKILL.md.agents/skills/pattern-creation/SKILL.md.agents/skills/testing/SKILL.md.agents/skills/troubleshooting/SKILL.md.agents/skills/typescript/SKILL.md.claude/hooks/format-prettier.sh.claude/hooks/guard-critical-files.sh.claude/hooks/guard-package-manager.sh.claude/hooks/lint-commitlint.sh.claude/hooks/lint-eslint.sh.claude/hooks/lint-stylelint.sh.claude/hooks/lint-vale.sh.claude/hooks/worktree-create.sh.claude/settings.json.codex/config.toml.codex/hooks.json.github/workflows/ci.yml.husky/commit-msgAGENTS.mdREADME.mdconfig/docker/ci.dockerfilepackage.jsonpnpm-workspace.yamlprojects/internals/tools/src/context/index.tsprojects/site/src/docs/internal/guidelines/troubleshooting.md
💤 Files with no reviewable changes (8)
- .claude/hooks/lint-eslint.sh
- .claude/hooks/lint-vale.sh
- .claude/hooks/guard-critical-files.sh
- .claude/hooks/lint-stylelint.sh
- .claude/hooks/lint-commitlint.sh
- .claude/hooks/format-prettier.sh
- .claude/hooks/worktree-create.sh
- .claude/hooks/guard-package-manager.sh
Comment on lines
+119
to
+129
| allowBuilds: | ||
| '@bundled-es-modules/glob': true | ||
| '@parcel/watcher': true | ||
| bun: true | ||
| esbuild: true | ||
| hugo-bin: true | ||
| lmdb: true | ||
| msgpackr-extract: true | ||
| rs-module-lexer: true | ||
| sharp: true | ||
| style-dictionary: true |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check .npmrc for any non-auth settings that must be migrated to pnpm-workspace.yaml
echo "=== .npmrc contents (if present) ==="
if [ -f .npmrc ]; then
cat .npmrc
else
echo ".npmrc not found"
fi
echo ""
echo "=== Looking for pnpm field in workspace package.json files (removed in v11) ==="
# pnpm v11 no longer reads config from the `pnpm` field in package.json
rg -l '"pnpm"\s*:' --type=json --include='package.json' -g '!**/node_modules/**' || echo "No package.json files with a 'pnpm' config field found"Repository: NVIDIA/elements
Length of output: 826
Migrate non-auth settings from .npmrc to pnpm-workspace.yaml for pnpm v11 compatibility.
The .npmrc file contains pnpm-specific configuration that pnpm v11 no longer reads:
engine-strict=truenode-options=--max-old-space-size=8192
These settings will be silently dropped. Move them to pnpm-workspace.yaml or ~/.config/pnpm/config.yaml. Keep registry and scoped registry settings in .npmrc (those remain supported).
🤖 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 `@pnpm-workspace.yaml` around lines 119 - 129, The project’s pnpm v11 will
ignore the non-auth entries in .npmrc (engine-strict and node-options), so add
them into pnpm-workspace.yaml instead: create a top-level config mapping and set
engine-strict: true and node-options: "--max-old-space-size=8192" (or
alternatively put them in ~/.config/pnpm/config.yaml); leave registry and scoped
registry entries in .npmrc. Locate pnpm-workspace.yaml (it already contains
allowBuilds) and add the new config keys there using the exact names
engine-strict and node-options.
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
projects/internals/tools/src/context/index.ts (1)
88-88:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse "set up" (two words) as a verb.
The description uses "is setup correctly" but the verb form should be two words: "is set up correctly".
✏️ Proposed fix
- description: 'Instructions for ensuring the Elements Design System is setup correctly', + description: 'Instructions for ensuring the Elements Design System is set up correctly',As per coding guidelines: TypeScript files must follow the Google developer documentation style guide for prose.
🤖 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 `@projects/internals/tools/src/context/index.ts` at line 88, The description string for the Elements Design System uses the incorrect verb form "is setup correctly"; update the value of the description property (the description key in this module) to use "is set up correctly" instead so the prose follows the Google developer documentation style guide.
♻️ Duplicate comments (2)
projects/internals/tools/src/context/index.ts (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPast review issue remains unresolved: "setup" should be "set up" (verb form).
A previous review flagged that "setup" (noun/adjective) should be "set up" (two words) when used as a verb. This issue is still present in the current code.
✏️ Proposed fix
-Elements provides a CLI and MCP server (\`nve\`) to help you create, setup, and validate projects. +Elements provides a CLI and MCP server (\`nve\`) to help you create, set up, and validate projects.As per coding guidelines: TypeScript files must follow the Google developer documentation style guide for prose.
🤖 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 `@projects/internals/tools/src/context/index.ts` at line 25, The sentence in the module description uses the noun "setup" where the verb form "set up" is required; update the string "Elements provides a CLI and MCP server (`nve`) to help you create, setup, and validate projects." to use "set up" instead of "setup" so it reads "...to help you create, set up, and validate projects." Make this change in the text constant or export where that exact sentence appears in projects/internals/tools/src/context/index.ts.package.json (1)
10-16:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
"onFail": "download"is not a recognized Corepack value.Corepack recognizes
ignore,error(or unset — throws on mismatch), andwarnor any other value (prints a warning)."download"falls under "some other value" and will only print a warning on mismatch, not enforce the pnpm version. If the intent is to match the strict enforcement previously provided by thepackageManagerroot field, change to"error".🛡️ Proposed fix
"devEngines": { "packageManager": { "name": "pnpm", "version": "11.0.8", - "onFail": "download" + "onFail": "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 `@package.json` around lines 10 - 16, The devEngines.packageManager.onFail value "download" is not a valid Corepack enforcement option and will only produce a warning; update devEngines.packageManager.onFail from "download" to "error" so Corepack strictly enforces the pnpm version (modify the devEngines -> packageManager -> onFail field).
🤖 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 @.agents/hooks/post-tool-use-edit-write.sh:
- Around line 25-28: The current conditional in the post-tool-use hook only
marks failure when both exit_code is non-zero and output is non-empty, allowing
tools to fail silently; update the condition in the blocks used by run_prettier,
run_vale, and run_stylelint so that any non-zero ${exit_code:-0} triggers
mark_failed (i.e., drop the -n "$output" requirement), but still only echo
"$output" when it is non-empty; locate the failing blocks that reference
exit_code and output and change the logic to call mark_failed whenever exit_code
!= 0 and separately guard the echo with a non-empty output test.
- Around line 61-65: The ESLint invocation and jq parsing are suppressing real
errors and failing on null ruleId; stop swallowing failures from the pnpm exec
eslint call (remove the unconditional 2>/dev/null || true and capture its exit
status), and make the jq expressions null-safe by coalescing ruleId to an empty
string before testing (e.g. use (.ruleId // "") | test($soft) to avoid "null
cannot be matched"); do the same null-safe change for both hard_errors and
total_errors calculations and ensure you default the shell variables to "0" if
jq fails so the hook accurately reflects ESLint exit status.
In @.github/workflows/ci.yml:
- Around line 18-22: Remove the unnecessary standalone: true setting from every
pnpm/action-setup@v4 invocation in the CI workflow (the shown block and the
similar blocks in the "lighthouse" and "release" jobs); edit the pnpm setup
steps (the uses: pnpm/action-setup@v4 steps) to omit the standalone key while
keeping version, run_install, and standalone-related behavior unchanged so pnpm
uses the Node runtime provided by actions/setup-node@v6.
In `@AGENTS.md`:
- Line 29: The setup step uses an unpinned corepack activation command
("corepack prepare --activate") which can install a pnpm version different from
the declared pnpm 11.0.8; update the activation command to explicitly pin the
version (use "corepack prepare pnpm@11.0.8 --activate" or equivalent) wherever
the unpinned "corepack prepare --activate" appears (the same change should be
applied to the repeated occurrence around the setup steps that reference pnpm),
so the declared pnpm 11.0.8 is guaranteed to be activated.
---
Outside diff comments:
In `@projects/internals/tools/src/context/index.ts`:
- Line 88: The description string for the Elements Design System uses the
incorrect verb form "is setup correctly"; update the value of the description
property (the description key in this module) to use "is set up correctly"
instead so the prose follows the Google developer documentation style guide.
---
Duplicate comments:
In `@package.json`:
- Around line 10-16: The devEngines.packageManager.onFail value "download" is
not a valid Corepack enforcement option and will only produce a warning; update
devEngines.packageManager.onFail from "download" to "error" so Corepack strictly
enforces the pnpm version (modify the devEngines -> packageManager -> onFail
field).
In `@projects/internals/tools/src/context/index.ts`:
- Line 25: The sentence in the module description uses the noun "setup" where
the verb form "set up" is required; update the string "Elements provides a CLI
and MCP server (`nve`) to help you create, setup, and validate projects." to use
"set up" instead of "setup" so it reads "...to help you create, set up, and
validate projects." Make this change in the text constant or export where that
exact sentence appears in projects/internals/tools/src/context/index.ts.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: f9ccab67-a14a-4fb2-99da-012eff093394
📒 Files selected for processing (39)
.agents/README.md.agents/hooks/lib/project-root.sh.agents/hooks/notification.sh.agents/hooks/post-tool-use-edit-write.sh.agents/hooks/pre-tool-use-bash.sh.agents/hooks/pre-tool-use-edit-write.sh.agents/hooks/session-start.sh.agents/hooks/stop.sh.agents/skills/accessibility/SKILL.md.agents/skills/api-design/SKILL.md.agents/skills/build-system/SKILL.md.agents/skills/code-review/SKILL.md.agents/skills/component-creation/SKILL.md.agents/skills/documentation/SKILL.md.agents/skills/examples/SKILL.md.agents/skills/pattern-creation/SKILL.md.agents/skills/testing/SKILL.md.agents/skills/troubleshooting/SKILL.md.agents/skills/typescript/SKILL.md.claude/hooks/format-prettier.sh.claude/hooks/guard-critical-files.sh.claude/hooks/guard-package-manager.sh.claude/hooks/lint-commitlint.sh.claude/hooks/lint-eslint.sh.claude/hooks/lint-stylelint.sh.claude/hooks/lint-vale.sh.claude/hooks/worktree-create.sh.claude/settings.json.codex/config.toml.codex/hooks.json.github/workflows/ci.yml.husky/commit-msgAGENTS.mdREADME.mdconfig/docker/ci.dockerfilepackage.jsonpnpm-workspace.yamlprojects/internals/tools/src/context/index.tsprojects/site/src/docs/internal/guidelines/troubleshooting.md
💤 Files with no reviewable changes (8)
- .claude/hooks/format-prettier.sh
- .claude/hooks/lint-commitlint.sh
- .claude/hooks/lint-eslint.sh
- .claude/hooks/guard-package-manager.sh
- .claude/hooks/worktree-create.sh
- .claude/hooks/lint-vale.sh
- .claude/hooks/guard-critical-files.sh
- .claude/hooks/lint-stylelint.sh
Comment on lines
+25
to
+28
| if [[ ${exit_code:-0} -ne 0 && -n "$output" ]]; then | ||
| echo "$output" >&2 | ||
| mark_failed | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n .agents/hooks/post-tool-use-edit-write.sh | head -150Repository: NVIDIA/elements
Length of output: 4862
Tool failures should fail even when output is empty.
Lines 25, 103, and 140 require both a non-zero exit code AND non-empty output to mark failure. A non-zero exit with empty output currently passes. This affects run_prettier, run_vale, and run_stylelint functions.
Suggested fix
- if [[ ${exit_code:-0} -ne 0 && -n "$output" ]]; then
- echo "$output" >&2
+ if [[ ${exit_code:-0} -ne 0 ]]; then
+ [[ -n "$output" ]] && echo "$output" >&2
mark_failed
fiAlso applies to: 103-106, 140-143
🤖 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 @.agents/hooks/post-tool-use-edit-write.sh around lines 25 - 28, The current
conditional in the post-tool-use hook only marks failure when both exit_code is
non-zero and output is non-empty, allowing tools to fail silently; update the
condition in the blocks used by run_prettier, run_vale, and run_stylelint so
that any non-zero ${exit_code:-0} triggers mark_failed (i.e., drop the -n
"$output" requirement), but still only echo "$output" when it is non-empty;
locate the failing blocks that reference exit_code and output and change the
logic to call mark_failed whenever exit_code != 0 and separately guard the echo
with a non-empty output test.
Comment on lines
+61
to
+65
| json_output=$(cd "$project_dir" && pnpm exec eslint -c ./eslint.config.js --no-warn-ignored --cache --cache-location .eslintcache/ --format json "$rel_path" 2>/dev/null) || true | ||
|
|
||
| hard_errors=$(echo "$json_output" | jq -r --arg soft "$soft_rules" ' | ||
| [.[].messages[] | select(.severity == 2) | select(.ruleId | test($soft) | not)] | length | ||
| ') 2>/dev/null || hard_errors="0" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the file
find . -name "post-tool-use-edit-write.sh" -type fRepository: NVIDIA/elements
Length of output: 103
🏁 Script executed:
# Examine the relevant section of the script (lines 50-80)
cat -n .agents/hooks/post-tool-use-edit-write.sh | sed -n '50,80p'Repository: NVIDIA/elements
Length of output: 1330
🏁 Script executed:
# Check if ruleId can be null in ESLint JSON output and test the jq expression
# First, let's see what an ESLint JSON output looks like and test jq behavior
python3 << 'EOF'
import json
import subprocess
# Create a sample ESLint JSON output with a null ruleId (which can happen for parsing errors)
sample_output = [
{
"filePath": "test.js",
"messages": [
{
"ruleId": None, # null ruleId from parsing error
"severity": 2,
"message": "Parsing error"
},
{
"ruleId": "no-unused-vars",
"severity": 2,
"message": "Variable unused"
}
]
}
]
# Test the original jq expression with null ruleId
json_str = json.dumps(sample_output)
print("Sample ESLint output with null ruleId:")
print(json_str)
print("\n" + "="*60 + "\n")
# Test original expression: [.[].messages[] | select(.severity == 2) | select(.ruleId | test($soft) | not)] | length
test_expr = r'[.[].messages[] | select(.severity == 2) | select(.ruleId | test($soft) | not)] | length'
try:
result = subprocess.run(['jq', '-r', '--arg', 'soft', 'some_pattern', test_expr],
input=json_str, text=True, capture_output=True)
print(f"Original expression result:\nStdout: {result.stdout}\nStderr: {result.stderr}\nReturn code: {result.returncode}")
except Exception as e:
print(f"Error running jq: {e}")
print("\n" + "="*60 + "\n")
# Test fixed expression: [.[].messages[] | select(.severity == 2) | select(((.ruleId // "") | test($soft)) | not)] | length
fixed_expr = r'[.[].messages[] | select(.severity == 2) | select(((.ruleId // "") | test($soft)) | not)] | length'
try:
result = subprocess.run(['jq', '-r', '--arg', 'soft', 'some_pattern', fixed_expr],
input=json_str, text=True, capture_output=True)
print(f"Fixed expression result:\nStdout: {result.stdout}\nStderr: {result.stderr}\nReturn code: {result.returncode}")
except Exception as e:
print(f"Error running jq: {e}")
EOFRepository: NVIDIA/elements
Length of output: 600
🏁 Script executed:
# Check the full context around lines 61-73 to understand the flow
cat -n .agents/hooks/post-tool-use-edit-write.sh | sed -n '55,75p'Repository: NVIDIA/elements
Length of output: 1090
🏁 Script executed:
# Look for similar patterns with other formatters/linters to see consistency
grep -n "2>/dev/null\||| true" .agents/hooks/post-tool-use-edit-write.shRepository: NVIDIA/elements
Length of output: 488
🏁 Script executed:
# Check the error handling pattern for other tools (Prettier, Vale, Stylelint)
cat -n .agents/hooks/post-tool-use-edit-write.sh | sed -n '20,30p'
cat -n .agents/hooks/post-tool-use-edit-write.sh | sed -n '100,110p'
cat -n .agents/hooks/post-tool-use-edit-write.sh | sed -n '135,145p'Repository: NVIDIA/elements
Length of output: 1136
Fix jq null-safety and error suppression in ESLint linting path.
Line 61 suppresses all errors with 2>/dev/null || true, and line 64's jq expression fails when ruleId is null (confirmed: null cannot be matched, as it is not a string). The fallback || hard_errors="0" masks this failure, causing the hook to incorrectly return success. This is inconsistent with Prettier, Vale, and Stylelint patterns in the same file, which explicitly check exit codes.
Suggested fix
- json_output=$(cd "$project_dir" && pnpm exec eslint -c ./eslint.config.js --no-warn-ignored --cache --cache-location .eslintcache/ --format json "$rel_path" 2>/dev/null) || true
+ local eslint_exit=0
+ json_output=$(cd "$project_dir" && pnpm exec eslint -c ./eslint.config.js --no-warn-ignored --cache --cache-location .eslintcache/ --format json "$rel_path" 2>&1) || eslint_exit=$?
+ if [[ $eslint_exit -eq 2 || -z "$json_output" ]]; then
+ echo "$json_output" >&2
+ mark_failed
+ return 0
+ fi
- [.[].messages[] | select(.severity == 2) | select(.ruleId | test($soft) | not)] | length
+ [.[].messages[] | select(.severity == 2) | select(((.ruleId // "") | test($soft)) | not)] | lengthAlso applies to: Line 67 (same null-handling issue in total_errors jq expression).
🤖 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 @.agents/hooks/post-tool-use-edit-write.sh around lines 61 - 65, The ESLint
invocation and jq parsing are suppressing real errors and failing on null
ruleId; stop swallowing failures from the pnpm exec eslint call (remove the
unconditional 2>/dev/null || true and capture its exit status), and make the jq
expressions null-safe by coalescing ruleId to an empty string before testing
(e.g. use (.ruleId // "") | test($soft) to avoid "null cannot be matched"); do
the same null-safe change for both hard_errors and total_errors calculations and
ensure you default the shell variables to "0" if jq fails so the hook accurately
reflects ESLint exit status.
57d5be4 to
62cba30
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (8)
projects/internals/tools/src/context/index.ts (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse "set up" (two words) as a verb.
The verb form is "set up", not "setup". Since this string appears in agent prompts, it should follow grammatical conventions.
✏️ Proposed fix
-Elements provides a CLI and MCP server (`nve`) to help you create, setup, and validate projects. +Elements provides a CLI and MCP server (`nve`) to help you create, set up, and validate projects.As per coding guidelines: follow the Google developer documentation style guide for prose in TypeScript files.
🤖 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 `@projects/internals/tools/src/context/index.ts` at line 25, The prompt string "Elements provides a CLI and MCP server (`nve`) to help you create, setup, and validate projects." uses the noun "setup" instead of the verb phrase "set up"; update that string literal to "set up" so it reads "Elements provides a CLI and MCP server (`nve`) to help you create, set up, and validate projects." Ensure you modify the exact string occurrence in projects/internals/tools/src/context/index.ts (the exported prompt/description string) so agent prompts follow the Google style guide..agents/hooks/post-tool-use-edit-write.sh (3)
75-82: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick winESLint is invoked twice per file — second run is redundant.
Line 61 runs ESLint with
--format jsonto classify errors programmatically. Line 75 re-runs ESLint without a format flag to produce human-readable output. Even with--cache, this doubles fork/analysis overhead per edited file. The human-readable representation can be derived from$json_outputviajqinstead.🤖 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 @.agents/hooks/post-tool-use-edit-write.sh around lines 75 - 82, The script currently runs ESLint twice: once into json_output (used to compute hard_errors) and again into readable, which is redundant; remove the second eslint invocation and instead generate the human-readable output from the existing $json_output using jq (or another JSON-to-text formatting step) so you reuse the initial ESLint result; update the block using readable/hard_errors/mark_failed to set readable by transforming $json_output (e.g., pretty-print messages, file paths and rule IDs), keep the same error-check logic that calls mark_failed when hard_errors != "0", and ensure any stderr/exit handling remains identical when printing readable (use the transformed output in place of the second eslint call).
61-69:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winESLint jq expression crashes on
nullruleId.Parse errors produce messages where
ruleIdisnull. The expressionselect(.ruleId | test($soft) | not)fails with "null cannot be matched, as it is not a string", jq exits 1, and the|| hard_errors="0"fallback masks actual errors — bothhard_errorsandtotal_errorsreset to"0", and the hook returns success. The same null-safety gap exists in thetotal_errorsexpression (line 67) and the ESLint invocation itself at line 61 silently discards all stderr with2>/dev/null.🛡️ Proposed fix
- json_output=$(cd "$project_dir" && pnpm exec eslint -c ./eslint.config.js --no-warn-ignored --cache --cache-location .eslintcache/ --format json "$rel_path" 2>/dev/null) || true + local eslint_exit=0 + json_output=$(cd "$project_dir" && pnpm exec eslint -c ./eslint.config.js --no-warn-ignored --cache --cache-location .eslintcache/ --format json "$rel_path" 2>&1) || eslint_exit=$? + if [[ $eslint_exit -eq 2 || (-z "$json_output") ]]; then + [[ -n "$json_output" ]] && echo "$json_output" >&2 + mark_failed + return 0 + fi - [.[].messages[] | select(.severity == 2) | select(.ruleId | test($soft) | not)] | length + [.[].messages[] | select(.severity == 2) | select(((.ruleId // "") | test($soft)) | not)] | length - [.[].messages[] | select(.severity == 2)] | length + [.[].messages[] | select(.severity == 2)] | length # total_errors already safe (no ruleId reference)🤖 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 @.agents/hooks/post-tool-use-edit-write.sh around lines 61 - 69, The jq expressions and ESLint call need null-safety and preserve stderr: when computing hard_errors and total_errors (variables json_output, hard_errors, total_errors) change the jq filters to guard or default ruleId (e.g., use (.ruleId // "") or select(.ruleId != null) before applying test) so null ruleId won't cause jq to exit 1, and adjust the ESLint invocation (json_output capture) to not throw away stderr so real errors propagate (remove or capture the 2>/dev/null redirection and handle nonzero exit properly) — update the filters around .messages[] and .ruleId to use the safe form and ensure jq failures don't get masked by the || fallbacks.
25-28:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTool failures with empty output silently succeed.
The
&& -n "$output"condition means a non-zero exit with no output still passes. This equally affectsrun_vale(line 103) andrun_stylelint(line 140).🛡️ Proposed fix (apply to all three callsites)
- if [[ ${exit_code:-0} -ne 0 && -n "$output" ]]; then - echo "$output" >&2 + if [[ ${exit_code:-0} -ne 0 ]]; then + [[ -n "$output" ]] && echo "$output" >&2 mark_failed fi🤖 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 @.agents/hooks/post-tool-use-edit-write.sh around lines 25 - 28, The current conditional if [[ ${exit_code:-0} -ne 0 && -n "$output" ]]; then ... mark_failed fi lets tools with a non‑zero exit code but empty output silently succeed; update all three callsites (the shown conditional block and the run_vale and run_stylelint invocations) to check only the exit code (if [[ ${exit_code:-0} -ne 0 ]]; then) and still echo "$output" if non‑empty (e.g., echo "$output" >&2 || true before/inside the branch), then call mark_failed so any non‑zero exit code triggers failure even when output is empty.pnpm-workspace.yaml (1)
119-129:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
engine-strictandnode-optionsin.npmrcare silently ignored by pnpm v11.pnpm v11 limits
.npmrcto auth/registry settings; all other settings must live inpnpm-workspace.yamlor the globalconfig.yaml.engine-strict=trueandnode-options=--max-old-space-size=8192in.npmrcare silently dropped.Add them to
pnpm-workspace.yaml:🛡️ Proposed fix
allowBuilds: '@bundled-es-modules/glob': true ... + +# migrated from .npmrc (pnpm v11 ignores non-auth .npmrc settings) +engine-strict: true +node-options: "--max-old-space-size=8192"🤖 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 `@pnpm-workspace.yaml` around lines 119 - 129, The pnpm v11 change means engine-strict and node-options set in .npmrc are ignored; update pnpm-workspace.yaml to include top-level keys engineStrict: true and nodeOptions: "--max-old-space-size=8192" (i.e., move engine-strict and node-options from .npmrc into pnpm-workspace.yaml), placing them as root-level fields near existing workspace config such as allowBuilds so pnpm will honor engineStrict and nodeOptions for the workspace..codex/hooks.json (2)
9-58:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
git rev-parse --show-toplevellacks error suppression andpwdfallback (re-introduced after being addressed in commit 59e7e7a).All five hook commands use
root="$(git rev-parse --show-toplevel)" &&— ifgit rev-parsefails, the&&short-circuits and the hook silently never runs. The equivalent in.claude/settings.jsonapplies2>/dev/null || pwdas a resilience layer. The fix landed in 59e7e7a appears to have been reverted.🛡️ Proposed fix (shown for `SessionStart`; apply to all five commands)
-"command": "root=\"$(git rev-parse --show-toplevel)\" && AGENTS_PROJECT_DIR=\"$root\" \"$root/.agents/hooks/session-start.sh\"", +"command": "root=\"$(git rev-parse --show-toplevel 2>/dev/null || pwd)\" && AGENTS_PROJECT_DIR=\"$root\" \"$root/.agents/hooks/session-start.sh\"",🤖 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 @.codex/hooks.json around lines 9 - 58, The hooks use root="$(git rev-parse --show-toplevel)" with && so if git rev-parse fails the hook silently aborts; update each command invocation that sets root to use a resilient fallback like root="$(git rev-parse --show-toplevel 2>/dev/null || pwd)" so the hook continues even when not in a git repo—apply this change to the five hook commands referencing session-start.sh, pre-tool-use-bash.sh, pre-tool-use-edit-write.sh, post-tool-use-edit-write.sh, and stop.sh.
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
"^startup$"matcher skips session-resume — environment setup will not run on resume.
startup|resume|clearare the validSessionStartvalues documented by Codex, and the official Codex example uses"matcher": "startup|resume"forSessionStart. With"^startup$",session-start.sh(nvm/corepack/pnpm install) is silently skipped when Codex resumes an existing session, potentially leaving the environment uninitialised.🛠️ Proposed fix
- "matcher": "^startup$", + "matcher": "startup|resume",🤖 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 @.codex/hooks.json at line 5, The current "matcher" value is anchored to "^startup$" which prevents SessionStart values like resume/clear from triggering the hook; update the "matcher" property to include the other valid SessionStart options (e.g., use "startup|resume" or "^(startup|resume|clear)$") so that session-start.sh (nvm/corepack/pnpm install) runs on resume/clear as well; modify the "matcher" string in .codex/hooks.json accordingly, ensuring the value includes resume (and optionally clear) instead of only startup..agents/hooks/stop.sh (1)
7-7:⚠️ Potential issue | 🟡 Minor | ⚡ Quick win
IS_CODEX_STOPoverly broad jq predicate (unresolved from prior review).Using
has("stop_hook_active") or has("last_assistant_message")alongsidehas("turn_id")is redundant: per the Codex hooks documentation,turn_idis the sole Codex-specific field and the correct discriminant. The OR conditions don't introduce false positives here but they do obscure the intent.🔧 Proposed fix
-IS_CODEX_STOP=$(jq -r 'if (.hook_event_name == "Stop" and (has("turn_id") or has("stop_hook_active") or has("last_assistant_message"))) then "1" else "0" end' <<<"$INPUT" 2>/dev/null || echo "0") +IS_CODEX_STOP=$(jq -r 'if (.hook_event_name == "Stop" and has("turn_id")) then "1" else "0" end' <<<"$INPUT" 2>/dev/null || echo "0")🤖 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 @.agents/hooks/stop.sh at line 7, The IS_CODEX_STOP jq predicate is too broad; change the condition to rely solely on the Codex discriminant turn_id instead of also checking stop_hook_active or last_assistant_message. Update the jq expression used to set IS_CODEX_STOP (the command that inspects hook_event_name and turn_id) so it tests hook_event_name == "Stop" and has("turn_id") only, removing the redundant has("stop_hook_active") and has("last_assistant_message") checks to make intent explicit and avoid ambiguity.
🤖 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 @.agents/hooks/stop.sh:
- Line 6: The stop hook currently silently exits when project root resolution
fails due to the trailing "|| exit 0" on the PROJECT_ROOT assignment; change
this to mirror session-start.sh by checking the result of resolve_project_root
(the PROJECT_ROOT assignment) and, if it fails, print an error message like
"Could not resolve project root." to stderr using >&2 before exiting with a
non-zero status so failures are visible; update the PROJECT_ROOT assignment in
stop.sh (the resolve_project_root invocation) and remove the silent exit
pattern.
In `@README.md`:
- Around line 34-40: Update the README setup commands to pin the pnpm version
when activating Corepack: replace the generic "corepack prepare --activate"
invocation with an explicit versioned prepare like "corepack prepare --activate
pnpm@11.0.8" so the repository's intended pnpm version (11.0.8) is
deterministically installed; ensure the line referencing corepack in the snippet
(the "corepack prepare --activate" command) is changed to include "pnpm@11.0.8".
---
Duplicate comments:
In @.agents/hooks/post-tool-use-edit-write.sh:
- Around line 75-82: The script currently runs ESLint twice: once into
json_output (used to compute hard_errors) and again into readable, which is
redundant; remove the second eslint invocation and instead generate the
human-readable output from the existing $json_output using jq (or another
JSON-to-text formatting step) so you reuse the initial ESLint result; update the
block using readable/hard_errors/mark_failed to set readable by transforming
$json_output (e.g., pretty-print messages, file paths and rule IDs), keep the
same error-check logic that calls mark_failed when hard_errors != "0", and
ensure any stderr/exit handling remains identical when printing readable (use
the transformed output in place of the second eslint call).
- Around line 61-69: The jq expressions and ESLint call need null-safety and
preserve stderr: when computing hard_errors and total_errors (variables
json_output, hard_errors, total_errors) change the jq filters to guard or
default ruleId (e.g., use (.ruleId // "") or select(.ruleId != null) before
applying test) so null ruleId won't cause jq to exit 1, and adjust the ESLint
invocation (json_output capture) to not throw away stderr so real errors
propagate (remove or capture the 2>/dev/null redirection and handle nonzero exit
properly) — update the filters around .messages[] and .ruleId to use the safe
form and ensure jq failures don't get masked by the || fallbacks.
- Around line 25-28: The current conditional if [[ ${exit_code:-0} -ne 0 && -n
"$output" ]]; then ... mark_failed fi lets tools with a non‑zero exit code but
empty output silently succeed; update all three callsites (the shown conditional
block and the run_vale and run_stylelint invocations) to check only the exit
code (if [[ ${exit_code:-0} -ne 0 ]]; then) and still echo "$output" if
non‑empty (e.g., echo "$output" >&2 || true before/inside the branch), then call
mark_failed so any non‑zero exit code triggers failure even when output is
empty.
In @.agents/hooks/stop.sh:
- Line 7: The IS_CODEX_STOP jq predicate is too broad; change the condition to
rely solely on the Codex discriminant turn_id instead of also checking
stop_hook_active or last_assistant_message. Update the jq expression used to set
IS_CODEX_STOP (the command that inspects hook_event_name and turn_id) so it
tests hook_event_name == "Stop" and has("turn_id") only, removing the redundant
has("stop_hook_active") and has("last_assistant_message") checks to make intent
explicit and avoid ambiguity.
In @.codex/hooks.json:
- Around line 9-58: The hooks use root="$(git rev-parse --show-toplevel)" with
&& so if git rev-parse fails the hook silently aborts; update each command
invocation that sets root to use a resilient fallback like root="$(git rev-parse
--show-toplevel 2>/dev/null || pwd)" so the hook continues even when not in a
git repo—apply this change to the five hook commands referencing
session-start.sh, pre-tool-use-bash.sh, pre-tool-use-edit-write.sh,
post-tool-use-edit-write.sh, and stop.sh.
- Line 5: The current "matcher" value is anchored to "^startup$" which prevents
SessionStart values like resume/clear from triggering the hook; update the
"matcher" property to include the other valid SessionStart options (e.g., use
"startup|resume" or "^(startup|resume|clear)$") so that session-start.sh
(nvm/corepack/pnpm install) runs on resume/clear as well; modify the "matcher"
string in .codex/hooks.json accordingly, ensuring the value includes resume (and
optionally clear) instead of only startup.
In `@pnpm-workspace.yaml`:
- Around line 119-129: The pnpm v11 change means engine-strict and node-options
set in .npmrc are ignored; update pnpm-workspace.yaml to include top-level keys
engineStrict: true and nodeOptions: "--max-old-space-size=8192" (i.e., move
engine-strict and node-options from .npmrc into pnpm-workspace.yaml), placing
them as root-level fields near existing workspace config such as allowBuilds so
pnpm will honor engineStrict and nodeOptions for the workspace.
In `@projects/internals/tools/src/context/index.ts`:
- Line 25: The prompt string "Elements provides a CLI and MCP server (`nve`) to
help you create, setup, and validate projects." uses the noun "setup" instead of
the verb phrase "set up"; update that string literal to "set up" so it reads
"Elements provides a CLI and MCP server (`nve`) to help you create, set up, and
validate projects." Ensure you modify the exact string occurrence in
projects/internals/tools/src/context/index.ts (the exported prompt/description
string) so agent prompts follow the Google style guide.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: 976a8cab-f361-46a2-bc89-44a97a6eb01c
📒 Files selected for processing (39)
.agents/README.md.agents/hooks/lib/project-root.sh.agents/hooks/notification.sh.agents/hooks/post-tool-use-edit-write.sh.agents/hooks/pre-tool-use-bash.sh.agents/hooks/pre-tool-use-edit-write.sh.agents/hooks/session-start.sh.agents/hooks/stop.sh.agents/skills/accessibility/SKILL.md.agents/skills/api-design/SKILL.md.agents/skills/build-system/SKILL.md.agents/skills/code-review/SKILL.md.agents/skills/component-creation/SKILL.md.agents/skills/documentation/SKILL.md.agents/skills/examples/SKILL.md.agents/skills/pattern-creation/SKILL.md.agents/skills/testing/SKILL.md.agents/skills/troubleshooting/SKILL.md.agents/skills/typescript/SKILL.md.claude/hooks/format-prettier.sh.claude/hooks/guard-critical-files.sh.claude/hooks/guard-package-manager.sh.claude/hooks/lint-commitlint.sh.claude/hooks/lint-eslint.sh.claude/hooks/lint-stylelint.sh.claude/hooks/lint-vale.sh.claude/hooks/worktree-create.sh.claude/settings.json.codex/config.toml.codex/hooks.json.github/workflows/ci.yml.husky/commit-msgAGENTS.mdREADME.mdconfig/docker/ci.dockerfilepackage.jsonpnpm-workspace.yamlprojects/internals/tools/src/context/index.tsprojects/site/src/docs/internal/guidelines/troubleshooting.md
💤 Files with no reviewable changes (8)
- .claude/hooks/guard-critical-files.sh
- .claude/hooks/lint-vale.sh
- .claude/hooks/worktree-create.sh
- .claude/hooks/guard-package-manager.sh
- .claude/hooks/lint-eslint.sh
- .claude/hooks/lint-commitlint.sh
- .claude/hooks/format-prettier.sh
- .claude/hooks/lint-stylelint.sh
coryrylan
force-pushed
the
topic-codex-fixes
branch
3 times, most recently
from
4a73807 to
99d94f5
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
projects/internals/tools/src/context/index.ts (1)
25-25:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winGrammar issue persists: use "set up" (verb form) instead of "setup".
This issue was previously flagged and marked as addressed, but the current code still contains "setup" as a verb. Per the Google developer documentation style guide, "setup" is a noun/adjective; the verb form is "set up" (two words).
✏️ Proposed fix
-Elements provides a CLI and MCP server (\`nve\`) to help you create, setup, and validate projects. +Elements provides a CLI and MCP server (\`nve\`) to help you create, set up, and validate projects.As per coding guidelines: Component JSDoc and documentation must follow Google developer documentation style guide.
🤖 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 `@projects/internals/tools/src/context/index.ts` at line 25, The JSDoc/README sentence containing "Elements provides a CLI and MCP server (`nve`) to help you create, setup, and validate projects." uses the noun "setup" incorrectly; update that text to use the verb form "set up" (i.e., change "create, setup, and validate" to "create, set up, and validate") wherever that exact string appears (search for the sentence or the string "create, setup, and validate projects" in the file to locate it).README.md (1)
37-40:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin
corepack preparetopnpm@11.0.8for deterministic setup.Line 39 still uses
corepack prepare --activate, which can install a different pnpm version than the documented11.0.8.🔧 Proposed doc fix
npm install -g corepack@0.34.7 corepack enable -corepack prepare --activate +corepack prepare pnpm@11.0.8 --activate pnpm i --frozen-lockfile --prefer-offline#!/bin/bash # Verify docs use pinned pnpm activation where corepack prepare appears rg -n "corepack prepare" README.md AGENTS.md projects/site/src/docs/internal/guidelines/troubleshooting.md rg -n "pnpm@11\\.0\\.8" README.md AGENTS.md projects/site/src/docs/internal/guidelines/troubleshooting.md🤖 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 `@README.md` around lines 37 - 40, Update the documented setup command so corepack activates a pinned pnpm version: replace the bare "corepack prepare --activate" invocation with an explicit "corepack prepare pnpm@11.0.8 --activate" (i.e., pin pnpm to 11.0.8) wherever the string "corepack prepare --activate" appears in the docs (e.g., README.md) and verify occurrences with a search for "corepack prepare".AGENTS.md (1)
50-53:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse version-pinned Corepack activation in the setup block.
Line 52 should explicitly activate
pnpm@11.0.8; otherwise setup can drift from the declared toolchain.🔧 Proposed fix
npm install -g corepack@0.34.7 corepack enable -corepack prepare --activate +corepack prepare pnpm@11.0.8 --activate pnpm i --frozen-lockfile --prefer-offline#!/bin/bash # Verify Corepack activation commands are pinned in docs rg -n "corepack prepare" AGENTS.md README.md projects/site/src/docs/internal/guidelines/troubleshooting.md rg -n "pnpm@11\\.0\\.8" AGENTS.md README.md projects/site/src/docs/internal/guidelines/troubleshooting.md🤖 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 `@AGENTS.md` around lines 50 - 53, Replace the unpinned Corepack activation command "corepack prepare --activate" with an explicit activation that pins pnpm to 11.0.8 (e.g., "corepack prepare --activate pnpm@11.0.8") so the toolchain cannot drift; update the line that currently contains "corepack prepare --activate" to include "pnpm@11.0.8".
🤖 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 @.agents/hooks/pre-tool-use-edit-write.sh:
- Around line 26-35: Move the constant PROTECTED_FILES array out of the per-file
loop so it is defined once before the while/read loop begins; locate the
PROTECTED_FILES variable definition and the loop that iterates over file paths
(the while ... read ... loop) and place the array declaration above that loop
(e.g., before line where the loop starts) to avoid reallocating it on each
iteration while preserving its usage in the existing checks.
In @.codex/hooks.json:
- Line 9: Update the hook command strings in .codex/hooks.json to use the same
defensive project-dir resolution as .claude/settings.json: replace the plain git
rev-parse --show-toplevel usage inside the command assigned to "command" with a
fallback expression that redirects stderr and falls back to pwd (i.e. the
`${AGENTS_PROJECT_DIR:-${CLAUDE_PROJECT_DIR:-$(git rev-parse --show-toplevel
2>/dev/null || pwd)}}` pattern) so the session-start.sh invocation still runs
when git rev-parse fails; apply this change to the "command" entries (the quoted
root=... && AGENTS_PROJECT_DIR=... session-start.sh invocation) for all five
hooks.
In `@projects/site/src/docs/internal/guidelines/troubleshooting.md`:
- Line 258: The troubleshooting doc's Corepack/pnpm command and the repository's
package manager are out of sync: update the package.json packageManager field
from "pnpm@10.27.0" to "pnpm@11.0.8" to match the docs (or alternatively change
the docs to use pnpm@10.27.0); locate the packageManager entry in package.json
and set it to "pnpm@11.0.8" so the command in troubleshooting.md ("corepack
prepare pnpm@11.0.8 --activate") and the repo config are consistent.
---
Duplicate comments:
In `@AGENTS.md`:
- Around line 50-53: Replace the unpinned Corepack activation command "corepack
prepare --activate" with an explicit activation that pins pnpm to 11.0.8 (e.g.,
"corepack prepare --activate pnpm@11.0.8") so the toolchain cannot drift; update
the line that currently contains "corepack prepare --activate" to include
"pnpm@11.0.8".
In `@projects/internals/tools/src/context/index.ts`:
- Line 25: The JSDoc/README sentence containing "Elements provides a CLI and MCP
server (`nve`) to help you create, setup, and validate projects." uses the noun
"setup" incorrectly; update that text to use the verb form "set up" (i.e.,
change "create, setup, and validate" to "create, set up, and validate") wherever
that exact string appears (search for the sentence or the string "create, setup,
and validate projects" in the file to locate it).
In `@README.md`:
- Around line 37-40: Update the documented setup command so corepack activates a
pinned pnpm version: replace the bare "corepack prepare --activate" invocation
with an explicit "corepack prepare pnpm@11.0.8 --activate" (i.e., pin pnpm to
11.0.8) wherever the string "corepack prepare --activate" appears in the docs
(e.g., README.md) and verify occurrences with a search for "corepack prepare".
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Enterprise
Run ID: c2e0b033-3ee4-4dc2-8ae3-9d0fac38011f
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (35)
.agents/README.md.agents/hooks/lib/project-root.sh.agents/hooks/notification.sh.agents/hooks/post-tool-use-edit-write.sh.agents/hooks/pre-tool-use-bash.sh.agents/hooks/pre-tool-use-edit-write.sh.agents/hooks/session-start.sh.agents/hooks/stop.sh.agents/skills/accessibility/SKILL.md.agents/skills/api-design/SKILL.md.agents/skills/build-system/SKILL.md.agents/skills/code-review/SKILL.md.agents/skills/component-creation/SKILL.md.agents/skills/documentation/SKILL.md.agents/skills/examples/SKILL.md.agents/skills/pattern-creation/SKILL.md.agents/skills/testing/SKILL.md.agents/skills/troubleshooting/SKILL.md.agents/skills/typescript/SKILL.md.claude/hooks/format-prettier.sh.claude/hooks/guard-critical-files.sh.claude/hooks/guard-package-manager.sh.claude/hooks/lint-commitlint.sh.claude/hooks/lint-eslint.sh.claude/hooks/lint-stylelint.sh.claude/hooks/lint-vale.sh.claude/hooks/worktree-create.sh.claude/settings.json.codex/config.toml.codex/hooks.jsonAGENTS.mdREADME.mdprojects/cli/package.jsonprojects/internals/tools/src/context/index.tsprojects/site/src/docs/internal/guidelines/troubleshooting.md
💤 Files with no reviewable changes (8)
- .claude/hooks/format-prettier.sh
- .claude/hooks/lint-eslint.sh
- .claude/hooks/lint-stylelint.sh
- .claude/hooks/guard-package-manager.sh
- .claude/hooks/worktree-create.sh
- .claude/hooks/lint-commitlint.sh
- .claude/hooks/lint-vale.sh
- .claude/hooks/guard-critical-files.sh
coryrylan
commented
May 7, 2026
| } | ||
| ' <<<"$command" | ||
| fi | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
Utility to normalize how codex/claude determine the current working directory
coryrylan
commented
May 7, 2026
| set -euo pipefail | ||
|
|
||
| if [[ "$(uname -s)" == "Darwin" ]] && command -v osascript >/dev/null 2>&1; then | ||
| osascript -e 'display notification "Agent needs your attention" with title "Agent"' |
Collaborator
Author
There was a problem hiding this comment.
Added extra guard for macos only notifications
coryrylan
commented
May 7, 2026
| exit 2 | ||
| fi | ||
|
|
||
| exit 0 |
Collaborator
Author
There was a problem hiding this comment.
This combined the four separate lint/format scripts into a single script/hook. Reason this doesnt just call the ci/lint scripts is it determines which files were edited relative to the package.json and only runs against that subset.
coryrylan
commented
May 7, 2026
| @@ -1,5 +1,5 @@ | |||
| --- | |||
| name: repo-accessibility | |||
Collaborator
Author
There was a problem hiding this comment.
Names have to match file name by convention
johnyanarella
approved these changes
May 7, 2026
coryrylan
force-pushed
the
topic-codex-fixes
branch
from
99d94f5 to
daa3c12
Compare
Signed-off-by: Cory Rylan <crylan@nvidia.com>
Signed-off-by: Cory Rylan <crylan@nvidia.com>
coryrylan
force-pushed
the
topic-codex-fixes
branch
from
daa3c12 to
5bd69e9
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Documentation
Chores
Refactor