chore: auto-rebuild all generated artifacts on commit + CI - #211
Merged
Conversation
When any file under editor-app/src/ is staged, the pre-commit hook now runs npm ci + npm run build and stages the updated app.js/app.css automatically — same pattern as the existing dist-bundle rebuild. https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
Pre-commit hook now covers admin-app (same pattern as editor-app) and regenerates docs/inventory when CSS sources change. CI gets a matching admin-app-freshness job. Every committed build artifact now has both a local auto-rebuild guard and a CI staleness check. https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
scripts/artifacts.json is now the one place to register a build artifact (source glob → build command → outputs). Both the pre-commit hook and CI read from it via scripts/check-artifacts.js: --fix (hook) rebuilds only affected artifacts and re-stages outputs --check (CI) rebuilds everything and fails on any stale output Replaces the three separate *-freshness CI jobs and the inline per-artifact logic in the pre-commit hook. https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
|
Warning Review limit reached
More reviews will be available in 51 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR introduces a centralized artifact verification system that replaces ad-hoc bundle rebuilding with a declarative manifest and script. The new ChangesArtifact Verification System
🎯 3 (Moderate) | ⏱️ ~20 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)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
.github/workflows/ci.yml (1)
41-54: ⚡ Quick winAdd a least-privilege
permissionsblock to the new job.The new
artifacts-freshnessjob inherits the workflow's default token permissions (flagged by zizmor as overly broad). It only needs read access to the checkout.🔒 Proposed change
artifacts-freshness: name: Verify all generated artifacts runs-on: ubuntu-latest + permissions: + contents: read steps:🤖 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 @.github/workflows/ci.yml around lines 41 - 54, The new artifacts-freshness job is inheriting broad default token permissions; add a least-privilege permissions block on the artifacts-freshness job to restrict the GITHUB_TOKEN to read-only for repository contents (e.g., set permissions: contents: read) so the job only has checkout/read access; locate the job named artifacts-freshness and add the permissions entry directly under it.scripts/check-artifacts.js (1)
31-33: 💤 Low valueStatic-analysis command-injection flags here are false positives, but quote paths for space-safety.
cmd/outputscome from the version-controlled manifest, so there's no untrusted input — the OpenGrepexec-jsfindings on lines 24/28 can be dismissed. However,git add ${out}/git diff ${out}break on any output path containing spaces or shell metacharacters. Switching git calls toexecFileSync('git', [...args])removes both the static-analysis noise and the quoting hazard.Also applies to: 45-47
🤖 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 `@scripts/check-artifacts.js` around lines 31 - 33, The git invocations currently build shell commands with interpolated paths (see isTracked calling git(`ls-files --error-unmatch ${path}`) and the uses that run git add ${out} / git diff ${out}), which breaks on paths with spaces and triggers command-injection warnings; change these to use child_process.execFileSync with the git binary and argument arrays (e.g., execFileSync('git', ['ls-files', '--error-unmatch', path])) for isTracked and similarly replace the git add/diff calls with execFileSync('git', ['add', out]) and execFileSync('git', ['diff', '--', out]) so paths are passed as separate args and static-analysis issues are resolved.scripts/artifacts.json (1)
17-17: 🏗️ Heavy lift
npm ciin the buildcmdmakes pre-commit (--fix) very slow.In
--fixmode thiscmdruns on every commit that toucheseditor-app/src/(oradmin-app/src/).npm ciwipes and reinstallsnode_modulesfrom scratch, adding tens of seconds to minutes to each commit. CI needsnpm ci, but the hook generally has deps installed already.Consider a mode-aware command (e.g.
npm cionly in--check, plainnpm run buildin--fix), or splitting the install step out of the rebuildcmd.Also applies to: 27-27
🤖 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 `@scripts/artifacts.json` at line 17, The build cmd currently uses "npm ci --silent && npm run build --silent" which causes pre-commit `--fix` to reinstall node_modules every commit; update the "cmd" entries (the JSON key "cmd" that currently contains that string for the editor/admin artifacts) to be mode-aware or split install from build: run `npm ci` only when running in CI/`--check` mode (or when no node_modules exist) and run just `npm run build --silent` for `--fix`/local hooks, or separate into two artifact steps (one for install used by CI, one for rebuild used by hooks) so pre-commit no longer calls `npm ci` on every commit.
🤖 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 `@scripts/check-artifacts.js`:
- Line 39: The current check uses prefix matching (artifact.srcGlobs.some(glob
=> [...staged].some(f => f.startsWith(glob))) ) so patterns in artifact.srcGlobs
(e.g., core/**/*.css) won't match; replace the startsWith logic with real glob
matching: import a glob matcher (e.g., picomatch or minimatch), compile each
pattern from artifact.srcGlobs into a matcher, and compute triggered as
artifact.srcGlobs.some(glob => [...staged].some(f => matcher(glob)(f))) or
equivalent API for the chosen library; update any references to srcGlobs
semantics (or rename to srcPrefixes if you prefer the prefix behavior) and
ensure bundle.js remains the source of truth for glob semantics.
- Around line 14-21: The script scripts/check-artifacts.js uses ESM features
(top-level import and import.meta.url) but the repo lacks "type":"module", so
either convert the file to CommonJS (replace import/ fileURLToPath usage with
require and __dirname) or declare "type":"module" in package.json so node can
run it as ESM; also adjust the git-staging logic that currently uses git
ls-files (which only returns tracked files) so newly generated/untracked outputs
are staged: stop filtering with git ls-files and instead run git add directly on
the generated output paths (or combine git ls-files with git ls-files --others
--exclude-standard to include untracked), and update the header text from “Skips
gitignored outputs silently” to “Skips untracked outputs silently” (references:
the top-level imports and import.meta.url usage, the mode variable checking
'--check', and the git staging code that uses git ls-files).
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 41-54: The new artifacts-freshness job is inheriting broad default
token permissions; add a least-privilege permissions block on the
artifacts-freshness job to restrict the GITHUB_TOKEN to read-only for repository
contents (e.g., set permissions: contents: read) so the job only has
checkout/read access; locate the job named artifacts-freshness and add the
permissions entry directly under it.
In `@scripts/artifacts.json`:
- Line 17: The build cmd currently uses "npm ci --silent && npm run build
--silent" which causes pre-commit `--fix` to reinstall node_modules every
commit; update the "cmd" entries (the JSON key "cmd" that currently contains
that string for the editor/admin artifacts) to be mode-aware or split install
from build: run `npm ci` only when running in CI/`--check` mode (or when no
node_modules exist) and run just `npm run build --silent` for `--fix`/local
hooks, or separate into two artifact steps (one for install used by CI, one for
rebuild used by hooks) so pre-commit no longer calls `npm ci` on every commit.
In `@scripts/check-artifacts.js`:
- Around line 31-33: The git invocations currently build shell commands with
interpolated paths (see isTracked calling git(`ls-files --error-unmatch
${path}`) and the uses that run git add ${out} / git diff ${out}), which breaks
on paths with spaces and triggers command-injection warnings; change these to
use child_process.execFileSync with the git binary and argument arrays (e.g.,
execFileSync('git', ['ls-files', '--error-unmatch', path])) for isTracked and
similarly replace the git add/diff calls with execFileSync('git', ['add', out])
and execFileSync('git', ['diff', '--', out]) so paths are passed as separate
args and static-analysis issues are resolved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 461ede9d-275d-4018-98b9-ef0bd29fd40b
📒 Files selected for processing (4)
.githooks/pre-commit.github/workflows/ci.ymlscripts/artifacts.jsonscripts/check-artifacts.js
Comment on lines
+14
to
+21
| import { execSync } from 'child_process'; | ||
| import { readFileSync } from 'fs'; | ||
| import { resolve, dirname } from 'path'; | ||
| import { fileURLToPath } from 'url'; | ||
|
|
||
| const root = resolve(dirname(fileURLToPath(import.meta.url)), '..'); | ||
| const artifacts = JSON.parse(readFileSync(resolve(root, 'scripts/artifacts.json'), 'utf8')); | ||
| const mode = process.argv.includes('--check') ? 'check' : 'fix'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the package.json governing scripts/ declares ESM.
fd -t f 'package.json' -d 2 -x sh -c 'echo "== {} =="; jq -r ".type // \"(no type → commonjs)\"" "{}"'Repository: codeslash-dev/SLASHED
Length of output: 107
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Find any package.json files that could affect module type resolution
echo "## package.json files"
fd -t f 'package.json' -d 5 > /tmp/pjs.txt
cat /tmp/pjs.txt
# Show all "type" values (if jq exists)
echo
echo "## package.json: .type"
while IFS= read -r p; do
echo "== $p =="
jq -r '.type // "(no type → commonjs)"' "$p" 2>/dev/null || echo "(jq missing or parse error)"
echo
done < /tmp/pjs.txt
# 2) Locate invocations of scripts/check-artifacts.js
echo "## invocations of scripts/check-artifacts.js"
rg -n --hidden --glob '!node_modules/**' 'check-artifacts\.js' .
# 3) Inspect the target script
echo
echo "## scripts/check-artifacts.js (top)"
sed -n '1,120p' scripts/check-artifacts.jsRepository: codeslash-dev/SLASHED
Length of output: 2604
Set correct Node module mode for scripts/check-artifacts.js (ESM import/meta)
- The file uses top-level ESM (
import .../import.meta.url) but the repo rootpackage.jsonhas no"type": "module", and CI/hook invoke it vianode scripts/check-artifacts.js ...—so it will fail withSyntaxError: Cannot use import statement outside a module. - In
--fixmode it onlygit adds outputs that are already tracked (git ls-files ...), so newly generated/untracked outputs won’t be staged; the header text “Skips gitignored outputs silently” is misleading (should describe “skips untracked outputs”).
🤖 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 `@scripts/check-artifacts.js` around lines 14 - 21, The script
scripts/check-artifacts.js uses ESM features (top-level import and
import.meta.url) but the repo lacks "type":"module", so either convert the file
to CommonJS (replace import/ fileURLToPath usage with require and __dirname) or
declare "type":"module" in package.json so node can run it as ESM; also adjust
the git-staging logic that currently uses git ls-files (which only returns
tracked files) so newly generated/untracked outputs are staged: stop filtering
with git ls-files and instead run git add directly on the generated output paths
(or combine git ls-files with git ls-files --others --exclude-standard to
include untracked), and update the header text from “Skips gitignored outputs
silently” to “Skips untracked outputs silently” (references: the top-level
imports and import.meta.url usage, the mode variable checking '--check', and the
git staging code that uses git ls-files).
- Convert to CommonJS (was ESM, root package.json has no type:module) - Rename srcGlobs → srcPrefixes to match actual prefix-matching behaviour - Split cmd → installCmd + buildCmd; pre-commit --fix skips installCmd so npm ci doesn't run on every local commit - Use execFileSync with argument arrays for all git calls (space-safe, no shell-injection surface) - Add permissions: contents: read to artifacts-freshness CI job https://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
This was referenced Jun 7, 2026
3 tasks
This was referenced Jul 3, 2026
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
scripts/artifacts.json— a single manifest mapping source globs → build command → outputs for every generated/compiled file in the reposcripts/check-artifacts.js— reads the manifest and either auto-rebuilds + stages (pre-commit,--fix) or rebuilds + asserts git diff clean (CI,--check).githooks/pre-committo call the script instead of enumerating artifacts inline*-freshnessCI jobs (docs-freshness,editor-app-freshness,admin-app-freshness) with a singleartifacts-freshnessjobHow to add a new build artifact going forward
Add one entry to
scripts/artifacts.json. Nothing else needs to change — the pre-commit hook and CI pick it up automatically.{ "name": "my-new-thing", "srcGlobs": ["path/to/src/"], "cwd": "path/to/build/dir", "cmd": "npm ci --silent && npm run build --silent", "outputs": ["path/to/output.js"] }Artifacts covered
docs/tokens.md,docs/classes.md,inventory.json,classes-hints.jsoncore/,optional/assets/editor-app/app.js+csseditor-app/src/assets/admin-app/app.js+cssadmin-app/src/Test plan
editor-app/src/and commit — confirm the hook auto-rebuilds and stagesapp.js/app.csscore/and commit — confirm docs/inventory files are regenerated and stagedartifacts-freshnessCI job passes on this PRhttps://claude.ai/code/session_01MR5eh7i7GRVpvudPrCjm2w
Generated by Claude Code
Summary by CodeRabbit