fix(local): inject --input KEY=VALUE into workflow env + idle-output watchdog#139
Merged
Merged
Conversation
…watchdog Two fixes so `ricky local --run` never stalls or fails spuriously, even with auto-fix enabled. 1. --input KEY=VALUE was silently dropped. `ricky local --spec-file _review.md --run --input TARGET_SPEC=<path>` parsed the flag nowhere — the value never reached the workflow runner subprocess env, so reusable review/fix workflows that read `process.env.TARGET_SPEC` failed with MISSING_ENV_VAR. The auto-fix loop then "repaired" the workflow repeatedly (adding an env loader that can't conjure a value that was never passed), burning attempts and wall-clock. Now `--input KEY=VALUE` (and `--input=KEY=VALUE`) parse into an `inputs` record that threads CLI → handoff → normalized request → coordinator launch env, so workflow scripts read them via process.env.KEY. Invalid keys / malformed pairs are reported as CLI errors. 2. No inactivity detection — a hung runner stalled for the full 12 h DEFAULT_RUN_TIMEOUT_MS. A healthy run constantly emits broker/agent output; total silence means it's hung (dead broker, half-open stdio pipe, a subprocess parked at 0% CPU). Added an idle-output watchdog (DEFAULT_RUN_IDLE_TIMEOUT_MS = 30 min, override via RICKY_RUN_IDLE_TIMEOUT_MS, 0 disables) that aborts the runner on prolonged silence so the run fails fast and the orchestrator moves on. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Warning Review limit reached
More reviews will be available in 4 minutes and 13 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR introduces CLI ChangesWorkflow Input Parameters and Inactivity Monitoring
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces an inactivity watchdog to abort hung workflow runner subprocesses early and adds support for passing custom environment variables via --input KEY=VALUE CLI flags. Feedback on these changes highlights two issues: a parsing bug where a trailing flag following --input could be skipped silently, and a potential resource leak where the watchdog's interval timer might not be cleared if the runner throws a synchronous error.
Comment on lines
+213
to
+225
| let pair: string | undefined; | ||
| if (arg === '--input') { | ||
| pair = argv[index + 1]; | ||
| index += 1; | ||
| } else if (arg.startsWith('--input=')) { | ||
| pair = arg.slice('--input='.length); | ||
| } else { | ||
| continue; | ||
| } | ||
| if (pair === undefined || pair.startsWith('--')) { | ||
| errors.push('--input requires a KEY=VALUE argument.'); | ||
| continue; | ||
| } |
There was a problem hiding this comment.
If the --input flag is followed by another flag starting with -- (for example, if the user forgets the argument or makes a typo like ricky local --input --run), the parser will consume --run as the pair value and increment index. This causes the next flag (--run) to be completely skipped in the next iteration of the loop, silently ignoring it.
We should validate that the next argument exists and does not start with -- before consuming it and incrementing the index.
Suggested change
| let pair: string | undefined; | |
| if (arg === '--input') { | |
| pair = argv[index + 1]; | |
| index += 1; | |
| } else if (arg.startsWith('--input=')) { | |
| pair = arg.slice('--input='.length); | |
| } else { | |
| continue; | |
| } | |
| if (pair === undefined || pair.startsWith('--')) { | |
| errors.push('--input requires a KEY=VALUE argument.'); | |
| continue; | |
| } | |
| let pair: string | undefined; | |
| if (arg === '--input') { | |
| const next = argv[index + 1]; | |
| if (next === undefined || next.startsWith('--')) { | |
| errors.push('--input requires a KEY=VALUE argument.'); | |
| continue; | |
| } | |
| pair = next; | |
| index += 1; | |
| } else if (arg.startsWith('--input=')) { | |
| pair = arg.slice('--input='.length); | |
| } else { | |
| continue; | |
| } | |
| if (pair === undefined || pair.startsWith('--')) { | |
| errors.push('--input requires a KEY=VALUE argument.'); | |
| continue; | |
| } |
Comment on lines
+447
to
+449
| ).finally(() => { | ||
| if (idleInterval) clearInterval(idleInterval); | ||
| }); |
There was a problem hiding this comment.
If this.runner throws a synchronous error (for example, during argument validation or initial setup before returning a promise), withTimeout is never called. As a result, the .finally() block is never attached, and idleInterval is leaked (never cleared).
To prevent this resource leak, we should ensure that any synchronous errors from this.runner are converted into a promise rejection so that withTimeout is always invoked and the .finally() block always runs.
For example, you can wrap the runner call in an async block or Promise.resolve().then(...) like this:
const runnerPromise = (async () => {
return this.runner(request.workflowFile, {
// ... options ...
});
})();
const runnerResult = await withTimeout(
runnerPromise,
request.timeoutMs ?? DEFAULT_RUN_TIMEOUT_MS,
() => abortController.abort(),
).finally(() => {
if (idleInterval) clearInterval(idleInterval);
});![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reviewed changes — two fixes for ricky local --run reliability: threading --input KEY=VALUE through the full CLI→handoff→runner env pipeline, and adding an idle-output watchdog that aborts the SDK script runner after 30 min of zero output.
--inputflag parsing and env injection —parseInputFlags()inpower-user-parser.tssupports both--input KEY=VALUEand--input=KEY=VALUEforms, with validation for valid env-var names and malformed pairs. Flows throughPowerUserParsedArgs→BaseHandoff→LocalInvocationRequest→RunRequest.env→ subprocessenv.- Idle-output watchdog —
setIntervalinSdkScriptWorkflowCoordinator.launch()tracks output activity and aborts the runner afterDEFAULT_RUN_IDLE_TIMEOUT_MS(30 min) of silence. Configurable viaRICKY_RUN_IDLE_TIMEOUT_MS;0disables.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/local/entrypoint.ts (1)
447-478:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPreserve idle-watchdog aborts as a distinct timeout signal.
If the watchdog aborts the run, the awaited runner promise rejects before Lines 450-452 execute, so the inactivity marker never reaches
stderr. The catch path then records onlyscript workflow run abortedand marks the run asfailed, which makes downstream blocker classification miss the idle timeout and fall back to generic failure handling.Suggested fix
- ).finally(() => { + ).finally(() => { if (idleInterval) clearInterval(idleInterval); }); - if (idleAborted) { - stderr.push(`Workflow runner aborted after ${Math.round(idleTimeoutMs / 1000)}s of inactivity (suspected hang).`); - } const reportedFailure = failureFromScriptWorkflowResult(runnerResult) ?? failureFromScriptWorkflowOutput(stdout, stderr); if (reportedFailure) { throw new Error(reportedFailure); } @@ } catch (error) { - const message = error instanceof Error ? error.message : String(error); - status = message.startsWith('timed out after ') ? 'timed_out' : 'failed'; - stderr.push(message); + const idleMessage = idleAborted + ? `Workflow runner aborted after ${Math.round(idleTimeoutMs / 1000)}s of inactivity (suspected hang).` + : undefined; + const message = idleMessage ?? (error instanceof Error ? error.message : String(error)); + status = idleAborted || message.startsWith('timed out after ') ? 'timed_out' : 'failed'; + stderr.push(message); emit(status === 'timed_out' ? 'timeout' : 'error', message, { error: message }); return coordinatorResultFromSdkRun({🤖 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 `@src/local/entrypoint.ts` around lines 447 - 478, The catch block needs to preserve watchdog aborts: detect if idleAborted is true and, if so, set message to `Workflow runner aborted after ${Math.round(idleTimeoutMs/1000)}s of inactivity (suspected hang).`, set status to 'timed_out', push that message to stderr and emit the 'timeout' event (instead of treating it as a generic failure); otherwise continue the existing error handling. Update the catch path where status is assigned and emit() is called so the idleAborted branch runs before the generic message/status logic.src/surfaces/cli/flows/power-user-parser.ts (1)
150-166:⚠️ Potential issue | 🟠 Major | ⚡ Quick winTreat
--inputas consuming the next argv token in legacyricky runparsing.
parseInputFlags()accepts--input KEY=VALUE, butreadRunArtifactPositional()still sees thatKEY=VALUEtoken as the artifact for commands likericky run --input TARGET_SPEC=spec.md workflows/generated/review.ts. That makes the new flag break the positional run form whenever it appears before the artifact.Suggested fix
function isValueForRunOption(previous: string | undefined, candidate: string): boolean { if (!previous) return false; + if (previous === '--input') return true; if ((previous === '--auto-fix' || previous === '--repair') && isAutoFixValue(candidate)) return true; return previous === '--start-from' || previous === '--previous-run-id' || previous === '--resume-from-run'; }🤖 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 `@src/surfaces/cli/flows/power-user-parser.ts` around lines 150 - 166, The parseInputFlags() changes made `--input KEY=VALUE` work but the legacy positional parser still treats the KEY=VALUE token as an artifact; update the CLI arg handling so `--input` is treated like a flag that consumes its next argv token during legacy parsing: add '--input' to the list passed to the loop that checks flags-with-values (the array iterated in this file) or update readFlagValue(effectiveArgv, flag) to recognize and consume the token after '--input', ensuring readRunArtifactPositional will not see the KEY=VALUE token as a positional artifact and leaving parseInputFlags and readRunArtifactPositional behavior consistent.
🤖 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 `@src/surfaces/cli/commands/cli-main.ts`:
- Around line 108-109: Add documentation for the new --input flag inside the
renderHelp() function so it appears in ricky --help; specifically, update the
help output generated by renderHelp() to include an entry describing "--input
KEY=VALUE" (referencing the inputs?: Record<string, string> option added to the
CLI command options) and explain that multiple --input flags set KEY=VALUE pairs
injected into the workflow runner env and how to use them. Ensure the help text
is concise and matches existing help formatting and examples used by
renderHelp().
---
Outside diff comments:
In `@src/local/entrypoint.ts`:
- Around line 447-478: The catch block needs to preserve watchdog aborts: detect
if idleAborted is true and, if so, set message to `Workflow runner aborted after
${Math.round(idleTimeoutMs/1000)}s of inactivity (suspected hang).`, set status
to 'timed_out', push that message to stderr and emit the 'timeout' event
(instead of treating it as a generic failure); otherwise continue the existing
error handling. Update the catch path where status is assigned and emit() is
called so the idleAborted branch runs before the generic message/status logic.
In `@src/surfaces/cli/flows/power-user-parser.ts`:
- Around line 150-166: The parseInputFlags() changes made `--input KEY=VALUE`
work but the legacy positional parser still treats the KEY=VALUE token as an
artifact; update the CLI arg handling so `--input` is treated like a flag that
consumes its next argv token during legacy parsing: add '--input' to the list
passed to the loop that checks flags-with-values (the array iterated in this
file) or update readFlagValue(effectiveArgv, flag) to recognize and consume the
token after '--input', ensuring readRunArtifactPositional will not see the
KEY=VALUE token as a positional artifact and leaving parseInputFlags and
readRunArtifactPositional behavior consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d1dcf161-5542-42e0-9e44-ca0d99acf583
📒 Files selected for processing (6)
src/local/entrypoint.tssrc/local/request-normalizer.tssrc/shared/constants.tssrc/surfaces/cli/commands/cli-main.tssrc/surfaces/cli/flows/power-user-parser.test.tssrc/surfaces/cli/flows/power-user-parser.ts
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found across 6 files
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Contributor
Ricky Eval ReviewRun: Passed: 11 | Needs human: 44 | Reviewable: 44 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesThese cases passed deterministic checks and include captured Ricky output for a human verdict against their REVIEW
|
| Category | Typical Symptom | What to Check | Quick Fix |
|---|---|---|---|
| Schema Mismatch | “Contract violation: input.type !== string” | Verify the input contract | |
| ...[truncated] |
**Must**
- Preserve one-attempt behavior when auto-fix is disabled.
- Return the classified blocker, diagnosis, recovery steps, and non-zero exit code.
- Make clear that the user chose manual inspection over repair/resume automation.
**Must Not**
- Start a repair loop despite `--no-auto-fix`.
- Suppress the diagnosis because no repair was attempted.
- Present the failure as a completed repair attempt.
**Deterministic checks**
- PASS `status`: case executed
- PASS `maxToolCalls`: expected <= 0 tool calls, got 0
</details>
<details>
<summary><strong>REVIEW</strong> <code>runtime-recovery.in-process-local-runner</code> (runtime-recovery/manual)</summary>
**User message**
> Explain how Ricky should execute a local TypeScript workflow artifact in the primary local path.
**Ricky output**
```text
**Ricky – Local Execution of a TypeScript Workflow Artifact**
*(Primary local path = `./workflows` – the default location for all workflow source files in the AgentWorkforce repo.)*
---
## 1. Preconditions (Contract)
| Item | Requirement | How to Verify |
|------|-------------|---------------|
| **Workspace** | You are in the root of the AgentWorkforce repo (`git rev‑parse --show-toplevel`). | `pwd` should end with the repo root. |
| **Node version** | `>= 18.0.0` (LTS) and `npm >= 9`. | `node -v && npm -v`. |
| **TypeScript compiler** | `tsc` available (installed via dev‑deps). | `npx tsc --version`. |
| **Workflow artifact** | A single `.ts` file under `./workflows` (e.g., `myWorkflow.ts`). | `ls ./workflows/*.ts`. |
| **Dependencies** | All `package.json` deps installed. | `npm ci` must succeed with a clean `node_modules`. |
| **Ricky config** | `ricky.yaml` present at repo root with `execution.localPath: "./workflows"`. | `cat ricky.yaml | grep execution.localPath`. |
If any contract item fails, **report a blocker** and abort execution.
---
## 2. Deterministic Verification Gates
| Gate | Command | Expected Outcome |
|------|---------|------------------|
| **G‑01: Lint*
...[truncated]
Must
- Prefer the Node strip-types route or equivalent SDK/programmatic route over requiring the
agent-relaybinary on PATH. - Precheck that Node and
@agent-relay/sdkare resolvable for the workflow. - Record the actual spawn command in execution evidence.
Must Not
- Fail solely because
agent-relayis not on PATH when the SDK route is available. - Hide the actual runtime command from evidence.
- Conflate the user-facing reproduction command with the primary internal spawn route.
Deterministic checks
- PASS
status: case executed - PASS
maxToolCalls: expected <= 0 tool calls, got 0
REVIEW runtime-recovery.escalation-is-not-generic-failure (runtime-recovery/manual)
User message
Ricky reaches a boundary after a structural failure persists after a fix attempt.
Ricky output
**Boundary‑Reached Protocol – Persistent Structural Failure**
When a structural failure remains after an attempted fix, the workflow must be halted, documented, and escalated in a deterministic, auditable way. Follow the contract below to keep the AgentWorkforce (AW) repository clean, reproducible, and ready for a coordinated recovery.
---
## 1. Immediate Blocker Declaration
1. **Create a blocker issue** in the `#workflow‑blockers` project board (or the repository’s `issues` with label `blocker`).
- **Title:** `BLOCKER – Persistent structural failure in <component‑name>`
- **Body template:**
```markdown
## Summary
Structural failure persists after fix attempt #<PR‑id>.
## Symptoms
- Symptom 1
- Symptom 2
- ...
## Attempted Fix
- PR/branch: <link>
- Changes: <short description>
- Verification results: <failed gate name> (details)
## Impact
- Downstream workflows affected: <list>
- SLA breach risk: <yes/no>
## Next Steps (proposed)
- Open a dedicated recovery branch `recovery/<component>-<date>`
- Assign a lead (see §2)
- Schedule a triage meeting (within 4 h)
## Attachmen
...[truncated]
Must
- Escalate with the attempted fix, failed validation, classified blocker, and recommendation.
- Distinguish escalation from a generic product failure.
- Preserve enough context for a human operator to continue.
Must Not
- Retry speculative fixes indefinitely.
- Collapse to "something went wrong" without the attempted actions.
- Discard evidence from failed repair attempts.
Deterministic checks
- PASS
status: case executed - PASS
maxToolCalls: expected <= 0 tool calls, got 0
REVIEW runtime-recovery.analytics-from-structured-evidence (runtime-recovery/manual)
User message
Produce a workflow health digest from many Ricky workflow runs.
Ricky output
**Ricky Workflow Health Digest – Template & Usage Guide**
Below is a ready‑to‑use, deterministic digest that aggregates the health signals from any number of Ricky workflow runs. Plug‑in the data from your CI/CD system (GitHub Actions, GitLab CI, Jenkins, etc.) and the digest will produce a concise, actionable report that respects the Ricky repository conventions (AGENTS.md, workflow standards, shared authoring rules).
---
## 1️⃣ Digest Contract (Input / Output)
| Element | Type | Description | Required |
|---------|------|-------------|----------|
| `run_id` | string | Unique identifier of the workflow run (e.g., GitHub run number). | ✅ |
| `branch` | string | Git ref the run executed on (e.g., `main`, `feature/x`). | ✅ |
| `started_at` | ISO‑8601 timestamp | When the run started. | ✅ |
| `ended_at` | ISO‑8601 timestamp | When the run finished. | ✅ |
| `status` | enum(`success`,`failure`,`cancelled`,`skipped`) | Final outcome. | ✅ |
| `stage_results` | array of objects | Per‑stage health (see **StageResult** below). | ✅ |
| `blockers` | array of strings | Human‑readable blocker messages (empty if none). | ✅ |
| `review_artifacts` | array of URLs | Links to PR reviews, covera
...[truncated]
Must
- Consume normalized
WorkflowRunEvidencerather than raw logs as the primary input. - Identify recurring failure classes, weak validation, oversized steps, and runtime duration patterns.
- Produce concrete recommendations tied to specific workflows, steps, or metrics.
Must Not
- Mutate evidence while analyzing it.
- Return generic advice like "improve your workflow" without references.
- Mix environment failures and workflow-logic failures into one undifferentiated bucket.
Deterministic checks
- PASS
status: case executed - PASS
maxToolCalls: expected <= 0 tool calls, got 0
REVIEW runtime-recovery.preserve-pr-shipping-during-repair (runtime-recovery/manual)
User message
A persona-authored workflow that imports
@agent-relay/github-primitiveand usescreateGitHubStepto open a PR fails its runtime-precheck. The auto-fix repair persona is invoked. Describe the contract the repaired artifact must satisfy.
Ricky output
**Repaired Workflow Contract**
The repaired artifact must be a **single, self‑contained workflow file** that satisfies the **runtime‑precheck** and the **deterministic verification pipeline** defined in the AgentWorkforce standards. Below is the contract broken into concrete sections that the auto‑fix persona must guarantee before the PR can be merged.
| # | Contract Element | Required State | How to Verify |
|---|------------------|----------------|---------------|
| 1 | **File Header & Metadata** | • `name: <workflow‑name>` <br>• `description: <short‑sentence>` <br>• `version: semver (≥ 1.0.0)` <br>• `author: <persona‑id>` | Lint rule `wf‑metadata‑header` checks presence and format. |
| 2 | **Import Statement** | Must import the **exact** package name and version that the runtime expects: <br>```ts\nimport { createGitHubStep } from '@agent‑relay/github‑primitive@^2.3.0';\n``` | The pre‑check parser resolves the import via the workspace lockfile; any mismatch fails `import‑resolution‑gate`. |
| 3 | **Step Factory Usage** | The workflow must call `createGitHubStep` **once** (or more, but each call must be wrapped in a `GitHubStep` object) and assign the result to a variable that
...[truncated]
Must
- Preserve the
@agent-relay/github-primitiveimport, theGitHubStepExecutorreference, and everycreateGitHubStep(...)invocation that the original artifact declared. - Keep the failing workflow runnable from the same path with the same
workflow(...)builder and.run({ cwd: process.cwd() })invocation. - Retain at least ceil(N / 2) of the original workflow's
.step(...)calls when the original declared four or more steps; a repair that collapses to a 2-3 step placeholder is a regression, not a fix. - Reject (do not apply) any repair output whose step list reduces to
prepare-context/runtime-precheck: true/final-signoff: echo placeholder; surface the regression diagnostic instead.
Must Not
- Strip
createGitHubStep,GitHubStepExecutor, or@agent-relay/github-primitivebecause the runtime-precheck failure mentioned PR-shipping or git side effects. The repair contract's "no commit / no push" constraint applies to the REPAIR AGENT's runtime behavior, not to the workflow's step declarations. - Emit a "minimal repair-safe master" or "simplified Ricky master" scaffold that passes the builder validator while doing none of the original work.
- Treat "the workflow now builds and runs" as success when the work it was supposed to ship is gone.
Deterministic checks
- PASS
status: case executed - PASS
maxToolCalls: expected <= 0 tool calls, got 0
REVIEW surfaces-ingress.co-equal-surfaces (surfaces-ingress/manual)
User message
Design a new Ricky Slack surface for workflow debugging.
Ricky output
**Ricky Slack Surface – “Workflow Debugger”**
---
## 1. Overview
The **Workflow Debugger** Slack surface gives engineers a deterministic, low‑latency way to inspect, step‑through, and mutate running Ricky workflows directly from a Slack channel or DM. It is built as a **Slack App** that talks to the existing Ricky API gateway and uses the **Workflow Execution Service (WES)** for introspection.
| Component | Responsibility |
|-----------|-----------------|
| **Slack Bot** (`ricky-debugger`) | Slash‑command handling, interactive blocks, modal dialogs, and result posting. |
| **Ricky API Extension** (`/debug/*`) | Auth‑checked endpoints for: <br>• `GET /debug/{runId}` – snapshot of current state <br>• `POST /debug/{runId}/step` – advance one step <br>• `POST /debug/{runId}/set` – inject variable/value <br>• `POST /debug/{runId}/abort` – terminate run |
| **Verification Service** | Executes deterministic “gate” functions before any state‑changing debug action (e.g., “step” only allowed if run is *paused*). |
| **Audit Log** | Immutable record of every debug command, user, timestamp, and before/after state diff. |
| **Permission Matrix** | Scoped to `ricky-debugger` r
---
_Comment truncated to stay within GitHub limits. Download the `ricky-eval-run` artifact for the full `human-review.md`._
- power-user-parser: don't consume a following flag as the --input value.
`--input --run` previously swallowed `--run`, silently dropping it.
Now only advance the index when the next token is a real value; report
an error otherwise. Empty pair ("") is also rejected.
- entrypoint idle watchdog: record the abort reason on stderr + events
inside the watchdog callback (before abort()), so it survives the runner
promise rejecting and surfaces as the real failure cause. The catch block
now classifies an idle abort as 'timed_out' and avoids double-logging.
- entrypoint: clear the idle interval in a finally block so a synchronous
throw from this.runner() (before withTimeout is reached) can't leak it.
- resolveIdleTimeoutMs: require a non-negative integer. A fractional value
like 0.5 previously floored to 0 and silently disabled the watchdog;
now it falls back to the default.
- cli-main: document --input KEY=VALUE in renderHelp().
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Two issues caused
ricky local --runto stall for hours in overnight batch runs, even with auto-fix enabled.1.
--input KEY=VALUEwas silently droppedThe overnight runner invokes reusable review/fix workflows like:
But
--inputwas parsed nowhere in ricky. The value never reached the workflow runner subprocess env, so the generated review workflow'sassertRickyWorkflowEnv('TARGET_SPEC')failed with:The auto-fix loop then tried to "repair" the workflow — adding a repo-local
.envloader — but no loader can conjure a value that was never passed in. Each repair spawns a fresh workflow gen+run (minutes each), burning the attempt budget and wall-clock.2. No inactivity detection
A hung runner (dead broker, half-open stdio pipe, a subprocess parked at 0% CPU) stalled for the full 12 h
DEFAULT_RUN_TIMEOUT_MSbefore the outer timeout fired. Observed in practice: anode --experimental-strip-typesworkflow process sat at 0% CPU for 8+ hours.Fix
1. Thread
--inputinto the runner env.--input KEY=VALUE(and--input=KEY=VALUE) now parse into aninputsrecord that flows CLI args →RawHandoff→ normalizedLocalInvocationRequest→coordinator.launchenv, so workflow scripts read them viaprocess.env.KEY. Invalid env-var keys and malformed pairs are reported as CLI errors. Empty values (--input TARGET_SPEC=) are allowed.2. Idle-output watchdog. A healthy run constantly emits broker/agent output; total silence for the idle window means it's hung. Added a watchdog (default
DEFAULT_RUN_IDLE_TIMEOUT_MS= 30 min, override viaRICKY_RUN_IDLE_TIMEOUT_MS,0disables) that aborts the runner on prolonged silence so the run fails fast and the orchestrator moves on instead of stalling.Together these mean: the review/fix workflows get their
TARGET_SPECand succeed, and any future hang is bounded to 30 minutes instead of 12 hours — so auto-fix can stay enabled without risk of multi-hour stalls.Verification
npm run typecheck→ cleannpm test→ 1383 passed (added 4--inputparser tests)🤖 Generated with Claude Code