Conversation
Treat nonzero pre-bash hook exits as hard guard failures so Codex app-server mode cannot implicitly approve commands when the wrapper loses structured hook output. Add a regression test that exercises a crashing pre-bash hook and verifies the approval request is declined. Constraint: Approval hooks must fail closed when guard execution degrades Rejected: Preserve pass fallback on malformed hook output | it silently disables the guard boundary in app-server mode Confidence: high Scope-risk: narrow Reversibility: clean Directive: Keep pre-bash approval handling fail-closed for hook crashes and unknown decisions Tested: cargo check --manifest-path "vg-helper/Cargo.toml"; cargo test --manifest-path "vg-helper/Cargo.toml"; bash tests/test_codex_runtime.sh Not-tested: Full repository shell test matrix Signed-off-by: majiayu000 <1835304752@qq.com>
![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: 0cfcc81a00
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if result.decision not in {"pass", "allow"}: | ||
| write_to_server({"id": msg_id, "result": {"decision": "decline"}}) |
There was a problem hiding this comment.
pre-bash-guard.sh has explicit non-blocking paths that emit {"decision":"warn"} and exit successfully (for example, the non-standard .md warning), but this condition now declines every decision other than pass/allow. In the app-server wrapper, that changes warning-only outcomes into hard denials, so commands that should still go to upstream approval are rejected. I verified this behavior against hooks/pre-bash-guard.sh where warn is treated as advisory rather than blocking.
Useful? React with 👍 / 👎.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hook Latency (P95)
Details
| Benchmark suite | Current: bb945e7 | Previous: 0bb421d | Ratio |
|---|---|---|---|
pre-edit-guard (P95) |
193 ms |
195 ms |
0.99 |
pre-write-guard (P95) |
221 ms |
219 ms |
1.01 |
pre-bash-guard (P95) |
266 ms |
274 ms |
0.97 |
post-edit-guard (100) (P95) |
308 ms |
299 ms |
1.03 |
post-write-guard (100) (P95) |
213 ms |
208 ms |
1.02 |
post-edit-guard (5000) (P95) |
319 ms |
316 ms |
1.01 |
post-write-guard (5000) (P95) |
221 ms |
217 ms |
1.02 |
stop-guard (5000) (P95) |
137 ms |
130 ms |
1.05 |
learn-evaluator (5000) (P95) |
142 ms |
136 ms |
1.04 |
This comment was automatically generated by workflow using github-action-benchmark.
Keep the app-server wrapper aligned with the pre-bash hook contract so warn-only decisions still pass through, and make hook launch errors decline approvals instead of silently failing open. Add runtime regressions for both paths so future wrapper changes keep the same boundary behavior. Constraint: Codex app-server approvals must honor the existing pre-bash hook decision contract Rejected: Treat every non-pass decision as decline | it regresses the documented warn-only path for non-standard .md creation Confidence: high Scope-risk: narrow Reversibility: clean Directive: When changing app-server approval interception, keep explicit tests for warn passthrough and hook launch failures Tested: bash tests/test_codex_runtime.sh; bash scripts/ci/validate-hooks.sh; VIBEGUARD_TEST_UPDATED_INPUT=1 bash tests/test_hooks.sh; python3 -m py_compile scripts/codex/app_server_wrapper.py Not-tested: Full CI matrix on GitHub Actions Signed-off-by: majiayu000 <1835304752@qq.com>
Surface pre-bash warn reasons through app-server warning notifications and cover malformed zero-exit hook decisions so approval warnings stay visible while unexpected hook output still fails closed. Signed-off-by: majiayu000 <1835304752@qq.com>
1 participant
Closes #108
Summary
pre-bash-guard.shexits nonzero in the Codex app-server wrapperTest plan
cargo check --manifest-path "vg-helper/Cargo.toml"cargo test --manifest-path "vg-helper/Cargo.toml"bash tests/test_codex_runtime.sh