Conversation
The runaway breaker latched `disabled_by_runaway` in the io thread, and only a scan that saw the mode off cleared it. Turning the mode back on before the next pty output therefore left the pane suppressed for good. The tab flag, which the main thread clears in the same step that sends the limit-reaching approval, is already the breaker's off switch, so drop the latch and start the io state over on that feedback.
Codex and agy command dialogs were approved whatever they ran, which
contradicts FR-11 ("never approve dangerous operations"). Check the
displayed command against a fixed denylist and leave a matching dialog
for the user, as if the mode were off.
The command is read off the grid, so the check has to survive what the
grid loses: a row boundary may be a wrap between words or inside one,
so words meeting at a boundary are tried both apart and glued; words
are split both shell-quoted and plainly; and rules look for flags and
git subcommands anywhere after the program rather than at fixed
positions. Every ambiguity resolves toward denying, since a false hit
costs only a manual approval.
An agent that exits without its end-of-session report (crash, kill) left its last explicit status on the card, so the sidebar and Overview kept showing it as running or waiting. Clear the status when the foreground leaves recognized agents. A switch from one agent straight to another keeps it: process changes post only on change, so the arriving agent's first report can land before the poll that sees it. Route the clearing through the Overview label invalidation an AgentStatus delta already uses.
The Claude Code prompt anchors were written against synthetic fixtures, and no real Claude permission dialog has been captured to confirm them. Leave the follow-up at the table so it is not lost.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 967ba92d04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "push" if after.iter().any(|arg| forced(arg) || arg.starts_with('+')) => { | ||
| "git push --force" | ||
| } | ||
| "reset" if after.contains(&"--hard") => "git reset --hard", |
There was a problem hiding this comment.
Git accepts unique long-option abbreviations, but this exact comparison allows git reset --har HEAD to be auto-approved even though Git executes it as --hard. I confirmed this behavior with the installed Git, and git reset -h describes --hard as resetting HEAD, the index, and the working tree. Similar accepted abbreviations such as GNU rm --rec also bypass the exact --recursive check, so the denylist must account for abbreviations before treating these commands as safe.
Useful? React with 👍 / 👎.
| fn agent_left(previous: Option<&str>, current: Option<&str>) -> bool { | ||
| let is_agent = |process: Option<&str>| { | ||
| process.is_some_and(|process| classify_agent(process) != AgentKind::Generic) | ||
| }; | ||
| is_agent(previous) && !is_agent(current) |
There was a problem hiding this comment.
When a macOS foreground-process poll transiently returns None, or argv lookup for a node/bun-hosted agent temporarily falls back to the generic wrapper name, this predicate treats that inconclusive result as proof that the agent exited and clears its explicit status. The next successful poll changes the process back to the recognized agent but deliberately preserves the now-empty status, so a one-off polling failure can erase a pending Permission/Input report until the agent emits another hook update. Only a confirmed transition to a known non-agent, or a suitably debounced failure, should clear the status.
Useful? React with 👍 / 👎.
1 participant
Auto-approve answered Codex and agy command dialogs no matter what they ran, which contradicts FR-11 ("never approve dangerous operations"). This PR adds a denylist for those dialogs and fixes two state bugs found along the way.
Four commits, each green on its own:
disabled_by_runawayin the io thread, and only a scan that saw the mode off cleared it. Turning the mode back on before the next pty output therefore left the pane suppressed for good. The tab flag is already the breaker's off switch: the main thread clears it in the same step that sends the approval that reaches the limit. The latch is dropped, and the io state starts over on that feedback.auto_approve/denylist.rs). The displayed command is checked before a Codex or agy command dialog is answered. A hit leaves the dialog for the user, as if the mode were off. The command is read from the grid, so the check has to survive what the grid loses:rm -/rf). The two words meeting at a boundary are kept apart and are also tried glued, which also handles both kinds of wrap mixed in one command. The check stays linear in the paragraph.-C "/tmp/my project",| 'bash'andsh -c 'rm -rf …'are all seen.git clean target -f,git -C dir reset --hard). Programs are matched by basename (/bin/rm).AgentStatusdelta uses.#TODO(agent): UNVERIFIEDat the signature table. The Claude Code anchors were written against synthetic fixtures, and no real Claude permission dialog has been captured to confirm them.Reviewer notes:
| grep shis denied, and so is a destructive command that appears only inside a commit message. A false hit costs one manual approval, but unattended Codex runs will stop more often.⋯ (n lines hidden)) or wraps inside a script can still pass. The specs record this (docs/specs/auto-approve-mode.md,docs/specs/agent-workflow.md).NOA_AUTO_APPROVE_TRACE=1logs the rule that matched.Validation:
rm -/rf, andgit/reset <sha> -/-hard). Each has a harmless control on the same layout that must still fire. The mid-token grid test was confirmed to fail without the glued-word handling.cargo test -p noa-appandcargo fmt --all -- --checkpass on each commit.cargo clippy -p noa-app --all-targetsadds no warnings in the touched files.cargo test --workspacepasses everywhere exceptnoa-ptyandnoa-ipc, which need unsandboxed device and socket access. Those crates are untouched and were not run outside the sandbox.Not addressed:
apply_session_delta.PermissionRequesthook is a separate decision.