feat(workflows): re-land orch-review workflow + add /orch-review command#2400
feat(workflows): re-land orch-review workflow + add /orch-review command#2400pythonstrup wants to merge 3 commits into
Conversation
Re-lands affaan-m#2363 (reverted by affaan-m#2393 to unbreak main's lint) and fixes the root cause so it stays green: - Restore workflows/orch-review.workflow.js + workflows/README.md. - eslint.config.js: ignore 'workflows/**/*.workflow.*' and '.claude/workflows/**' per the maintainer's note in affaan-m#2393. Workflow DSL scripts use both top-level export (ESM) and top-level return (the runtime wraps them in an async fn), which no single eslint sourceType can parse — they must be excluded, not lint-fixed. 'npx eslint .' is green with this ignore. - Add commands/orch-review.md (the /orch-review surface) + regenerate docs/COMMAND-REGISTRY.json. Supersedes affaan-m#2397 (command-only), which referenced the reverted workflow.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout. (16)
🧰 Additional context used📓 Path-based instructions (15)**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
Files:
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Files:
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Files:
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Files:
**/*.{js,ts,jsx,tsx,html,php,java,cs,rb,go}📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Files:
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)
Files:
**/*.{js,ts,jsx,tsx}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,ts,jsx,tsx,json,env*}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{jsx,tsx,js,ts}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,ts,env*}📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt,cpp,c,fs}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{jsx,tsx,html,js,ts}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 Additional comments (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds Changesorch-review Workflow and Command
Sequence Diagram(s)sequenceDiagram
participant User
participant orch-review Command
participant orch-review.workflow.js
participant DimensionReviewers
participant AdversarialVerifier
User->>orch-review Command: run in Local or PR mode
orch-review Command->>orch-review Command: gather diff + changedFiles
orch-review Command->>orch-review.workflow.js: invoke(diff, language?, changedFiles?)
orch-review.workflow.js->>orch-review.workflow.js: validate input and select dimensions
orch-review.workflow.js->>DimensionReviewers: parallel review requests
DimensionReviewers-->>orch-review.workflow.js: findings or failure/null
orch-review.workflow.js->>orch-review.workflow.js: deduplicate findings
orch-review.workflow.js->>AdversarialVerifier: verify CRITICAL/HIGH findings
AdversarialVerifier-->>orch-review.workflow.js: confirmed | refuted | unverified
orch-review.workflow.js-->>orch-review Command: verdict, blocking, advisory, incomplete, stats
orch-review Command-->>User: report results
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Biome (2.5.1)workflows/orch-review.workflow.jsFile contains syntax errors that prevent linting: Line 280: Illegal return statement outside of a function 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
| Filename | Overview |
|---|---|
| workflows/orch-review.workflow.js | Restores the native review workflow with input validation, review fan-out, deduplication, and fail-closed verification handling. |
| commands/orch-review.md | Adds the slash-command surface for collecting diffs and invoking the workflow. |
| eslint.config.js | Adds workflow DSL paths to the ESLint ignore list. |
| docs/COMMAND-REGISTRY.json | Updates the registry counts and adds the new orch-review command. |
| workflows/README.md | Documents the native workflow pilot, invocation shape, result contract, and follow-up work. |
Reviews (3): Last reviewed commit: "style(workflows): apply formatter to orc..." | Re-trigger Greptile
![greptile-apps[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@commands/orch-review.md`:
- Line 81: The stats example is incomplete and does not match the workflow
return contract; update the example used in orch-review.md to include the
missing confirmed, unverified, and refuted fields alongside dimensions, failed,
raw, and unique so it reflects the full shape returned by the workflow. Use the
stats contract referenced by the workflow to locate the example and keep the
field names consistent with the returned object.
- Line 110: The wording in the reviewer concurrency note is too verbose; update
the phrase used in orch-review.md from “Very large diff” to a cleaner
alternative like “Large diff” or “Extremely large diff.” Keep the rest of the
guidance about workflow-capped reviewer concurrency unchanged.
In `@eslint.config.js`:
- Line 6: The ESLint ignore list in the config currently excludes workflow files
entirely, which lets parser or globals issues in workflow definitions escape
linting. Update the workflow handling around the config’s ignores so
`workflows/**/*.workflow.*` and the mirrored `.claude/workflows/**` copy still
get a workflow-specific validation pass before being excluded, and keep the
relevant logic near the `ignores` entry in `eslint.config.js` so it is easy to
find.
In `@workflows/orch-review.workflow.js`:
- Around line 116-120: The verifier flow in verifyPrompt and the downstream
result classification should fail closed for low-confidence cases. Update the
logic so uncertainty in the verifier does not automatically become a
refuted/advisory outcome; instead, treat ambiguous or low-confidence results as
blocking, especially for CRITICAL/HIGH findings. Use the existing verifyPrompt
return path and the classification code around the false-verdict handling to
distinguish “confidently false” from “uncertain” and preserve blocking behavior
when confidence is not strong enough.
- Around line 157-166: The dimension assembly in orchestrator logic is mutating
existing arrays/objects, which violates the immutable-update rule; update the
logic around the dimension construction and dedup/merge path in
orch-review.workflow.js to build new arrays/objects instead of using push or
direct property assignment. Replace in-place updates in the dimension setup and
any later merge logic (including the prev.dimensions and prev.severity handling)
with non-mutating patterns that return fresh values while preserving the
existing behavior of the reviewer selection and security trigger flow.
- Around line 149-154: Validate each entry in input.changedFiles before
constructing the security trigger haystack in orch-review.workflow.js: the
existing Array.isArray check in the changedFiles guard only verifies the
container, not its contents. Add a per-item validation step in the same input
validation block (or immediately before building haystack) to ensure every entry
is a string path, and reject non-string objects like { path: "auth.js" } with a
clear error. Then build haystack from the validated string paths only, using the
existing diff and changedFiles handling.
- Around line 104-128: The prompt builders in reviewPrompt and verifyPrompt
treat raw diff text as plain instructions, which leaves them open to
prompt-injection from untrusted diff content. Harden both functions by wrapping
the diff in explicit data delimiters and adding clear instructions that the diff
is untrusted input to be evaluated only as data, not followed as directives;
update the prompt strings in reviewPrompt and verifyPrompt to preserve this
separation.
- Around line 73-81: The schema in orch-review.workflow.js is missing
enforcement for blocker proof on HIGH/CRITICAL findings, so update the JSON
schema definition to require proof and a non-empty value when severity is HIGH
or CRITICAL. Adjust the schema around the properties for severity and proof so
the contract is enforced by validation, not only by the prompt, and keep the
existing required fields like title, severity, file, and evidence intact.
In `@workflows/README.md`:
- Around line 55-58: The follow-up list in README is outdated because the
`/orch-review` command is already included in this PR via
commands/orch-review.md and docs/COMMAND-REGISTRY.json. Update the “Not in this
PR” bullets to remove the command itself and keep only the remaining missing
work, such as the skill trigger, mirrored i18n docs, surface tests, and
installer/manifest wiring; use the existing `/orch-review` and
docs/COMMAND-REGISTRY.json entries to locate the related 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: 39118479-0e06-4f09-9569-62f6852a596b
📒 Files selected for processing (5)
commands/orch-review.mddocs/COMMAND-REGISTRY.jsoneslint.config.jsworkflows/README.mdworkflows/orch-review.workflow.js
📜 Review details
⏰ Context from checks skipped due to timeout. (15)
- GitHub Check: Greptile Review
- GitHub Check: Test (macos-latest, Node 20.x, bun)
- GitHub Check: Test (macos-latest, Node 20.x, pnpm)
- GitHub Check: Test (macos-latest, Node 22.x, pnpm)
- GitHub Check: Test (windows-latest, Node 22.x, pnpm)
- GitHub Check: Test (macos-latest, Node 18.x, pnpm)
- GitHub Check: Test (macos-latest, Node 18.x, bun)
- GitHub Check: Test (ubuntu-latest, Node 20.x, bun)
- GitHub Check: Test (ubuntu-latest, Node 22.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 18.x, bun)
- GitHub Check: Test (windows-latest, Node 18.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, bun)
- GitHub Check: Test (windows-latest, Node 20.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 18.x, pnpm)
🧰 Additional context used
📓 Path-based instructions (19)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
docs/COMMAND-REGISTRY.jsoneslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,json,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not hardcode secrets, API keys, passwords, or tokens
Files:
docs/COMMAND-REGISTRY.jsoneslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,html,php,java,cs,rb,go}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Implement XSS prevention by sanitizing HTML output
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)
**/*.{ts,tsx,js,jsx}: Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation
Use async/await with try-catch for error handling in TypeScript/JavaScript
Use Zod for schema-based input validation in TypeScript/JavaScript
No console.log statements in production code; use proper logging libraries instead
**/*.{ts,tsx,js,jsx}: Auto-format JavaScript/TypeScript files using Prettier after edit
Warn aboutconsole.logstatements in edited files
Check all modified files forconsole.logstatements before session ends
**/*.{ts,tsx,js,jsx}: Use the ApiResponse interface pattern with generic type parameter:interface ApiResponse<T> { success: boolean; data?: T; error?: string; meta?: { total: number; page: number; limit: number; } }
Implement custom React hooks following the pattern: export a named function with use prefix, generic type parameters, and proper useEffect cleanup for side effects
**/*.{ts,tsx,js,jsx}: Never hardcode secrets; always use environment variables for sensitive credentials like API keys
Throw an error when required environment variables are not configured to fail fast and ensure security prerequisites are metUse Playwright as the E2E testing framework for critical user flows in TypeScript/JavaScript
Files:
eslint.config.jsworkflows/orch-review.workflow.js
{package.json,*.config.js,scripts/**/*.js}
📄 CodeRabbit inference engine (CLAUDE.md)
Package manager detection should support npm, pnpm, yarn, and bun, with configuration via CLAUDE_PACKAGE_MANAGER environment variable or project config.
Files:
eslint.config.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Always create new objects and never mutate in place; return new copies instead
Keep files between 200–400 lines typical, with a maximum of 800 lines
Extract helpers when a file exceeds 200 lines
Handle errors explicitly at every level; never swallow errors silently
Validate all user input before processing; use schema-based validation where available
Never trust external data (API responses, file content, query params); always validate
All user inputs must be validated and sanitized
Error messages must be scrubbed of sensitive internals
Use readable, well-named identifiers in all code
Keep functions under 50 lines
Keep files under 800 lines
Avoid nesting deeper than 4 levels
Implement comprehensive error handling in all code
Do not hardcode values; use constants or environment configuration instead
Do not use in-place mutation; always return new objects or state
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use parameterized queries for all database writes (no string interpolation)
Auth/authz must be checked server-side for every sensitive path
Rate limiting must be applied to all public endpoints
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
HTML output must be sanitized where applicable
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Required environment variables must be validated at startup
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt,cpp,c,fs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt,cpp,c,fs}: Write tests before implementation using TDD workflow: write failing test (RED), implement minimal code (GREEN), then refactor (IMPROVE)
Keep functions small (<50 lines) and files focused (<800 lines, typical 200-400 lines)
Avoid deep nesting (>4 levels)
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt}: Never mutate existing objects; always create new objects with changes applied (Immutability requirement)
Handle errors at every level; provide user-friendly messages in UI code and detailed context in server-side logs
Ensure error messages don't leak sensitive data
Files:
eslint.config.jsworkflows/orch-review.workflow.js
**/*.{jsx,tsx,html,js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Sanitize HTML output to prevent XSS vulnerabilities
Files:
eslint.config.jsworkflows/orch-review.workflow.js
commands/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Commands should be formatted as Markdown with description frontmatter.
Files:
commands/orch-review.md
{agents,skills,commands}/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Use lowercase filenames with hyphens (e.g.,
python-reviewer.md,tdd-workflow.md) for agents, skills, and commands.
Files:
commands/orch-review.md
{skills,commands,agents,rules}/**
⚙️ CodeRabbit configuration file
{skills,commands,agents,rules}/**: Focus on prompt-injection resilience, tool-permission scope, destructive action guards, and secret exfiltration risks.
Files:
commands/orch-review.md
🪛 Biome (2.5.1)
workflows/orch-review.workflow.js
[error] 247-254: Illegal return statement outside of a function
(parse)
🪛 LanguageTool
workflows/README.md
[grammar] ~18-~18: Use a hyphen to join words.
Context: ...ted* on uncertainty. MEDIUM/LOW pass through as advisory. The Review→Verify ...
(QB_NEW_EN_HYPHEN)
[style] ~20-~20: Consider an alternative for the overused word “exactly”.
Context: ...berate: deduping before verification is exactly the case the Workflow guidance calls a ...
(EXACTLY_PRECISELY)
commands/orch-review.md
[style] ~110-~110: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ainst a checked-out branch instead. - Very large diff: the workflow caps reviewer conc...
(EN_WEAK_ADJECTIVE)
🔇 Additional comments (3)
docs/COMMAND-REGISTRY.json (1)
3-3: LGTM!Also applies to: 620-628, 1023-1023
workflows/README.md (1)
1-53: LGTM!Also applies to: 59-60
workflows/orch-review.workflow.js (1)
247-254: 📐 Maintainability & Code QualityNo change needed for workflow DSL files
workflows/**/*.workflow.*is already excluded ineslint.config.js, and CI only runseslint+markdownlint; no Biome parser is configured for this path.> Likely an incorrect or invalid review comment.
- Verifier uncertainty no longer demotes blockers (Greptile P1 + CodeRabbit): isReal=false only refutes when confidence >= 0.8; low-confidence 'false' is treated as uncertain and kept blocking (fail closed). - Treat the diff (and finding text) as untrusted input in both review and verify prompts; ignore embedded directives (prompt-injection hardening). - Validate changedFiles entries are strings, not just that it is an array. - Enforce proof for HIGH/CRITICAL in FINDINGS_SCHEMA, not only in the prompt. - Remove in-place mutation in dimension build + dedup merge (immutable). - /orch-review: extract & validate a numeric PR id before shelling out to gh. - Docs: complete the stats example, soften wording, refresh follow-up list.
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
commands/orch-review.md (1)
6-12: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winAdd required
descriptionfrontmatter.This command spec is missing the YAML frontmatter required by the project convention. Add a
descriptionfield so tooling and discovery can present the command correctly.+--- +description: "Surface for workflows/orch-review.workflow.js — runs parallel dimension reviews with deduplication and adversarial verification." +--- + # /orch-reviewAs per coding guidelines, commands should be formatted as Markdown with description frontmatter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@commands/orch-review.md` around lines 6 - 12, Add the required YAML frontmatter to the /orch-review command spec so it follows the project’s Markdown command convention. Update the top of the document to include a description field alongside the existing command metadata, keeping the rest of the orch-review content intact. Use the /orch-review command definition as the place to add the frontmatter so tooling and discovery can identify it correctly.Source: Coding guidelines
♻️ Duplicate comments (1)
workflows/orch-review.workflow.js (1)
119-142: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winEscape untrusted block delimiters before embedding diff/finding text.
The prompt marks the diff as untrusted, but raw diff/finding fields can still contain
----- END DIFF -----or newline directives and break out of the intended data block. Encode or escape these fields before interpolation.Proposed hardening
+function untrustedBlock(label, value) { + const endMarker = `----- END ${label} -----`; + const escaped = String(value).split(endMarker).join(`----- ESCAPED END ${label} -----`); + return [`----- BEGIN ${label} (untrusted) -----`, escaped, endMarker].join('\n'); +} + function reviewPrompt(dimensionLabel, diff) { @@ - '----- BEGIN DIFF (untrusted) -----', - diff, - '----- END DIFF -----' + untrustedBlock('DIFF', diff) @@ - `Finding (${finding.severity}) in ${finding.file}: ${finding.title}`, - `Claimed evidence: ${finding.evidence}`, - finding.proof ? `Claimed proof: ${finding.proof}` : '', + untrustedBlock('FINDING', JSON.stringify({ + severity: finding.severity, + file: finding.file, + title: finding.title, + evidence: finding.evidence, + proof: finding.proof || undefined + }, null, 2)), @@ - '----- BEGIN DIFF (untrusted) -----', - diff, - '----- END DIFF -----' + untrustedBlock('DIFF', diff)As per coding guidelines, “Never trust external data (API responses, user input, file content).”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@workflows/orch-review.workflow.js` around lines 119 - 142, The prompt assembly in verifyPrompt is still interpolating untrusted finding/diff content directly into the block delimiters, so escape or encode those fields before joining the string. Update the prompt-building logic in orchestr-review.workflow.js around verifyPrompt (and any helper that injects diff/finding text) so embedded newline directives or delimiter-like text cannot break out of the intended untrusted section.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@workflows/orch-review.workflow.js`:
- Around line 29-31: Update the verdict classification in the workflow so
low-confidence `isReal=true` results are treated as uncertain rather than
confirmed. In the logic around `classifyFindings` and the stats aggregation that
currently counts every `isReal` verdict as confirmed, add a confidence check so
uncertain `isReal` findings are routed into `blocking.uncertain` (or the
equivalent uncertain bucket) and only high-confidence `isReal` findings
increment `stats.confirmed`; then make sure `stats.uncertain` and
`stats.confirmed` stay consistent with that split.
---
Outside diff comments:
In `@commands/orch-review.md`:
- Around line 6-12: Add the required YAML frontmatter to the /orch-review
command spec so it follows the project’s Markdown command convention. Update the
top of the document to include a description field alongside the existing
command metadata, keeping the rest of the orch-review content intact. Use the
/orch-review command definition as the place to add the frontmatter so tooling
and discovery can identify it correctly.
---
Duplicate comments:
In `@workflows/orch-review.workflow.js`:
- Around line 119-142: The prompt assembly in verifyPrompt is still
interpolating untrusted finding/diff content directly into the block delimiters,
so escape or encode those fields before joining the string. Update the
prompt-building logic in orchestr-review.workflow.js around verifyPrompt (and
any helper that injects diff/finding text) so embedded newline directives or
delimiter-like text cannot break out of the intended untrusted section.
🪄 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: afe4699e-4f08-4035-8506-8d50c2ef067a
📒 Files selected for processing (3)
commands/orch-review.mdworkflows/README.mdworkflows/orch-review.workflow.js
📜 Review details
⏰ Context from checks skipped due to timeout. (11)
- GitHub Check: Greptile Review
- GitHub Check: Test (macos-latest, Node 22.x, bun)
- GitHub Check: Test (windows-latest, Node 22.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 18.x, bun)
- GitHub Check: Test (windows-latest, Node 20.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, bun)
- GitHub Check: Test (windows-latest, Node 18.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 18.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 22.x, pnpm)
- GitHub Check: Test (ubuntu-latest, Node 20.x, bun)
🧰 Additional context used
📓 Path-based instructions (18)
commands/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Commands should be formatted as Markdown with description frontmatter.
Files:
commands/orch-review.md
{agents,skills,commands}/**/*.md
📄 CodeRabbit inference engine (CLAUDE.md)
Use lowercase filenames with hyphens (e.g.,
python-reviewer.md,tdd-workflow.md) for agents, skills, and commands.
Files:
commands/orch-review.md
{skills,commands,agents,rules}/**
⚙️ CodeRabbit configuration file
{skills,commands,agents,rules}/**: Focus on prompt-injection resilience, tool-permission scope, destructive action guards, and secret exfiltration risks.
Files:
commands/orch-review.md
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}
📄 CodeRabbit inference engine (.cursor/rules/common-coding-style.md)
**/*.{js,ts,jsx,tsx,py,java,cs,go,rb,php,scala,kt}: Always create new objects, never mutate existing ones. Use immutable patterns to prevent hidden side effects and enable safe concurrency
Organize code into many small files (200-400 lines typical, 800 lines max) organized by feature/domain rather than by type
Always handle errors explicitly at every level and never silently swallow errors
Always validate all user input before processing at system boundaries
Use schema-based validation where available
Fail fast with clear error messages when validation fails
Never trust external data (API responses, user input, file content)
Ensure code is readable and well-named
Keep functions small (less than 50 lines)
Keep files focused (less than 800 lines)
Avoid deep nesting (more than 4 levels)
Do not use hardcoded values; use constants or configuration instead
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
No hardcoded secrets (API keys, passwords, tokens) - validate before any commit
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php}: All user inputs must be validated
Enable CSRF protection on all state-changing endpoints
Verify authentication and authorization for all protected endpoints
Implement rate limiting on all endpoints to prevent abuse
Ensure error messages do not leak sensitive data in responses
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,sql}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Use parameterized queries to prevent SQL injection
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,html,php,java,cs,rb,go}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
Implement XSS prevention by sanitizing HTML output
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,cs,rb,go,php,swift,kt,rs,c,cpp,h,hpp,properties,yml,yaml,json,env,config}
📄 CodeRabbit inference engine (.cursor/rules/common-security.md)
NEVER hardcode secrets in source code - ALWAYS use environment variables or a secret manager
Files:
workflows/orch-review.workflow.js
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.cursor/rules/typescript-coding-style.md)
**/*.{ts,tsx,js,jsx}: Use spread operator for immutable updates in TypeScript/JavaScript instead of direct mutation
Use async/await with try-catch for error handling in TypeScript/JavaScript
Use Zod for schema-based input validation in TypeScript/JavaScript
No console.log statements in production code; use proper logging libraries instead
**/*.{ts,tsx,js,jsx}: Auto-format JavaScript/TypeScript files using Prettier after edit
Warn aboutconsole.logstatements in edited files
Check all modified files forconsole.logstatements before session ends
**/*.{ts,tsx,js,jsx}: Use the ApiResponse interface pattern with generic type parameter:interface ApiResponse<T> { success: boolean; data?: T; error?: string; meta?: { total: number; page: number; limit: number; } }
Implement custom React hooks following the pattern: export a named function with use prefix, generic type parameters, and proper useEffect cleanup for side effects
**/*.{ts,tsx,js,jsx}: Never hardcode secrets; always use environment variables for sensitive credentials like API keys
Throw an error when required environment variables are not configured to fail fast and ensure security prerequisites are metUse Playwright as the E2E testing framework for critical user flows in TypeScript/JavaScript
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts,jsx,tsx}: Always create new objects and never mutate in place; return new copies instead
Keep files between 200–400 lines typical, with a maximum of 800 lines
Extract helpers when a file exceeds 200 lines
Handle errors explicitly at every level; never swallow errors silently
Validate all user input before processing; use schema-based validation where available
Never trust external data (API responses, file content, query params); always validate
All user inputs must be validated and sanitized
Error messages must be scrubbed of sensitive internals
Use readable, well-named identifiers in all code
Keep functions under 50 lines
Keep files under 800 lines
Avoid nesting deeper than 4 levels
Implement comprehensive error handling in all code
Do not hardcode values; use constants or environment configuration instead
Do not use in-place mutation; always return new objects or state
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,json,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Do not hardcode secrets, API keys, passwords, or tokens
Files:
workflows/orch-review.workflow.js
**/*.{js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{js,ts}: Use parameterized queries for all database writes (no string interpolation)
Auth/authz must be checked server-side for every sensitive path
Rate limiting must be applied to all public endpoints
Files:
workflows/orch-review.workflow.js
**/*.{jsx,tsx,js,ts}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
HTML output must be sanitized where applicable
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,env*}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Required environment variables must be validated at startup
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt,cpp,c,fs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt,cpp,c,fs}: Write tests before implementation using TDD workflow: write failing test (RED), implement minimal code (GREEN), then refactor (IMPROVE)
Keep functions small (<50 lines) and files focused (<800 lines, typical 200-400 lines)
Avoid deep nesting (>4 levels)
Files:
workflows/orch-review.workflow.js
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx,py,java,go,rs,kt}: Never mutate existing objects; always create new objects with changes applied (Immutability requirement)
Handle errors at every level; provide user-friendly messages in UI code and detailed context in server-side logs
Ensure error messages don't leak sensitive data
Files:
workflows/orch-review.workflow.js
**/*.{jsx,tsx,html,js,ts}
📄 CodeRabbit inference engine (AGENTS.md)
Sanitize HTML output to prevent XSS vulnerabilities
Files:
workflows/orch-review.workflow.js
🔇 Additional comments (5)
commands/orch-review.md (3)
41-45: LGTM!
80-89: LGTM! The stats example now includes all fields from the workflow return contract (confirmed,unverified,uncertain,refuted). Past comment resolved.
110-119: LGTM! The "Very large diff" wording fix addresses the previous style comment. Past comment resolved.workflows/README.md (1)
55-65: LGTM!workflows/orch-review.workflow.js (1)
82-90: LGTM!Also applies to: 167-185, 230-234
|
ECC bundle files are already tracked in this repository. Skipping generation of another bundle PR. |
1 participant
What Changed
Re-lands the orch-review native Workflow (#2363, reverted by #2393) and fixes the root cause that forced the revert, plus adds the
/orch-reviewcommand surface — all in one PR somainstays green this time.workflows/orch-review.workflow.js+workflows/README.md(the feat(workflows): add orch-review native Workflow pilot #2363 content, unchanged).eslint.config.js— ignore'workflows/**/*.workflow.*'and'.claude/workflows/**', exactly the one-line follow-up @affaan-m recommended in fix(ci): unbreak main after dependabot batch (checkout SHA + lint) #2393.commands/orch-review.md— the/orch-reviewsurface that gathers a diff and invokes the workflow.docs/COMMAND-REGISTRY.json— regenerated for the new command.Why the eslint ignore (not a source fix)
#2393 reverted #2363 because, after the dependabot batch, the
Lintjob runseslint .and the workflow script fails to parse. Workflow DSL scripts use both top-levelexport const meta(ESM-only syntax) and top-levelreturn result(the runtime wraps the script in anasync function, so the result is captured from the return). No single eslintsourceTypecan parse a file with both:exportreturnmodulecommonjs(repo default)Both are mandated by the Workflow runtime, so neither can be removed, and a fatal parse error can't be silenced with an inline
eslint-disable. Excluding the file type is the correct, generalizable fix (it covers all future*.workflow.*files), which is why the maintainer recommended it.Testing Done
npx eslint .— exit 0 (workflow file matched by the new ignore; this is the exact job that triggered the fix(ci): unbreak main after dependabot batch (checkout SHA + lint) #2393 revert).node scripts/ci/validate-commands.js— 93 command files validated.npm run command-registry:check— registry up to date.npx markdownlint commands/orch-review.md workflows/README.md— clean.CHANGES_REQUESTED).Type of Change
feat:New featureNotes
maincurrently also has annpm cilockfile drift (eslint 9 in lock vs 10 inpackage.json) and a missing PyYAML in the Python Tests job. Those break install/test for all PRs and need a separate maintainer fix; this PR does not touchpackage-lock.jsonor Python deps.Follow-ups (not in this PR)
docs/<locale>/commands/orch-review.md(not CI-enforced)./orch-reviewinto theorch-pipelineReview phase as the native option.🤖 Generated with Claude Code