Repository navigation
Explain Codex sandbox restrictions before pstack setup probes [#124] - #129
Conversation
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}
| const message = errors.join("\n") || nullableString(result.error) | ||
| || nullableString(object(result.error)?.message) || nullableString(result.result) | ||
| || `grok reported ${result.subtype}`; | ||
| const cancelled = result.stop_reason === "cancelled" || result.stop_reason === "canceled" | ||
| || result.subtype === "cancelled" || result.subtype === "canceled" | ||
| || /^(?:User cancel(?:led|ed) the execution|PermissionCancelled)/i.test(message); | ||
| throw new ProviderResultError(message, cancelled ? "cancelled" : "child-failed", metadata); |
There was a problem hiding this comment.
When Grok sends an error event without a reason, parseGrok makes up a message such as grok reported api_error. The runner treats it as Grok's reason and skips the Codex sandbox hint. That leaves the operator with a generic error instead of the likely cause. Only a reason Grok actually sent should suppress the hint.
Knowledge Base Used: Poteto runner and orchestration
✅ Accepted riskAccepted by ericlitman: GH-124 (authority 48fa77b) 🟡 Medium · Restored Codex marker breaks later runner test 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.
🛑 1 blocking finding
🟡 Medium · Grok nonzero exits with non-terminal stdout become malformed-output
Check
| if (result.cancelledBy !== null || result.timedOut || providerFailure !== null | ||
| || (result.exitCode !== 0 && parseError === null)) { |
There was a problem hiding this comment.
🟡 Medium · Grok nonzero exits with non-terminal stdout become malformed-output
Lines 810-811 parse every Grok run that exits nonzero with non-empty stdout. If parsing fails with a generic error, (result.exitCode !== 0 && parseError === null) on line 821 is false. The failure branch is skipped, and line 862 rethrows the error as malformed-output/65. Trigger: Grok streams assistant or tool events, then crashes, is OOM-killed, or aborts on a sandbox or network error and exits 1 without a type:"result" event. Printing a plain-text error line to stdout has the same effect. - Before this diff, any nonzero exit went through unavailableStatus. That yields child-failed/70, or unauthenticated/unavailable-model. - Now the receipt says malformed-output with the message "grok result did not contain a terminal event" (or "non-JSON event"), and unavailableStatus is never consulted. - The new Codex-sandbox hint is also suppressed, because status is no longer child-failed. This contradicts the doc's "sandbox-startup refusal remains child-failed (exit 70)" whenever Grok has written any stdout first. The existing exit-1 test covers only a present-but-invalid result event, not the case where no terminal event exists.
Explain effective parent permissions before assigned external probes, preserve an explicit native-only choice, and guard actual configuration destinations independently. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Keep provider-reported reasons authoritative and preserve child-failed receipts while surfacing the confirmed network-disabled parent marker after execution. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
a9e8ac9 to
e257bad
Compare
|
Rebased only the two #124 commits onto issue #78 head New candidate: Acceptance checks rerun at this exact candidate, all exit 0: frozen-lockfile install; fake-child regression; focused Codex diagnostics; runner tests (64 pass); full Bun tests (196 pass); strict typecheck; static invariants; manifest JSON validation; strict Claude plugin validation; setup-prose review; Real Codex/Grok live acceptance remains pending operator execution after review; it is not claimed. The permission-information-unavailable branch remains prose-reviewed only. Exact-head live evidence must be collected for this new candidate. The #56 shared-prerequisites coordination remains unchanged. |
There was a problem hiding this comment.
🛑 1 blocking finding
🟡 Medium · Exit-1 malformed-terminal test fails when run inside a Codex network-disabled sandbox
Check
| 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 · Restored Codex marker breaks later runner test
When the suite starts with CODEX_SANDBOX_NETWORK_DISABLED=1, this nested cleanup restores that marker after the diagnostics block, while the file-level setup does not clear it. The later exit-1 malformed-terminal test then receives the newly appended sandbox hint but still expects exactly child exited with status 1, causing the repository-required bun run test command to fail in a Codex network-disabled sandbox. This reproduces as 202 passing and 1 failing test. Also reported for this defect: - Marker isolation scoped to new block; pre-existing exact-message test fails in Codex sandbox (plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:288-291): Carried repair v1:2937dc7d (run.test.ts ~623) still holds at this head. The runner now appends the sandbox hint whenever parent is codex and the inherited CODEX_SANDBOX_NETWORK_DISABLED is "1" (run.ts:574-575, 852). The save/clear of that variable exists only in this describe block's hooks (288-299). The file-level beforeEach (228-257) does not clear it, and neither does the issue78 block. That block's test keeps malformed terminal data distinct after child exit 1 runs a codex-parent Grok lane that exits 1 and asserts error.message is exactly "child exited with status 1". Scenario: a Codex agent in the default workspace-write session runs bun run test as AGENTS.md requires; that session exports the marker (provider-dispatch.md:76). The message becomes "child exited with status 1; likely cause: Codex parent sandbox has network disabled; ...", so the assertion fails. The suite exits nonzero there and passes elsewhere. run.test.ts changes are confined to lines 285-461, so nothing visible repairs ~623; a fix outside the visible diff cannot be ruled out, but the PR body lists it as not repaired. Any other pre-existing exact child-failed assertion with a Codex parent is exposed the same way. - Inherited Codex marker makes the Bun suite fail (plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:623): When bun run test runs inside Codex's default network-disabled sandbox, CODEX_SANDBOX_NETWORK_DISABLED=1 remains set outside the new diagnostics block. The later keeps malformed terminal data distinct after child exit 1 case runs a Grok lane with parent codex, so the runner correctly appends the new sandbox hint, but this line still exact-matches child exited with status 1. The repository-required test suite therefore fails in that environment despite the behavior being valid. - Exit-1 malformed-terminal test fails when run inside a Codex network-disabled sandbox (plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:623-624): The new test keeps malformed terminal data distinct after child exit 1 runs a Grok lane with parent codex. It asserts error.message is exactly "child exited with status 1". The neighbouring "Codex parent sandbox diagnostics" block saves and clears CODEX_SANDBOX_NETWORK_DISABLED. The issue78 terminal results block does not, and neither does the file-level beforeEach. Codex exports CODEX_SANDBOX_NETWORK_DISABLED=1 to commands in its default workspace-write session. Scenario: a Codex agent runs bun run test, as AGENTS.md requires before opening a PR. - executeLane computes parentNetworkDisabled = true. - The generic parse error on exit 1 yields plain child-failed. - The message becomes "child exited with status 1; likely cause: Codex parent sandbox has network disabled; see provider-dispatch.md#host-and-parent-prerequisites". - The toMatchObject assertion fails, so the suite exits nonzero in that environment and passes elsewhere. The hint and this exact-message assertion are both introduced by this diff.
Now at plugins/pstack/skills/poteto-mode/scripts/runner/run.test.ts:297
Also affects:
|
Unfret waiver for head e257bad. Ticket Finding v1:2937dc7d7daeff05c0951a9eb2a799ec236e1a83a01ec477effee72b6e3d86a0 ("Exit-1 malformed-terminal test fails when run inside a Codex network-disabled sandbox") is class fails-loudly.
|
Live test, Codex half (head e257bad)Result: the Codex half passes. The Claude half has not run yet, so I have not set the Installed version. Open Pstack 1.5.0 from ref
Not covered on the real surface:
Review state: Unfret shows Still pending: the Claude Code parent check. It needs a logged-in Claude Code CLI. |
Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com> # Conflicts: # plugins/pstack/skills/poteto-mode/references/provider-dispatch.md
|
Merged New tested and normally pushed head:
Local logs: |
Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
|
Merged latest Diff against main remains only #124 changes in its four files (204 insertions, 5 deletions). Main launcher updates are retained.
Logs: |
There was a problem hiding this comment.
🛑 1 blocking finding
🟡 Medium · Restored Codex marker breaks later runner test
Check
|
Waiver re-applied for head cb1cfbd (queue update of e257bad). Unfret waiver for head e257bad. Ticket Finding v1:2937dc7d7daeff05c0951a9eb2a799ec236e1a83a01ec477effee72b6e3d86a0 ("Exit-1 malformed-terminal test fails when run inside a Codex network-disabled sandbox") is class fails-loudly.
|
Live tests and live-gate (head cb1cfbd)This head is level with Codex
Claude Code
|
Merge Queue Status
This pull request spent 8 seconds in the queue, including 1 second running CI. Required conditions to merge
|
Build the combined setup fix on the approved ericlitman#126/ericlitman#129 dependency history. Co-Authored-By: mastra-platform[bot] <284800079+mastra-platform[bot]@users.noreply.github.com>
Summary
Make
/setup-pstackexplain blocked or unknown Codex-parent permissions before external CLI checks/probes, offer an explicitly confirmed native-only configuration or deliberate restart, and independently check both actual configuration destinations before writing. HonorCODEX_HOMErather than treating all outside-workspace writes as forbidden.After execution, enrich only plain
child-failedresults without a provider-reported reason when the runner's own Codex-parent environment hasCODEX_SANDBOX_NETWORK_DISABLED=1. Preserve child launch, schema, status/exit 70, requested models/efforts, and raw sidecars. No permission escalation, execution skip, fallback, or new runtime module.Fixes #124 (implementation; required exact-head live gates remain pending before merge).
Coordination
21e134ffd993ab4f1952382dd1c2717e51b3c3cf. Its provider-reported failure reason remains authoritative and suppresses this hint. Preserve Grok terminal failures and complete runner evidence [#78] #126 is still open; this PR currently includes its ancestor commits. Merge that dependency first and rebase if necessary; repeat exact-head evidence after any head change.Build handoff
factory/issue-124; configured base:main.a9e8ac95538b342c147f9aec22cf19623f5cbb56.5978003550(Codex CLI 0.160.0, restricted vs. full access); captured expected pre-fix fake-child failure without pushing a failing commit.fb3cb90b48821b0e7fac1c46dadfba61d9106428(setup and shared prerequisite documentation).a9e8ac95538b342c147f9aec22cf19623f5cbb56(post-failure diagnostics and boundary tests).Test plan and actual automated evidence
Executed on the pushed HEAD using Bun 1.4.0 and Claude Code 2.1.283. Script commands run from
plugins/pstack/skills/poteto-mode/scripts; other commands from repository root.bun test runner/run.test.ts -t 'names likely Codex sandbox cause after child failure'child exited with status 1; exit 1bun install --frozen-lockfilebun test runner/run.test.tsbun run testbun run typecheckPSTACK_STATIC_ONLY=1 bash tests/skill-collision-repro.shbun -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())'claude plugin validate ./plugins/pstack --strictgit diff --checkFocused diagnostics suite: 13 tests cover parent-marker eligibility, scrubbed child environment with actual child execution, bounded evidence prefix, raw sidecars, provider-reason precedence, cancellation, successful children, missing/non-1 markers, Claude parents, unavailable-model, unauthenticated, and timed-out results. Initial focused test fixtures were corrected before the final passing run.
Raw command output and exit statuses are preserved locally under
.git/issue-124-evidence/:pre-fix-runner-regression.log,runner-regression.log,runner-tests.log,bun-tests.log,strict-typecheck.log,static-invariants.log,manifest-validation.log,plugin-validation.log,setup-prose-review.log, andpermission-evidence.json. These are untracked local artifacts, not shipped files. The interrupted typecheck stream was followed by log verification and a fresh successful acceptance run.Live acceptance: PENDING, not claimed
The operator's Phase 1 observation establishes the marker and required-path behavior, not candidate acceptance. Codex and Grok CLIs are absent in this build sandbox. Per the operator's request, run the matrix after review, with the exact candidate installed; do not merge until required evidence and live-gate status are posted.
workspace-write, network off, external assignments: explanation and two choices precede external CLI invocation; no configuration writes.CODEX_HOMEdestinations.Record installed version, candidate SHA, surface, action, permission evidence, tool invocation order, confirmed choices, and configuration outcomes without secrets. Repeat after rebases/head changes.
Assumptions and open questions
GITHUB-1345013588-100at.mergify.yml:queue_rules: queue-head invalidation does not cause the sandbox setup defect. Existing exact-head live gates still apply.Findings
None
Known observation (not repaired)
run.test.ts,issue78 terminal results: the exit-1 malformed-terminal test asserts an exact message. It fails when the suite runs inside a Codex network-disabled sandbox, because the new sandbox hint is appended. The fix is to clearCODEX_SANDBOX_NETWORK_DISABLEDin that block, as the neighbouring diagnostics block does. It is waived as fails-loudly at head e257bad and will be fixed with the next change to these files.