Repository navigation
Support Claude event-array results and fail-closed Grok API-key setup [#58] - #130
Conversation
Select the last terminal result while preserving legacy objects and terminal metadata. Reject errors before requiring final text. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Require bounded model availability and reject explicit authentication or unsupported-model failures before execution; verify compatibility with typed terminal failures and sidecars. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Merge Protections🟢 All 2 merge protections satisfied — ready to merge. Show 2 satisfied protections🟢 Queue ready pull requestsAutomatically queue non-draft pull requests targeting main after verify, Unfret, and live-gate pass on the head.
🟢 🚦 Auto-queueWhen all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
|
 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| return unavailableStatus(combined) === "child-failed" | ||
| && /\blogged in\b|\bYou are using XAI_API_KEY\./i.test(combined) | ||
| && grokModelAvailable(combined, model); |
There was a problem hiding this comment.
If grok models prints an informational line such as Authentication: API key alongside You are using XAI_API_KEY. and the requested model, this new check rejects the response. unavailableStatus treats the word authentication alone as a failed login. The runner then retries and exits 77 without starting the lane. Check for a stated authentication failure instead of any mention of authentication.
Knowledge Base Used: Poteto runner and orchestration
✅ Accepted riskAccepted by ericlitman: GH-58 (authority 48fa77b) 🟡 Medium · Malformed-result test inherits the parent sandbox marker 🟡 Medium · Long rejected Grok preflights get the wrong status Advisories (1)
DetailsRun: @unfret review · @unfret status · @unfret help |
![unfret-eal[bot]](https://avatars.githubusercontent.com/u/28211?s=60&v=4){kind=small}
There was a problem hiding this comment.
🛑 2 blocking findings
🟡 Medium · Isolate the parent network marker in the malformed-result test
🟡 Medium · Long rejected Grok preflights receive the wrong status
Check
| status: exitCode === 0 ? "malformed-output" : "child-failed", | ||
| exitCode, reportedModel: null, modelVerified: false, | ||
| sessionId: null, error: { message: exitCode === 0 | ||
| ? "grok result did not contain a valid terminal status" : "child exited with status 1" }, |
There was a problem hiding this comment.
🟡 Medium · Malformed-result test inherits the parent sandbox marker
The exit-1 assertion expects exactly child exited with status 1, but options("grok") selects a Codex parent and the file-level hooks do not clear CODEX_SANDBOX_NETWORK_DISABLED. When the suite runs from a supported Codex workspace-write session with the marker set to 1, the documented sandbox hint is appended and this test fails; reproduced with CODEX_SANDBOX_NETWORK_DISABLED=1 bun test .../run.test.ts (84 pass, 1 fail). Also reported for this defect: - Ambient sandbox marker breaks the Bun test suite (plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:638): When the suite runs from a Codex workspace with CODEX_SANDBOX_NETWORK_DISABLED=1, this malformed-result test invokes runLane in the same process and expects exactly "child exited with status 1". The marker is cleared only inside the separate diagnostics describe block, so the runner appends the new sandbox hint and this assertion fails, breaking the required bun run test gate. - Isolate the parent network marker in the malformed-result test (plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:638, plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:637-638): With CODEX_SANDBOX_NETWORK_DISABLED=1, the exit-1 case correctly receives the documented sandbox hint, but this assertion requires the unhinted message. The marker is cleared only inside the separate diagnostics describe block, so this test inherits it from a restricted Codex parent. Reproduced with CODEX_SANDBOX_NETWORK_DISABLED=1 bun run test: 213 pass, 1 fail. Clear and restore the marker for this test's unhinted scenario. Also reported for this defect: - Test expects no sandbox hint but inherits the ambient marker (plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:637-638): The new hint in run.ts reads the runner process's own CODEX_SANDBOX_NETWORK_DISABLED, and the tests call runLane in the same process. Only the "Codex parent sandbox diagnostics" describe clears that variable; the file-level beforeEach/afterEach do not. The test "keeps malformed terminal data distinct after child exit 1" uses options("grok") and requires error.message to equal exactly "child exited with status 1". That path is a plain child-failed with no providerFailure, so the hint applies. Trigger: bun test runs from a default Codex workspace-write session, which the diff's own doc says exports CODEX_SANDBOX_NETWORK_DISABLED=1. AGENTS.md also requires agents to run Bun tests before opening a PR. If options() gives grok a codex parent, the message becomes "child exited with status 1; likely cause: Codex parent sandbox…" and the assertion fails, breaking the pre-PR test gate. The codex default is likely: the claude test at line 326 gets the hint without overriding the parent. However, the options() helper is outside the visible lines, so the grok default cannot be confirmed.
Now at plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:638-639
Also affects:
| return unavailableStatus(combined) === "child-failed" | ||
| && /\blogged in\b|\bYou are using XAI_API_KEY\./i.test(combined) | ||
| && grokModelAvailable(combined, model); |
There was a problem hiding this comment.
🟡 Medium · Long rejected Grok preflights get the wrong status
This veto examines complete stdout/stderr, but retry and final classification later receive rawPreflightEvidence truncated to 4,000 characters. If an authenticated exit-0 preflight lists the requested model, emits over 4,000 characters, then reports model grok-4.6 is not supported on stderr, the veto rejects it while classification loses the reason, waits five seconds, retries, and returns unauthenticated/77 instead of unavailable-model/69; this was reproduced on the current head. Also reported for this defect: - Long rejected Grok preflights get the wrong status (plugins/pstack/skills/poteto-mode/scripts/runner/run.ts:412): The new veto examines the complete stdout/stderr, but downstream retry and failure classification still use evidence truncated to 4,000 characters. If an exit-0 preflight prints authentication and the requested model early, more than 4,000 characters of output, then an unsupported-model reason on stderr, this check rejects it while classification misses the reason; the runner unnecessarily retries and returns unauthenticated/77 instead of unavailable-model/69. - Rejected Grok preflight misclassified as unauthenticated (plugins/pstack/skills/poteto-mode/scripts/runner/run.ts:412-414): Carried demand, keep-confirmed. The run.ts hunks are unchanged since the earlier reproduction, and nothing visible passes full streams to classification. The new veto at lines 412-414 scans the complete stdout and stderr. Retry selection and preflightFailureStatus (line 440) still classify the preflight evidence after it is truncated to 4,000 characters. That call site is outside the diff, so I cannot see it directly. Failure scenario: - An exit-0 grok models run prints You are logged in.\nAvailable models: grok-4.6 followed by more than 4,000 characters on stdout. - Stderr then reports model grok-4.6 is not supported. - The veto rejects the preflight. - The truncated evidence lacks the rejection but still contains the exact model token, so classification falls through to unauthenticated. - The runner waits through the 5-second retry and returns 77, not the unavailable-model/69 the PR promises. Same root cause at line 440: if truncation cuts grok-4.6-build right after grok-4.6, the token check passes on the evidence while the full-stream check failed. This also yields a retry and 77. - Long rejected Grok preflights receive the wrong status (plugins/pstack/skills/poteto-mode/scripts/runner/run.ts:412-414): The new veto examines complete stdout/stderr, but retry and final failure classification still receive evidence truncated to 4,000 characters. When an authenticated exit-0 preflight lists the requested model, emits more than 4,000 characters on stdout, then reports model grok-4.6 is not supported on stderr, the veto rejects it while classification loses the rejection reason. Reproduced under both authentication modes: the runner retries and returns unauthenticated/77 instead of unavailable-model/69. Classification needs the full streams, independently of receipt truncation.
Now at plugins/pstack/skills/poteto-mode/scripts/runner/run.ts:412-414
Also affects:
|
Unfret waiver for head 277029a. Ticket
Neither finding involves security, secret exposure, data loss, or gate or waiver integrity. |
Live test, partial (head 277029a)Result: a Codex regression check passed, but #58's two headline behaviors have not run on the real surface. I have not set the
Blocked, still required before
Review state: Unfret shows |
Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
There was a problem hiding this comment.
🛑 2 blocking findings
🟡 Medium · Malformed-result test inherits the parent sandbox marker
🟡 Medium · Long rejected Grok preflights get the wrong status
Check
|
Waiver re-applied for head 5d75866 (queue update of 277029a). Unfret waiver for head 277029a. Ticket
Neither finding involves security, secret exposure, data loss, or gate or waiver integrity. |
Live tests and live-gate (head 5d75866)This head is level with
|
Merge Queue Status
This pull request spent 4 minutes 55 seconds in the queue, including 4 minutes 42 seconds running CI. Required conditions to merge
|
Live tests and live-gate for the queue-created head 927a7b6Mergify merged
The installed |
Bring in ericlitman/open-pstack main at c3a177d: Claude terminal event arrays and Grok API-key preflight (ericlitman#58). Grok preflight keeps the local Kimi effort and Cursor listed-model checks beside upstream's whole-token model match. Version 1.5.1-local.8. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary
Fixes #58 by supporting current Claude JSON event-array results and Grok API-key authentication without weakening model availability checks.
type: "result"event, reject terminal errors before accepting text, and preserve legacy object parsing plus model/session/usage/cost metadata.You are using XAI_API_KEY.authentication only with bounded exact-model evidence and no explicit authentication or unsupported-model failure. Rejected models exit 69 without launching; authentication failures retain the bounded retry and exit 77.Dependencies and scope
Merged
factory/issue-124ate257bad2c7567bd5108c5c654ac27dc920efa5ccintofactory/issue-58, without rebase or force-push. This includes #126 and #129. Do not land this PR before both dependencies. They remain open at publication; after they land, merge updatedorigin/maininto this branch and repeat the affected gates.Relative to that dependency base, implementation changes are limited to
runner/parse-output.ts,runner/parse-output.test.ts,runner/run.ts, andrunner/run.test.ts. No setup-contract or stdin transport changes.#94 stdin: not reproducible on available hosts (Bun 1.4.0, macOS, Codex sandboxes); left open
The approved revision-11 verdict removes the proposed stdin transport phase.
runProcessis unchanged, and this PR does not claim to fix #94.Test plan and actual results
Run with Bun 1.4.0 against pushed candidate
277029a694c0077532400e97f1ae7e1293c22b8c. Automated gates completed before opening this PR.bun test runner/parse-output.test.ts runner/run.test.tson merged prerequisite basebun test runner/parse-output.test.tsbun test runner/run.test.ts runner/parse-output.test.tsbun run testbun run typecheckbun -e 'for (const path of [".claude-plugin/marketplace.json", ".agents/plugins/marketplace.json", "plugins/pstack/.claude-plugin/plugin.json", "plugins/pstack/.codex-plugin/plugin.json"]) JSON.parse(await Bun.file(path).text())'PSTACK_STATIC_ONLY=1 bash tests/skill-collision-repro.shgit diff --checkand dependency-base range checkRetained logs:
/tmp/open-pstack-issue-58/{pre-fix-tests,claude-tests,focused-tests,full-tests,typecheck,manifests,static-collision}.login the build sandbox. No live credentials were used by repository tests.Remaining acceptance evidence — after review
Per approval, live verification is deferred, not claimed complete:
XAI_API_KEY; blocked unless the operator provides a key through the runtime environment, never the repository.Pushed phase checkpoints
9a2dfd0f9ac4f40ec44a59b63d31aec484ba0a04.20af5b22e359a622b60b2702521b6e0acf3b7d96.277029a694c0077532400e97f1ae7e1293c22b8c.Findings
None