feat(persona-kit): suppress codex 'are you sure?' via single bypass flag#86
Merged
Conversation
…ttings The two-flag form (--sandbox danger-full-access + --ask-for-approval never) still trips codex's interactive "are you sure?" startup confirmation. The combined --dangerously-bypass-approvals-and-sandbox flag suppresses it. Expose it as a first-class HarnessSettings field, mutually exclusive with sandboxMode / approvalPolicy / workspaceWriteNetworkAccess at parse time so personas cannot half-set both shapes. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds Codex Bypass Approvals and Sandbox Option
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🟡 Warning messages for claude/opencode omit newly added dangerouslyBypassApprovalsAndSandbox setting
The hasCodexLaunchSettings() function at packages/persona-kit/src/interactive-spec.ts:83-92 now detects dangerouslyBypassApprovalsAndSandbox, but the warning messages emitted by the claude (line 233) and opencode (line 303) branches still enumerate only the original four settings: sandboxMode, approvalPolicy, workspaceWriteNetworkAccess, and webSearch. If a persona sets only dangerouslyBypassApprovalsAndSandbox: true with a non-codex harness, the trigger fires but the warning names settings the user never set, making it confusing rather than helpful.
(Refers to line 233)
Was this helpful? React with 👍 or 👎 to provide feedback.
Contributor
There was a problem hiding this comment.
🟡 Warning message for opencode omit newly added dangerouslyBypassApprovalsAndSandbox setting
Same issue as the claude branch: the opencode warning at line 303 lists the four original codex-only settings but not the newly added dangerouslyBypassApprovalsAndSandbox. This triggers when hasCodexLaunchSettings() returns true due to the new field, producing a misleading warning.
(Refers to line 303)
Was this helpful? React with 👍 or 👎 to provide feedback.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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)
packages/persona-kit/src/interactive-spec.ts (1)
83-91:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCodex-only warning logic misses explicit bypass-field presence and warning text is incomplete.
hasCodexLaunchSettingsonly treats the new field as set when truthy, so an explicitly providedfalsewon’t trigger claude/opencode warnings. Also, the warning message should name this field too.Proposed fix
function hasCodexLaunchSettings(settings: HarnessSettings | undefined): boolean { if (!settings) return false; return Boolean( settings.sandboxMode || settings.approvalPolicy || settings.workspaceWriteNetworkAccess !== undefined || settings.webSearch || - settings.dangerouslyBypassApprovalsAndSandbox + settings.dangerouslyBypassApprovalsAndSandbox !== undefined ); }- 'persona declares codex-only harnessSettings but the claude harness ignores sandboxMode, approvalPolicy, workspaceWriteNetworkAccess, and webSearch.' + 'persona declares codex-only harnessSettings but the claude harness ignores sandboxMode, approvalPolicy, workspaceWriteNetworkAccess, webSearch, and dangerouslyBypassApprovalsAndSandbox.'- 'persona declares codex-only harnessSettings but the opencode harness ignores sandboxMode, approvalPolicy, workspaceWriteNetworkAccess, and webSearch.' + 'persona declares codex-only harnessSettings but the opencode harness ignores sandboxMode, approvalPolicy, workspaceWriteNetworkAccess, webSearch, and dangerouslyBypassApprovalsAndSandbox.'Also applies to: 232-234, 302-304
🤖 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 `@packages/persona-kit/src/interactive-spec.ts` around lines 83 - 91, hasCodexLaunchSettings currently treats dangerouslyBypassApprovalsAndSandbox only when truthy so an explicit false is ignored; update the predicate in hasCodexLaunchSettings to treat dangerouslyBypassApprovalsAndSandbox as "present" when !== undefined (same approach used for workspaceWriteNetworkAccess) and update the corresponding warning text locations (the other checks noted around the second and third occurrences) to explicitly mention "dangerouslyBypassApprovalsAndSandbox" in their messages so callers see that this field (whether true or false) was provided.
🧹 Nitpick comments (1)
packages/persona-kit/src/parse.test.ts (1)
198-224: ⚡ Quick winAdd a regression test for explicit
falsewith conflicting legacy fields.Given the exclusivity contract is shape-based, it’s worth locking this with a
dangerouslyBypassApprovalsAndSandbox: falseconflict test too.Suggested test addition
+test('parseHarnessSettings rejects dangerouslyBypassApprovalsAndSandbox:false with conflicting fields', () => { + assert.throws( + () => + parseHarnessSettings( + { + reasoning: 'high', + timeoutSeconds: 60, + dangerouslyBypassApprovalsAndSandbox: false, + sandboxMode: 'workspace-write' + }, + 'rt' + ), + /mutually exclusive with: .*sandboxMode/ + ); +});🤖 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 `@packages/persona-kit/src/parse.test.ts` around lines 198 - 224, Add a regression test to ensure parseHarnessSettings treats an explicitly-present dangerouslyBypassApprovalsAndSandbox: false as conflicting with legacy fields (shape-based exclusivity). Mirror the existing test 'parseHarnessSettings rejects dangerouslyBypassApprovalsAndSandbox with conflicting fields' but set dangerouslyBypassApprovalsAndSandbox to false and assert that parseHarnessSettings(overlay, 'rt') throws with a RegExp containing the conflicting field (use the same conflict loop over 'sandboxMode', 'approvalPolicy', 'workspaceWriteNetworkAccess' and the same error pattern). This keeps the check tied to the parseHarnessSettings behavior and covers the explicit-false case.
🤖 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 `@packages/persona-kit/src/parse.ts`:
- Around line 136-150: The code currently only checks for conflicts with
sandboxMode/approvalPolicy/workspaceWriteNetworkAccess when
dangerouslyBypassApprovalsAndSandbox is true; change it so that as soon as
dangerouslyBypassApprovalsAndSandbox is present (after the type check) you
validate mutual exclusion regardless of its boolean value: inside the existing
block that handles dangerouslyBypassApprovalsAndSandbox !== undefined (and after
verifying it's a boolean), collect conflicts into the conflicts array using
sandboxMode, approvalPolicy, and workspaceWriteNetworkAccess and throw the same
Error(`${context}.dangerouslyBypassApprovalsAndSandbox is mutually exclusive
with: ...`) if conflicts.length > 0; keep the type check and no other behavior
changes.
---
Outside diff comments:
In `@packages/persona-kit/src/interactive-spec.ts`:
- Around line 83-91: hasCodexLaunchSettings currently treats
dangerouslyBypassApprovalsAndSandbox only when truthy so an explicit false is
ignored; update the predicate in hasCodexLaunchSettings to treat
dangerouslyBypassApprovalsAndSandbox as "present" when !== undefined (same
approach used for workspaceWriteNetworkAccess) and update the corresponding
warning text locations (the other checks noted around the second and third
occurrences) to explicitly mention "dangerouslyBypassApprovalsAndSandbox" in
their messages so callers see that this field (whether true or false) was
provided.
---
Nitpick comments:
In `@packages/persona-kit/src/parse.test.ts`:
- Around line 198-224: Add a regression test to ensure parseHarnessSettings
treats an explicitly-present dangerouslyBypassApprovalsAndSandbox: false as
conflicting with legacy fields (shape-based exclusivity). Mirror the existing
test 'parseHarnessSettings rejects dangerouslyBypassApprovalsAndSandbox with
conflicting fields' but set dangerouslyBypassApprovalsAndSandbox to false and
assert that parseHarnessSettings(overlay, 'rt') throws with a RegExp containing
the conflicting field (use the same conflict loop over 'sandboxMode',
'approvalPolicy', 'workspaceWriteNetworkAccess' and the same error pattern).
This keeps the check tied to the parseHarnessSettings behavior and covers the
explicit-false case.
🪄 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: 68c8ae76-48dc-49ba-aa51-e54ce4b69230
📒 Files selected for processing (5)
packages/persona-kit/src/interactive-spec.test.tspackages/persona-kit/src/interactive-spec.tspackages/persona-kit/src/parse.test.tspackages/persona-kit/src/parse.tspackages/persona-kit/src/types.ts
Address feedback on #86: - hasCodexLaunchSettings now treats an explicit `false` as "field present" via !== undefined, matching workspaceWriteNetworkAccess's shape semantics. - Claude and opencode warning enumerations name the new field so the message lines up with what the predicate actually detects. - parseHarnessSettings enforces mutual exclusion whenever the field is present, not only when true — co-declaring the two-flag fields with an explicit `false` was still a contradictory shape. - New regression test covers the explicit-false conflict path. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
willwashburn
approved these changes
May 11, 2026
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/persona-kit/src/parse.test.ts (1)
186-196: ⚡ Quick winAdd an explicit happy-path assertion for
dangerouslyBypassApprovalsAndSandbox: false.Given the recent semantic change around field presence, it’s worth locking in that
falsealone parses and is preserved in output.Proposed test update
-test('parseHarnessSettings accepts dangerouslyBypassApprovalsAndSandbox alone', () => { - const ok = parseHarnessSettings( - { - reasoning: 'high', - timeoutSeconds: 60, - dangerouslyBypassApprovalsAndSandbox: true - }, - 'rt' - ); - assert.equal(ok.dangerouslyBypassApprovalsAndSandbox, true); -}); +test('parseHarnessSettings accepts dangerouslyBypassApprovalsAndSandbox alone', () => { + for (const value of [true, false]) { + const ok = parseHarnessSettings( + { + reasoning: 'high', + timeoutSeconds: 60, + dangerouslyBypassApprovalsAndSandbox: value + }, + 'rt' + ); + assert.equal(ok.dangerouslyBypassApprovalsAndSandbox, value); + } +});🤖 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 `@packages/persona-kit/src/parse.test.ts` around lines 186 - 196, Add a second happy-path unit test to ensure parseHarnessSettings preserves an explicit false value for dangerouslyBypassApprovalsAndSandbox: call parseHarnessSettings with dangerouslyBypassApprovalsAndSandbox: false (same other fields as the existing test) and assert that the returned object's dangerouslyBypassApprovalsAndSandbox === false; use a clear test name like "parseHarnessSettings accepts dangerouslyBypassApprovalsAndSandbox false" and reference the same parseHarnessSettings helper used in the current test so the change validates the new semantic that explicit false is preserved.
🤖 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.
Nitpick comments:
In `@packages/persona-kit/src/parse.test.ts`:
- Around line 186-196: Add a second happy-path unit test to ensure
parseHarnessSettings preserves an explicit false value for
dangerouslyBypassApprovalsAndSandbox: call parseHarnessSettings with
dangerouslyBypassApprovalsAndSandbox: false (same other fields as the existing
test) and assert that the returned object's dangerouslyBypassApprovalsAndSandbox
=== false; use a clear test name like "parseHarnessSettings accepts
dangerouslyBypassApprovalsAndSandbox false" and reference the same
parseHarnessSettings helper used in the current test so the change validates the
new semantic that explicit false is preserved.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ab54203d-e51d-442d-9444-fbabfc333254
📒 Files selected for processing (3)
packages/persona-kit/src/interactive-spec.tspackages/persona-kit/src/parse.test.tspackages/persona-kit/src/parse.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/persona-kit/src/interactive-spec.ts
Lock in that `dangerouslyBypassApprovalsAndSandbox: false` parses and is preserved on output, matching the field-presence semantics introduced in 9fb0eab. Addresses PR #86 review feedback. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
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
2 participants
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.
Summary
dangerouslyBypassApprovalsAndSandbox?: booleantoHarnessSettings. When set, the codex spec emits the single--dangerously-bypass-approvals-and-sandboxflag instead of the--sandbox danger-full-access+--ask-for-approval neverpair. The two-flag form still trips codex's interactive "are you sure?" startup prompt; the combined flag suppresses it.sandboxMode/approvalPolicy/workspaceWriteNetworkAccessso personas can't half-set both shapes.hasCodexLaunchSettingsupdated so claude/opencode runtimes still warn when a persona carries the new field.Persona shape:
Test plan
pnpm -r typecheckpnpm -r test(persona-kit + cli + workload-router + agentworkforce all green)🤖 Generated with Claude Code