[codex] Harden agent design prepare contract - #160
Conversation
Why: agents need a single reliable pre-edit UI contract and the plan/spec/docs must make the wrapper/read-only boundary explicit. What: add the prepare north-star spec and execution plan, flip agent workflow docs toward prepare-first usage, and document gold-example and wrapper guardrails. Validation: git diff --cached --check -> pass Co-authored-by: Codex <[email protected]>
Why: the prepare north-star plan needed the final reviewer hardening captured as a completed, machine-verifiable record.
What: mark the plan review-green, add machine evidence requirements and locators, tighten wrapper reliability/read-only wording, resolve settled spec decisions, and update FORJAMIE with the current plan lane status.
Impact/Risk: docs-only; clarifies that the silent pnpm wrapper is build-backed setup while astudio design prepare remains the read-only operation contract.
Validation: awk '/^```jsonl$/{flag=1;next}/^```$/{if(flag){flag=0}}flag' docs/plans/2026-04-30-agent-design-prepare-north-star-plan.md | jq -c . -> pass
Validation: pnpm docs:lint -> pass
Validation: git diff --check -- FORJAMIE.md docs/plans/2026-04-30-agent-design-prepare-north-star-plan.md docs/specs/2026-04-30-agent-design-prepare-north-star-spec.md -> pass
Review: adversarial-reviewer -> GREEN; adversarial-document-reviewer -> GREEN; agent-native-reviewer -> GREEN
Co-authored-by: Codex <[email protected]>
Why: the prepare command is the project north-star pre-edit contract for agent UI work, so payload, wrapper, and validation evidence need to fail closed and be machine-consumable. What: add semantic token contract loading, timing evidence, deterministic prepare/guidance/routing/package errors, targeted validation-command package-script checks with realpath workspace boundaries, prepare schema hardening, JSON-safe wrapper builds, and masking coverage for public token contracts. Impact/Risk: prepare payloads now include required token and timing fields and reject malformed validation metadata more strictly. Wrapper scripts build CLI workspace dependencies before invoking the read-only prepare operation. Validation: - git diff --check -> pass - pnpm lint -> pass - pnpm -C packages/agent-design-engine type-check -> pass - pnpm -C packages/agent-design-engine test -> pass - pnpm -C packages/cli build -> pass - pnpm -C packages/cli test -> pass - pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json -> pass - pnpm agent-design:lint -> pass - pnpm generated-source:check -> fail first run regenerated stale manifests; pass on rerun Co-authored-by: Codex <[email protected]>
Why: the completed prepare north-star plan still pointed final parser hardening evidence at the earlier planning commit, which made the ledger less useful for future agents. What: add final P2 hardening evidence locators for commit 47aae7b and explicitly name the parser and symlink-boundary cases covered by that commit. Impact/Risk: documentation-only evidence tightening; no runtime behavior changes. Validation: - pnpm docs:lint -> pass - git diff --check -> pass Co-authored-by: Codex <[email protected]>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughImplements a canonical read-only "astudio design prepare" pre-edit contract: expands PreparePayload (token contract, evidence, constrained mode), adds deterministic DesignEngineError codes, pnpm validation-command normalization, a token-contract validator, CLI masking for public token contracts, extensive tests, docs/spec/plan, and package/script wiring. ChangesAgent Design Prepare (single cohesive change DAG)
Sequence DiagramsequenceDiagram
participant Agent as Agent
participant CLI as astudio CLI
participant Engine as Design Engine
participant Guidance as Guidance Loader
participant Routes as Routing/Lifecycle Loader
participant TokenSys as Token Contract Validator
Agent->>CLI: astudio design prepare --surface <path> --json
CLI->>Engine: buildPreparePayload(surface, rootDir)
Engine->>Guidance: read/parse DESIGN.md + guidance, validate mode
Guidance-->>Engine: guidance data or DesignEngineError
Engine->>Routes: load routing, lifecycle, coverage, raw validationCommands
Routes-->>Engine: routes + raw validationCommands
Engine->>TokenSys: buildDesignTokenContract(rootDir)
TokenSys-->>Engine: validated designTokenContract
Engine->>Engine: normalize validationCommands (pnpm parsing, resolve scripts)
Engine->>Engine: compute openDecisions (with nextAction), assemble PreparePayload
Engine-->>CLI: PreparePayload (astudio.design.prepare.v1)
CLI->>CLI: validate payload against schema, apply masking
CLI-->>Agent: JSON response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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)❌ Error committing Unit Tests locally.
Review rate limit: 1/5 review remaining, refill in 41 minutes and 36 seconds. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agent-design-engine/src/prepare.ts (1)
662-680:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMissing route examples should still block automatic implementation.
E_DESIGN_ROUTE_EXAMPLE_MISSINGis still downgraded towarn, sookandsafeForAutomaticImplementationstay true for routes that lost their required example evidence. The stop rule for JSC-241 and this plan both treat missing examples as fail-closed, so agents can currently proceed without the example inventory this contract is supposed to guarantee.Suggested fix
function routeDecisions( routeResult: ReturnType<typeof resolveRouteForSurface>, surfaceScope: PrepareSurfaceScope, ): PrepareOpenDecision[] { const decisions: PrepareOpenDecision[] = routeResult.diagnostics.map((diagnostic) => ({ code: diagnostic.code, message: diagnostic.message, - severity: diagnostic.code === "E_DESIGN_ROUTE_EXAMPLE_MISSING" ? "warn" : "error", + severity: "error", }));As per coding guidelines, "Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable."
Also applies to: 767-770
🤖 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 662 - 680, The routeDecisions function currently downgrades the E_DESIGN_ROUTE_EXAMPLE_MISSING diagnostic to "warn", which allows routes missing required examples to be treated as ok/safeForAutomaticImplementation; change the severity for code === "E_DESIGN_ROUTE_EXAMPLE_MISSING" from "warn" to "error" inside routeDecisions (and update the other similar conditional occurrence referenced in the review) so missing route examples fail-closed; update any tests or callers that assume the old severity if present.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CONTRIBUTING.md`:
- Around line 198-200: The Table of Contents in CONTRIBUTING.md is missing the
new "## Link Policy" section; update the TOC to include an entry (e.g., "Link
Policy") pointing to the "## Link Policy" heading so navigation is in
sync—insert the TOC line in the existing Table of Contents near related policy
sections and ensure the anchor text matches the heading (Link Policy) exactly.
In `@docs/guides/AGENT_DESIGN_WORKFLOW.md`:
- Line 28: The ordered list items using explicit incremental numbers (e.g., the
line starting "2. In this repo, the clean-checkout convenience wrapper is:" and
the list block spanning the items at lines 36–40) violate markdownlint MD029;
update those ordered-list entries to use a consistent numbering style—preferably
use "1." for every ordered item (or renumber sequentially) so all list markers
match the chosen style across the document (e.g., change "2." to "1." and make
the items in the 36–40 block all "1." or properly sequential).
In `@docs/specs/2026-04-30-agent-design-prepare-north-star-spec.md`:
- Line 31: The Table of Contents entry for "Open Questions" points to the
nonexistent fragment "#open-questions"; either add a corresponding heading "##
Open Questions" (exact text and spacing) to the document or change the TOC line
to point to an existing heading (e.g., update the fragment in the line "- [Open
Questions](`#open-questions`)" to match the actual heading slug used elsewhere).
Ensure the heading text exactly matches the TOC label so the generated
fragment/slug aligns with the link.
In `@FORJAMIE.md`:
- Around line 19-21: The document contains two conflicting "Last updated" dates:
the top-level "Last updated:" entry showing "2026-04-30" and a later occurrence
showing "2026-05-01"; pick the authoritative date (e.g., 2026-05-01) and update
the earlier "Last updated:" field so both occurrences match, ensuring the single
canonical date appears consistently wherever "Last updated:" is used in
FORJAMIE.md.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 118-168: The contract currently returns many semantic roles
without verifying they exist in the parsed sources; update
buildDesignTokenContract() to only emit roles that are validated by the
preflight checks (or re-derive the contract entirely from parsed sources).
Concretely: extend assertThemeSource(), assertAliasMapSource(), and
assertDtcgSource() (or add new validation helpers called from
buildDesignTokenContract()) to explicitly check for each advertised role pattern
(e.g., surface.card, text.secondary, border.strong, and all status.* entries) by
parsing themeSourcePath content for CSS variables, tokenAliasMap exports for
alias keys, and the parsed DTCG JSON for color groups/status tokens; if any
advertised role is missing, call tokenContractAmbiguous() for that specific role
so buildDesignTokenContract() only returns roles that passed validation. Ensure
prepare()/buildDesignTokenContract() consumes the validated set rather than
unconditionally emitting hard-coded roles.
In
`@packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json`:
- Around line 437-440: The schema's designContractMode property currently allows
any non-empty string but parseGuidanceConfig() only accepts "legacy" or
"design-md"; update the designContractMode schema to restrict values to the
engine's union (enum: "legacy" and "design-md") so fixture validation matches
parseGuidanceConfig() and prevents unsupported modes from passing validation.
- Around line 373-390: The openDecision object schema lacks a machine-readable
nextAction, so add a required "nextAction" property to the openDecision
definition (update its "required" array to include "nextAction") and define
"nextAction" as a string enum with the prescribed action categories (e.g.,
"stop", "escalate", "diagnose") so downstream agents can make deterministic
decisions; locate the openDecision schema block and add the nextAction enum
property and include it in required.
---
Outside diff comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 662-680: The routeDecisions function currently downgrades the
E_DESIGN_ROUTE_EXAMPLE_MISSING diagnostic to "warn", which allows routes missing
required examples to be treated as ok/safeForAutomaticImplementation; change the
severity for code === "E_DESIGN_ROUTE_EXAMPLE_MISSING" from "warn" to "error"
inside routeDecisions (and update the other similar conditional occurrence
referenced in the review) so missing route examples fail-closed; update any
tests or callers that assume the old severity if present.
🪄 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: 76ec1ef2-0dcf-496d-84a2-7ed00356880f
📒 Files selected for processing (20)
.gitignoreCONTRIBUTING.mdFORJAMIE.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/design-system-guidance/src/core.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). (3)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (23)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Add a Table of Contents for documentation files
Files:
CONTRIBUTING.mddocs/design-system/GOLD_EXAMPLES.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdREADME.mddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mdFORJAMIE.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.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:
CONTRIBUTING.mddocs/design-system/GOLD_EXAMPLES.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdREADME.mddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mdFORJAMIE.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.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:
CONTRIBUTING.mdpackages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsdocs/design-system/GOLD_EXAMPLES.mdpackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tsdocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/cli/src/utils/mask.tsREADME.mdpackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tspackages/agent-design-engine/tests/engine.test.mjsdocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mdFORJAMIE.mdpackages/agent-design-engine/src/prepare.tsdocs/specs/2026-04-30-agent-design-prepare-north-star-spec.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:
CONTRIBUTING.mdpackages/cli/tests/mask.test.mjspackages/design-system-guidance/src/core.tsdocs/design-system/GOLD_EXAMPLES.mdpackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/types.tsdocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/cli/src/utils/mask.tsREADME.mdpackage.jsonpackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tsdocs/design-system/GOLD_EXAMPLES.jsonpackages/agent-design-engine/tests/engine.test.mjsdocs/plans/2026-04-30-agent-design-prepare-north-star-plan.mdFORJAMIE.mdpackages/agent-design-engine/src/prepare.tsdocs/specs/2026-04-30-agent-design-prepare-north-star-spec.md
**/*.{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.tspackages/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.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.tspackages/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.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.tspackages/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.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/cli.test.mjspackages/agent-design-engine/src/types.tspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/cli/src/utils/mask.tspackage.jsonpackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/routes.tsdocs/design-system/GOLD_EXAMPLES.jsonpackages/agent-design-engine/tests/engine.test.mjspackages/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.tspackages/cli/tests/cli.test.mjspackages/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/tests/engine.test.mjspackages/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.tspackages/cli/tests/cli.test.mjspackages/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/tests/engine.test.mjspackages/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.tspackages/cli/tests/cli.test.mjspackages/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/tests/engine.test.mjspackages/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.tspackages/cli/tests/cli.test.mjspackages/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/tests/engine.test.mjspackages/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.tspackages/cli/tests/cli.test.mjspackages/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/tests/engine.test.mjspackages/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.tspackages/cli/tests/cli.test.mjspackages/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/tests/engine.test.mjspackages/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/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.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.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.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.jsonpackage.jsondocs/design-system/GOLD_EXAMPLES.json
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
🧠 Learnings (1)
📚 Learning: 2026-04-07T12:07:28.929Z
Learnt from: CR
Repo: jscraik/Design-System PR: 0
File: docs/agents/05-dev-reference.md:0-0
Timestamp: 2026-04-07T12:07:28.929Z
Learning: Stable public API includes chat UI components (ChatUIRoot, ChatHeader, ChatSidebar, ChatMessages, ChatInput), UI primitives (Button, Dialog, Tabs, Tooltip), Icons adapter, DesignSystemPage, and utilities like useControllableState
Applied to files:
README.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)
🪛 LanguageTool
docs/plans/2026-04-30-agent-design-prepare-north-star-plan.md
[style] ~194-~194: 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] ~467-~467: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...for every command that ran. - surface is required for prepare invocations; `fail...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~468-~468: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...nistic error-path checks. - commitSha is required once work is committed; use `l...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~525-~525: 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)
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)
🪛 markdownlint-cli2 (0.22.1)
docs/guides/AGENT_DESIGN_WORKFLOW.md
[warning] 28-28: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1
(MD029, ol-prefix)
[warning] 36-36: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/2/3
(MD029, ol-prefix)
[warning] 37-37: Ordered list item prefix
Expected: 2; Actual: 4; Style: 1/2/3
(MD029, ol-prefix)
[warning] 38-38: Ordered list item prefix
Expected: 3; Actual: 5; Style: 1/2/3
(MD029, ol-prefix)
[warning] 39-39: Ordered list item prefix
Expected: 4; Actual: 6; Style: 1/2/3
(MD029, ol-prefix)
[warning] 40-40: Ordered list item prefix
Expected: 5; Actual: 7; Style: 1/2/3
(MD029, ol-prefix)
docs/specs/2026-04-30-agent-design-prepare-north-star-spec.md
[warning] 31-31: Link fragments should be valid
(MD051, link-fragments)
🔍 Remote MCP Linear
Perfect! I now have comprehensive context. Let me compile the findings:
Summary of Relevant Context for PR #160 Review
Related Linear Issues & Epic Structure
This PR directly implements work tracked under JSC-238 ("Agent-native design-system command layer"), a high-priority epic that is now complete. The PR specifically fulfills:
- JSC-241 (Done): "Agent-native design system: prepare payload model" — The core requirement to build the semantic payload behind
astudio design preparewith deterministic contract fields - JSC-240 (Done): Routing table contract — Machine-readable component routing dependency
- JSC-239 (Done): Contract wiring — DESIGN.md and guidance config foundational work
- JSC-242 (Done): Read-only CLI commands — Public surface exposure
The prepare payload model specification calls for:
- Resolving and digesting DESIGN.md, guidance config, routing JSON, lifecycle, coverage
- Deterministic digests and validation commands
- Fail-closed behavior for unknown scope, missing routes/lifecycle/coverage, or missing examples
safeForAutomaticImplementationdecision gate (mentioned as critical stopping point before agent UI edits)- Symmetric safety classification for validation commands
Key Technical Requirements from Related Issues (JSC-241)
The stop rule for JSC-241 states: "Do not expose CLI commands until the engine model has deterministic fixtures for protected, warn, exempt, unknown, overlap, and symlink-normalized scopes."
This directly relates to the PR's work on symlink boundary checking and package-script validation in packages/agent-design-engine/src/prepare.ts and packages/agent-design-engine/src/routes.ts.
Epic Completion Status (JSC-238)
All seven execution slices are now complete:
- ✅ Contract wiring
- ✅ Machine-readable routing table
- ✅ Prepare payload model (this PR)
- ✅ Read-only CLI command surface
- ✅ Actionable remediation output
- ✅ Gold example inventory
- ✅ New abstraction proposal gate
Validation Requirements from Spec
Per JSC-241 scope, validation should confirm:
pnpm -C packages/agent-design-engine testpasses (73 tests confirmed passing in PR metadata)- Type checking succeeds
- Deterministic error codes are enforced
- Symlink and workspace boundary behavior is hardened
- All fixtures cover protected/warn/exempt/unknown/overlap scope scenarios
This context confirms the PR's focus on:
- Deterministic payload validation with explicit error codes
- Workspace boundary enforcement via
realpathand symlink escape rejection - Safety gatekeeping through
safeForAutomaticImplementationflag - Read-only validation commands with metadata normalization
- Schema hardening across prepare command variants
🔇 Additional comments (8)
.gitignore (1)
134-134: Good root-scoped ignore for local Codex conformance artifact.
/.codex/hook-conformance.jsonis appropriately scoped and prevents local machine metadata from being committed.packages/design-system-guidance/src/core.ts (1)
1621-1628: Dry-run lock re-check is a solid fail-closed hardening step.The pre/post-read lock checks in Line 1621-Line 1628 improve deterministic behavior when a migration is already active.
package.json (1)
42-43: Wrapper dependency ordering looks correct and intentional.The prepare/lint wrapper changes preserve a clean contract by building prerequisites first and keeping surface pinning confined to the smoke script.
Also applies to: 131-131
packages/cli/tests/mask.test.mjs (1)
53-86: Good coverage for public token-contract masking behavior.This test cleanly locks in the intended “preserve public metadata, redact secrets” contract.
README.md (1)
113-130: Agent pre-edit workflow documentation is clear and actionable.The prepare-first flow, gating on
safeForAutomaticImplementation, and silent-wrapper guidance are well specified.packages/cli/tests/cli.test.mjs (1)
517-579: Excellent fail-closed contract coverage fordesign prepare.These tests materially strengthen payload-schema drift detection and wrapper execution-order guarantees.
Also applies to: 581-609
docs/design-system/GOLD_EXAMPLES.md (1)
40-47: Deferred promotion criteria are now much more enforceable.The added proof requirements make promotion gates explicit and machine-auditable.
docs/design-system/GOLD_EXAMPLES.json (1)
156-161: Good hardening of deferred-promotion evidence requirements.The updated prerequisites are concrete and aligned with route-need linkage expectations.
Also applies to: 170-175, 184-189
Why: CodeRabbit review found prepare contract and docs gaps that could let agents treat unsafe or drifting payloads as usable. What: fail closed on missing route examples, add machine nextAction metadata to open decisions, narrow prepare schema contract modes, validate every advertised token role against runtime CSS, and repair docs lint issues. Validation: pnpm -C packages/agent-design-engine test -> pass; pnpm -C packages/cli test -> pass; pnpm docs:lint -> pass; pnpm lint -> pass; pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json -> pass; git diff --check -> pass. Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/agent-design-engine/src/prepare.ts (2)
736-752:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftFail closed on guidance/contract mode drift.
We validate that
guidance.designContract.modeis one of the allowed strings, but we never verify that it matches whatparseDesignContract()actually found. A stale guidance file can therefore serialize a contradictorydesignContractModeinstead of stopping, which reintroduces contract drift into the payload.As per coding guidelines, "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."
Also applies to: 791-797
🤖 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 736 - 752, The guidance file's declared designContract mode (guidance.designContract.mode) is not validated against the actual contract parsed by parseDesignContract(), which allows stale guidance to silently drift the payload; update prepare.ts to compare guidance.designContract.mode to the mode/value returned by parseDesignContract(designSource, ...) (and the analogous check around the later block at lines ~791-797) and throw/abort via signal.throwIfAborted() or a clear error when they differ so the process fails closed instead of continuing with contradictory designContractMode.
722-725:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWrap the two prerequisite contract reads in stable engine errors.
digestFile()now maps missing sources to deterministic codes, but these initialreadText()calls still leak rawENOENT/EACCESerrors for missingDESIGN.mdor.design-system-guidance.json. That breaks the fail-closed, machine-consumable contract on the very first read path.🤖 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 722 - 725, Wrap the Promise.all(...) that calls readText(resolvedRoot, designPath, signal) and readText(resolvedRoot, guidancePath, signal) in a try/catch and convert filesystem errors into deterministic engine errors (the same style used by digestFile) instead of letting raw ENOENT/EACCES bubble up; inspect err.code and rethrow a stable error code (e.g., MISSING_DESIGN_SOURCE / MISSING_GUIDANCE_SOURCE or PERMISSION_DENIED_DESIGN_SOURCE) with contextual fields (which path: designPath/guidancePath and original message) so callers can rely on machine-consumable failure codes while preserving the original error text for debugging.
🤖 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 28-34: The wrapper example command "pnpm --silent
agent-design:prepare --surface <path>" omits the required --json flag; update
the example to append --json so the wrapper (script name agent-design:prepare)
emits machine-readable JSON consistent with the direct CLI example (astudio
design prepare) and the rest of the workflow docs.
In `@FORJAMIE.md`:
- Around line 27-29: The status table row "Open PRs | 1 | PR `#158`" is stale;
update the PR reference to the current branch's PR number (PR `#160`) so the
living status block matches the surrounding updated north-star prepare review;
locate the table row string "Open PRs" or the exact cell containing "PR `#158`" in
FORJAMIE.md and replace it with "PR `#160`" (or the correct active PR number if
different) to keep documentation consistent.
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 589-598: The current code returns Set(Object.keys(parsed.scripts))
which accepts entries like { "test": false } or { "lint": {} }; change the logic
in prepare.ts where parsed.scripts is processed to only include script names
whose values are valid runnable script strings (e.g., typeof value === "string"
and value.trim() !== ""), by iterating Object.entries(parsed.scripts), filtering
for string/non-empty values, and returning a Set of those keys so packageScript
values accepted by prepare match what pnpm run will actually execute.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 9-10: The tokenSourceRefs array only lists implementation files
(tokenSourceRefs) and misses authored policy/prose authorities referenced by
allowedRoles.useFor/avoidFor and forbiddenTokenPatterns; update tokenSourceRefs
to include the authoritative docs/contract sources (the
DESIGN/PROFESSIONAL_UI_CONTRACT and any harness/validation contract files) so
provenance is captured for allowedRoles and forbiddenTokenPatterns, ensuring
requiredAliasCategories remains unchanged and the same authoritative sources are
added wherever tokenSourceRefs is referenced (lines around tokenSourceRefs,
allowedRoles, forbiddenTokenPatterns).
---
Outside diff comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 736-752: The guidance file's declared designContract mode
(guidance.designContract.mode) is not validated against the actual contract
parsed by parseDesignContract(), which allows stale guidance to silently drift
the payload; update prepare.ts to compare guidance.designContract.mode to the
mode/value returned by parseDesignContract(designSource, ...) (and the analogous
check around the later block at lines ~791-797) and throw/abort via
signal.throwIfAborted() or a clear error when they differ so the process fails
closed instead of continuing with contradictory designContractMode.
- Around line 722-725: Wrap the Promise.all(...) that calls
readText(resolvedRoot, designPath, signal) and readText(resolvedRoot,
guidancePath, signal) in a try/catch and convert filesystem errors into
deterministic engine errors (the same style used by digestFile) instead of
letting raw ENOENT/EACCES bubble up; inspect err.code and rethrow a stable error
code (e.g., MISSING_DESIGN_SOURCE / MISSING_GUIDANCE_SOURCE or
PERMISSION_DENIED_DESIGN_SOURCE) with contextual fields (which path:
designPath/guidancePath and original message) so callers can rely on
machine-consumable failure codes while preserving the original error text for
debugging.
🪄 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: d2b86867-44a7-4135-9f7d-9cbcf9bb809c
📒 Files selected for processing (9)
CONTRIBUTING.mdFORJAMIE.mddocs/guides/AGENT_DESIGN_WORKFLOW.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdpackages/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.mjspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json
📜 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 (19)
**/*.md
📄 CodeRabbit inference engine (AGENTS.md)
Add a Table of Contents for documentation files
Files:
CONTRIBUTING.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.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:
CONTRIBUTING.mddocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.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:
CONTRIBUTING.mdpackages/agent-design-engine/src/token-contract.tsdocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdpackages/agent-design-engine/src/types.tsdocs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.mdpackages/agent-design-engine/tests/engine.test.mjspackages/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:
CONTRIBUTING.mdpackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/agent-design-engine/src/token-contract.tsdocs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdpackages/agent-design-engine/src/types.tsdocs/guides/AGENT_DESIGN_WORKFLOW.mdFORJAMIE.mdpackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.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.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:
packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.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.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.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.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.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.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.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
🪛 LanguageTool
docs/specs/2026-04-30-agent-design-prepare-north-star-spec.md
[style] ~268-~268: 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)
🔍 Remote MCP Linear
Summary of additional, review-relevant facts from Linear issues:
-
PR implements slice work under epic "Agent-native design-system command layer" (JSC-238). Epic completed and lists canonical plan location in repo docs/plans.
-
The Prepare Payload Model slice (JSC-241) requires deterministic PreparePayload, fail-closed behavior for unknown scope/missing route/lifecycle/coverage/ambiguous routing, and a stop rule forbidding CLI exposure until fixtures exist for protected/warn/exempt/unknown/overlap/symlink-normalized scopes. Validation commands to run: type-check, package tests, boundary checks, git diff checks.
-
The Routing Table Contract slice (JSC-240) mandates authored AGENT_UI_ROUTING.json as v1 source of truth (no silent rewrites), seeded canonical needs, and exports to resolve routes; routing drift detection is required. This PR’s routing/manifest changes must align with that contract.
-
The Contract Wiring slice (JSC-239) requires that design guidance and guidance config be connected and validated (design-system-guidance contract mode consistency); migration/validation steps are specified. Ensure token/contract mode in guidance JSON matches the engine’s capabilities.
-
The Read-Only CLI Commands slice (JSC-242) specifies that only prepare is the public full context entrypoint, that prepare/components/coverage are read-only, and that selector conflicts must return E_DESIGN_SELECTOR_CONFLICT; wrapper/script ordering and build-before-run semantics are required by validation steps. Tests/build commands listed for validation.
-
Done/completion state: each of the above child issues (JSC-239–242) are marked completed per Linear; the PR should still satisfy the validation commands and stop rules documented in those issues before merge.
Relevant reviewer actions implied by these facts:
- Verify fail-closed behavior for missing/ambiguous routing/examples and the exact error codes (per JSC-241 stop rule).
- Confirm AGENT_UI_ROUTING.json and any routing changes respect the authored-v1 no-rewrite rule and that drift checks pass.
- Validate that design-system-guidance contract mode and token sources are consistent with token-contract builders.
- Ensure wrapper script ordering (build prerequisites) and that the public CLI surface exposes only the permitted commands; run the listed build/test commands as part of validation.
🔇 Additional comments (2)
CONTRIBUTING.md (2)
17-17: LGTM — Table of Contents entry for “Link Policy” is correctly wired.The TOC link at Line 17 uses
(#link-policy), which matches the## Link Policyheading that follows, so navigation should work cleanly.
199-202: LGTM — “Link Policy” section looks well-formed and consistent.The new
## Link Policysection at Lines 199-201 is clearly formatted, concise, and placed consistently with the rest of the contributing policy doc.
Why: CodeRabbit found remaining prepare-contract reliability gaps around JSON wrapper docs, source provenance, prerequisite reads, guidance mode drift, and package script validation. What: Add deterministic source-read errors, fail closed on guidance/contract mode mismatch, validate package scripts as runnable strings, include policy docs in token contract provenance, and update workflow/FORJAMIE status text. Impact/Risk: Prepare now rejects contradictory or malformed project metadata earlier; callers receive stable DesignEngineError codes instead of raw filesystem failures. Validation: pnpm -C packages/agent-design-engine test -> pass (80/80) Validation: pnpm -C packages/cli test -> pass (116/116) Validation: pnpm docs:lint -> pass Validation: pnpm lint -> pass Validation: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json > /tmp/agent-design-prepare-review.json -> pass Validation: jq '.status, .data.designTokenContract.sourceRefs, .data.designContractMode' /tmp/agent-design-prepare-review.json -> pass Validation: git diff --check -> pass Co-authored-by: Codex <[email protected]>
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 618-620: The current code quietly treats non-object parsed.scripts
as an empty set; instead, update the scripts validation in prepare.ts (the block
using parsed.scripts and isObject) to reject malformed metadata by throwing an
error with code E_DESIGN_PACKAGE_JSON when parsed.scripts is present but not an
object; specifically, replace the early return new Set() for the case where
parsed.scripts exists but isObject(parsed.scripts) === false with throwing a
DesignError (or the project's standard error type) that includes
E_DESIGN_PACKAGE_JSON and a brief message identifying parsed.scripts as
malformed so downstream logic no longer misreports a missing script.
- Around line 382-418: The pnpmSubcommands set used by the command parser is
missing mutating commands causing a fail-open; update the pnpmSubcommands Set
(the constant named pnpmSubcommands) to include "export", "ls", "rm", and
"uninstall" and ensure any other duplicate declaration/usage of pnpmSubcommands
elsewhere in this file is updated too; additionally add the flag
"--filter-omit-pkg-dep" to the run/flag options set (pnpmRunOptionsWithValues or
the appropriate flags collection) so it’s treated as a known pnpm option rather
than a generic script flag.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 127-139: The current check in assertThemeSource uses
content.includes(`${role.cssVariable}:`) which can match commented-out CSS;
update assertThemeSource to strip/ignore comments from the readTokenSource
result before validating roles (i.e., remove CSS block comments /* ... */ and
any line comments that might appear) and then test for an actual declaration
(use a regex like `${role.cssVariable}\\s*:` on the uncommentedContent) so only
live declarations satisfy the check; keep existing symbols: assertThemeSource,
readTokenSource, semanticRoles, role.cssVariable, themeSourcePath and continue
to throw tokenContractAmbiguous when a role is missing.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: a9c56e64-e1e7-4284-8686-346e06c1b700
📒 Files selected for processing (5)
FORJAMIE.mddocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/tests/engine.test.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: Analyze (javascript)
- GitHub Check: build (macos-latest)
🧰 Additional context used
📓 Path-based instructions (18)
**/*.{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.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/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.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/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.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/token-contract.tsdocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/agent-design-engine/tests/engine.test.mjspackages/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/agent-design-engine/src/token-contract.tsdocs/guides/AGENT_DESIGN_WORKFLOW.mdpackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tsFORJAMIE.md
**/*.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
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
Relevant context retrieved
-
Linear issue JSC-241 ("Agent-native design system: prepare payload model") is present and marked Done; its description references the slice plan at docs/plans/2026-04-28-agent-native-design-system-plan.md and lists validation/stop-rule scope for the Prepare Payload Model. URL: https://linear.app/jscraik/issue/JSC-241/agent-native-design-system-prepare-payload-model.
-
Linear issue JSC-238 ("Agent-native design-system command layer") is present and marked Done; its description references the canonical plan at docs/plans/2026-04-28-agent-native-design-system-plan.md and appears to be the parent epic of JSC-241. URL: https://linear.app/jscraik/issue/JSC-238/agent-native-design-system-command-layer.
-
Metadata from the Linear responses: JSC-241 lists parentId = JSC-238 and both issues show high priority and completion timestamps (completed late 2026-04-29), indicating the PR’s changes implement completed slices from that plan.
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 3 file(s) based on 3 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Why: CodeRabbit found prepare could still misclassify pnpm subcommands, malformed scripts metadata, and commented CSS token declarations. What: Reject unsupported pnpm subcommands and filter-omit selectors in validation commands. Treat present non-object package scripts as deterministic package metadata errors. Ignore commented CSS when validating advertised token role variables. Add focused regressions for the reviewed edge cases. Validation: - Command: pnpm -C packages/agent-design-engine test -> pass (84/84) - Command: pnpm -C packages/cli test -> pass (116/116) - Command: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json > /tmp/agent-design-prepare-review-2.json -> pass - Command: jq '.status, .data.designTokenContract.mode, (.data.validationCommands | length)' /tmp/agent-design-prepare-review-2.json -> pass - Command: pnpm lint -> pass - Command: git diff --check -> pass Co-authored-by: Codex <[email protected]>
Fixed 3 file(s) based on 3 unresolved review comments. Co-authored-by: CodeRabbit <[email protected]>
![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: 0ba61c8691
ℹ️ 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: The PR branch advanced with a CodeRabbit autofix commit while local review fixes were validating. What: Merge the remote branch, keep the local regression tests, preserve CodeRabbit's tsconfig compatibility update, and resolve overlapping prepare parser changes without duplicated subcommand entries. Validation: - Command: pnpm -C packages/agent-design-engine test -> pass (84/84) - Command: pnpm lint -> pass - Command: git diff --check -> pass Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
packages/agent-design-engine/src/token-contract.ts (1)
184-188:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftValidate the remaining published roles against authored token sources.
buildDesignTokenContract()still returnsfocus.ringand allstatus.*roles, but Lines 184-188 only prove DTCG groups forbackground,text,border,accent, andinteractive. If the authored status/focus tokens drift out of the token sources,preparewill still publish those roles as canonical instead of failing closed. Extend the preflight so every emitted role is validated against its authority before it is returned. As per coding guidelines, "Prioritize behavioral risk, missing validation, and governance drift over style-only feedback."Also applies to: 219-225
🤖 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 184 - 188, buildDesignTokenContract() currently only validates a subset of alias groups (via requiredAliasCategories and the color lookup) and misses emitted roles like focus.ring and status.*; extend the preflight checks to verify every role that will be published is present in the authored token sources and fail closed if not. Concretely, after you compute the contract (buildDesignTokenContract) and where you validate the color groups (the loop using requiredAliasCategories and the color variable), iterate the full set of emitted roles (including focus.ring and all status.* entries the contract returns) and for each role confirm it exists on the source token maps; if a role is missing, throw tokenContractAmbiguous(dtcgSourcePath, `<role>` ) to stop publishing. Apply the same extra validation logic in the second validation block referenced around 219-225 so both preflight checks cover all emitted roles.
🤖 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 789-790: The canonical prepare payload currently contains
nondeterministic timestamps via startedAt, startedMs and durationMs; remove
ambient clock/perf usage from core payload by either accepting an injected
clock/perf provider (e.g., pass a clock.now() and perf.now() into the function
that sets startedAt/startedMs and computes durationMs) or by moving those fields
out of the canonical payload into wrapper metadata returned alongside it; update
the function that constructs startedAt/startedMs/durationMs to use the injected
interfaces (or to emit timing only in the caller) so identical inputs produce
identical canonical payloads.
- Around line 371-373: commandTokens currently splits on whitespace and breaks
quoted arguments; replace its implementation (function commandTokens) with a
shell-aware tokenizer that treats single-quoted and double-quoted segments as
atomic tokens (removing the surrounding quotes) and honors backslash escapes
inside double quotes and for escaped characters, then trims and filters empty
tokens; this will ensure callers like inferPackageScript receive correctly
parsed args for commands such as pnpm --dir "./packages/app shell" run lint.
In `@tsconfig.base.json`:
- Line 12: Remove the workspace-wide deprecation suppression by deleting the
"ignoreDeprecations": "5.0" compiler option from tsconfig.base.json so
TypeScript deprecation warnings from the baseline (5.9+) are not masked; locate
the "ignoreDeprecations" property in the root config and remove the entire
key/value entry (or set it to false/omit it) to restore normal deprecation
reporting.
---
Duplicate comments:
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 184-188: buildDesignTokenContract() currently only validates a
subset of alias groups (via requiredAliasCategories and the color lookup) and
misses emitted roles like focus.ring and status.*; extend the preflight checks
to verify every role that will be published is present in the authored token
sources and fail closed if not. Concretely, after you compute the contract
(buildDesignTokenContract) and where you validate the color groups (the loop
using requiredAliasCategories and the color variable), iterate the full set of
emitted roles (including focus.ring and all status.* entries the contract
returns) and for each role confirm it exists on the source token maps; if a role
is missing, throw tokenContractAmbiguous(dtcgSourcePath, `<role>` ) to stop
publishing. Apply the same extra validation logic in the second validation block
referenced around 219-225 so both preflight checks cover all emitted roles.
🪄 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: a93c60ae-637a-4a8c-8173-6be48436e721
📒 Files selected for processing (3)
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.tstsconfig.base.json
📜 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/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/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/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.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/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/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/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/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.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/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/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.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/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/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/token-contract.tspackages/agent-design-engine/src/prepare.ts
🪛 ast-grep (0.42.1)
packages/agent-design-engine/src/token-contract.ts
[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(${role.cssVariable.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&')}\\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)
🪛 GitHub Actions: CI
packages/agent-design-engine/src/token-contract.ts
[error] 133-140: Biome formatter check failed. Formatter would have reformatted string quotes (single to double) in .replace(...) and wrapped a new RegExp(...) template string into a multi-line call.
packages/agent-design-engine/src/prepare.ts
[error] 624-630: Biome formatter check failed. Formatter would have reformatted the throw new DesignEngineError(...) call to a multi-line argument/placeholder style.
🔍 Remote MCP Linear
Additional relevant context found
-
Linear issue JSC-241 ("Agent-native design system: prepare payload model") — Slice 3 implements the engine model for
astudio design prepare: adds engine types, resolves/digests DESIGN.md/guidance/routing/lifecycle/coverage, classifies surface scope, uses deterministic JSON serialization, and "fail closed" behavior for unknown/ambiguous scope; includes validation commands and stop rules. Status: Done; completed 2026-04-29. -
Linear issue JSC-238 ("Agent-native design-system command layer") — Canonical plan and execution slices (contract wiring, routing table, prepare payload model, read-only CLI surface, remediation output, gold examples, abstraction gate). Status: Done; completed 2026-04-29.
Why: A review thread found inferPackageScript skipped value-bearing pnpm flags before run without consuming their values, so valid commands could be misread as scripts. What: Consume known value-bearing pnpm flags in the pre-run parser path and add a regression for pnpm --resume-from ui run agent-design:lint. Validation: - Command: pnpm -C packages/agent-design-engine test -> pass (85/85) - Command: pnpm lint -> pass - Command: git diff --check -> pass - Command: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json > /tmp/agent-design-prepare-review-3.json -> pass - Command: jq '.status, .data.validationCommands[0].packageScript' /tmp/agent-design-prepare-review-3.json -> pass Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (3)
packages/agent-design-engine/src/prepare.ts (2)
371-373:⚠️ Potential issue | 🟠 MajorUse a shell-aware tokenizer for pnpm command parsing.
split(/\s+/)still breaks quoted--dirvalues and other shell-quoted arguments, so valid commands can be misparsed or rejected based only on formatting.🤖 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 371 - 373, The current commandTokens function naively splits on whitespace and thus misparses quoted or escaped arguments (e.g., quoted --dir values); update commandTokens to use a shell-aware tokenizer that understands single/double quotes and backslash escapes (for example, replace the split logic with a parser from a library like 'shell-quote' or implement a small stateful tokenizer) so quoted arguments are returned as single tokens while preserving trimming and filtering behavior; keep the exported function name commandTokens and ensure tests cover quoted, escaped, and unquoted cases.
796-797:⚠️ Potential issue | 🟠 MajorKeep ambient time out of the canonical prepare payload.
startedAtanddurationMsmake identical inputs produce different payloads on every run, which undercuts the PR's canonical/JSON-stable payload goal.As per coding guidelines, "No ambient randomness/time in core logic; inject seeds/clocks/IDs"
Also applies to: 888-890
🤖 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 796 - 797, The canonical prepare payload currently includes ambient time fields (startedAt, startedMs, durationMs) which makes identical inputs produce different outputs; remove those from the payload and instead accept an injectable clock/timer via the existing prepare function (or its options) so callers can provide deterministic timestamps or omit them. Replace direct new Date()/performance.now() references (startedAt, startedMs) with calls to an injected now() and perfNow() functions (or an optional startedAt/startedMs parameter) and stop writing durationMs into the canonical payload; if a duration is required for non-canonical telemetry, compute it locally and do not include it in the canonical/JSON-stable output. Ensure you update any uses in the prepare function and the other spot where durationMs is set so only the injected values are used.packages/agent-design-engine/src/token-contract.ts (1)
188-202:⚠️ Potential issue | 🟠 MajorValidate the exported token policy text, not just the markdown headings.
These checks only prove the docs still contain
"Token Notes"/"Token discipline", whileallowedRoles[*].useFor/avoidForandforbiddenTokenPatternsare still hard-coded below. A contradictory edit under the same headings would pass validation and ship stale guidance as authoritative.As per coding guidelines, "Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable."
Also applies to: 215-228
🤖 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 188 - 202, The current assertPolicySources only checks for headings; update it to validate the actual exported token policy content: in assertPolicySources (and the same logic referenced around lines 215-228) read the token policy blocks from readTokenSource(designSourcePath) and readTokenSource(professionalContractSourcePath), extract/parse the structured policy payload (e.g., fenced code block or YAML/JSON under the "Token Notes"/"Token discipline" headings), and compare the parsed values against the runtime constants used in this module (allowedRoles[*].useFor / avoidFor and forbiddenTokenPatterns). If parsing fails or any parsed field contradicts the in-code values, throw tokenContractAmbiguous with a clear message; ensure the check runs after signal?.throwIfAborted() and covers both sources so docs must match the exported policy, not just the heading text.
🤖 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 672-703: The code accepts commands where
inferPackageScript(command.command) returns undefined (non-pnpm shell commands),
which lets unparsed arbitrary shell commands slip through; update the validation
in the loop inside prepare() to reject any command where inferPackageScript(...)
is undefined unless the command explicitly matches a safe allowlist: call
inferPackageScript(command.command) and if it returns undefined then throw a
DesignEngineError (same shape/code "E_DESIGN_VALIDATION_COMMAND_INVALID")
explaining non-pnpm commands are disallowed (or alternatively require an
explicit allowlist check) before calling getPackageScripts(...) and before
pushing into normalized; reference inferPackageScript, getPackageScripts,
normalized, commands and DesignEngineError so reviewers can locate and update
the check.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 160-185: In assertDtcgSource, after you fetch each category group
from color (loop over requiredAliasCategories and the local variable group),
validate that it contains the required mode branches (e.g. "light", "dark",
"highContrast" or the project’s required modes) and that each mode branch is a
non-empty object (not null/array). If any mode is missing or not an object/token
map, throw tokenContractAmbiguous(dtcgSourcePath, `${category} ${mode} token
group`) (or a similar message) so malformed authority objects like
{"color":{"background":{}}} fail; update the validation logic to check per-mode
existence and type inside the existing for loop in assertDtcgSource.
- Around line 145-157: assertAliasMapSource currently validates raw file text so
commented-out exports or buildModeMap entries pass; fix by stripping JS/TS
comments from the content returned by readTokenSource before running the regex
and includes checks. In assertAliasMapSource, after reading content
(readTokenSource(...)), remove line/block comments (both // and /* */) and then
run the /\bexport\s+const\s+tokenAliasMap\b/ test and the
requiredAliasCategories.includes(`${category}: buildModeMap("${category}")`)
checks against the comment-free string; preserve existing error throws via
tokenContractAmbiguous(aliasMapSourcePath, ...) when checks fail. Ensure the
comment-stripping logic handles template strings and does not corrupt source
encoding.
---
Duplicate comments:
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 371-373: The current commandTokens function naively splits on
whitespace and thus misparses quoted or escaped arguments (e.g., quoted --dir
values); update commandTokens to use a shell-aware tokenizer that understands
single/double quotes and backslash escapes (for example, replace the split logic
with a parser from a library like 'shell-quote' or implement a small stateful
tokenizer) so quoted arguments are returned as single tokens while preserving
trimming and filtering behavior; keep the exported function name commandTokens
and ensure tests cover quoted, escaped, and unquoted cases.
- Around line 796-797: The canonical prepare payload currently includes ambient
time fields (startedAt, startedMs, durationMs) which makes identical inputs
produce different outputs; remove those from the payload and instead accept an
injectable clock/timer via the existing prepare function (or its options) so
callers can provide deterministic timestamps or omit them. Replace direct new
Date()/performance.now() references (startedAt, startedMs) with calls to an
injected now() and perfNow() functions (or an optional startedAt/startedMs
parameter) and stop writing durationMs into the canonical payload; if a duration
is required for non-canonical telemetry, compute it locally and do not include
it in the canonical/JSON-stable output. Ensure you update any uses in the
prepare function and the other spot where durationMs is set so only the injected
values are used.
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 188-202: The current assertPolicySources only checks for headings;
update it to validate the actual exported token policy content: in
assertPolicySources (and the same logic referenced around lines 215-228) read
the token policy blocks from readTokenSource(designSourcePath) and
readTokenSource(professionalContractSourcePath), extract/parse the structured
policy payload (e.g., fenced code block or YAML/JSON under the "Token
Notes"/"Token discipline" headings), and compare the parsed values against the
runtime constants used in this module (allowedRoles[*].useFor / avoidFor and
forbiddenTokenPatterns). If parsing fails or any parsed field contradicts the
in-code values, throw tokenContractAmbiguous with a clear message; ensure the
check runs after signal?.throwIfAborted() and covers both sources so docs must
match the exported policy, not just the heading text.
🪄 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: 79ee4ae7-2826-472f-89d4-920b31d69e88
📒 Files selected for processing (3)
packages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/token-contract.tspackages/agent-design-engine/tests/engine.test.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: Analyze (javascript)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
🧰 Additional context used
📓 Path-based instructions (15)
**/*.{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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/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/tests/engine.test.mjspackages/agent-design-engine/src/prepare.tspackages/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/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
**/*.{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
🔍 Remote MCP Linear
Additional relevant facts for PR review
-
Linear issue JSC-241 ("Agent-native design system: prepare payload model") — Slice 3 implemented the engine model for
astudio design prepare: adds types, resolves/digests DESIGN.md/guidance/routing/lifecycle/coverage, classifies surface scope with protected>warn>exempt>unknown precedence, uses deterministic JSON serialization, and enforces fail-closed behavior for unknown/ambiguous scope. Validation commands are safety-classed and runtime budget telemetry is emitted. Status: Done (completed 2026-04-29). -
Linear issue JSC-238 ("Agent-native design-system command layer") — Canonical plan for the command layer covering contract wiring, routing table, prepare payload model, read-only CLI surface, remediation output, gold example inventory, and abstraction proposal gate. The plan is marked Done (completed 2026-04-29) and is the parent of JSC-241.
Why: CodeRabbit found prepare still had shell-tokenization drift, ambient timing in the canonical payload, and a TypeScript deprecation suppression added by autofix.
What: Parse quoted command tokens before pnpm script inference, remove nondeterministic timing from the prepare payload contract, remove the workspace-wide ignoreDeprecations setting, and add regressions for quoted package dirs plus stable prepare serialization.
Validation:\n- Command: pnpm -C packages/agent-design-engine test -> pass (86/86)\n- Command: pnpm -C packages/cli test -> pass (116/116)\n- Command: pnpm lint -> pass\n- Command: git diff --check -> pass\n- Command: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json > /tmp/agent-design-prepare-review-4.json -> pass\n- Command: jq '.status, has("timing"), (.data | has("timing")), .data.validationCommands[0].packageScript' /tmp/agent-design-prepare-review-4.json -> pass
Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 994cc567bd
ℹ️ 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: Fresh PR review found remaining fail-open paths in prepare validation and token authority checks.
What: Require validation commands to be parsed pnpm package-script invocations, consume reporter-valued pnpm options, read prepare sources in deterministic order, strip comments before alias-map authority checks, and require DTCG color mode groups for every advertised alias category.
Validation:\n- Command: pnpm -C packages/agent-design-engine test -> pass (91/91)\n- Command: pnpm -C packages/cli test -> pass (116/116)\n- Command: pnpm lint -> pass\n- Command: git diff --check -> pass\n- Command: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json > /tmp/agent-design-prepare-review-5.json -> pass\n- Command: jq '.status, (.data.validationCommands[] | select(.command == "pnpm agent-design:lint") | .packageScript), (.data | has("timing"))' /tmp/agent-design-prepare-review-5.json -> pass
Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 01ed0810c9
ℹ️ 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: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/agent-design-engine/src/types.ts (1)
328-351:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe canonical prepare payload is still missing the evidence locators called out for this PR.
PreparePayloadis the source-of-truth shape forastudio.design.prepare.v1, but it still has no wrapper-evidence or final-plan-evidence locator fields. That omission propagates into the payload builder and schema, so downstream agents still cannot rely on those provenance links before editing. Please add them here as required fields and thread them through the schema/tests in the same change.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 `@packages/agent-design-engine/src/types.ts` around lines 328 - 351, The PreparePayload interface is missing the evidence locator fields required by the new PR; update the PreparePayload type to include two required fields (e.g., wrapperEvidenceLocator and finalPlanEvidenceLocator) with appropriate types (string or a dedicated EvidenceLocator type) and then propagate these fields through the payload builder and JSON schema and tests (update code that constructs PreparePayload, validation/schema definitions, and unit/integration tests that assert the prepare payload shape) so downstream agents can rely on those provenance links; reference the PreparePayload interface, the payload builder function that emits astudio.design.prepare.v1, and the schema/tests that validate this message when making the changes.
♻️ Duplicate comments (1)
packages/agent-design-engine/src/token-contract.ts (1)
253-267:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftValidate the token policy you publish, not just the section headings.
assertPolicySources()only proves those docs still contain"Token Notes"/"Token discipline", but the returned contract still hard-codes theuseFor,avoidFor, andforbiddenTokenPatternsprose. A contradictory edit under the same headings would still pass and ship stale guidance as authoritative. Please either derive these strings from the authored docs or assert the exact statements you emit.As per coding guidelines, "Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable."
Also applies to: 280-294
🤖 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 253 - 267, assertPolicySources currently only checks for the presence of headings by calling readTokenSource for designSourcePath and professionalContractSourcePath and throwing tokenContractAmbiguous if headings are missing, but it doesn't validate or extract the actual policy prose used to populate useFor, avoidFor, and forbiddenTokenPatterns; update assertPolicySources (or add a new helper called from it) to parse and extract the explicit policy statements from designSource and professionalContractSource (e.g., the exact sentences/blocks for useFor, avoidFor, forbiddenTokenPatterns) and either (a) return those strings for downstream code to populate the token contract instead of hard-coding them, or (b) assert equality between the extracted statements and the current emitted strings and throw tokenContractAmbiguous if they differ, ensuring the code references the extracted policy text rather than only verifying headings.
🤖 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 609-645: resolvePackageJsonPath currently realpaths the package
directory but returns path.join(resolvedPackageDir, "package.json") without
verifying the actual package.json file isn't a symlink to outside the workspace;
update resolvePackageJsonPath to compute the packageJsonPath =
path.join(resolvedPackageDir, "package.json"), call realpath(packageJsonPath)
(or equivalent) to get realPackageJson, and then re-run the workspace boundary
check (e.g., isWithinDirectory(realRoot, realPackageJson)) and throw the same
E_DESIGN_VALIDATION_COMMAND_INVALID/E_DESIGN_PACKAGE_JSON errors if it fails;
also apply the same realpath+isWithinDirectory guard in the related location
referenced by loadPackageScripts so package scripts cannot escape the workspace.
In
`@packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json`:
- Around line 102-132: The shared "validationCommand" schema leaves
packageScript, expectedOutcome, and timeoutClass optional, allowing AJV to
accept prepare payloads that lack metadata canonicalized by buildPreparePayload;
create a prepare-specific command schema (e.g., "prepareValidationCommand") or
update the references in preparePayload and recommendedRoutes to require
["command","safetyClass","reason","packageScript","expectedOutcome","timeoutClass"]
so AJV enforces those fields when validationCommand is used for prepare
payloads; update schema references to point to the new/updated definition and
ensure the required list matches the canonicalization performed by
buildPreparePayload.
---
Outside diff comments:
In `@packages/agent-design-engine/src/types.ts`:
- Around line 328-351: The PreparePayload interface is missing the evidence
locator fields required by the new PR; update the PreparePayload type to include
two required fields (e.g., wrapperEvidenceLocator and finalPlanEvidenceLocator)
with appropriate types (string or a dedicated EvidenceLocator type) and then
propagate these fields through the payload builder and JSON schema and tests
(update code that constructs PreparePayload, validation/schema definitions, and
unit/integration tests that assert the prepare payload shape) so downstream
agents can rely on those provenance links; reference the PreparePayload
interface, the payload builder function that emits astudio.design.prepare.v1,
and the schema/tests that validate this message when making the changes.
---
Duplicate comments:
In `@packages/agent-design-engine/src/token-contract.ts`:
- Around line 253-267: assertPolicySources currently only checks for the
presence of headings by calling readTokenSource for designSourcePath and
professionalContractSourcePath and throwing tokenContractAmbiguous if headings
are missing, but it doesn't validate or extract the actual policy prose used to
populate useFor, avoidFor, and forbiddenTokenPatterns; update
assertPolicySources (or add a new helper called from it) to parse and extract
the explicit policy statements from designSource and professionalContractSource
(e.g., the exact sentences/blocks for useFor, avoidFor, forbiddenTokenPatterns)
and either (a) return those strings for downstream code to populate the token
contract instead of hard-coding them, or (b) assert equality between the
extracted statements and the current emitted strings and throw
tokenContractAmbiguous if they differ, ensuring the code references the
extracted policy text rather than only verifying headings.
🪄 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: af04db3d-8f62-4605-9800-21cd72079180
📒 Files selected for processing (7)
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.mjspackages/cli/tests/cli.test.mjspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.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). (3)
- GitHub Check: build (macos-latest)
- GitHub Check: Analyze (javascript)
- GitHub Check: build (ubuntu-latest)
🧰 Additional context used
📓 Path-based instructions (16)
**/*.{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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/types.tspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/cli/tests/cli.test.mjspackages/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/types.tspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/token-contract.tspackages/cli/tests/cli.test.mjspackages/agent-design-engine/src/prepare.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.json
🔍 Remote MCP Linear
Relevant Linear context for PR #160
-
JSC-241 "Agent-native design system: prepare payload model" — Done. Confirms scope implemented by this PR: add astudio.design.prepare.v1 engine types, deterministic JSON serializer, fail‑closed behavior for unknown/ambiguous scope, safety‑classed validation commands, runtime budget telemetry, and required inputs (DESIGN.md, guidance, routing, lifecycle, coverage, contract). Also lists validation steps used by the author.
-
JSC-240 "Agent-native design system: routing table contract" — Done. Establishes authored route authority (docs/design-system/AGENT_UI_ROUTING.json), canonical needs, validation commands, and exported resolver functions that PR changes rely on; JSC-241 is blocked by this issue (ordering confirmed).
-
JSC-238 "Agent-native design-system command layer" — Done. Parent plan describing execution slices that map directly to this PR (contract wiring, routing table, prepare payload model, read‑only CLI, remediation output, gold examples, abstraction gate) and references review artifacts used to harden the plan.
Why: PR review found prepare validation still handled edge pnpm commands and package metadata inconsistently.
What: Accept the pnpm run-script alias, validate script names after pnpm run as package scripts, constrain package.json realpaths inside the workspace, and require normalized prepare validation command metadata in the schema.
Validation:
- pnpm -C packages/agent-design-engine test -> pass (94/94)
- pnpm -C packages/cli test -> pass (116/116)
- pnpm lint -> pass
- git diff --check -> pass
- pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json > /tmp/agent-design-prepare-review-6.json -> pass
- jq '.status, (.data.validationCommands[0] | has("packageScript") and has("expectedOutcome") and has("timeoutClass"))' /tmp/agent-design-prepare-review-6.json -> pass ("success", true)
Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da04a85a8f
ℹ️ 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 still had unresolved parser blockers for pnpm -C=dir and recursive run aliases. What: Parse -C=dir consistently, support recursive/multi/m run aliases as script-bearing validation commands, and add fixture coverage. Impact/Risk: Narrow prepare validation-command parser change; keeps fail-closed package-script validation and updates FORJAMIE. Validation: pnpm -C packages/agent-design-engine test -> pass (96/96). Validation: pnpm docs:lint -> pass. Validation: git diff --check -> pass. Co-authored-by: Codex <[email protected]>
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`:
- Around line 223-229: The changelog for the prepare contract incorrectly states
that PreparePayload includes timing evidence/fields; update the FORJAMIE.md
prepare-contract section to remove any references to timing being carried in
PreparePayload (references to "timing evidence", "timing fields", or similar) so
the doc matches the implemented contract and the earlier guidance note; ensure
mentions of PreparePayload, prepare-contract, and the changelog paragraph are
edited to reflect that timing was moved out of the canonical public payload and
leave a short note pointing readers to the new timing/location if needed.
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 758-795: The current loop only verifies the pnpm syntax and
existence of the script (inferPackageScript, getPackageScripts) but trusts the
route-provided safetyClass; update prepare.ts to treat "script exists" as
insufficient by validating the resolved packageScript against an explicit
read-only allowlist/manifest or repository-derived evidence (e.g., a validation
manifest or harness contracts) before accepting a read-only safetyClass: add a
lookup call (e.g., isScriptReadOnly or
getValidationManifest/getTrustedValidationWrappers) using inferred.packageDir
and packageScript, and if the script is not explicitly marked read-only in that
trusted source, either downgrade/override safetyClass to a conservative value or
throw a DesignEngineError (fail-closed); ensure normalized still includes
packageScript but only marks it read-only when the trusted check passes.
In
`@packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json`:
- Around line 370-375: The schema allows an empty validationCommands array, so
update the two "validationCommands" array schemas (the ones with "items": {
"$ref": "#/definitions/prepareValidationCommand" } at both the top-level and the
route-level) to include "minItems": 1 so the array cannot be empty; keep the
existing items ref and only add the minItems constraint to both occurrences
referenced in the diff.
- Around line 418-544: The preparePayload schema disallows unknown fields
(additionalProperties:false) but is missing the new evidence locator properties;
add the engine’s canonical locator properties "wrapper-evidence" and
"final-plan-evidence" to the preparePayload "properties" block and include those
same property names in the "required" array so AJV will accept and validate the
new wrapper-evidence and final-plan-evidence locators (update the preparePayload
object that currently lists "kind", "ok", ... "openDecisions" to include these
two locator entries).
🪄 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: 6c477710-114f-4d29-bd99-2bf4fa6ca179
📒 Files selected for processing (5)
FORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/tests/cli.test.mjspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json
📜 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 (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: Analyze (javascript)
🧰 Additional context used
📓 Path-based instructions (19)
**/*.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
**/*.{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/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/cli/tests/cli.test.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/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/cli/tests/cli.test.mjsFORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{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/cli.test.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/cli/tests/cli.test.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/cli/tests/cli.test.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/cli/tests/cli.test.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.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:
packages/cli/tests/cli.test.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/cli/tests/cli.test.mjspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/cli/tests/cli.test.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/cli/tests/cli.test.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/cli/tests/cli.test.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/cli/tests/cli.test.mjsFORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
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
**/*.{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
**/*.{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
**/*.{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
🔍 Remote MCP Linear
Relevant context found
-
JSC-241 "Agent-native design system: prepare payload model" — confirms scope implemented by this PR: add engine types for astudio.design.prepare.v1, resolve/digest DESIGN.md/guidance/routing/lifecycle/coverage, use deterministic JSON serializer, and fail-closed behavior for unknown/ambiguous scope. Validation commands listed (type-check, tests, boundaries, git diff) and Stop rule: do not expose CLI until deterministic fixtures exist.
-
JSC-240 "Agent-native design system: routing table contract" — establishes AGENT_UI_ROUTING.json as authored v1 route authority, required exports (resolveRouteForNeed/surface/remediation), canonical needs list, and drift detection expectations. Relevant because PR tightens routing/validation command handling and relies on this routing authority.
-
JSC-238 "Agent-native design-system command layer" — parent plan and execution slices that the PR implements: contract wiring, routing table, prepare payload model, read-only CLI, remediation output, gold examples, and proposal gate. Includes review-evidence artifacts and acceptance criteria that the PR is expected to meet (e.g., deterministic serialization, read-only guarantees).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 30edd1eb09
ℹ️ 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 that prepare verified package-script existence but still trusted route-authored read_only metadata, and missed compact pnpm -C syntax plus schema guardrails for validation and evidence fields. What: add an explicit read-only package-script allowlist, parse compact -C<dir> pnpm forms, emit wrapper/final plan evidence locators, require non-empty prepare validation commands in the CLI schema, and remove stale timing wording from FORJAMIE. Validation: - pnpm -C packages/agent-design-engine test -> pass (98/98) - pnpm -C packages/cli test -> pass (116/116) - cat packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json | jq . >/dev/null -> pass - pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json >/tmp/agent-design-prepare-ci-fix.json -> pass - cat /tmp/agent-design-prepare-ci-fix.json | jq '.data["wrapper-evidence"], .data["final-plan-evidence"], (.data.validationCommands | length)' -> pass\n- pnpm docs:lint -> pass\n- git diff --check -> pass Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1927d549ea
ℹ️ 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: 2
🤖 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 473-479: trustedReadOnlyPackageScripts is currently keyed only by
bare script names allowing any workspace package to spoof a "trusted" read-only
script; change the trust key to be anchored to the resolved package and script
(e.g., `${packageDir}#${packageScript}`) or to the resolved script body/wrapper
evidence before granting trust. Update the data structure and any checks that
reference trustedReadOnlyPackageScripts (the Set and places that call it) to
build and compare fully-qualified keys using the package directory path
(packageDir) plus the script name, or alternatively compute and compare a
hash/signature of the resolved script wrapper, and ensure all places that
previously matched by name use the new fully-qualified resolver so trust is
bound to the package origin rather than the bare script name.
- Around line 974-989: The code currently allows building a prepare payload with
zero validation commands; fix by validating after normalization: when computing
recommendedRoutes (use normalizeValidationCommands and onlyReadOnly) ensure that
for any route provided the resulting validationCommands array is non-empty and,
if empty, throw a deterministic engine error (include route identifier/name in
the message). Also ensure the top-level prepare payload's validationCommands
(preparePayload) is validated the same way—if preparePayload exists or is
required and its normalized validationCommands are empty, throw an error rather
than returning an empty array. Reference normalizeValidationCommands,
onlyReadOnly, recommendedRoutes, validationCommands and preparePayload when
adding these checks and error throws.
🪄 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: 1c843453-7e25-4234-8b84-a754b0dfbe50
📒 Files selected for processing (5)
FORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json
📜 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 (19)
**/*.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
**/*.{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/fixtures/design-schemas/astudio-design-command.v1.schema.jsonpackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.jsonFORJAMIE.mdpackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.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
**/*.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
**/*.{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.mdpackages/agent-design-engine/src/types.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/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/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/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/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/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/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/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/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/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/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/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/prepare.ts
🔇 Additional comments (10)
FORJAMIE.md (10)
179-179: LGTM!The guidance correctly reflects the design decision to keep ambient timing out of the canonical PreparePayload contract, as confirmed by the PreparePayload interface definition.
217-217: LGTM!The hardening lane guidance accurately reflects the
safeForAutomaticImplementationcontract as implemented in the prepare.ts logic, requiring both successful validation (ok=true) and known scope.
221-221: LGTM!The date-based section structure follows the established changelog pattern in the document.
223-229: Past review concern appears resolved.The past review comment flagged this section for containing stale timing references in PreparePayload, but the current text (Lines 223-229) contains no mention of timing fields or timing evidence being part of the payload. The guidance now correctly aligns with Line 179's statement that timing is excluded from the canonical public contract.
226-226: LGTM!The
designTokenContractfield is correctly documented and confirmed to exist in the PreparePayload interface definition.
230-230: LGTM!The description of
safeForAutomaticImplementationfailing closed when error-severity open decisions exist correctly reflects the implemented logic whereok = openDecisions.every((decision) => decision.severity !== "error").
460-464: LGTM!The machine-readable metadata is now internally consistent, with
last_updated: 2026-05-01matching the human-readable date on Line 19, resolving the previous date-alignment concern.
79-81: Files are present; no action needed.Both referenced files (
docs/specs/2026-04-30-agent-design-prepare-north-star-spec.mdanddocs/plans/2026-04-30-agent-design-prepare-north-star-plan.md) exist in the repository and are correctly linked in FORJAMIE.md.
226-227: All documented error codes are present in the implementation. No action required.
214-214: Wrapper script definition and package dependencies are accurate.Verification confirms all claims in line 214:
agent-design:preparecorrectly buildsagent-design-engine,design-system-guidance,skill-ingestion, and the CLI before running the prepare operation- No default
--surfaceis hardcoded; callers must provide it explicitly- The
--silentpattern for JSON capture is demonstrated in theagent-design:prepare:smokescriptAll guidance is correct and well-documented.
Why: PR review found that prepare validation trust was still script-name based and that malformed designTokenContract payloads could bypass masking. What: bind trusted validation scripts to packageDir#script, keep stopped prepare payloads schema-valid with a trusted lint fallback, reject routes with no trusted read-only validation command, and redact malformed designTokenContract payloads. Validation: pnpm -C packages/agent-design-engine test -> pass (100/100) Validation: pnpm -C packages/cli test -> pass (117/117) Validation: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx --json >/tmp/agent-design-prepare-final-tighten.json -> pass Validation: cat /tmp/agent-design-prepare-final-tighten.json | jq '(.data.validationCommands | length), .data.validationCommands[0].packageScript, .data.recommendedRoutes[0].validationCommands[0].packageScript' -> pass (1, agent-design:lint, agent-design:lint) Validation: pnpm docs:lint -> pass Validation: git diff --check -> pass Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1f7566d96
ℹ️ 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: 6
🤖 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 27: Update the table row for "Agent Design Prepare plan" so it explicitly
acknowledges that while the reviewer loop is green the PR remains merge-blocked
pending required checks; change the status wording to something like "Green —
review green, merge blocked pending required checks" (or append “review green,
merge-blocked pending required checks”) to avoid contradiction between the lane
status and the PR objective; modify the single table cell text for the "Agent
Design Prepare plan" row in FORJAMIE.md to include that brief note.
- Line 214: Update the documentation example to use the actual wrapper
invocation without duplicating the --json flag: reference the wrapper script
name agent-design:prepare (which already appends --json in package.json) and
show callers using pnpm --silent agent-design:prepare --surface <path> (no extra
--json), keep the note about building workspace packages
(packages/agent-design-engine, packages/design-system-guidance,
packages/skill-ingestion) before the CLI like pnpm agent-design:lint does, and
emphasize using --silent when capturing JSON to avoid lifecycle banners.
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 1009-1013: Currently fallbackPrepareValidationCommands is
normalized eagerly via normalizeValidationCommands into
fallbackValidationCommands before the code determines if the fallback is needed;
defer calling normalizeValidationCommands until after the decision point so the
fallback is normalized only when actually used. Move the
normalizeValidationCommands(fallbackPrepareValidationCommands, resolvedRoot,
signal) call into the branch where the code decides to use the fallback (replace
uses of fallbackValidationCommands there), keeping the same arguments
(resolvedRoot, signal) and preserving behavior when the fallback is required.
- Around line 553-575: In readPnpmRunScript(), tighten parsing to fail-closed:
explicitly detect and reject recursive/multi forms ("-r", "--recursive",
"recursive", "multi", "m") wherever flags/subcommands are inspected (similar to
how filter forms are rejected) and throw the same kind of rejection; also
disallow any trailing tokens after the script name by treating the first
non-flag, non-value token as the only accepted script and throwing if additional
tokens follow. Update the token-handling logic that currently uses
pnpmRunOptionsWithValues and the "-" checks so that these new recursive tokens
are handled explicitly and extra trailing tokens cause an error instead of being
ignored.
In `@packages/cli/src/utils/mask.ts`:
- Around line 89-105: maskPublicDesignTokenRole currently returns non-object or
array values unchanged (e.g., a stray string in allowedRoles), risking secret
leakage; change the early-return branch in maskPublicDesignTokenRole so
malformed role entries are redacted instead of returned — call the existing
maskFieldValue helper (e.g., maskFieldValue('allowedRoles', value, masks,
inDebugMode) or similar) when value is null, not an object, or an array; keep
publicDesignTokenRoleKeys logic for proper objects and ensure behavior matches
maskPublicDesignTokenContract's fail-closed handling.
In `@packages/cli/tests/mask.test.mjs`:
- Around line 53-86: Add an assertion to verify that maskObject preserves the
designTokenContract.sourceRefs field: after calling maskObject(input) assert
that masked.designTokenContract.sourceRefs deep-equals
input.designTokenContract.sourceRefs (or matches the expected array),
referencing maskObject and the designTokenContract.sourceRefs property to ensure
this allowlisted field is covered by the test.
🪄 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: f23be6c8-55e2-40c2-b2fc-120dfa6dba1e
📒 Files selected for processing (5)
FORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/src/utils/mask.tspackages/cli/tests/mask.test.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 (22)
**/*.{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/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.ts
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
**/*.{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/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.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/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.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/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.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/cli/src/utils/mask.tspackages/cli/tests/mask.test.mjsFORJAMIE.mdpackages/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/cli/src/utils/mask.tspackages/cli/tests/mask.test.mjsFORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
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
🔍 Remote MCP Linear
Perfect. Now I have enough context. Let me compile the relevant findings for the PR review.
Summary of Additional Context for PR #160 Review
Core Dependencies & Execution Scope
This PR directly fulfills JSC-241 ("Agent-native design system: prepare payload model"), which is Slice 3 of the larger JSC-238 ("Agent-native design-system command layer") execution plan. The work is part of a sequenced initiative to build the semantic payload behind astudio design prepare inside packages/agent-design-engine before exposing the CLI command.
Parent issue JSC-238 defines the complete goal: Turn the existing agent-readable design-system guidance into an agent-native command layer where agents receive one canonical prepare command that returns deterministic routing, state, example, validation, safety, and recovery guidance before editing UI. This PR represents a key hardening phase following foundational contract-wiring (JSC-239), routing-table (JSC-240), and payload-model work.
Key Requirements from JSC-241 Stop Rule
The Linear issue explicitly states validation requirements that align closely with the PR changes:
- Add engine types for
astudio.design.prepare.v1✓ - Resolve and digest
DESIGN.md, guidance config, routing JSON, lifecycle, coverage, and professional UI contract ✓ - Classify surface scope/kind with protected > warn > exempt > unknown precedence ✓
- Use the canonical engine JSON serializer for deterministic JSON-mode output ✓
- 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 ✓
The issue also specifies a stop rule: "Do not expose CLI commands until the engine model has deterministic fixtures for protected, warn, exempt, unknown, overlap, and symlink-normalized scopes"—which the test expansion in packages/agent-design-engine/tests/engine.test.mjs (+776 lines) directly addresses.
Related Specification Context
JSC-238's description references the canonical technical plan at docs/plans/2026-04-28-agent-native-design-system-plan.md and companion spec at docs/specs/2026-04-28-agent-native-design-system-spec.md. Both of these spec documents are added in this PR (referenced in the AI summary as 548 and 754 lines respectively). The PR hardening addresses specifically:
- No public
contextcommand — only read-onlypreparepathway - Deterministic serialization — including token contract construction priorities, timing stability, and comment-stripping for alias-map authority checks
- Typed waivers and XOR selector semantics — as foundational to the contract
- Missing-example diagnostics (not fatal) — mapping to
openDecisionsentries with deterministic error codes - Validation command schema — including pnpm package-script parsing with compact
-Cforms and workspace constraints
Test Hardening Specifics
The PR expands packages/agent-design-engine/tests/engine.test.mjs by 776 lines, adding:
-
Deterministic negative test suite covering:
- Missing/ambiguous token contract sources
- Malformed DTCG JSON structures
- Missing design vs guidance contract sources with explicit precedence
- Guidance contract mode drift
- Invalid validation command definitions
- pnpm command-string parsing and normalization rules
- Security failures (symlinks escaping repo)
- Fixture-schema validation (invalid guidance JSON/schema, routing schema, lifecycle schema, coverage schema)
-
Routing diagnostics tightening — enforcing:
- Route examples missing →
E_DESIGN_ROUTE_EXAMPLE_MISSINGwithseverity: "error"andnextAction: "stop" - Unknown surface scopes →
E_DESIGN_SURFACE_SCOPE_UNKNOWNwithseverity: "error"andnextAction: "escalate"
- Route examples missing →
-
Serialization determinism — asserting identical serialized output across two consecutive
buildPreparePayloadruns
Validation Checkpoint: CLI Contract Schema
The PR introduces a new preparePayload JSON Schema definition in packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json (+559 lines) with:
- Required prepare-result fields (routing metadata,
recommendedRoutes,requiredStates,forbiddenPatterns,validationCommands, digest fields,openDecisions) - Constrained safety class and timeout class enums for validation commands
- Design token role/contract object validation with
mode: "semantic-only" - New
prepareDigestformat with SHA256 enforcement
This schema is enforced by a new root-level allOf conditional branch in the command envelope that routes data.kind = "astudio.design.prepare.v1" payloads through explicit preparePayload validation.
Masking & Publication Rules
The CLI masking refactor in packages/cli/src/utils/mask.ts (+79 lines) implements specialized handling for "public design token contract" objects:
- Preserves fixed allowlist of contract keys (
mode,forbiddenTokenPatterns,allowedRoles) - Maps each role through
maskPublicDesignTokenRoleto preserve role-specific allowlist while redacting secret-bearing fields - Fully redacts malformed/non-object
designTokenContractvalues
This directly supports the spec's safety invariant: "default returned validation commands must be safetyClass: 'read_only'" and ensures the public contract cannot accidentally leak sensitive token data.
Commit-Level Validation
The final commit "fix(agent-design): bind prepare validation trust" cites passing validation:
- Agent-design-engine test suite passed
- CLI test suite passed
- CLI prepare run produced expected JSON with correct
validationCommandslength andpackageScriptvalues - Docs lint passed
- Git diff checks passed
Risk & Scope
This PR represents stricter prepare-payload validation and documentation updates; it does not add new CLI surfaces, only hardens the existing prepare contract. The rollback plan is straightforward: revert if downstream consumers depend on the previous looser contract.
🔇 Additional comments (5)
packages/cli/src/utils/mask.ts (4)
55-65: LGTM!The
getKeySegmentshelper correctly handles camelCase-to-snake_case conversion and provides consistent key normalization for bothshouldMaskFieldandisPublicDesignTokenContractKey. The regex splitting approach properly isolates segments like["design", "token", "contract"]fromdesignTokenContract.
80-87: LGTM!The allowlists correctly mirror the
DesignTokenContractandDesignTokenRoleinterfaces defined inpackages/agent-design-engine/src/types.ts. This ensures all non-secret fields are preserved while unknown fields are masked.
107-127: LGTM!The
maskPublicDesignTokenContractfunction correctly implements fail-closed behavior for malformed top-level contracts (line 112-114), preserves allowlisted contract keys, and delegates role masking appropriately. The special handling forallowedRolesarrays ensures each role object is processed individually.
141-150: LGTM!The integration into
maskValueRecursivecorrectly detects design token contract keys viaisPublicDesignTokenContractKeyand delegates to the specialized masking path while falling back tomaskFieldValuefor all other properties.packages/cli/tests/mask.test.mjs (1)
88-100: LGTM!Good fail-closed test coverage for malformed
designTokenContractpayloads. Testing both primitive string and array cases ensures the masking logic handles unexpected input shapes securely.
Why: PR review found remaining fail-open paths in prepare validation command parsing and token-contract masking. What: Reject recursive and argument-forwarding pnpm invocations, normalize fallback checks only when needed, and recursively sanitize malformed public design-token contract fields. Impact/Risk: Stricter prepare validation may reject route metadata that forwards args or runs scripts recursively; this is intended for the agent-safe contract. Validation: - Command: pnpm -C packages/agent-design-engine test -> pass - Command: pnpm -C packages/cli test -> pass - Command: pnpm docs:lint -> pass - Command: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx > /tmp/agent-design-prepare-review-tighten.json -> pass - Command: jq '.kind, .data.validationCommands[0].packageScript' /tmp/agent-design-prepare-review-tighten.json -> pass - Command: git diff --check -> pass Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 27: Update the status cell for the "Agent Design Prepare plan" row in
FORJAMIE.md so it no longer claims "Green"; change the status text to "review
green, merge-blocked pending required checks" (or equivalent phrasing used
elsewhere in the doc) to reflect that reviewer loop items are satisfied but the
PR is still blocked by required checks; ensure the row text remains consistent
with other status-lane phrasing in the file.
In `@packages/cli/src/utils/mask.ts`:
- Around line 127-139: The branch handling allowedRoles currently only treats
array-shaped values as special and falls back to generic maskFieldValue for
malformed inputs; update the allowedRoles branch so it validates that val is an
array of strings before mapping through maskPublicDesignTokenRole and otherwise
calls maskMalformedPublicValue(val, masks, inDebugMode); this ensures malformed
allowedRoles (e.g., a raw string secret) is redacted like the other
contract-special fields rather than passed to maskFieldValue. Use the existing
symbols allowedRoles, maskPublicDesignTokenRole, maskMalformedPublicValue,
masks, and inDebugMode to implement 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: a981bc75-05fd-496c-8342-b2d4e4877fa9
📒 Files selected for processing (5)
FORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/src/utils/mask.tspackages/cli/tests/mask.test.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 (22)
**/*.{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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tsFORJAMIE.mdpackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tsFORJAMIE.mdpackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.ts
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
**/*.{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/cli/src/utils/mask.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/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.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
**/*.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
🔍 Remote MCP Linear
Additional context retrieved (concise, review-relevant)
-
Linear issue JSC-241 ("Agent-native design system: prepare payload model") records the PR's exact goals and stop-rule used for validation: build the semantic payload for
astudio design prepare, add engine types forastudio.design.prepare.v1, resolve/digest DESIGN.md/guidance/routing/lifecycle/coverage, classify surface scope (protected>warn>exempt>unknown), use deterministic JSON serialization, and fail-closed for unknown scope / missing route/lifecycle/coverage / ambiguous route. Validation steps and the stop rule are listed., -
JSC-241 is a child of JSC-238 ("Agent-native design-system command layer"), which documents the canonical plan and execution slices this PR implements (contract wiring, routing table, prepare payload model, read-only CLI surface, remediation output, gold examples, proposal gate). The plan enforces no public
contextcommand, deterministic serialization, typed waivers, missing-example diagnostics, and a read-only prepare pathway.
Sources:
- Linear list/search for JSC-241 (issue found) —
- Linear full issue fetch JSC-241 —
- Linear full issue fetch JSC-238 (parent) —
🔇 Additional comments (5)
FORJAMIE.md (1)
223-225: Wrapper invocation examples drift from repo-owned wrapper contractThese lines still show
pnpm --silent agent-design:prepare --surface <path> --json, while repo evidence and Line 214 establish that the wrapper already appends--json. This reintroduces the same double-flag drift in a high-visibility section.As per coding guidelines, "Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions."
Also applies to: 234-234
packages/cli/tests/mask.test.mjs (1)
53-122: Good regression coverage for contract masking paths.These tests now clearly validate both allowlisted public fields and malformed-payload redaction behavior, including
sourceRefsand malformedallowedRolesentries.packages/agent-design-engine/src/prepare.ts (2)
683-738: Workspace boundary hardening forpackage.jsonresolution looks good.Realpath +
isWithinDirectorychecks on both package directory and resolvedpackage.jsonpath correctly close symlink-escape paths.
535-541: No fix required: pnpm boolean flags do not support=valuesyntax.The review's concern about
--recursive=truebypassing the guard is not valid. In pnpm 10.x, the--recursive(or-r) flag is a boolean flag set by presence, not by value assignment. The syntax--recursive=trueis not recognized by pnpm and does not activate recursive execution. The current code's exact-token checks (-r,--recursive,recursive,multi,m) are sufficient to catch all valid recursive invocation forms.The proposed helper function and refactoring are unnecessary.
> Likely an incorrect or invalid review comment.packages/agent-design-engine/tests/engine.test.mjs (1)
899-1714: Excellent fail-closed regression coverage for prepare contract hardening.The new cases for pnpm parsing, trust binding, symlink escapes, schema failures, and deterministic serialization align tightly with the hardened contract.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 390ee065b7
ℹ️ 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: Fresh PR review threads found that absolute package directories could miss read-only trust keys and malformed allowedRoles could bypass design-token contract masking. What: Normalize package directories to repo-relative realpaths before comparing trusted validation-script keys, redact malformed allowedRoles values, and cover both paths with focused tests. Update FORJAMIE with the final trust and masking behavior. Validation: pnpm -C packages/agent-design-engine test -> pass Validation: pnpm -C packages/cli test -> pass Validation: pnpm docs:lint -> pass Validation: git diff --check -> pass Validation: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx >/tmp/agent-design-prepare-final-tighten.json -> pass Validation: jq '.data.validationCommands[0].packageScript' /tmp/agent-design-prepare-final-tighten.json -> pass Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/utils/mask.ts (1)
43-52:⚠️ Potential issue | 🟠 Major | ⚡ Quick winNormalize every mask rule against segmented keys.
shouldMaskField()only usesgetKeySegments()for"token". Common variants likeapiKeyorapi-keystill miss the"api_key"default rule and will pass through unredacted, which weakens the masking guarantees for error payloads.💡 Localized fix
function shouldMaskField(key: string, masks: FieldMask[]): FieldMask | undefined { const keySegments = getKeySegments(key); - const lowerKey = key.toLowerCase(); + const normalizedKey = keySegments.join("_"); return masks.find((mask) => { - const field = mask.field.toLowerCase(); - if (field === "token") { - return keySegments.includes("token"); - } - return lowerKey.includes(field); + const normalizedField = getKeySegments(mask.field).join("_"); + return normalizedKey.includes(normalizedField); }); }As per coding guidelines,
JavaScript/TypeScript variables/functions: camelCase.Also applies to: 55-60
🤖 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 43 - 52, shouldMaskField currently only segments the input key for the special case "token", so rules like "apiKey" or "api-key" don't match; update shouldMaskField (and the similar logic around lines 55-60) to normalize the mask rule by running mask.field through getKeySegments (or otherwise normalize to the same segmented/lowercase form) and then compare segments (or the normalized joined form) against the keySegments/lowerKey so variants like apiKey, api-key, and api_key all match; reference the shouldMaskField function and FieldMask.mask.field to locate where to apply the normalization and matching change.
♻️ Duplicate comments (2)
FORJAMIE.md (2)
27-30:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winStatus block still overstates merge readiness.
This section says the required checks are green and blockers are none, but the PR objective for this branch says merge is still blocked pending required checks. Please align the status lane and blocker row with the actual PR state. 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 27 - 30, Update the status table in FORJAMIE.md to accurately reflect that merge is blocked: change the "Agent Design Prepare plan" status from "Green" to a value like "Pending checks" or "Blocked" (so it no longer claims required checks are green), and update the "Blockers" cell from "None" to note that PR `#160` has required checks pending and must pass before merge; also ensure the "Open PRs" row (PR `#160`) mentions the specific check/policy that is blocking if available.
223-225:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRecent-changes entries still show the wrapper with an extra
--json.Line 214 already says
agent-design:prepareappends--jsonitself, but these history entries still documentpnpm --silent agent-design:prepare --surface <path> --json. That leaves the changelog contradicting the repo-owned wrapper contract. As per coding guidelines, "Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions."Also applies to: 234-234
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@FORJAMIE.md` around lines 223 - 225, The recent-changes entries incorrectly document the wrapper as "pnpm --silent agent-design:prepare --surface <path> --json" even though the package script "agent-design:prepare" appends --json itself; update the three affected sections ("Agent Design Prepare north-star spec", "Agent Design Prepare north-star plan", and "Agent Design Prepare plan review hardening") to remove the explicit "--json" from the documented wrapper invocation (or rephrase to note that the wrapper appends --json) so the changelog matches the repo-owned wrapper contract and the symbol "agent-design:prepare" is the single source of truth.
🤖 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`:
- Around line 231-233: The changelog entry in FORJAMIE.md incorrectly states
that `pnpm recursive run`/`multi`/`m` are accepted, but the implementation in
packages/agent-design-engine/src/prepare.ts and the added tests explicitly
reject those aliases; update the FORJAMIE.md bullet to state that those alias
forms are rejected (fail-closed) by the prepare parser and tests, and align the
language with the behavior described in prepare.ts and the related test names so
documentation matches code.
In `@packages/agent-design-engine/src/prepare.ts`:
- Around line 541-612: Both readPnpmRunScript and inferPackageScript duplicate
the pnpm option-scanning logic; extract that common logic into a shared helper
(e.g., parsePnpmOption or consumePnpmToken) that takes (tokens, index, command,
currentPackageDir) and returns an object with updated packageDir, newIndex, and
an action (continue / throw / return script) so both functions call it instead
of reimplementing -C/--dir, -w/--workspace-root, recursive flags, --filter
variants, pnpmRunOptionsWithValues handling, and generic flag skipping; reuse
normalizePackageDir, unsupportedPnpmFilter, pnpmRunOptionsWithValues, and
invalidValidationCommand inside the helper and update readPnpmRunScript and
inferPackageScript to delegate to this helper to keep behavior identical and
avoid drift.
- Around line 179-192: The catch block in readPrepareSource (the try calling
readText) appends raw filesystem error messages that can leak absolute paths;
change it to produce deterministic, path-safe details by checking for Node fs
error codes (e.g., error.code === "ENOENT" || error.code === "EACCES") and
include only the error.code (or a short stable token like "[not-found]" /
"[permission-denied]") instead of error.message, while preserving full
error.message for non-fs errors; update the DesignEngineError construction in
the catch (the thrown instance) to use this sanitized detail so prepare.ts (the
catch around readText) no longer emits machine-specific paths.
In `@packages/cli/src/utils/mask.ts`:
- Around line 80-88: The current maskMalformedPublicValue function routes any
non-string to maskValueRecursive which lets malformed or unknown public-contract
fields slip through; change it to fail-closed by validating the input against
the expected closed public contract shape (using the FieldMask entries) and
treating any unknown keys or values that don't match the contract as
sensitive—i.e., return maskValue(..., "redact") for those cases; update
maskMalformedPublicValue to inspect the FieldMask definitions (and use the same
matching logic used by maskValueRecursive) and call maskValue(value, "redact")
when a field is missing from the allowlist, has an unexpected type/shape, or
contains malformed nested entries, otherwise delegate to maskValueRecursive for
known-good shapes.
---
Outside diff comments:
In `@packages/cli/src/utils/mask.ts`:
- Around line 43-52: shouldMaskField currently only segments the input key for
the special case "token", so rules like "apiKey" or "api-key" don't match;
update shouldMaskField (and the similar logic around lines 55-60) to normalize
the mask rule by running mask.field through getKeySegments (or otherwise
normalize to the same segmented/lowercase form) and then compare segments (or
the normalized joined form) against the keySegments/lowerKey so variants like
apiKey, api-key, and api_key all match; reference the shouldMaskField function
and FieldMask.mask.field to locate where to apply the normalization and matching
change.
---
Duplicate comments:
In `@FORJAMIE.md`:
- Around line 27-30: Update the status table in FORJAMIE.md to accurately
reflect that merge is blocked: change the "Agent Design Prepare plan" status
from "Green" to a value like "Pending checks" or "Blocked" (so it no longer
claims required checks are green), and update the "Blockers" cell from "None" to
note that PR `#160` has required checks pending and must pass before merge; also
ensure the "Open PRs" row (PR `#160`) mentions the specific check/policy that is
blocking if available.
- Around line 223-225: The recent-changes entries incorrectly document the
wrapper as "pnpm --silent agent-design:prepare --surface <path> --json" even
though the package script "agent-design:prepare" appends --json itself; update
the three affected sections ("Agent Design Prepare north-star spec", "Agent
Design Prepare north-star plan", and "Agent Design Prepare plan review
hardening") to remove the explicit "--json" from the documented wrapper
invocation (or rephrase to note that the wrapper appends --json) so the
changelog matches the repo-owned wrapper contract and the symbol
"agent-design:prepare" is the single source of truth.
🪄 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: a39162fd-83b0-4a60-9b2f-abe7a176daec
📒 Files selected for processing (5)
FORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/src/utils/mask.tspackages/cli/tests/mask.test.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 (22)
**/*.{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/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.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:
packages/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
**/*.{ts,tsx,jsx,js,py,rs,mjs}
📄 CodeRabbit inference engine (CODESTYLE.md)
Types/components: PascalCase
Files:
packages/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjspackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjsFORJAMIE.mdpackages/cli/src/utils/mask.tspackages/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/cli/tests/mask.test.mjsFORJAMIE.mdpackages/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjs
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
**/*.{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/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.ts
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
**/*.{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/cli/src/utils/mask.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/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.ts
🔍 Remote MCP Linear
Relevant additional context (concise)
-
JSC-241 ("Agent-native design system: prepare payload model") — Goal: build the semantic prepare payload for
astudio design prepare(astudio.design.prepare.v1), resolve/digest DESIGN.md + guidance/routing/lifecycle/coverage, classify surface scope (protected>warn>exempt>unknown), use deterministic JSON serialization, and fail-closed for unknown scope / missing route/lifecycle/coverage / ambiguous route. Validation commands and a stop rule (do not expose CLI until deterministic fixtures exist) are recorded. -
JSC-241 validation checklist (explicit): run type-check/tests in packages/agent-design-engine, run agent-design:boundaries, and git diff --check; stop rule: do not expose CLI until fixtures exist for protected/warn/exempt/unknown/overlap/symlink-normalized scopes.
-
JSC-238 ("Agent-native design-system command layer") — Canonical plan and execution slices that this PR implements: contract wiring, routing table, prepare payload model, read-only CLI surface, remediation output, gold example inventory, and abstraction-proposal gate; plan enforces deterministic serialization, validation command schema, read-only prepare guarantees, missing-example diagnostics, and review-evidence requirements.
Sources used:
- Linear issue JSC-241 — Agent-native design system: prepare payload model.
- Linear issue JSC-238 — Agent-native design-system command layer.
Why: PR review found remaining drift in malformed public contract masking, prepare source error determinism, and duplicated pnpm option parsing. What: fail closed on malformed or unknown designTokenContract fields, sanitize prepare source read errors, share pnpm option scanning between parser paths, and correct the living changelog wording for rejected recursive aliases. Validation: pnpm -C packages/agent-design-engine test -> pass, 104 tests Validation: pnpm -C packages/cli test -> pass, 117 tests Validation: pnpm lint -> pass Validation: pnpm docs:lint -> pass Validation: git diff --check -> pass Validation: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx >/tmp/agent-design-prepare-review-tighten.json -> pass Co-authored-by: Codex <[email protected]>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0eeab26b67
ℹ️ 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: 1
🤖 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 473-479: Remove the synthetic ".#list" entry from the
trustedReadOnlyPackageScriptKeys Set so the parser-only test case does not
expand the runtime trust surface; update the Set in prepare.ts
(trustedReadOnlyPackageScriptKeys) to exclude ".#list" and leave other entries
unchanged, keeping the synthetic `scripts.list` case covered only by the
parser-focused tests rather than the production allowlist.
🪄 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: ad315a3e-dd7e-494c-a169-883bbe98f29c
📒 Files selected for processing (5)
FORJAMIE.mdpackages/agent-design-engine/src/prepare.tspackages/agent-design-engine/tests/engine.test.mjspackages/cli/src/utils/mask.tspackages/cli/tests/mask.test.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 (22)
**/*.{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/cli/src/utils/mask.tspackages/agent-design-engine/src/prepare.ts
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
**/*.{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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjspackages/agent-design-engine/tests/engine.test.mjspackages/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/src/utils/mask.tspackages/cli/tests/mask.test.mjsFORJAMIE.mdpackages/agent-design-engine/tests/engine.test.mjspackages/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/cli/src/utils/mask.tspackages/cli/tests/mask.test.mjsFORJAMIE.mdpackages/agent-design-engine/tests/engine.test.mjspackages/agent-design-engine/src/prepare.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
**/*.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
🔍 Remote MCP Linear
Additional factual context for PR review
-
PR implements items from the Agent-native command-layer plan and the Prepare Payload Model: contract wiring, routing table, prepare payload model, read-only CLI surface, remediation output, gold-example gating, and proposal gate — matching execution slices in the canonical plan.
-
The Prepare Payload Model explicitly requires deterministic serializer output, digesting DESIGN.md + guidance/routing/lifecycle/coverage, surface scope classification (protected>warn>exempt>unknown), and a fail-closed behavior for unknown scope / missing route/lifecycle/coverage / ambiguous route. These are validation requirements the PR aims to satisfy.
-
Validation checklist referenced by the plan (tests/type-check/build gates to run before exposing CLI):
- pnpm -C packages/agent-design-engine type-check
- pnpm -C packages/agent-design-engine test
- pnpm agent-design:boundaries
- git diff --check
Ensure CI includes these checks for merging.
-
Stop rule from the Prepare Payload Model: do not expose CLI until deterministic fixtures exist for protected, warn, exempt, unknown, overlap, and symlink-normalized scopes. This explains why the PR tightens payload/schema validation and enforces fail-closed diagnostics.
-
Review artifacts expected by the command-layer plan (round-2 adversarial/architecture/API reviewers) are listed as required review evidence; the PR summary notes CodeRabbit/Codex reviews and Semgrep artifacts are pending — confirm those review artifacts are produced before merge.
Sources:
- Linear issue JSC-238 “Agent-native design-system command layer” — execution slices and review-evidence expectations.
- Linear issue JSC-241 “Agent-native design system: prepare payload model” — goals, validation checklist, and stop rule.
🔇 Additional comments (2)
FORJAMIE.md (2)
27-30: Status table still overstates merge readiness.This still says required checks are green and blockers are none, but the PR context for
#160says merge remains blocked pending required checks and review artifacts. Please keep the living status block aligned with the actual gate state.As per coding guidelines, "Treat contradictions between code, docs, and workflow policy as review-worthy even when code changes look reasonable."
223-225: Recent-change notes reintroduced the wrapper--jsonduplication.These bullets document
pnpm --silent agent-design:prepare --surface <path> --json, but line 214 correctly says the wrapper appends--jsonitself. Keep the changelog entries on the repo-owned invocation form so the history section does not drift from the actual wrapper contract.As per coding guidelines, "Use repository evidence first: package scripts, tracked docs, harness contracts, and validation wrappers over assumptions."
Also applies to: 234-234
Why: PR review found remaining edge cases in designTokenContract masking, pnpm command tokenization, trusted validation scripts, and live status wording. What: Redact malformed non-object token contracts, preserve literal backslashes in pnpm package-dir tokens, remove the synthetic list script from trusted read-only validation commands, update parser regression coverage, and stop overclaiming required-check status in FORJAMIE.md. Impact/Risk: The prepare contract stays stricter and more path-safe. A root list script will no longer be treated as trusted read-only validation without explicit evidence. Validation: pnpm -C packages/agent-design-engine test -> pass Validation: pnpm -C packages/cli test -> pass Validation: pnpm lint -> pass Validation: pnpm docs:lint -> pass Validation: git diff --check -> pass Validation: pnpm --silent agent-design:prepare --surface packages/ui/src/app/settings/AppsPanel/AppsPanel.tsx -> pass Co-authored-by: Codex <[email protected]>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Caution Docstrings generation - FAILED An unexpected error occurred while creating a local commit: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
|
Note Unit test generation is a public access feature. Expect some limitations and changes as we gather feedback and continue to improve it. Generating unit tests... This may take up to 20 minutes. |
Docstrings generation was requested by @jscraik. The following files were modified: * `packages/agent-design-engine/src/prepare.ts` * `packages/agent-design-engine/src/routes.ts` * `packages/agent-design-engine/src/token-contract.ts` * `packages/cli/src/utils/mask.ts` * `packages/design-system-guidance/src/core.ts` These files were ignored: * `packages/agent-design-engine/tests/engine.test.mjs` * `packages/cli/tests/cli.test.mjs` * `packages/cli/tests/mask.test.mjs` These file types are not supported: * `.gitignore` * `CONTRIBUTING.md` * `FORJAMIE.md` * `README.md` * `docs/design-system/GOLD_EXAMPLES.json` * `docs/design-system/GOLD_EXAMPLES.md` * `docs/guides/AGENT_DESIGN_WORKFLOW.md` * `docs/plans/2026-04-30-agent-design-prepare-north-star-plan.md` * `docs/specs/2026-04-30-agent-design-prepare-north-star-spec.md` * `package.json` * `packages/cli/tests/fixtures/design-schemas/astudio-design-command.v1.schema.json`
|
❌ Failed to commit unit tests locally: An unexpected error occurred while creating a commit: Not Found - https://docs.github.com/rest/git/refs#get-a-reference |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2680be569f
ℹ️ 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".
| token === "recursive" || | ||
| token === "multi" || | ||
| token === "m" |
There was a problem hiding this comment.
run script names before recursive-alias rejection
The consumePnpmOption guard rejects bare recursive/multi/m tokens in all positions, and readPnpmRunScript calls this helper before deciding a token is the script name. That means commands like pnpm run recursive (or pnpm run m) are rejected as unsupported recursive execution even when they are legitimate script names. Per pnpm run semantics, options must come before the script name, so once parsing is in pnpm run ... and sees a non-flag token, it should be interpreted as the script token rather than a recursive alias.
Useful? React with 👍 / 👎.
1 participant
Pull request checklist
Summary
astudio design prepareis the agent-first UI pre-edit contract, so agents need a fail-closed, JSON-safe payload with canonical components, semantic tokens, validation commands, and proposal stops before editing UI.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.Testing
pnpm -C packages/agent-design-engine test-> passgit diff --check-> passpnpm lint-> passpnpm docs:lint-> passgit push -u origin codex/agent-design-prepare-contract-> pass after running normal hook-enforced push outside sandboxbash scripts/validate-codestyle.sh-> blocked locally by sandboxed Chromium launch in Playwright browser checks before escalated push; normal pre-push hook passed outside sandboxpnpm check-> blocked locally by sandboxed Chromium launch in Playwright browser checks before escalated push; normal pre-push hook passed outside sandboxtest -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-> blocked,memory.jsonis not present in this checkoutpnpm -C packages/agent-design-engine test: pass, 73 tests passedgit diff --check: passpnpm lint: pass, Biome checked 1501 filespnpm docs:lint: pass, Vale and markdown links passedgit push -u origin codex/agent-design-prepare-contract: pass, pre-push passed outside sandbox and branch was publishedbash scripts/validate-codestyle.sh: blocked in sandbox by Chromium Mach port permission error during Playwright checks; equivalent pre-push hook passed outside sandboxpnpm check: blocked in sandbox by Chromium Mach port permission error during Playwright checks; equivalent pre-push hook passed outside sandboxmemory.jsonjq gate: blocked becausememory.jsondoes not exist in this repo checkoutMachPortRendezvousServerduring Playwright checks;memory.jsonis absent in this checkout. The normal hook-enforced push was rerun outside the sandbox and passed.bash scripts/validate-codestyle.sh-> blocked locally; equivalent pre-push hook passed outside sandboxpnpm check-> blocked locally; equivalent pre-push hook passed outside sandboxtest -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-> blocked,memory.jsonis absentpnpm -C packages/agent-design-engine test-> pass;pnpm lint-> pass;pnpm docs:lint-> pass;git diff --check-> pass;git push -u origin codex/agent-design-prepare-contract-> passReview artifacts
coderabbit auth status --agentreturned not authenticated35ee1e21Notes
This PR makes
preparebehave like the actual north-star agent contract: before an AI coding agent edits UI, the command now returns stricter implementation context and rejects ambiguous validation metadata instead of allowing payload drift. The remaining merge gates should come from GitHub/CircleCI/CodeRabbit because local browser-based gates require non-sandboxed Chromium permissions.