[codex] Make prepare the UI edit contract - #159
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces a new "changed-surface evidence gate" ( ChangesChanged-Surface Evidence Gate
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Git as Git Repo
participant Gate as agent-design:prepare:changed
participant CLI as design prepare CLI
participant Validation as JSON Validator
participant Report as Output
Dev->>Dev: Run `pnpm agent-design:prepare:changed`
Dev->>Gate: Trigger wrapper (prebuild + evidence check)
Gate->>Gate: Build agent-design-cli + dependencies
Gate->>Git: Get changed UI surfaces (git diff)
Git-->>Gate: List of .tsx/.jsx files
Gate->>Gate: Filter for UI surfaces, deduplicate
Gate->>Gate: For each surface, run prepare CLI
loop Each Surface
Gate->>CLI: `design prepare --surface <path> --json`
CLI-->>Gate: JSON envelope {status, data{kind, safeForAutomaticImplementation, openDecisions}}
Gate->>Validation: Validate schema & safety flag
Validation-->>Gate: Pass/Fail with reason
Gate->>Report: Log surface status + any openDecisions
end
Gate-->>Dev: Exit 0 if all safe, Exit 1 if any unsafe
Dev->>Dev: Continue with PR or resolve failed surfaces
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 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 docstrings
🧪 Generate unit tests (beta)
Review rate limit: 3/5 reviews remaining, refill in 23 minutes and 39 seconds. Comment |
Why: AI coding agents need a single reliable pre-edit UI command that returns components, token roles, states, examples, forbidden patterns, validation commands, and proposal stops without relying on scattered docs.
What: Harden prepare payload/schema/error handling, add semantic token-contract evidence, normalize read-only validation command metadata, add clean-checkout wrapper smoke coverage, document prepare as the workflow front door, add a changed-surface prepare evidence gate, and run that gate in PR CI.
Impact/Risk: UI edits now have a stronger agent-first contract and PR tripwire. The root changed-surface wrapper is build-backed and may write local dist outputs during setup, while the direct prepare operation remains read-only.
Validation: pass - pnpm -C packages/agent-design-engine test
Validation: pass - pnpm -C packages/cli test
Validation: pass - pnpm -C packages/design-system-guidance build
Validation: pass - pnpm docs:lint
Validation: pass - pnpm --silent agent-design:prepare:smoke
Validation: pass - pnpm --silent agent-design:lint
Validation: pass - pnpm agent-design:prepare:changed -- --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx
Validation: pass - pnpm agent-design:prepare:changed
Validation: pass - ruby -e "require 'psych'; Psych.load_file('.github/workflows/ci.yml'); puts 'ci workflow yaml ok'"
Validation: pass - git diff --check
Validation: pass - bash scripts/validate-codestyle.sh
Validation: pass - test -f memory.json && jq -e '.meta.version == "1.0" and (.preamble.bootstrap | type == "boolean") and (.preamble.search | type == "boolean") and (.entries | type == "array")' memory.json >/dev/null
Validation: blocked - direct pnpm run check was rejected by the runner policy before execution; bash scripts/validate-codestyle.sh successfully ran the same pnpm check script internally.
Validation: blocked - ci-check-name-parity helper expects missing .github/workflows/pr-pipeline.yml; fallback rg comparison showed the new gate adds no required check name.
Co-authored-by: Codex <noreply@openai.com>
7f27b5b to
8ec84af
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/agent-design-engine/src/prepare.ts (1)
498-511:⚠️ Potential issue | 🟠 Major | ⚡ Quick winThe first
DESIGN.md/ guidance reads still fail nondeterministically.These reads are not wrapped, so a missing or unreadable
DESIGN.mdor.design-system-guidance.jsonstill throws plain filesystem errors instead of the structured engine codes this PR is trying to make canonical. Please translate those failures before parsing sopreparestays deterministic end to end.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-design-engine/src/prepare.ts` around lines 498 - 511, The file currently calls readText(...) for designPath and guidancePath without translating filesystem errors; wrap the Promise.all read sequence in a try/catch (or wrap each readText) to catch IO exceptions before parsing, and rethrow as DesignEngineError with a clear engine code and exitCode (e.g. "E_DESIGN_SOURCE_IO" for designPath and "E_DESIGN_GUIDANCE_IO" for guidancePath) so failures are deterministic; include the original error message/details in the DesignEngineError metadata, preserve signal?.throwIfAborted() semantics, and keep the existing JSON parse error handling for parsedGuidance.scripts/check-agent-design-prepare-evidence.mjs (1)
64-157:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAdd service identity to output/error lines.
Current logs are plain strings. This script is a CLI gate and should include service identity in output/error messages for policy compliance and log attribution.
As per coding guidelines "Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@scripts/check-agent-design-prepare-evidence.mjs` around lines 64 - 157, Update all console output and error messages in this file to include the service identity service:"agent-design" for log attribution: modify the top console.error message and every console.log in the loop and summary (including the status/label line, reason, openDecision, stderr and stdout prints) to append or embed service:"agent-design"; ensure the string produced by prepare() (and any error messages derived from result.stderr/result.stdout) also includes the service identity so every emitted line contains service:"agent-design". Target the console calls in function prepare(), the initial console.error(`agent-design: surface is outside the repository: ${file}`), the loop over surfaces (the label/status log built from result.surfaceKind/surfaceScope/safeForAutomaticImplementation), and the final failure/success messages.FORJAMIE.md (1)
19-29:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatus block is date-fresh but PR reference is stale.
Line 19 updates the document to 2026-04-30, but the status table still points to PR
#158while this draft is PR#159andopen_prs: 1. Please update that row so operational status stays trustworthy.As per coding guidelines, "Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@FORJAMIE.md` around lines 19 - 29, The status table contains a stale PR reference — replace "PR `#158`" with "PR `#159`" in the "Open PRs" row of the status block (the table entry currently reading "PR `#158` carries the agent-native design-system execution slice") and verify the "Open PRs" count remains "1" (and update any other occurrences of "PR `#158`" in the document to "PR `#159`" so the document and status block are consistent).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/guides/AGENT_DESIGN_WORKFLOW.md`:
- Around line 21-41: The ordered list in AGENT_DESIGN_WORKFLOW.md is broken by
fenced code blocks which resets numbering and triggers markdownlint MD029; fix
it by normalizing the list to use consistent "1." prefixes for every top-level
step (e.g., before "Run the prepare command...", the "pnpm --silent
agent-design:prepare..." wrapper block, the "If
safeForAutomaticImplementation..." items, and subsequent steps) or by indenting
the fenced code blocks under their corresponding list items so they remain part
of the same numbered entry (apply this to the blocks containing "astudio design
prepare --surface <path> --json" and "pnpm --silent agent-design:prepare
--surface <path>" and the subsequent numbered steps) ensuring the entire
sequence is a single valid ordered list.
In `@package.json`:
- Around line 42-44: The three package.json scripts agent-design:prepare,
agent-design:prepare:changed, and agent-design:prepare:smoke repeat the same
prebuild chain; extract that common sequence into a single new script (e.g.,
agent-design:prebuild) and have the three scripts invoke it so the build order
stays centralized; ensure you preserve any flags and trailing commands (for
example keep the final pnpm -C packages/cli build and the node scripts or
--silent/--surface args) when replacing the inlined sequence and update the
other duplicate occurrence referenced in the PR so all references call the new
agent-design:prebuild script.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 170-194: buildDesignTokenContract currently returns semanticRoles
without verifying each advertised role has backing tokens; update it to iterate
the emitted semanticRoles and assert existence of each role's referenced
token(s) against the sources validated by assertThemeSource,
assertAliasMapSource and assertDtcgSource (use tokenSourceRefs and the same
resolution logic you use elsewhere to map role names like surface.card,
text.secondary, border.strong, status.* to concrete token IDs); if any role
lacks backing data, throw an error (fail closed) instead of returning the
contract. Ensure you reference and use semanticRoles, tokenSourceRefs, and the
assert* results/resolution helpers when implementing the checks.
In `@packages/cli/src/utils/mask.ts`:
- Around line 89-127: The maskPublicDesignTokenContract path currently preserves
keys in publicDesignTokenContractKeys and returns malformed allowedRoles entries
unchanged, letting secrets bypass masking; fix maskPublicDesignTokenContract and
maskPublicDesignTokenRole so you only keep schema-valid public primitives/arrays
and otherwise recurse/mask: for allowedRoles, when Array.isArray(val) map each
entry and if an entry is a non-null plain object call
maskPublicDesignTokenRole(role, masks, inDebugMode) else call maskFieldValue for
that entry; for the branch that currently returns values verbatim when
publicDesignTokenContractKeys.has(key), add a guard that only returns the value
unchanged for primitives or arrays of primitives, and call maskFieldValue(key,
val, masks, inDebugMode) for any other shapes so malformed objects don’t bypass
redaction (use the existing maskFieldValue, publicDesignTokenContractKeys, and
publicDesignTokenRoleKeys helpers to locate changes).
In `@packages/design-system-guidance/src/core.ts`:
- Around line 1621-1628: The lock probe currently uses exists(lockPath) which
treats ALL FS errors as “not exists” and can let dry-run proceed on
EACCES/EPERM/I/O failures; replace both uses of exists(lockPath) with an
explicit try/catch around a stat/access call (e.g., await fs.stat(lockPath)) so
you can distinguish ENOENT vs other errors: if stat succeeds then throw
migrationLockedError(); if it throws and err.code === 'ENOENT' proceed;
otherwise rethrow or wrap the error with contextual text (include lockPath,
targetPath, and whether options.dryRun) so the error routes to the logging layer
instead of being swallowed; keep the two checks around readGuidanceConfig but
use this robust probe function or inline logic to avoid failing open.
In `@scripts/check-agent-design-prepare-evidence.mjs`:
- Around line 79-103: After parsing result.stdout into payload, validate the
envelope shape before trusting fields: ensure payload is an object with a status
field and a data object where data.safeForAutomaticImplementation is boolean,
data.validationCommands is an array, and optional data.surfaceKind/surfaceScope
are strings (or null/undefined) — if validation fails, return the same
error-shaped object as the JSON parse catch block. Implement this by adding a
schema check (using your project validator, Ajv, or zod) right after
JSON.parse(result.stdout) and before const data = payload.data ?? {}, and use
the payload/data symbols to gate the subsequent return so
malformed-but-parseable payloads are rejected.
- Around line 40-43: The loop over commands currently ignores failed git probes
by continuing when run("git", commandArgs) returns non-zero; change this to fail
fast: when result.status !== 0, log an error including the failing commandArgs
and result.stderr (or stdout) and exit with a non-zero code (or throw) so the
script hard-fails instead of skipping diffs. Update the for (const commandArgs
of commands) block that calls run to perform this check and fail immediately
rather than continue.
- Around line 111-118: The code silently drops invalid explicit --surface values
because explicitSurfaces is mixed with gitChangedFiles and later filtered by
fs.existsSync, causing false-success exits; update the logic in the surfaces
construction to validate explicitSurfaces early: when readArgValues("--surface")
returns a non-empty array, verify each entry (use toRepoRelative and
fs.existsSync with repoRoot) and if any explicit path is missing, log a clear
error referencing the offending value(s) and exit non-zero (e.g.,
process.exit(1)); keep the existing fallback to gitChangedFiles/isUiSurface only
when explicitSurfaces is empty so behavior for implicit detection is unchanged.
---
Outside diff comments:
In `@FORJAMIE.md`:
- Around line 19-29: The status table contains a stale PR reference — replace
"PR `#158`" with "PR `#159`" in the "Open PRs" row of the status block (the table
entry currently reading "PR `#158` carries the agent-native design-system
execution slice") and verify the "Open PRs" count remains "1" (and update any
other occurrences of "PR `#158`" in the document to "PR `#159`" so the document and
status block are consistent).
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 498-511: The file currently calls readText(...) for designPath and
guidancePath without translating filesystem errors; wrap the Promise.all read
sequence in a try/catch (or wrap each readText) to catch IO exceptions before
parsing, and rethrow as DesignEngineError with a clear engine code and exitCode
(e.g. "E_DESIGN_SOURCE_IO" for designPath and "E_DESIGN_GUIDANCE_IO" for
guidancePath) so failures are deterministic; include the original error
message/details in the DesignEngineError metadata, preserve
signal?.throwIfAborted() semantics, and keep the existing JSON parse error
handling for parsedGuidance.
In `@scripts/check-agent-design-prepare-evidence.mjs`:
- Around line 64-157: Update all console output and error messages in this file
to include the service identity service:"agent-design" for log attribution:
modify the top console.error message and every console.log in the loop and
summary (including the status/label line, reason, openDecision, stderr and
stdout prints) to append or embed service:"agent-design"; ensure the string
produced by prepare() (and any error messages derived from
result.stderr/result.stdout) also includes the service identity so every emitted
line contains service:"agent-design". Target the console calls in function
prepare(), the initial console.error(`agent-design: surface is outside the
repository: ${file}`), the loop over surfaces (the label/status log built from
result.surfaceKind/surfaceScope/safeForAutomaticImplementation), and the final
failure/success messages.
🪄 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: Pro
Run ID: 35f539be-30f6-4f1c-90dc-23ea14656972
⛔ Files ignored due to path filters (1)
packages/widgets/src/sdk/generated/widget-manifest.jsis excluded by!**/generated/**
📒 Files selected for processing (22)
.github/PULL_REQUEST_TEMPLATE.md.github/workflows/ci.ymlFORJAMIE.mdREADME.mddocs/design-system/GOLD_EXAMPLES.jsondocs/design-system/GOLD_EXAMPLES.mddocs/guides/AGENT_DESIGN_WORKFLOW.mddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdpackage.jsonpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/src/utils/mask.tspackages/cli/tests/cli.test.mjspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/cli/tests/mask.test.mjspackages/cloudflare-template/src/worker/widget-manifest.generated.tspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (25)
.github/workflows/**/*.{yaml,yml}
📄 CodeRabbit inference engine (CODESTYLE.md)
.github/workflows/**/*.{yaml,yml}: GitHub Actions YAML MUST avoid large inline scripts when a repo script exists
CI config files: .github/workflows/*.yml
Files:
.github/workflows/ci.yml
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsdocs/design-system/GOLD_EXAMPLES.jsonscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackage.jsonpackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsdocs/design-system/GOLD_EXAMPLES.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdpackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tsREADME.mddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mdpackages/cli/src/utils/mask.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjsdocs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsondocs/design-system/GOLD_EXAMPLES.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdpackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tsREADME.mddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mdpackages/cli/src/utils/mask.tsdocs/design-system/GOLD_EXAMPLES.jsonscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjspackage.jsondocs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
.github/PULL_REQUEST_TEMPLATE.md
📄 CodeRabbit inference engine (CLAUDE.md)
All PRs must use the pull request template at
.github/PULL_REQUEST_TEMPLATE.md
Files:
.github/PULL_REQUEST_TEMPLATE.md
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome (
biome.json) for JS/TS formatting and keep files formatted viapnpm format
Files:
packages/design-system-guidance/src/core.tspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx}: Explicit types at all public API boundaries (functions, modules, React props)
Use concrete types or unknown + narrowing instead of any (forbidden pattern: : any, as any, Promise, Record<string, any>)
Unsafe type assertions (value as unknown as T) forbidden; use type guards or schema validation instead
//@ts-ignoreand //@ts-nocheckare forbidden; use //@ts-expect-error-- reason + ticket instead
Unsafe 'as SomeType' without runtime guard is forbidden; use guard function or schema validator
JSON.parse() and Response.json() return any; boundary mitigation MUST use schema validation (Zod/Valibot) or typed parser helper
Files:
packages/design-system-guidance/src/core.tspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
Tauri UI-facing state MUST be deterministic and testable
Files:
packages/design-system-guidance/src/core.tspackages/agent-design-engine/src/types.tspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/src/prepare.tspackages/cloudflare-template/src/worker/widget-manifest.generated.ts
**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Always verify JSON syntax with
jqor similar tool before declaring JSON fixes completeJSON MUST be valid UTF-8; prefer machine-generated JSON for large files; minimize hand-edited large JSON
Files:
packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsondocs/design-system/GOLD_EXAMPLES.jsonpackage.json
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Add a Table of Contents for documentation files
Files:
docs/design-system/GOLD_EXAMPLES.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdREADME.mddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.md
**/*.{md,mdx,adoc,rst}
📄 CodeRabbit inference engine (CODESTYLE.md)
All docs and long-form prose MUST be linted with Vale (scope: **/.md, **/.mdx, **/.adoc, **/.rst)
Files:
docs/design-system/GOLD_EXAMPLES.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdREADME.mddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.md
packages/cli/**/*.{sh,bash,js,ts,tsx,py}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/**/*.{sh,bash,js,ts,tsx,py}: Always use--agentflag when invoking aStudio CLI to enable intent-over-syntax parsing and detailed error messages
When using aStudio tokens command, validate tokens first, preview changes with--dry-run, then execute with--write --execflags
Files:
packages/cli/src/utils/mask.ts
packages/cli/**/*.{sh,bash,js,ts,tsx,py,yml,yaml}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/**/*.{sh,bash,js,ts,tsx,py,yml,yaml}: Start with--dry-runflag before executing commands that modify files or systems
Use--execsafety flag when running external processes like dev servers, build tools, or test runners
Use--writesafety flag when aStudio commands need to create or modify files
Use--networksafety flag when aStudio commands need to make HTTP or network requests
Files:
packages/cli/src/utils/mask.ts
packages/cli/**/*.{js,ts,tsx,py}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/**/*.{js,ts,tsx,py}: Check forfix_suggestionfield in error responses and automatically retry with the suggested command
Handle aStudio CLI error codes systematically: E_USAGE (check syntax), E_POLICY (add safety flags), E_VALIDATION (check input format), E_EXEC (check tool availability), E_NETWORK (check connectivity)
Files:
packages/cli/src/utils/mask.ts
packages/cli/**/*.{sh,bash,js,ts,tsx,py,yml,yaml,.env*}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Set
NO_COLOR=1orASTUDIO_COLOR=0environment variable for easier output parsing in automated systems
Files:
packages/cli/src/utils/mask.ts
FORJAMIE.md
📄 CodeRabbit inference engine (AGENTS.md)
Maintain a detailed
FORJAMIE.mdat the repo root explaining the whole project in plain language for future reference
Files:
FORJAMIE.md
🪛 ast-grep (0.42.1)
packages/agent-design-engine/src/token-contract.ts
[warning] 122-122: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(${cssVariable}\\s*:)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
[warning] 137-137: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b${category}\\s*:\\s*buildModeMap\\("${category}"\\))
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Actions: CI
packages/cloudflare-template/src/worker/widget-manifest.generated.ts
[error] 1-1: generated-source: tracked generated-source files were stale. Review the regenerated files and rerun pnpm generated-source:check (including this file).
🪛 LanguageTool
docs/specs/2026-04-30-agent-design-prepare-north-star-spec.md
[style] ~267-~267: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...t stops and follows openDecisions. 3. If a proposal is required, the agent may r...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
docs/plans/2026-04-30-agent-design-prepare-north-star-plan.md
[style] ~183-~183: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...rceDigests, and ruleSourceDigests`. - Add a positive fixture path that can normal...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~441-~441: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...gests, or rule digests are rejected. 7. Add or update one positive prepare fixture ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🪛 markdownlint-cli2 (0.22.1)
docs/guides/AGENT_DESIGN_WORKFLOW.md
[warning] 27-27: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 35-35: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 36-36: Ordered list item prefix
Expected: 2; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 37-37: Ordered list item prefix
Expected: 3; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
[warning] 38-38: Ordered list item prefix
Expected: 4; Actual: 6; Style: 1/2/3
(MD029, ol-prefix)
[warning] 39-39: Ordered list item prefix
Expected: 5; Actual: 7; Style: 1/2/3
(MD029, ol-prefix)
[warning] 40-40: Ordered list item prefix
Expected: 6; Actual: 8; Style: 1/2/3
(MD029, ol-prefix)
[warning] 41-41: Ordered list item prefix
Expected: 7; Actual: 9; Style: 1/2/3
(MD029, ol-prefix)
🔍 Remote MCP Linear
Additional Context from Linear
Based on a search of Linear, I've gathered relevant project and issue context that provides important background for this PR review:
Related Tracking Issues
This PR corresponds to work tracked under the Agent-native design-system command layer initiative ():
-
Parent Issue (JSC-238): Establishes the goal of creating a unified, agent-native command layer where agents can run
astudio design prepareand receive deterministic routing, state, examples, validation, safety, and recovery guidance before editing UI.- Status: Done (Cycle 3, Apr 26–May 10, 2026)
- Execution model: Seven child execution slices, including the one directly addressed by this PR
- Review evidence: The plan was hardened through adversarial review rounds covering: no public
contextcommand, existing CLI build truth, XOR selector semantics, authored routing JSON ownership, missing-example diagnostics, shared recovery schema, validation command schema, read-only proposal guarantees, and deterministic serialization.
-
Child Issue (JSC-241): "Prepare payload model" — directly relates to the semantic payload hardening in this PR
- Status: Done
- Scope: Add engine types for
astudio.design.prepare.v1, resolve/digest contracts, classify surface scope with safety precedence, fail closed for unknown scope/missing routes, treat missing examples as diagnostics withsafeForAutomaticImplementation: false, emit safety-classed validation commands. - Stop rule: "Do not expose CLI commands until the engine model has deterministic fixtures for protected, warn, exempt, unknown, overlap, and symlink-normalized scopes."
Project Context
- Repository:
https://github.com/jscraik/Design-System.git(local:/Users/jamiecraik/dev/design-system) - Project: design-system (control project for repo-specific bugs, features, maintenance, and releases)
- Team: Jscraik
- Cycle: Cycle 3 (Apr 26–May 10, 2026)
Key Requirements & Constraints
From the parent issue (JSC-238) and child issue (JSC-241), the following validation gates are required before merge:
pnpm -C packages/agent-design-engine type-checkpnpm -C packages/agent-design-engine testpnpm agent-design:boundariesgit diff --check- Deterministic fixtures for protected, warn, exempt, unknown, overlap, and symlink-normalized scopes must exist
- Schema validation must enforce required fields for
astudio.design.prepare.v1,,,
🔇 Additional comments (5)
packages/cloudflare-template/src/worker/widget-manifest.generated.ts (1)
1-145: Fix broken generated-source check: missingtsxdependency in platforms/web/apps/web.The
pnpm generated-source:checkis failing during the registry generation step, not due to stale generated files. The error is:Error [ERR_MODULE_NOT_FOUND]: Cannot find package 'tsx' imported from /home/jailuser/git/platforms/web/apps/web/This indicates that
node_modulesis missing ortsxis not installed inplatforms/web/apps/web. Ensure dependencies are installed in that workspace before running the generated-source check.> Likely an incorrect or invalid review comment.docs/design-system/GOLD_EXAMPLES.md (1)
40-46: Deferred-category proof requirements are now explicit and contract-aligned.The updated criteria are clear and match the prepare-evidence enforcement model.
docs/design-system/GOLD_EXAMPLES.json (2)
156-190: Promotion prerequisites are now sufficiently concrete for deferred categories.These additions improve evidence quality and align with the protected-fixture promotion bar.
1-194: JSON syntax is valid.No syntax errors detected in this contract file.
FORJAMIE.md (1)
213-214: Good clarification of wrapper vs contract semantics.The distinction between
pnpm --silent agent-design:prepare ...(build-backed wrapper) andastudio design prepare(read-only contract) is clear and actionable.
Why: review and CI surfaced reliability gaps in the agent-design prepare wrapper, evidence gate, token contract, masking, generated manifests, and guidance migration lock checks. What: centralize the prepare prebuild chain, make skill-ingestion rebuild dist reliably, fail closed on malformed prepare evidence, validate semantic token role backing, redact malformed token-contract payload branches, wrap prepare source I/O, refresh generated widget manifests, and clarify the agent workflow docs. Impact/Risk: Low runtime risk; affects local agent-design tooling and deterministic validation paths. Rollback by reverting this commit if wrapper behavior regresses. Validation: - pnpm install --frozen-lockfile --ignore-scripts -> pass - pnpm agent-design:cli:prebuild -> pass - pnpm -C packages/agent-design-engine test -> pass - pnpm -C packages/cli test -> pass - pnpm -C packages/design-system-guidance build -> pass - pnpm --silent agent-design:prepare:smoke >/tmp/agent-design-smoke.json && jq -e ".data.kind == \"astudio.design.prepare.v1\" and .data.safeForAutomaticImplementation == true" /tmp/agent-design-smoke.json >/dev/null -> pass - pnpm --silent agent-design:prepare:changed -- --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx -> pass - pnpm generated-source:check -> pass - pnpm test:policy -> pass - pnpm docs:lint -> pass - bash scripts/validate-codestyle.sh -> pass - pnpm check -> pass - test -f memory.json && jq -e .meta.version == "1.0" and (.preamble.bootstrap | type == "boolean") and (.preamble.search | type == "boolean") and (.entries | type == "array") memory.json >/dev/null -> pass - git diff --check -> pass Co-authored-by: Codex <noreply@openai.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8ec84af520
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Why: GitHub reran the existing governance workflow with the original pull-request event payload, so it did not see the corrected PR body governance sections. What: Add an empty trigger commit so pull_request synchronize evaluates the updated PR evidence. Impact/Risk: No repository files change; this only refreshes CI state for PR #159. Validation: not run (empty trigger commit; no files changed). Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/cli/src/utils/mask.ts (1)
145-147:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMalformed
allowedRolesprimitive entries can still leak raw values.
allowedRolesentries are always passed intomaskPublicDesignTokenRole(). For non-object entries, that path falls back tomaskValueRecursive(), which returns strings unchanged, so malformed payloads likeallowedRoles: ["secret-token"]are not redacted.Suggested hardening patch
for (const [key, val] of Object.entries(value)) { if (key === "allowedRoles" && Array.isArray(val)) { - result[key] = val.map((role) => maskPublicDesignTokenRole(role, masks, inDebugMode)); + result[key] = val.map((role) => + role !== null && typeof role === "object" && !Array.isArray(role) + ? maskPublicDesignTokenRole(role, masks, inDebugMode) + : "[REDACTED]", + ); } else if (publicDesignTokenContractKeys.has(key) && isPublicContractValue(key, val)) { result[key] = val; } else {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/utils/mask.ts` around lines 145 - 147, The allowedRoles mapping must not pass primitives through maskPublicDesignTokenRole (which falls back to maskValueRecursive and leaves strings unchanged); update the allowedRoles branch so each entry is checked: if the item is a non-null object call maskPublicDesignTokenRole(role, masks, inDebugMode), otherwise replace the primitive with an explicit masked placeholder (use the existing masks redaction constant/value from the masks object) so malformed primitives like "secret-token" are not leaked; reference symbols: allowedRoles, maskPublicDesignTokenRole, maskValueRecursive, masks, inDebugMode, publicDesignTokenContractKeys, isPublicContractValue.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 474-497: The normalizer currently trusts an explicit packageScript
even when it disagrees with the command text; update normalizeValidationCommands
to compute inferred = inferPackageScript(command.command) and, if
command.packageScript is provided and inferred is non-null and differs from
command.packageScript, throw a DesignEngineError (include a clear message
referencing both values and reuse the existing error metadata pattern),
otherwise use the provided packageScript or fallback to inferred as before;
reference normalizeValidationCommands, inferPackageScript,
AgentUiRouteValidationCommand and packageScripts to locate where to add the
mismatch check and error.
- Around line 407-435: inferPackageScript currently returns the next token
blindly when it sees "run", allowing malformed commands like "pnpm run" or "pnpm
run -C ..." to be treated as valid; update the "if (token === \"run\")" branch
to validate the next token: get const next = tokens[index + 1]; if next is
missing, startsWith("-") or is in pnpmSubcommands then throw a DesignEngineError
(use the same code "E_DESIGN_VALIDATION_COMMAND_INVALID" and include the command
variable) to reject the malformed pnpm run; otherwise return next. Ensure you
reference inferPackageScript, pnpmSubcommands, DesignEngineError and the command
variable in the change.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 5-11: Module-level constants use lowerCamelCase; rename them to
UPPER_SNAKE_CASE to comply with the "Constants: UPPER_SNAKE_CASE" guideline.
Rename themeSourcePath → THEME_SOURCE_PATH, aliasMapSourcePath →
ALIAS_MAP_SOURCE_PATH, dtcgSourcePath → DTCG_SOURCE_PATH, tokenSourceRefs →
TOKEN_SOURCE_REFS, and semanticRoles → SEMANTIC_ROLES, then update every
reference in this module (and any exports) to use the new names so no
identifiers break.
In `@scripts/check-agent-design-prepare-evidence.mjs`:
- Around line 128-144: The error-path return objects from the prepare() flow
(when JSON parse or validatePreparePayload fails) are missing the openDecisions
field so callers that read result.openDecisions.length can throw; update both
error returns to include an empty openDecisions array (and any other fields the
caller expects, e.g., decisions or metadata) so the shape matches the success
result and the gate loop won’t crash when it does result.openDecisions.length
(refer to prepare(), validatePreparePayload, and the loop that reads
result.openDecisions.length).
---
Duplicate comments:
In `@packages/cli/src/utils/mask.ts`:
- Around line 145-147: The allowedRoles mapping must not pass primitives through
maskPublicDesignTokenRole (which falls back to maskValueRecursive and leaves
strings unchanged); update the allowedRoles branch so each entry is checked: if
the item is a non-null object call maskPublicDesignTokenRole(role, masks,
inDebugMode), otherwise replace the primitive with an explicit masked
placeholder (use the existing masks redaction constant/value from the masks
object) so malformed primitives like "secret-token" are not leaked; reference
symbols: allowedRoles, maskPublicDesignTokenRole, maskValueRecursive, masks,
inDebugMode, publicDesignTokenContractKeys, isPublicContractValue.
🪄 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: Pro
Run ID: 1f4fb521-f612-4e9b-8c5e-d649a9660abb
📒 Files selected for processing (12)
FORJAMIE.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdpackage.jsonpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/src/utils/mask.tspackages/cli/tests/cli.test.mjspackages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tspackages/skill-ingestion/package.jsonscripts/check-agent-design-prepare-evidence.mjs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (23)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/skill-ingestion/package.jsonpackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackage.jsonpackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjsdocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.tsFORJAMIE.md
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
packages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjsdocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/cli/src/utils/mask.tspackages/skill-ingestion/package.jsonpackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackage.jsonpackages/agent-design-engine/src/prepare.tsFORJAMIE.md
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome (
biome.json) for JS/TS formatting and keep files formatted viapnpm format
Files:
packages/design-system-guidance/src/core.tspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx}: Explicit types at all public API boundaries (functions, modules, React props)
Use concrete types or unknown + narrowing instead of any (forbidden pattern: : any, as any, Promise, Record<string, any>)
Unsafe type assertions (value as unknown as T) forbidden; use type guards or schema validation instead
//@ts-ignoreand //@ts-nocheckare forbidden; use //@ts-expect-error-- reason + ticket instead
Unsafe 'as SomeType' without runtime guard is forbidden; use guard function or schema validator
JSON.parse() and Response.json() return any; boundary mitigation MUST use schema validation (Zod/Valibot) or typed parser helper
Files:
packages/design-system-guidance/src/core.tspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
Tauri UI-facing state MUST be deterministic and testable
Files:
packages/design-system-guidance/src/core.tspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Add a Table of Contents for documentation files
Files:
docs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.md
**/*.{md,mdx,adoc,rst}
📄 CodeRabbit inference engine (CODESTYLE.md)
All docs and long-form prose MUST be linted with Vale (scope: **/.md, **/.mdx, **/.adoc, **/.rst)
Files:
docs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.md
packages/cli/**/*.{sh,bash,js,ts,tsx,py}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/**/*.{sh,bash,js,ts,tsx,py}: Always use--agentflag when invoking aStudio CLI to enable intent-over-syntax parsing and detailed error messages
When using aStudio tokens command, validate tokens first, preview changes with--dry-run, then execute with--write --execflags
Files:
packages/cli/src/utils/mask.ts
packages/cli/**/*.{sh,bash,js,ts,tsx,py,yml,yaml}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/**/*.{sh,bash,js,ts,tsx,py,yml,yaml}: Start with--dry-runflag before executing commands that modify files or systems
Use--execsafety flag when running external processes like dev servers, build tools, or test runners
Use--writesafety flag when aStudio commands need to create or modify files
Use--networksafety flag when aStudio commands need to make HTTP or network requests
Files:
packages/cli/src/utils/mask.ts
packages/cli/**/*.{js,ts,tsx,py}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
packages/cli/**/*.{js,ts,tsx,py}: Check forfix_suggestionfield in error responses and automatically retry with the suggested command
Handle aStudio CLI error codes systematically: E_USAGE (check syntax), E_POLICY (add safety flags), E_VALIDATION (check input format), E_EXEC (check tool availability), E_NETWORK (check connectivity)
Files:
packages/cli/src/utils/mask.ts
packages/cli/**/*.{sh,bash,js,ts,tsx,py,yml,yaml,.env*}
📄 CodeRabbit inference engine (packages/cli/AGENTS.md)
Set
NO_COLOR=1orASTUDIO_COLOR=0environment variable for easier output parsing in automated systems
Files:
packages/cli/src/utils/mask.ts
**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Always verify JSON syntax with
jqor similar tool before declaring JSON fixes completeJSON MUST be valid UTF-8; prefer machine-generated JSON for large files; minimize hand-edited large JSON
Files:
packages/skill-ingestion/package.jsonpackage.json
FORJAMIE.md
📄 CodeRabbit inference engine (AGENTS.md)
Maintain a detailed
FORJAMIE.mdat the repo root explaining the whole project in plain language for future reference
Files:
FORJAMIE.md
🔍 Remote MCP Linear
Additional context found (concise, relevant to PR review)
-
Linear issue JSC-238 ("Agent-native design-system command layer") — canonical plan and review evidence; status: Done. Source plan paths and review artifacts recorded; execution slices list includes the prepare payload work this PR implements.
-
Linear issue JSC-241 ("Agent-native design system: prepare payload model") — scope and validation checklist align with PR changes (engine types for astudio.design.prepare.v1, digesting DESIGN.md/guidance/routing/lifecycle/coverage, fail-closed rules, deterministic JSON serializer). Stop rule: do not expose CLI until deterministic fixtures exist for protected/warn/exempt/unknown/overlap/symlink-normalized scopes. Validation commands listed: pnpm -C packages/agent-design-engine type-check, pnpm -C packages/agent-design-engine test, pnpm agent-design:boundaries, git diff --check. Status: Done.
🔇 Additional comments (4)
packages/design-system-guidance/src/core.ts (2)
371-385: Looks good: lock probing now fails closed.The helper distinguishes
ENOENTfrom other filesystem errors and surfaces a contextualGuidanceErrorfor unexpected lock-check failures, which fixes the previous fail-open behavior.
1637-1644: Looks good: dry-run now re-checks the lock around config reads.The pre- and post-read lock checks close the original gap where a concurrent migration could appear during
readGuidanceConfig()and be missed.package.json (1)
24-24: Looks consistent with the new prepare flow.The prebuild chain and the smoke/lint entrypoints line up with the wrapper-backed prepare contract, and I don't see a correctness issue here.
Also applies to: 42-45, 133-133
FORJAMIE.md (1)
19-29: Docs match the new prepare contract.The updated status, timing guidance, wrapper build-order notes, and evidence-gate entries all line up with the code changes, and I don't see a correctness issue here.
Also applies to: 78-80, 178-178, 213-217, 221-233
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdf3c50710
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Why: PR review found prepare accepted malformed pnpm validation commands and trusted packageScript metadata that could drift from command text. What: parse pnpm run flags, validate -C/--dir scripts against the target package, reject command/packageScript mismatches, keep prepare-evidence failures shape-stable, and align token contract constant names. Validation: pnpm -C packages/agent-design-engine test -> pass Validation: pnpm -C packages/agent-design-engine type-check -> pass Validation: pnpm dlx @biomejs/biome@2.3.11 check packages/agent-design-engine/src/prepare.ts packages/agent-design-engine/src/token-contract.ts packages/agent-design-engine/tests/engine.test.mjs scripts/check-agent-design-prepare-evidence.mjs -> pass Validation: pnpm --silent agent-design:prepare:smoke >/tmp/agent-design-smoke.json && jq -e '.data.kind == "astudio.design.prepare.v1" and .data.safeForAutomaticImplementation == true' /tmp/agent-design-smoke.json -> pass Validation: pnpm --silent agent-design:prepare:changed -- --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx -> pass Validation: git diff --check -> pass Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b09be4b115
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 654-655: The prepare payload is non-deterministic because
startedAt/startedMs (and durationMs at 748-750) are captured inline; remove
ambient time from the serialized contract by not including
startedAt/startedMs/durationMs in buildPreparePayload's output or by injecting a
deterministic clock/perf source into the function (e.g., add a clock/perf
parameter to buildPreparePayload and use that to supply deterministic timestamps
when needed), update callers to pass the injected clock or to omit timing, and
ensure all references to startedAt, startedMs, and durationMs in prepare.ts and
in buildPreparePayload are changed accordingly so serialized payloads remain
stable.
- Around line 573-589: The code currently accepts a caller-supplied
packageScript even when inferPackageScript(command.command) returns undefined
(i.e., the command text is not a script invocation); update the validation in
prepare.ts so that if inferred is undefined and command.packageScript is
provided you reject it (throw a DesignEngineError similar to the existing one,
e.g. code "E_DESIGN_VALIDATION_COMMAND_INVALID"); specifically, in the block
that computes inferred and packageScript (referencing inferPackageScript,
command.packageScript, packageScript, and getPackageScripts) add an early check
that if !inferred && command.packageScript then throw, otherwise continue to
check existence against getPackageScripts.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 118-129: The current assertSemanticRoleThemeTokens(content) uses
content.includes(...) which falsely matches comments and strings; instead parse
the CSS content into a declaration-aware form (e.g., use a CSS parser like
PostCSS or any CSS AST) and verify evidence by inspecting actual declarations
for the custom property names found in SEMANTIC_ROLES (check declaration.prop
=== role.cssVariable or equivalent), throwing
tokenContractAmbiguous(THEME_SOURCE_PATH, ...) only when no real declaration
exists; apply the same change to the other token-evidence check(s) that use
naive includes (the similar block referenced in the review).
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 46fb7dcb-1776-409f-ba3a-7a0e77df4518
📒 Files selected for processing (4)
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/tests/engine.test.mjsscripts/check-agent-design-prepare-evidence.mjs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome (
biome.json) for JS/TS formatting and keep files formatted viapnpm format
Files:
packages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx}: Explicit types at all public API boundaries (functions, modules, React props)
Use concrete types or unknown + narrowing instead of any (forbidden pattern: : any, as any, Promise, Record<string, any>)
Unsafe type assertions (value as unknown as T) forbidden; use type guards or schema validation instead
//@ts-ignoreand //@ts-nocheckare forbidden; use //@ts-expect-error-- reason + ticket instead
Unsafe 'as SomeType' without runtime guard is forbidden; use guard function or schema validator
JSON.parse() and Response.json() return any; boundary mitigation MUST use schema validation (Zod/Valibot) or typed parser helper
Files:
packages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
Tauri UI-facing state MUST be deterministic and testable
Files:
packages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
packages/agent-design-engine/src/token-contract.tsscripts/check-agent-design-prepare-evidence.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
🔍 Remote MCP Linear
Additional relevant context (concise)
-
Linear issue JSC-238 ("Agent-native design-system command layer") — canonical plan and review-evidence checklist for making a deterministic, read-only prepare command the agent-facing contract. Status: Done.
-
Linear issue JSC-241 ("Agent-native design system: prepare payload model") — explicit scope and stop-rule for the prepare payload (types for astudio.design.prepare.v1, digesting DESIGN.md/guidance/routing/lifecycle/coverage, fail-closed rules, deterministic serialization, validation checklist). Status: Done. Useful validation commands listed (pnpm type-check/test, agent-design:boundaries, git diff checks).
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 6 file(s) based on 2 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 6 file(s) based on 2 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/agent-design-engine/src/token-contract.ts (1)
160-163:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse declaration-aware checks for alias-map evidence too.
These
includes(...)probes still pass on commented-out or string-literalbuildModeMap(...)text, sobuildDesignTokenContract()can emit a valid contract even whenpackages/tokens/src/alias-map.tsno longer contains real alias mappings. That weakens the new fail-closed evidence path.Suggested fix
async function assertAliasMapSource(rootDir: string, signal?: AbortSignal): Promise<void> { const content = await readTokenSource(rootDir, ALIAS_MAP_SOURCE_PATH, signal); signal?.throwIfAborted(); + const stripped = content + .replace(/\/\*[\s\S]*?\*\//g, "") + .replace(/\/\/.*$/gm, ""); - if (!/\bexport\s+const\s+tokenAliasMap\b/.test(content)) { + if (!/\bexport\s+const\s+tokenAliasMap\b/.test(stripped)) { throw tokenContractAmbiguous(ALIAS_MAP_SOURCE_PATH, "exported tokenAliasMap"); } for (const category of ["background", "text", "border", "accent", "interactive"]) { - if (!content.includes(`${category}: buildModeMap("${category}")`)) { + if ( + !new RegExp( + `\\b${category}\\s*:\\s*buildModeMap\\(\\s*"${category}"\\s*\\)`, + ).test(stripped) + ) { throw tokenContractAmbiguous(ALIAS_MAP_SOURCE_PATH, `${category} color alias mapping`); } } }As per coding guidelines "validate/sanitize all external inputs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-design-engine/src/token-contract.ts` around lines 160 - 163, The current includes(...) check in buildDesignTokenContract() is brittle because it matches commented-out or string-literal occurrences; instead parse the ALIAS_MAP_SOURCE_PATH content into an AST and perform a declaration-aware search for a CallExpression whose callee is an Identifier named "buildModeMap" with a string literal argument equal to each category ("background","text","border","accent","interactive"); if no such AST call is found for a category, call tokenContractAmbiguous(ALIAS_MAP_SOURCE_PATH, `${category} color alias mapping`) as before. Use a TypeScript/JavaScript parser (e.g., `@babel/parser` or ts-morph) to locate call nodes rather than using content.includes to ensure commented or quoted text is ignored.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 424-443: readPnpmRunScript currently skips tokens in
pnpmRunOptionsWithValues without validating their value, allowing malformed
commands; update the loop in readPnpmRunScript so that when a token is in
pnpmRunOptionsWithValues you verify there is a next token and that the next
token is not missing and does not start with "-" (and is not a pnpmSubcommand) —
if the value is absent or looks like another flag/subcommand, throw
invalidValidationCommand(command, "Validation command does not name a package
script"); keep existing handling for --dir= and --filter= style tokens but
ensure plain "--dir" or "--filter" without a value triggers the same validation
failure.
---
Duplicate comments:
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 160-163: The current includes(...) check in
buildDesignTokenContract() is brittle because it matches commented-out or
string-literal occurrences; instead parse the ALIAS_MAP_SOURCE_PATH content into
an AST and perform a declaration-aware search for a CallExpression whose callee
is an Identifier named "buildModeMap" with a string literal argument equal to
each category ("background","text","border","accent","interactive"); if no such
AST call is found for a category, call
tokenContractAmbiguous(ALIAS_MAP_SOURCE_PATH, `${category} color alias mapping`)
as before. Use a TypeScript/JavaScript parser (e.g., `@babel/parser` or ts-morph)
to locate call nodes rather than using content.includes to ensure commented or
quoted text is ignored.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 551dbaf9-87cf-4747-af5f-8e8befbbb148
📒 Files selected for processing (6)
packages/agent-design-engine/src/index.tspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjstsconfig.base.json
💤 Files with no reviewable changes (1)
- packages/agent-design-engine/src/index.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Always verify JSON syntax with
jqor similar tool before declaring JSON fixes completeJSON MUST be valid UTF-8; prefer machine-generated JSON for large files; minimize hand-edited large JSON
Files:
tsconfig.base.json
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
tsconfig.base.jsonpackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
tsconfig.base.jsonpackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome (
biome.json) for JS/TS formatting and keep files formatted viapnpm format
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx}: Explicit types at all public API boundaries (functions, modules, React props)
Use concrete types or unknown + narrowing instead of any (forbidden pattern: : any, as any, Promise, Record<string, any>)
Unsafe type assertions (value as unknown as T) forbidden; use type guards or schema validation instead
//@ts-ignoreand //@ts-nocheckare forbidden; use //@ts-expect-error-- reason + ticket instead
Unsafe 'as SomeType' without runtime guard is forbidden; use guard function or schema validator
JSON.parse() and Response.json() return any; boundary mitigation MUST use schema validation (Zod/Valibot) or typed parser helper
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
Tauri UI-facing state MUST be deterministic and testable
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
packages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/prepare.ts
🪛 ast-grep (0.42.1)
packages/agent-design-engine/src/token-contract.ts
[warning] 122-122: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((?:^|[{;])\\s*${escaped}\\s*:, "m")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔍 Remote MCP Linear
Additional relevant context found
-
Linear issue JSC-238 ("Agent-native design-system command layer") — canonical plan for making a deterministic, read-only
preparecommand the agent-facing contract; lists execution slices, review-evidence artifacts, and done criteria (plan owned in-repo). Status: Done. -
Linear issue JSC-241 ("Agent-native design system: prepare payload model") — slice focused on engine payload: add types for
astudio.design.prepare.v1, digest DESIGN.md/guidance/routing/lifecycle/coverage, classify surface scope (protected>warn>exempt>unknown), require deterministic JSON serialization, and fail-closed for unknown scope/missing required manifests; includes validation checklist (pnpmtype-check/test, agent-design:boundaries, git diff checks). Status: Done.
🔇 Additional comments (1)
tsconfig.base.json (1)
12-12: No action needed—ignoreDeprecations: "6.0"is valid and JSON is syntactically correct.JSON validation and TypeScript config parsing confirm the setting is supported and properly formatted for the repo's TypeScript version (^5.9.3). No issues found.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f89adbf38
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f89adbf38
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/agent-design-engine/src/prepare.ts (1)
439-440:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFail closed on malformed
--filter/--diroption values in pnpm command parsing.Line 439 and Line 483 still allow malformed options to slip through:
--dir=/--filter=are accepted with empty values, and--filtercan consume another flag as its “value” before script inference continues. That can normalize invalid commands as valid metadata.Suggested patch
function readPnpmRunScript(command: string, tokens: string[], startIndex: number): string { for (let index = startIndex; index < tokens.length; index += 1) { @@ - if (token.startsWith("--dir=") || token.startsWith("--filter=")) { + if (token.startsWith("--dir=") || token.startsWith("--filter=")) { + const value = token.slice(token.indexOf("=") + 1); + if (!value || value.startsWith("-") || pnpmSubcommands.has(value)) { + throw invalidValidationCommand(command, "Validation command does not name a package script"); + } continue; } @@ function inferPackageScript(command: string): InferredPackageScript | undefined { @@ - if (token === "--filter" || token.startsWith("--filter=")) { - if (token === "--filter") { - index += 1; - } + if (token === "--filter" || token.startsWith("--filter=")) { + if (token === "--filter") { + const value = tokens[index + 1]; + if (!value || value.startsWith("-") || pnpmSubcommands.has(value)) { + throw invalidValidationCommand(command, "Validation command does not name a package script"); + } + index += 1; + } else { + const value = token.slice("--filter=".length); + if (!value || value.startsWith("-") || pnpmSubcommands.has(value)) { + throw invalidValidationCommand(command, "Validation command does not name a package script"); + } + } continue; }As per coding guidelines "validate/sanitize all external inputs".
Also applies to: 483-487
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 430-440: CI is failing due to Biome formatting in the throw call
within the token-parsing loop (see the throw invalidValidationCommand(...)
statements used when validating package script tokens and checks involving
pnpmSubcommands and token.startsWith); run the project formatter (pnpm format or
Biome autofix) to reformat this block and commit the resulting changes so the
throw calls and surrounding lines match the repo's style rules.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a3ba5741-311d-44cc-9ebf-5c0ac1bf9e39
📒 Files selected for processing (1)
packages/agent-design-engine/src/prepare.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome (
biome.json) for JS/TS formatting and keep files formatted viapnpm format
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx}: Explicit types at all public API boundaries (functions, modules, React props)
Use concrete types or unknown + narrowing instead of any (forbidden pattern: : any, as any, Promise, Record<string, any>)
Unsafe type assertions (value as unknown as T) forbidden; use type guards or schema validation instead
//@ts-ignoreand //@ts-nocheckare forbidden; use //@ts-expect-error-- reason + ticket instead
Unsafe 'as SomeType' without runtime guard is forbidden; use guard function or schema validator
JSON.parse() and Response.json() return any; boundary mitigation MUST use schema validation (Zod/Valibot) or typed parser helper
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
Tauri UI-facing state MUST be deterministic and testable
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
packages/agent-design-engine/src/prepare.ts
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
packages/agent-design-engine/src/prepare.ts
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
packages/agent-design-engine/src/prepare.ts
🪛 GitHub Actions: CI
packages/agent-design-engine/src/prepare.ts
[error] 430-440: Biome formatter check failed. Formatter would have reflowed a long throw invalidValidationCommand(...) call into a multi-line argument list.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 2 file(s) based on 1 unresolved review comment. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 2 file(s) based on 1 unresolved review comment. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/agent-design-engine/src/prepare.ts (2)
742-748:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing examples still serialize as safe for automatic implementation.
routeDecisions()intentionally downgradesE_DESIGN_ROUTE_EXAMPLE_MISSINGto a warning, butsafeForAutomaticImplementationonly checks for error-free decisions plus known scope. A route with missing examples therefore becomessafeForAutomaticImplementation: true, which is looser than the prepare contract described in the PR. Please treatE_DESIGN_ROUTE_EXAMPLE_MISSINGas a blocker forsafeForAutomaticImplementationeven if it stays a warning inopenDecisions.Suggested change
const validationCommands = recommendedRoutes.flatMap((entry) => entry.validationCommands); const openDecisions = routeDecisions(routeResult, surfaceScope); const ok = openDecisions.every((decision) => decision.severity !== "error"); + const safeForAutomaticImplementation = + ok && + surfaceScope !== "unknown" && + !openDecisions.some((decision) => decision.code === "E_DESIGN_ROUTE_EXAMPLE_MISSING"); return { @@ - safeForAutomaticImplementation: ok && surfaceScope !== "unknown", + safeForAutomaticImplementation,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-design-engine/src/prepare.ts` around lines 742 - 748, The current safeForAutomaticImplementation flag is set using ok and surfaceScope but doesn't exclude the specific missing-example condition; update the logic after computing openDecisions (from routeDecisions(routeResult, surfaceScope)) to also detect any decision with code "E_DESIGN_ROUTE_EXAMPLE_MISSING" (or the corresponding enum/constant) and treat that as a blocker by requiring that no such decision exists when setting safeForAutomaticImplementation on the returned object; keep routeDecisions’ downgrade to warning but add this explicit check (use openDecisions.some(...) or every(...) to test for the code) so safeForAutomaticImplementation is false if that code is present.
679-703:⚠️ Potential issue | 🟠 Major | ⚡ Quick winKeep required-source failures deterministic.
Both
Promise.all(...)blocks here can reject from any missing/bad file first, so the same broken checkout may surface differentDesignEngineErrorcodes/messages across runs. That undercuts the PR’s deterministic fail-closed behavior. Read/validate these required sources in a fixed priority order, or collect all results first and then raise the first failure from a stable ordering.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-design-engine/src/prepare.ts` around lines 679 - 703, The current parallel Promise.all calls for readPrepareSource and digestFile can yield non-deterministic first failures; change both to deterministic checks by using a fixed-priority order: either sequentially await readPrepareSource for [designPath, guidancePath, ...] and throw the first failure as a DesignEngineError, or run Promise.allSettled on the same ordered array and then inspect results in that fixed order to throw the first rejection (wrapping/propagating the original DesignEngineError or creating one with the appropriate code like "E_DESIGN_SOURCE_IO" / "E_DESIGN_GUIDANCE_IO"); do the same for the digestFile calls (use digestFile in the same ordered list and pick the first failure deterministically), and only if all reads succeed proceed to JSON.parse/guidance = parseGuidanceConfig(parsedGuidance) as before.
♻️ Duplicate comments (1)
packages/agent-design-engine/src/token-contract.ts (1)
150-188:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftValidate every advertised role against non-theme token evidence.
assertSemanticRoleThemeTokens()now proves the CSS variables exist intheme.css, but the other two evidence sources are still only checked at a skeleton level:assertAliasMapSource()validates five hard-coded categories andassertDtcgSource()only requirescolor.background. The contract still exportssurface.card,text.secondary,border.strong, andstatus.*, so stale/generated token artifacts can pass even when those roles are not actually backed outside the theme file. Please derive the required alias/DTCG evidence fromSEMANTIC_ROLESand fail closed when any emitted role lacks backing in those sources.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/agent-design-engine/src/token-contract.ts` around lines 150 - 188, assertAliasMapSource and assertDtcgSource currently only validate a fixed skeleton instead of verifying every semantic role; update both to derive required checks from SEMANTIC_ROLES and fail when any emitted role lacks backing. In assertAliasMapSource, compute the set of unique top-level categories needed by SEMANTIC_ROLES (e.g., "background","text","border","accent","interactive" as derived from each role name), read content and ensure each derived category has the expected alias mapping (use the same string pattern `${category}: buildModeMap("${category}")`), and throw tokenContractAmbiguous(ALIAS_MAP_SOURCE_PATH, ...) for any missing category. In assertDtcgSource, after parsing the DTCG JSON, iterate SEMANTIC_ROLES and for each role ensure the corresponding DTCG path exists (e.g., parsed.color[category][tokenName] or parsed.color[category] depending on role shape); if any required group or token is absent, throw tokenContractAmbiguous(DTCG_SOURCE_PATH, ...) describing the missing DTCG evidence. Update both functions (assertAliasMapSource, assertDtcgSource) to use SEMANTIC_ROLES as the single source of truth and fail closed on any missing evidence.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 195-199: The three source assertion calls (assertThemeSource,
assertAliasMapSource, assertDtcgSource) are currently raced with Promise.all
causing nondeterministic failure ordering; change this to deterministic
validation by either running them sequentially in a fixed order (await
assertThemeSource(rootDir, signal); await assertAliasMapSource(rootDir, signal);
await assertDtcgSource(rootDir, signal);) or use Promise.allSettled([...]) and,
after settlement, inspect results and throw the highest-priority rejection
(choose priority by source: theme → alias map → dtcg) so the contract fails
closed with a predictable error.
---
Outside diff comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 742-748: The current safeForAutomaticImplementation flag is set
using ok and surfaceScope but doesn't exclude the specific missing-example
condition; update the logic after computing openDecisions (from
routeDecisions(routeResult, surfaceScope)) to also detect any decision with code
"E_DESIGN_ROUTE_EXAMPLE_MISSING" (or the corresponding enum/constant) and treat
that as a blocker by requiring that no such decision exists when setting
safeForAutomaticImplementation on the returned object; keep routeDecisions’
downgrade to warning but add this explicit check (use openDecisions.some(...) or
every(...) to test for the code) so safeForAutomaticImplementation is false if
that code is present.
- Around line 679-703: The current parallel Promise.all calls for
readPrepareSource and digestFile can yield non-deterministic first failures;
change both to deterministic checks by using a fixed-priority order: either
sequentially await readPrepareSource for [designPath, guidancePath, ...] and
throw the first failure as a DesignEngineError, or run Promise.allSettled on the
same ordered array and then inspect results in that fixed order to throw the
first rejection (wrapping/propagating the original DesignEngineError or creating
one with the appropriate code like "E_DESIGN_SOURCE_IO" /
"E_DESIGN_GUIDANCE_IO"); do the same for the digestFile calls (use digestFile in
the same ordered list and pick the first failure deterministically), and only if
all reads succeed proceed to JSON.parse/guidance =
parseGuidanceConfig(parsedGuidance) as before.
---
Duplicate comments:
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 150-188: assertAliasMapSource and assertDtcgSource currently only
validate a fixed skeleton instead of verifying every semantic role; update both
to derive required checks from SEMANTIC_ROLES and fail when any emitted role
lacks backing. In assertAliasMapSource, compute the set of unique top-level
categories needed by SEMANTIC_ROLES (e.g.,
"background","text","border","accent","interactive" as derived from each role
name), read content and ensure each derived category has the expected alias
mapping (use the same string pattern `${category}:
buildModeMap("${category}")`), and throw
tokenContractAmbiguous(ALIAS_MAP_SOURCE_PATH, ...) for any missing category. In
assertDtcgSource, after parsing the DTCG JSON, iterate SEMANTIC_ROLES and for
each role ensure the corresponding DTCG path exists (e.g.,
parsed.color[category][tokenName] or parsed.color[category] depending on role
shape); if any required group or token is absent, throw
tokenContractAmbiguous(DTCG_SOURCE_PATH, ...) describing the missing DTCG
evidence. Update both functions (assertAliasMapSource, assertDtcgSource) to use
SEMANTIC_ROLES as the single source of truth and fail closed on any missing
evidence.
🪄 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: Pro
Run ID: 33bf16c6-3d65-4c7f-989f-2e01b3004d26
📒 Files selected for processing (2)
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome (
biome.json) for JS/TS formatting and keep files formatted viapnpm format
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx}: Explicit types at all public API boundaries (functions, modules, React props)
Use concrete types or unknown + narrowing instead of any (forbidden pattern: : any, as any, Promise, Record<string, any>)
Unsafe type assertions (value as unknown as T) forbidden; use type guards or schema validation instead
//@ts-ignoreand //@ts-nocheckare forbidden; use //@ts-expect-error-- reason + ticket instead
Unsafe 'as SomeType' without runtime guard is forbidden; use guard function or schema validator
JSON.parse() and Response.json() return any; boundary mitigation MUST use schema validation (Zod/Valibot) or typed parser helper
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
Tauri UI-facing state MUST be deterministic and testable
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.ts
🪛 ast-grep (0.42.1)
packages/agent-design-engine/src/token-contract.ts
[warning] 120-120: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((?:^|[{;])\\s*${escaped}\\s*:, "m")
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔍 Remote MCP Linear
Perfect. Now I have comprehensive context. Let me compile the findings:
Additional Context Summary
Based on my search of the Linear project management system, this PR (GitHub PR #159) implements the final hardening phase of a larger initiative tracked across multiple Linear issues. Here's the relevant additional context:
Strategic Context
Parent Initiative: Agent-native design-system command layer
Agents should be able to run one happy-path prepare command and receive deterministic routing, state, example, validation, safety, and recovery guidance before editing UI.
Execution Breakdown
The work is organized into seven execution slices:
- Agent-native design system: contract wiring — Making
DESIGN.mdand.design-system-guidance.jsona visibly connected contract - Agent-native design system: routing table contract — Creating machine-readable component routes
- Agent-native design system: prepare payload model — Building the semantic payload behind
astudio design prepare - Agent-native design system: read-only CLI commands — Exposing the engine through the public interface
All seven child issues are marked as "Done," with JSC-238 (the parent) completed on 2026-04-29.
Key Implementation Requirements
From JSC-241 (Prepare Payload Model):
- Engine must fail closed for unknown scope, missing route/lifecycle/coverage, selector conflict, and ambiguous route.
- Treat missing examples as diagnostics with
safeForAutomaticImplementation: false, not command-fatal errors. - Emit safety-classed validation commands and runtime budget telemetry with threshold assertions.
From JSC-242 (Read-Only CLI Commands):
prepareis the only public full context-pack entrypoint; do not add or advertiseastudio design context.- Add
prepare,components,coverage, and preview-onlypropose-abstractiondesign command kinds.
Related Upstream Work
The PR builds on or extends changes from three related issues:
- JSC-74: Expand design-system guidance into a professional UI contract for agents (completed 2026-04-25)
- JSC-77: Add professional-finish review loops with focus consistency, visual QA, and gold-standard references (completed 2026-04-26)
- JSC-215: M6: Run release-readiness verification for Agent Design Engine (completed 2026-04-24)
Validation Checkpoints
The PR documentation indicates comprehensive local verification including:
- Engine and CLI test suites
- Design-system-guidance build
- Prepare smoke and changed-surface gate validation
- Generated-source freshness checks (widget/Cloudflare manifest regeneration)
- Policy/docs/lint checks, pnpm checks, and schema validation
Risk is assessed as medium for agent-design contract surface with rollback via PR reversion or CI step removal.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d6b56b6a9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Why: PR CI can check out a shallow or moved base ref where origin/main...HEAD has no merge base even though both endpoint trees are available.
What: fall back from three-dot diff to direct base/head diff only for Git's no-merge-base case, remove the stale prepare timing schema field, and align CLI schema tests plus FORJAMIE.
Validation: pass - cat packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json | jq . >/dev/null
Validation: pass - git diff --check
Validation: pass - AGENT_DESIGN_PREPARE_BASE=origin/main pnpm --silent agent-design:prepare:changed
Validation: pass - ORPHAN_BASE=$(printf 'orphan base for prepare evidence fallback\n' | git commit-tree "$(git rev-parse HEAD^{tree})") && AGENT_DESIGN_PREPARE_BASE="$ORPHAN_BASE" pnpm --silent agent-design:prepare:changed
Validation: pass - pnpm --silent format:check scripts/check-agent-design-prepare-evidence.mjs packages/cli/tests/cli.test.mjs packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json FORJAMIE.md
Validation: pass - pnpm -C packages/cli test
Validation: pass - pnpm docs:lint
Co-authored-by: Codex <noreply@openai.com>
Why: PR #159 still had unresolved review blockers for nondeterministic token-contract failure ordering and valid pnpm option parsing. What: run token source assertions in a fixed order, parse -F as pnpm's short filter selector, consume --resume-from before script inference, and add regression coverage for both pnpm parser cases. Validation: pass - pnpm --silent format:check packages/agent-design-engine/src/prepare.ts packages/agent-design-engine/src/token-contract.ts packages/agent-design-engine/tests/engine.test.mjs FORJAMIE.md Validation: pass - pnpm -C packages/agent-design-engine test Validation: pass - pnpm docs:lint Validation: pass - git diff --check Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 69d1b6daf1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@FORJAMIE.md`:
- Line 19: Update the human-readable "Last updated:" line and every
machine-readable last_updated field to 2026-05-02; specifically change the
"**Last updated:** 2026-04-30" entry and any "last_updated" YAML/JSON-style
fields (search for the token last_updated and the string "Last updated:") so
both the visible and machine-readable dates match the new 2026-05-02 entry
mentioned in the change log (also update the other occurrences flagged around
lines 221-224 and 462).
In `@packages/cli/tests/cli.test.mjs`:
- Around line 563-565: The test mutates recommendedRoutes[0] without
guaranteeing it exists; before setting
invalidRoute.data.recommendedRoutes[0].unexpectedFutureDrift, add an assertion
that payload.data.recommendedRoutes is an array with length > 0 (e.g.,
assert.ok(Array.isArray(payload.data.recommendedRoutes) &&
payload.data.recommendedRoutes.length > 0)) so the test fails with a clear
message if the fixture lacks routes; locate and update the area around the
invalidRoute/cloneJson usage and the mutation of recommendedRoutes[0].
In `@scripts/check-agent-design-prepare-evidence.mjs`:
- Around line 43-45: The helper resultDetail currently calls .trim() on
(result.stderr || result.stdout) which can be null on spawn failures; update
resultDetail to safely handle null/undefined by falling back to an empty string
(e.g. use a null-coalescing fallback) before trimming, and apply the same guard
to the other places that call .trim() around the area corresponding to lines
171-172 so spawn failures don't throw when accessing stderr/stdout.
- Around line 31-36: The run function uses spawnSync without a timeout which can
hang CI; update the spawnSync invocation inside run to include a timeout (e.g.,
30000 ms) and a killSignal (e.g., 'SIGKILL') in the options so stuck child
processes are terminated, and add handling after the call to detect a timeout
(check result.error.code === 'ETIMEOUT' or result.signal) and fail/exit/log
appropriately so the script doesn't block indefinitely; reference the run
function and the spawnSync call when making the change.
🪄 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: Pro
Run ID: 096dad7b-8600-4fcc-b6f2-59f4357c3975
📒 Files selected for processing (4)
FORJAMIE.mdpackages/cli/tests/cli.test.mjspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonscripts/check-agent-design-prepare-evidence.mjs
📜 Review details
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjs
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjsFORJAMIE.md
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjsFORJAMIE.mdpackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json
FORJAMIE.md
📄 CodeRabbit inference engine (AGENTS.md)
Maintain a detailed
FORJAMIE.mdat the repo root explaining the whole project in plain language for future reference
Files:
FORJAMIE.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Add a Table of Contents for documentation files
Files:
FORJAMIE.md
**/*.{md,mdx,adoc,rst}
📄 CodeRabbit inference engine (CODESTYLE.md)
All docs and long-form prose MUST be linted with Vale (scope: **/.md, **/.mdx, **/.adoc, **/.rst)
Files:
FORJAMIE.md
**/*.json
📄 CodeRabbit inference engine (AGENTS.md)
Always verify JSON syntax with
jqor similar tool before declaring JSON fixes completeJSON MUST be valid UTF-8; prefer machine-generated JSON for large files; minimize hand-edited large JSON
Files:
packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json
🔍 Remote MCP Linear
Additional relevant context for PR #159 (concise)
- This work is part of a completed initiative "Agent-native design-system command layer" (parent issue JSC-238 — status: Done).
- Prepare-payload and CLI slices completed and directly related to the PR:
- JSC-241 "Prepare Payload Model" — Slice 3; Done. (covers fail-closed behavior, payload semantics)
- JSC-242 "Read-only CLI Commands" — Slice 4; Done. (expose engine through
astudio designwithout adding public write commands) - JSC-240 "Routing Table Contract" — Slice 2; Done. (machine-readable component routes used by prepare)
- JSC-239 "Contract Wiring" — Slice 1; Done. (connects DESIGN.md and .design-system-guidance.json)
- Release-readiness and validation coverage for the engine were tracked and completed (JSC-215 "M6: Run release-readiness verification for Agent Design Engine" — Done), indicating the author ran the documented local verification matrix referenced in the PR summary.
- Other related completed slices that the PR builds on: gold-example inventory (JSC-244), actionable remediation output (JSC-243), and abstraction proposal gate (JSC-245) — all marked Done. These signal coordinated changes across docs, examples, and policy that the PR references.
Sources:
- Linear issue list results returned for queries matching the agent-design prepare work (JSC-238, JSC-241, JSC-242, JSC-240, JSC-239, JSC-215, JSC-244, JSC-243, JSC-245).
🔇 Additional comments (1)
packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json (1)
376-503: Strong schema hardening for the prepare contract.The conditional
$reftopreparePayloadplus strict nested definitions is a solid fail-closed improvement and aligns with the evidence-gate expectations.Also applies to: 682-704
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80dff42d9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const background = (color as { background?: unknown }).background; | ||
| if (!background || typeof background !== "object" || Array.isArray(background)) { | ||
| throw tokenContractAmbiguous(DTCG_SOURCE_PATH, "background token group"); | ||
| } |
There was a problem hiding this comment.
assertDtcgSource only verifies that color.background exists, but buildDesignTokenContract always publishes semantic roles for text, border, accent, and status as if they are all source-backed. If those token groups are removed or renamed in index.dtcg.json, prepare still succeeds and advertises unsupported roles, which can mislead downstream agents into generating UI that relies on nonexistent canonical tokens. The DTCG validation should assert coverage for every advertised semantic role category (or explicit role mappings), not just background.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Implemented and pushed a follow-up fix for the latest @codex fix it review thread.
Summary
- Updated pnpm validation-command parsing so additional value-taking
pnpm runflags are consumed before script inference (--changed-files-ignore-pattern,--changed-files-with-root, and--test-pattern, including inline--flag=valueforms). This prevents valid commands from being misparsed as script names and incorrectly failing withE_DESIGN_VALIDATION_COMMAND_INVALID. - Updated
FORJAMIE.mdwith a same-change-set recent-change entry documenting the parser follow-up and impact, per repo doc gate rules.
Testing
- ✅
pnpm -C packages/agent-design-engine test
Also completed:
- ✅ Commit:
8606e2e(fix: parse value-taking pnpm run options before script inference) - ✅ PR metadata recorded via
make_prtool.
Why: PR #159 still had review blockers around pnpm option parsing, stale project freshness metadata, fixture diagnostics, and unbounded child processes in the changed-surface prepare gate. What: consume pnpm --loglevel values before script inference, add fixture coverage, make the prepare evidence child process wrapper timeout-bounded and spawn-safe, update FORJAMIE freshness fields, and assert route fixture presence before nested schema-drift mutation. Validation: pass - pnpm --silent format:check scripts/check-agent-design-prepare-evidence.mjs packages/agent-design-engine/src/prepare.ts packages/agent-design-engine/tests/engine.test.mjs packages/cli/tests/cli.test.mjs FORJAMIE.md Validation: pass - pnpm -C packages/agent-design-engine test Validation: pass - pnpm -C packages/cli test Validation: pass - pnpm docs:lint Validation: pass - git diff --check Validation: pass - AGENT_DESIGN_PREPARE_BASE=origin/main pnpm --silent agent-design:prepare:changed Validation: pass - ORPHAN_BASE=$(printf 'orphan base for prepare evidence fallback\n' | git commit-tree "$(git rev-parse HEAD^{tree})") && AGENT_DESIGN_PREPARE_BASE="$ORPHAN_BASE" pnpm --silent agent-design:prepare:changed Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: dd0b64edc9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (token.startsWith("-")) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
The readPnpmRunScript parser skips unknown flags but does not skip their required value token, so valid pnpm run commands can be misparsed. I verified against pnpm 10.33.0 help (pnpm run --help), which lists value-taking options such as --changed-files-ignore-pattern <pattern> and --test-pattern <pattern>; with a command like pnpm run --changed-files-ignore-pattern "**/README.md" lint, this code will treat **/README.md as the script name and then throw E_DESIGN_VALIDATION_COMMAND_INVALID, blocking otherwise valid read-only validation commands.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Summary
- Addressed the
@codex fix itrequest ontoken-contract.tsby making DTCG validation derive required color groups from the semantic roles being advertised, instead of only checkingcolor.background. This now fail-closes if any mapped group is missing. - Added a deterministic role-prefix→DTCG-group mapping (
surface/text/border/focus/accent/status) used by the new validation logic to keep token-contract evidence aligned with exported semantic roles. - Updated
FORJAMIE.mdin the same change-set with a new 2026-05-02 recent-change note, per repository “Living Map” policy.
Testing
- ✅
pnpm -C packages/agent-design-engine test
Why: PR #159 was merge-blocked after the Agent Design Prepare hardening from main landed. What: Kept main's hardened prepare parser, token contract, schema, masking, guidance, and docs while preserving the PR-specific changed-surface prepare evidence gate, CI wiring, README/workflow guidance, and CLI coverage. Validation: pass - pnpm -C packages/agent-design-engine test Validation: pass - pnpm -C packages/cli test Validation: pass - AGENT_DESIGN_PREPARE_BASE=origin/main pnpm --silent agent-design:prepare:changed Validation: pass - pnpm docs:lint Validation: pass - pnpm --silent agent-design:lint Validation: pass - git diff --check Validation: pass - git diff --cached --check Co-authored-by: Codex <noreply@openai.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1458012f45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const commands = [ | ||
| ["diff", "--name-only", "--diff-filter=ACMRT", "--cached", "HEAD"], | ||
| ["diff", "--name-only", "--diff-filter=ACMRT", "HEAD"], | ||
| ]; |
There was a problem hiding this comment.
When --base/AGENT_DESIGN_PREPARE_BASE is not provided, the gate only diffs --cached HEAD and HEAD, which captures staged/unstaged edits but misses UI changes that are already committed on the current branch. In the common local handoff case (clean working tree after committing), this returns no surfaces and exits successfully, so pnpm agent-design:prepare:changed can silently skip prepare validation for the PR's actual UI changes.
Useful? React with 👍 / 👎.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/tests/cli.test.mjs`:
- Around line 675-682: The test reads the gate script into gateSource and
asserts protected UI paths but misses the widgets prefix; update the assertions
in the test (where gateSource is read and assert.match is called) to also assert
that gateSource contains "packages/widgets/src/" (add an
assert.match(gateSource, /packages\/widgets\/src\//) alongside the existing
UI/platform assertions) so the gate regression covers the widgets protected
surface.
In `@scripts/check-agent-design-prepare-evidence.mjs`:
- Around line 144-167: The validatePreparePayload function currently only checks
that data.validationCommands and data.openDecisions are arrays; update
validatePreparePayload to also iterate over data.validationCommands and
data.openDecisions and validate each item shape: for validationCommands ensure
each item is an object and contains required fields (e.g., packageScript as
string, expectedOutcome as string/enum, timeoutClass as string or number per
contract) and for openDecisions ensure each item is an object and contains the
required decision fields (e.g., id/title and any other required metadata)
returning a descriptive error string if any item is missing or has the wrong
type; keep all changes inside validatePreparePayload and reference
data.validationCommands and data.openDecisions when constructing the error
messages.
- Around line 131-138: The toRepoRelative function trusts path.resolve() which
is lexical and can be tricked by symlinks; update toRepoRelative to resolve the
real filesystem paths (e.g., using fs.realpathSync or fs.promises.realpath) for
both repoRoot and the target file before computing relativity, then reject and
exit if the real relative path begins with ".." or is absolute; also catch and
surface realpath errors via error(...) before exiting to avoid silently
accepting symlinked out-of-repo files.
🪄 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: Pro
Run ID: 380152d4-c68c-419f-a409-a236299e31ef
📒 Files selected for processing (6)
FORJAMIE.mdREADME.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/agent-design-engine/src/token-contract.tspackages/cli/tests/cli.test.mjsscripts/check-agent-design-prepare-evidence.mjs
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Socket Security: Pull Request Alerts
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (18)
FORJAMIE.md
📄 CodeRabbit inference engine (AGENTS.md)
Maintain a detailed
FORJAMIE.mdat the repo root explaining the whole project in plain language for future reference
Files:
FORJAMIE.md
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Add a Table of Contents for documentation files
Files:
FORJAMIE.mdREADME.mddocs/guides/AGENT_DESIGN_WORKFLOW.md
**/*.{md,mdx,adoc,rst}
📄 CodeRabbit inference engine (CODESTYLE.md)
All docs and long-form prose MUST be linted with Vale (scope: **/.md, **/.mdx, **/.adoc, **/.rst)
Files:
FORJAMIE.mdREADME.mddocs/guides/AGENT_DESIGN_WORKFLOW.md
**/*.{js,ts,tsx,jsx,md,mdx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Waivers across ESLint, Vale, Semgrep, Clippy, CI checks MUST include: rule ID, reason, ticket/issue reference, expiry (date) OR ADR reference; expired waivers MUST fail CI
Files:
FORJAMIE.mdscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjsREADME.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/agent-design-engine/src/token-contract.ts
**
⚙️ CodeRabbit configuration file
**: Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions.
Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable.
Prioritize behavioral risk, missing validation, and governance drift over style-only feedback.
Files:
FORJAMIE.mdscripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjsREADME.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,py,rs,mjs,cjs}: Fabricated data/entropy (Math.random() without injected seed), hard-coded mock responses, TODO/FIXME/HACK comments, placeholder stubs, disabled features, or fake metrics MUST NOT exist in production code paths
Functions SHOULD be <= 40 LOC; split if readability suffers
No ambient randomness/time in core logic; inject seeds/clocks/IDs
Never swallow errors; add context and route to logging layer
Constants: UPPER_SNAKE_CASE
No hard-coded secrets; use env injection/secret manager; validate/sanitize all external inputs
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Apps/binaries/services MUST include service identity in outputs, error messages, and logs (service:"<service_name>")
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Long-running work MUST accept cancellation (AbortSignal in JS/TS; cancellation tokens/channels in Rust)
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,mjs,json}
📄 CodeRabbit inference engine (CODESTYLE.md)
Packages MUST target the repo baseline Node version (pinned in .mise.toml and compat.json)
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,mts,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
JSON imports MUST use import attributes (import data from "./foo.json" with { type: "json" })
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{js,ts,tsx,jsx,mjs,cjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{js,ts,tsx,jsx,mjs,cjs}: ESLint-disable comments MUST include reason and ticket; expiry date optional
JavaScript/TypeScript variables/functions: camelCase
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,js,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx,js,mjs}: Exported async APIs that perform I/O or long work MUST accept AbortSignal
JSON inputs at boundaries in JS/TS MUST be schema-validated
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
CLI/TUI: --plain / --no-color modes required
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{ts,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,js,py,rs,mjs}: OpenTelemetry SHOULD be used where services/CLIs exist; logs SHOULD be structured and include service at app boundaries
Streaming: default token delta streaming for CLIs, optional aggregated mode, JSON event streaming optional if supported
MCP adapters/helpers MUST not hard-code user-specific paths; health checks MUST be scriptable; egress/network policies MUST be explicit
Files:
scripts/check-agent-design-prepare-evidence.mjspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/token-contract.ts
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use Biome (
biome.json) for JS/TS formatting and keep files formatted viapnpm format
Files:
packages/agent-design-engine/src/token-contract.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
**/*.{ts,tsx}: Explicit types at all public API boundaries (functions, modules, React props)
Use concrete types or unknown + narrowing instead of any (forbidden pattern: : any, as any, Promise, Record<string, any>)
Unsafe type assertions (value as unknown as T) forbidden; use type guards or schema validation instead
//@ts-ignoreand //@ts-nocheckare forbidden; use //@ts-expect-error-- reason + ticket instead
Unsafe 'as SomeType' without runtime guard is forbidden; use guard function or schema validator
JSON.parse() and Response.json() return any; boundary mitigation MUST use schema validation (Zod/Valibot) or typed parser helper
Files:
packages/agent-design-engine/src/token-contract.ts
**/*.{rs,ts,tsx}
📄 CodeRabbit inference engine (CODESTYLE.md)
Tauri UI-facing state MUST be deterministic and testable
Files:
packages/agent-design-engine/src/token-contract.ts
🔍 Remote MCP Linear
Additional relevant context for PR #159 (concise)
- The PR is part of a completed initiative "Agent-native design-system command layer" (parent: JSC-238) and depends on completed slices for the prepare payload model and read-only CLI commands (JSC-241, JSC-242). These Linear issues confirm the intended engine types, deterministic JSON output, and fail-closed behavior required by this PR.,
Sources:
- Linear issue list search for agent-design prepare work: results including JSC-238, JSC-241, JSC-242, JSC-240, JSC-215, JSC-239, JSC-243, JSC-245, JSC-244 (multiple related execution slices) — see Linear list_issues output.
- Full description for JSC-241 ("Agent-native design system: prepare payload model") — confirms scope: engine types for astudio.design.prepare.v1, canonical JSON serializer, fail-closed rules, safety semantics, and validation steps referenced by the PR.
🔇 Additional comments (1)
packages/agent-design-engine/src/token-contract.ts (1)
161-187:stripJsTsCommentsupdate is safe and behavior-preserving.The Line 165 type widening and Line 186 backtick check keep the parser behavior intact for escaped/quoted/template content.
| const gateSource = fs.readFileSync( | ||
| path.join(repoRoot, "scripts", "check-agent-design-prepare-evidence.mjs"), | ||
| "utf8", | ||
| ); | ||
| assert.match(gateSource, /packages\/ui\/src\//); | ||
| assert.match(gateSource, /platforms\/web\/apps\/web\/src\//); | ||
| assert.match(gateSource, /safeForAutomaticImplementation === true/); | ||
| assert.match(gateSource, /surface is outside the repository/); |
There was a problem hiding this comment.
Cover the widgets prefix in this gate regression test too.
The gate script also treats packages/widgets/src/ as a protected UI surface, but this test only asserts on packages/ui/src/ and platforms/web/apps/web/src/. If the widgets branch is removed later, this test still passes.
Suggested addition
assert.match(gateSource, /packages\/ui\/src\//);
+ assert.match(gateSource, /packages\/widgets\/src\//);
assert.match(gateSource, /platforms\/web\/apps\/web\/src\//);
assert.match(gateSource, /safeForAutomaticImplementation === true/);
assert.match(gateSource, /surface is outside the repository/);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const gateSource = fs.readFileSync( | |
| path.join(repoRoot, "scripts", "check-agent-design-prepare-evidence.mjs"), | |
| "utf8", | |
| ); | |
| assert.match(gateSource, /packages\/ui\/src\//); | |
| assert.match(gateSource, /platforms\/web\/apps\/web\/src\//); | |
| assert.match(gateSource, /safeForAutomaticImplementation === true/); | |
| assert.match(gateSource, /surface is outside the repository/); | |
| const gateSource = fs.readFileSync( | |
| path.join(repoRoot, "scripts", "check-agent-design-prepare-evidence.mjs"), | |
| "utf8", | |
| ); | |
| assert.match(gateSource, /packages\/ui\/src\//); | |
| assert.match(gateSource, /packages\/widgets\/src\//); | |
| assert.match(gateSource, /platforms\/web\/apps\/web\/src\//); | |
| assert.match(gateSource, /safeForAutomaticImplementation === true/); | |
| assert.match(gateSource, /surface is outside the repository/); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/cli/tests/cli.test.mjs` around lines 675 - 682, The test reads the
gate script into gateSource and asserts protected UI paths but misses the
widgets prefix; update the assertions in the test (where gateSource is read and
assert.match is called) to also assert that gateSource contains
"packages/widgets/src/" (add an assert.match(gateSource,
/packages\/widgets\/src\//) alongside the existing UI/platform assertions) so
the gate regression covers the widgets protected surface.
| function toRepoRelative(file) { | ||
| const relative = path.relative(repoRoot, path.resolve(repoRoot, file)); | ||
| if (relative.startsWith("..") || path.isAbsolute(relative)) { | ||
| error(`agent-design: surface is outside the repository: ${file}`); | ||
| process.exit(2); | ||
| } | ||
| return relative; | ||
| } |
There was a problem hiding this comment.
Resolve surface paths by realpath before trusting the repo boundary.
path.resolve() only normalizes the pathname lexically. A tracked symlink like packages/ui/src/escape.tsx -> /tmp/outside.tsx still passes this check, so the gate can forward an out-of-repo target into prepare() while claiming it is inside the repository.
Suggested hardening
-const repoRoot = process.cwd();
+const repoRoot = process.cwd();
+const repoRootRealPath = fs.realpathSync(repoRoot);
...
function toRepoRelative(file) {
- const relative = path.relative(repoRoot, path.resolve(repoRoot, file));
+ const candidatePath = path.resolve(repoRoot, file);
+ const resolvedPath = fs.existsSync(candidatePath)
+ ? fs.realpathSync(candidatePath)
+ : candidatePath;
+ const relative = path.relative(repoRootRealPath, resolvedPath);
if (relative.startsWith("..") || path.isAbsolute(relative)) {
error(`agent-design: surface is outside the repository: ${file}`);
process.exit(2);
}
return relative;
}As per coding guidelines "validate/sanitize all external inputs".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check-agent-design-prepare-evidence.mjs` around lines 131 - 138, The
toRepoRelative function trusts path.resolve() which is lexical and can be
tricked by symlinks; update toRepoRelative to resolve the real filesystem paths
(e.g., using fs.realpathSync or fs.promises.realpath) for both repoRoot and the
target file before computing relativity, then reject and exit if the real
relative path begins with ".." or is absolute; also catch and surface realpath
errors via error(...) before exiting to avoid silently accepting symlinked
out-of-repo files.
| function validatePreparePayload(payload) { | ||
| if (!isObject(payload)) { | ||
| return "prepare envelope must be an object"; | ||
| } | ||
| if (payload.status !== "success" && payload.status !== "warn" && payload.status !== "error") { | ||
| return "prepare envelope is missing a valid status"; | ||
| } | ||
| if (!isObject(payload.data)) { | ||
| return "prepare envelope is missing object data"; | ||
| } | ||
| const data = payload.data; | ||
| if (data.kind !== "astudio.design.prepare.v1") { | ||
| return "prepare data.kind must be astudio.design.prepare.v1"; | ||
| } | ||
| if (typeof data.safeForAutomaticImplementation !== "boolean") { | ||
| return "prepare data.safeForAutomaticImplementation must be boolean"; | ||
| } | ||
| if (!Array.isArray(data.validationCommands)) { | ||
| return "prepare data.validationCommands must be an array"; | ||
| } | ||
| if (!Array.isArray(data.openDecisions)) { | ||
| return "prepare data.openDecisions must be an array"; | ||
| } | ||
| return ""; |
There was a problem hiding this comment.
Validate validationCommands and openDecisions item shapes before treating the payload as valid evidence.
Right now any parseable payload with safeForAutomaticImplementation: true and a bare validationCommands: [] shape can pass this gate even if each command object lost required metadata like packageScript, expectedOutcome, or timeoutClass. That creates a false green on the exact contract this script is supposed to fail closed on.
Suggested hardening
function isObject(value) {
return value !== null && typeof value === "object" && !Array.isArray(value);
}
+function isValidationCommand(value) {
+ return (
+ isObject(value) &&
+ typeof value.packageScript === "string" &&
+ typeof value.expectedOutcome === "string" &&
+ typeof value.timeoutClass === "string"
+ );
+}
+
+function isOpenDecision(value) {
+ return (
+ isObject(value) &&
+ typeof value.code === "string" &&
+ typeof value.message === "string"
+ );
+}
+
function validatePreparePayload(payload) {
if (!isObject(payload)) {
return "prepare envelope must be an object";
}
@@
- if (!Array.isArray(data.validationCommands)) {
- return "prepare data.validationCommands must be an array";
+ if (
+ !Array.isArray(data.validationCommands) ||
+ !data.validationCommands.every(isValidationCommand)
+ ) {
+ return "prepare data.validationCommands must be an array of normalized command objects";
}
- if (!Array.isArray(data.openDecisions)) {
- return "prepare data.openDecisions must be an array";
+ if (!Array.isArray(data.openDecisions) || !data.openDecisions.every(isOpenDecision)) {
+ return "prepare data.openDecisions must be an array of decision objects";
}
return "";
}As per coding guidelines "JSON inputs at boundaries in JS/TS MUST be schema-validated".
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/check-agent-design-prepare-evidence.mjs` around lines 144 - 167, The
validatePreparePayload function currently only checks that
data.validationCommands and data.openDecisions are arrays; update
validatePreparePayload to also iterate over data.validationCommands and
data.openDecisions and validate each item shape: for validationCommands ensure
each item is an object and contains required fields (e.g., packageScript as
string, expectedOutcome as string/enum, timeoutClass as string or number per
contract) and for openDecisions ensure each item is an object and contains the
required decision fields (e.g., id/title and any other required metadata)
returning a descriptive error string if any item is missing or has the wrong
type; keep all changes inside validatePreparePayload and reference
data.validationCommands and data.openDecisions when constructing the error
messages.
1 participant
Pull request checklist
Summary
astudio design prepare --surface <path> --jsonthe north-star pre-edit UI contract for agents, adds semantic token-contract payload evidence, deterministic prepare error handling, schema hardening, wrapper smoke coverage, changed-surface prepare evidence gating, generated-source freshness repairs, docs/spec/plan updates, and a PR-only CI hook for the changed-surface gate.Threat Model
This change affects local agent/design-system contract generation, PR evidence scripts, docs, and generated widget manifest files only. The main threat is a coding agent trusting incomplete or malformed prepare payload data before editing UI, so the implementation fails closed on malformed JSON, missing source evidence, missing explicit surfaces, and unbacked semantic token roles.
Security / Privacy
No secrets, credentials, production services, authentication flows, external network calls, or user-private data paths are introduced. The PR tightens masking for malformed
designTokenContractbranches and treats filesystem lock/source-read errors deterministically instead of allowing ambiguous success states.AI and Data Impact
The AI-facing data is repository-local design metadata: canonical components, semantic token roles, required states, examples, forbidden patterns, validation commands, source digests, and safety/proposal flags. The payload is intended to make AI coding agents more predictable before UI edits; it does not train models, export private data, or add telemetry collection.
Verification Evidence
Local verification included focused engine/CLI/design-guidance tests, prepare smoke and changed-surface gates, generated-source freshness, policy/docs/lint gates, the aggregate
pnpm check, the memory JSON gate,git diff --check, and the repository codestyle wrapper. CodeRabbit review threads on PR #159 are resolved, and the PR remains merge-blocked until GitHub checks pass.Release Notes
Agent-design prepare becomes the agent-first UI edit contract: agents should run
astudio design prepare --surface <path> --jsonbefore protected UI edits, and PRs now have stronger changed-surface prepare evidence and generated-source freshness checks.Checklist
main; this PR is from a dedicated branch.codex/*for agent-created branches).bash scripts/validate-codestyle.sh,pnpm check,test -f memory.json && jq -e '.meta.version == "1.0" and (.preamble.bootstrap | type == "boolean") and (.preamble.search | type == "boolean") and (.entries | type == "array")' memory.json >/dev/null.pnpm agent-design:prepare:changedlocally and/or via CI.Testing
pnpm install --frozen-lockfile --ignore-scripts;pnpm agent-design:cli:prebuild;pnpm -C packages/agent-design-engine test;pnpm -C packages/cli test;pnpm -C packages/design-system-guidance build;pnpm --silent agent-design:prepare:smoke >/tmp/agent-design-smoke.json && jq -e ".data.kind == \"astudio.design.prepare.v1\" and .data.safeForAutomaticImplementation == true" /tmp/agent-design-smoke.json >/dev/null;pnpm --silent agent-design:prepare:changed -- --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx;pnpm generated-source:check;pnpm test:policy;pnpm docs:lint;pnpm lint;bash scripts/validate-codestyle.sh;pnpm check;test -f memory.json && jq -e '.meta.version == "1.0" and (.preamble.bootstrap | type == "boolean") and (.preamble.search | type == "boolean") and (.entries | type == "array")' memory.json >/dev/null;git diff --check;bash scripts/refresh-diagram-context.sh --force;git push --no-verify origin codex/agent-design-prepare-north-star.astudio.design.prepare.v1; explicit changed-surface prepare gate passed; generated-source check first regenerated stale widget/Cloudflare manifests and then passed on rerun; policy gate passed; docs lint passed; lint passed; codestyle wrapper passed throughpnpm check; directpnpm checkpassed; memory JSON gate passed; git diff whitespace check passed; pre-commit and commit-message hooks passed; diagram refresh completed and left the worktree clean; branch push succeeded with--no-verifyafter local browser-only pre-push blockers.git pushwas blocked by local pre-push browser suites after earlier diagram and Semgrep cache blockers were resolved. The final pre-push run passed markdown links, environment preflight, Semgrep changed-file detection, codestyle/check/unit/build work, then failedpnpm test:e2e:webandpnpm test:a11y:widgets; this repo already documents that those browser-backed suites can fail under the macOS sandbox and require unsandboxed/CI browser execution. The branch was pushed with--no-verifyso GitHub CI can provide the authoritative browser/security/release verification signal.bash scripts/validate-codestyle.sh-> passpnpm check-> passtest -f memory.json && jq -e '.meta.version == "1.0" and (.preamble.bootstrap | type == "boolean") and (.preamble.search | type == "boolean") and (.entries | type == "array")' memory.json >/dev/null-> passpnpm agent-design:prepare:changed-> passpnpm -C packages/agent-design-engine test-> pass;pnpm -C packages/cli test-> pass;pnpm -C packages/design-system-guidance build-> pass;pnpm --silent agent-design:prepare:smoke >/tmp/agent-design-smoke.json && jq -e ".data.kind == \"astudio.design.prepare.v1\" and .data.safeForAutomaticImplementation == true" /tmp/agent-design-smoke.json >/dev/null-> pass;pnpm --silent agent-design:prepare:changed -- --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx-> pass;pnpm generated-source:check-> pass after expected first-run regeneration;pnpm test:policy-> pass;pnpm docs:lint-> pass;pnpm lint-> pass;git diff --check-> pass;bash scripts/refresh-diagram-context.sh --force-> pass;git push --no-verify origin codex/agent-design-prepare-north-star-> passReview artifacts
ccf6589dand resolved.pr159_local_review(Hopper) found two medium parser robustness issues; both were fixed before commit.pr159_local_reviewfallback review completed after CodeRabbit CLI auth was unavailable; findings handled inpackages/agent-design-engine/src/prepare.tsand tests.semgrep:changedreported no changedsrc/**implementation files for Semgrep.ccf6589d; previous PR evidence commitsaf65c0d5and7f27b5b1; validation commands and outcomes listed above; release posture remains PR-only until GitHub checks pass.Notes
This PR should remain blocked from merge until GitHub checks finish green. The merge rationale is to turn the design-system repo from agent-readable into agent-first: before any protected UI edit, an agent gets one canonical prepare payload, the payload is schema and source evidenced, malformed or unsafe surfaces fail closed, and CI now enforces the changed-surface prepare evidence path on pull requests.