Feat/caveman response style#1920
Conversation
- Add Beans CLI integration and UI - Thread quality gate failures into provider turns - Handle aborted turns and update docs
[codex] Add Kiro and Amazon Q providers
[codex] Fix orchestration recovery and work log detail popup
Added personal acknowledgment and thanks to contributors.
[codex] Add Kiro editor option
[codex] Fix session completion and diff rendering performance
Add change-proof requirements to commit and push flows, surface the required intent and validation inputs in the app, and harden Bun-based quality-gate execution on Windows. Intent: Tie significant git actions to a clear why and to functional proof before code is committed or pushed, while keeping the app quality gate able to run bun scripts reliably. Functional Validation: - bun fmt - bun lint - bun typecheck - bun run --cwd apps/server test -- src/processRunner.test.ts
…nal-test-enforcement [codex] Enforce intent and functional validation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
|
Sorry, my agent wanted to publish this to the original! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit ed3d320. Configure here.
| ), | ||
| Effect.forkDetach, | ||
| ); | ||
| } |
There was a problem hiding this comment.
Interrupt causes duplicate turn completion and abort events
High Severity
When interruptTurn calls Fiber.interrupt on the active turn fiber, Effect.catchCause in forkTurn catches the Interrupt cause and runs completeTurnFailure, which emits runtime.error and turn.completed(failed). After the fiber finishes, interruptTurn then emits turn.aborted and sets the session to "ready". This produces conflicting lifecycle events (turn.completed and turn.aborted) for the same turn, which the ProviderRuntimeIngestion layer processes independently, leading to inconsistent session state.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ed3d320. Configure here.
| env: options.env, | ||
| stdio: "pipe", | ||
| shell: process.platform === "win32", | ||
| shell: false, |
There was a problem hiding this comment.
Removing shell mode breaks Windows .cmd shim resolution
Medium Severity
Changing shell from process.platform === "win32" to false breaks Windows execution of commands distributed as .cmd shims (e.g., beans). Node.js spawn without shell: true cannot execute .cmd files on Windows. While the cli.ts build script added resolveCliBinary for its own bun/npm calls, other callers like beans/cli.ts (calling runProcess("beans", ...)) still rely on shell-based .cmd resolution.
Reviewed by Cursor Bugbot for commit ed3d320. Configure here.
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| @@ -226,12 +239,10 @@ const publishCmd = Command.make( | |||
|
|
|||
| yield* Effect.log(`[cli] Running: npm ${args.join(" ")}`); | |||
| yield* runCommand( | |||
There was a problem hiding this comment.
🟡 Medium scripts/cli.ts:241
On Windows, resolveCliBinary("npm") returns a path to npm.cmd, but the shell: true option was removed in this diff. Spawning a .cmd file without shell mode fails because .cmd files are batch scripts that require cmd.exe to interpret them. The npm publish command will fail on Windows.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/scripts/cli.ts around line 241:
On Windows, `resolveCliBinary("npm")` returns a path to `npm.cmd`, but the `shell: true` option was removed in this diff. Spawning a `.cmd` file without shell mode fails because `.cmd` files are batch scripts that require `cmd.exe` to interpret them. The `npm publish` command will fail on Windows.
Evidence trail:
1. apps/server/scripts/cli.ts lines 37-50 (REVIEWED_COMMIT): `resolveCliBinary` function returns `npm.cmd` for npm on Windows (line 48)
2. git_diff MERGE_BASE..REVIEWED_COMMIT apps/server/scripts/cli.ts: shows removal of `shell: process.platform === "win32"` and the comment "Windows needs shell mode to resolve .cmd shims"
3. apps/server/scripts/cli.ts line 242 (REVIEWED_COMMIT): `ChildProcess.make(resolveCliBinary("npm"), [...args], {...})` without shell option
4. External evidence: GitHub issue anomalyco/opencode#18043 documents the exact same bug pattern with `.cmd` files and `spawn()` without shell option causing ENOENT on Windows
| function toMessage(cause: unknown, fallback: string): string { | ||
| return cause instanceof Error && cause.message.length > 0 ? cause.message : fallback; | ||
| } |
There was a problem hiding this comment.
🟡 Medium Layers/CliAgentAdapterOps.ts:78
result.failure from Effect.result is a Cause<E>, not an Error, so cause instanceof Error always fails and the function always returns the fallback string. Use Cause.squash or Cause.failureOrCause to extract the actual error message from the Cause structure.
-function toMessage(cause: unknown, fallback: string): string {
- return cause instanceof Error && cause.message.length > 0 ? cause.message : fallback;
-}Also found in 1 other location(s)
apps/server/src/provider/Layers/CliAgentAdapterRuntime.ts:407
In
Effect.catchCause, thecauseparameter is aCause.Cause<E>from Effect, not a JavaScriptError. ThetoMessage(cause, ...)call will always return the fallback message becausecause instanceof Erroris alwaysfalsefor Effect'sCausetype. This loses actual error information. To extract error details from aCause, use Effect'sCauseutilities likeCause.findErroror inspectcause.reasons.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Layers/CliAgentAdapterOps.ts around lines 78-80:
`result.failure` from `Effect.result` is a `Cause<E>`, not an `Error`, so `cause instanceof Error` always fails and the function always returns the fallback string. Use `Cause.squash` or `Cause.failureOrCause` to extract the actual error message from the Cause structure.
Evidence trail:
apps/server/src/provider/Layers/CliAgentAdapterOps.ts lines 78-79 (toMessage function definition), lines 108-118 (Effect.result usage and result.failure passed to toMessage). Effect-ts library documentation confirms Exit.Failure.failure is of type Cause<E>, not the error E itself.
Also found in 1 other location(s):
- apps/server/src/provider/Layers/CliAgentAdapterRuntime.ts:407 -- In `Effect.catchCause`, the `cause` parameter is a `Cause.Cause<E>` from Effect, not a JavaScript `Error`. The `toMessage(cause, ...)` call will always return the fallback message because `cause instanceof Error` is always `false` for Effect's `Cause` type. This loses actual error information. To extract error details from a `Cause`, use Effect's `Cause` utilities like `Cause.findError` or inspect `cause.reasons`.
| it.effect("returns unauthenticated when the Kiro auth probe reports no session", () => | ||
| Effect.gen(function* () { | ||
| const status = yield* checkCliAgentProviderStatus(KIRO_PROVIDER_CONFIG); | ||
| assert.strictEqual(status.provider, "kiro"); | ||
| assert.strictEqual(status.status, "error"); | ||
| assert.strictEqual(status.installed, true); | ||
| assert.strictEqual(status.auth.status, "unauthenticated"); | ||
| assert.strictEqual( | ||
| status.message, | ||
| 'Kiro is not authenticated. Run `wsl.exe --exec bash -lc "exec \\"$@\\"" bash kiro-cli login` and try again.', | ||
| ); |
There was a problem hiding this comment.
🟢 Low Layers/CliAgentProvider.test.ts:250
The test asserts status.message equals a WSL-wrapped login command (wsl.exe --exec bash -lc ...), but ServerSettingsService.layerTest() is called without any executionMode or wslDistro settings. The actual message will be the plain kiro-cli login command instead of the WSL variant, causing the assertion to fail when the test runs.
- assert.strictEqual(
- status.message,
- 'Kiro is not authenticated. Run `wsl.exe --exec bash -lc "exec \\"$@\\"" bash kiro-cli login` and try again.',
- );
+ assert.strictEqual(
+ status.message,
+ 'Kiro is not authenticated. Run `kiro-cli login` and try again.',
+ );🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Layers/CliAgentProvider.test.ts around lines 250-260:
The test asserts `status.message` equals a WSL-wrapped login command (`wsl.exe --exec bash -lc ...`), but `ServerSettingsService.layerTest()` is called without any `executionMode` or `wslDistro` settings. The actual message will be the plain `kiro-cli login` command instead of the WSL variant, causing the assertion to fail when the test runs.
Evidence trail:
1. apps/server/src/provider/Layers/CliAgentProvider.test.ts lines 249-270: Test uses `ServerSettingsService.layerTest()` with no overrides, expects WSL-wrapped message
2. packages/contracts/src/settings.test.ts lines 11-16: `DEFAULT_SERVER_SETTINGS.providers.kiro` has `executionMode: "auto"`
3. packages/shared/src/cliAgentCommand.ts lines 65-66: `effectiveExecutionMode = executionMode === "auto" ? (platform === "win32" ? "wsl" : "host") : executionMode`
4. packages/shared/src/cliAgentCommand.ts lines 70-77: When `effectiveExecutionMode !== "wsl"`, returns plain command
5. apps/server/src/provider/Layers/CliAgentProvider.test.ts lines 146-153: Contrast with test that explicitly sets `executionMode: "wsl"` for WSL testing
| const decoded = decodeBeansJson( | ||
| result.stdout.trim(), | ||
| BeansCreateEnvelope, | ||
| "create", | ||
| "Beans CLI returned invalid create JSON.", | ||
| ); | ||
| return Schema.decodeUnknownSync(BeansCreateResult)({ | ||
| bean: decoded.bean, | ||
| message: decoded.message?.trim() || "Bean created", | ||
| }); |
There was a problem hiding this comment.
🟢 Low beans/cli.ts:239
BeansCreateEnvelope decodes a success field from the CLI response, but createBean never checks it. If the CLI returns { success: false, bean: {...}, message: "error" }, the function proceeds to return decoded.bean and the error message as if the operation succeeded. Consider verifying decoded.success === true before extracting the bean, and throwing an appropriate error when the CLI reports failure.
- const decoded = decodeBeansJson(
+ const decoded = decodeBeansJson(
result.stdout.trim(),
BeansCreateEnvelope,
"create",
"Beans CLI returned invalid create JSON.",
);
+ if (!decoded.success) {
+ throw new BeansCommandError({
+ operation: "create",
+ detail: decoded.message?.trim() || "Bean creation failed",
+ cause: new Error(decoded.message || "Beans CLI returned success: false"),
+ });
+ }
return Schema.decodeUnknownSync(BeansCreateResult)({
bean: decoded.bean,
message: decoded.message?.trim() || "Bean created",🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/beans/cli.ts around lines 239-248:
`BeansCreateEnvelope` decodes a `success` field from the CLI response, but `createBean` never checks it. If the CLI returns `{ success: false, bean: {...}, message: "error" }`, the function proceeds to return `decoded.bean` and the error message as if the operation succeeded. Consider verifying `decoded.success === true` before extracting the bean, and throwing an appropriate error when the CLI reports failure.
Evidence trail:
apps/server/src/beans/cli.ts lines 26-30: `BeansCreateEnvelope` schema with `success: Schema.Boolean` field.
apps/server/src/beans/cli.ts lines 239-248: `createBean` function decodes with `BeansCreateEnvelope` but only extracts `decoded.bean` and `decoded.message`, never checking `decoded.success`.
ApprovabilityVerdict: Needs human review 2 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
1 participant
What Changed
Why
UI Changes
Checklist
Note
High Risk
High risk because it changes core orchestration/session lifecycle handling and adds new commit/push gating that can block developer workflows if misconfigured. It also alters process spawning behavior (disables shell) and introduces new CLI-based provider adapters, increasing cross-platform/runtime surface area.
Overview
Adds workflow guardrails across commits, pushes, and agent turns. Git actions can now require per-change proof (Intent + functional validation commands), run those validation commands during
commit, append proof to commit bodies, and blockpush/PR creation when outgoing commits are missing required proof.Integrates a post-turn quality gate into provider runtime ingestion: turns with file changes trigger evaluation, record pass/fail activities, set the thread session to
erroron failure, and automatically prepend the failure detail to the next user prompt until a passing gate is recorded.Improves reliability and UX of long-running sessions by treating
turn.abortedas terminal, reconciling staleactiveTurnId/latest turn state when sessions settle, and rejectingthread.turn.startwhile a turn is already running.Expands provider/editor tooling: adds CLI-based adapters and health checks for Kiro and Amazon Q (including WSL execution and Amazon Q IAM Identity Center auth detection), adds Kiro to editor launch detection, and updates documentation/Beans metadata (new
.beansconfig + bean files, README refresh).Reviewed by Cursor Bugbot for commit ed3d320. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Add Kiro and Amazon Q provider support with quality guardrails, Beans UI, and Caveman response style
ProviderKindschema, model selection, settings UI, adapter layers, provider registry, and editor/picker integrationCliAgentAdapterandCliAgentProviderimplementations that invokekiroandqbinaries via CLI (host or WSL mode) to handle turns and detect install/auth readinessQualityGateservice that evaluates file-changing turns against configurable rules (format, lint, typecheck, file/function length, cyclomatic complexity); failures are surfaced asQUALITY_GATE_FAILEDactivities and block the session with alastErrorresponseStyle: off/lite/full/ultra) that prepends style instructions to turn inputs and developer instructions before routing to providersBeansControl) backed by new RPC endpoints and React Query hooks for browsing, creating, editing, and tracking project beansGitManager, with proof appended to commit messages and parsed on pushlatestTurnfinalization when a running session settlesprocessRunnernow spawns withshell: falseon all platforms including Windows, changing process invocation behavior📊 Macroscope summarized ed3d320. 59 files reviewed, 8 issues evaluated, 2 issues filtered, 4 comments posted
🗂️ Filtered Issues
apps/server/src/provider/Layers/CliAgentAdapterRuntime.ts — 0 comments posted, 1 evaluated, 1 filtered
Effect.catchCause, thecauseparameter is aCause.Cause<E>from Effect, not a JavaScriptError. ThetoMessage(cause, ...)call will always return the fallback message becausecause instanceof Erroris alwaysfalsefor Effect'sCausetype. This loses actual error information. To extract error details from aCause, use Effect'sCauseutilities likeCause.findErroror inspectcause.reasons. [ Cross-file consolidated ]packages/contracts/src/beans.ts — 0 comments posted, 1 evaluated, 1 filtered
BeansArchiveInputonly containscwdbut lacks anidfield to identify which bean to archive. Comparing toBeansUpdateInput(lines 77-85) which includes bothcwdandidto target a specific bean, this schema appears incomplete. Without an identifier, the archive operation cannot target a specific bean. [ Out of scope (triage) ]