Greptile Summary

This PR fixes /bash chat sessions not inheriting the configured tools.exec policy, which caused them to fall back to approval-pending defaults even when operators had explicitly set fields like security=full and ask=off. The fix threads security, ask, pathPrepend, safeBins, safeBinTrustedDirs, and safeBinProfiles from the config's ExecToolConfig into the createExecTool() call, and a targeted regression test verifies all six fields are forwarded correctly.

• The core bug fix is correct and well-scoped — the named policy fields are now properly inherited.
• A new execPolicy local variable is introduced to avoid repeated optional-chain traversal, but the three pre-existing fields (timeoutSec, notifyOnExit, notifyOnExitEmptySuccess) still use the old params.cfg.tools?.exec?. form instead of the new variable (minor inconsistency).
• approvalRunningNoticeMs from ExecToolConfig/ExecToolDefaults is not forwarded, leaving a small gap in the policy inheritance (falls back to the 10 000 ms default silently).
• The regression test is well-structured, uses vi.hoisted + top-level dynamic import correctly for ESM mocking, and covers all the key assertions.

Confidence Score: 4/5

• Safe to merge; the fix is correct and narrowly scoped with no regressions introduced.
• The core policy-inheritance fix is correct — all six named fields are properly threaded into createExecTool. The only deductions are a minor style inconsistency (three fields bypass the new execPolicy variable) and one missing field (approvalRunningNoticeMs) that has low practical impact since /bash sessions with ask=off rarely hit the approval flow. The regression test is solid and covers the key scenario described in the PR.
• No files require special attention beyond the minor style notes in bash-command.ts.

This is a comment left during a code review.
Path: src/auto-reply/reply/bash-command.ts
Line: 337-339

Comment:
**Unused `execPolicy` variable for three fields**

`execPolicy` was introduced on line 336 to avoid repeated optional-chain lookups, but the three lines below still access `params.cfg.tools?.exec?.` directly instead of using it. This is a minor inconsistency worth cleaning up.

```suggestion
    const timeoutSec = execPolicy?.timeoutSec;
    const notifyOnExit = execPolicy?.notifyOnExit;
    const notifyOnExitEmptySuccess = execPolicy?.notifyOnExitEmptySuccess;
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/auto-reply/reply/bash-command.ts
Line: 349

Comment:
**`approvalRunningNoticeMs` not forwarded from exec policy**

`ExecToolConfig` has an `approvalRunningNoticeMs` field that controls how long before a running notice is emitted when an approval-backed exec runs for a long time. `ExecToolDefaults` accepts this field as well. Since this PR is about inheriting the full exec policy into `/bash` sessions, it is worth forwarding this field here alongside the other policy fields. If omitted, the tool silently falls back to its internal default (10 000 ms), which may not match what the operator configured.

Consider adding `approvalRunningNoticeMs: execPolicy?.approvalRunningNoticeMs` to the `createExecTool` call.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "fix: inherit exec po..."}

Codex review: keeping this open for maintainer follow-up; there is still a little grit to resolve.

Keep this PR open. Current main still does not implement the intended /bash exec policy inheritance, and the PR remains a relevant focused fix candidate, but it should be updated for full policy parity before merge.

Best possible solution:

Keep this PR open and update it, or apply an equivalent current-main patch, so /bash builds createExecTool defaults from the effective exec policy. The fix should include security, ask, PATH prepend, safe-bin settings/profiles, strict inline eval, approval running notice timing, timeout, and notification defaults, with focused regression coverage that also confirms the existing bash and elevated gates remain required.

What I checked:

• Current /bash caller still omits most exec policy defaults: On current main, handleBashChatCommand passes only scope/background/session/timeout/notification/elevated defaults into createExecTool; it does not forward configured tools.exec.security, ask, pathPrepend, safeBins, strictInlineEval, safeBinTrustedDirs, safeBinProfiles, or approvalRunningNoticeMs. (src/auto-reply/reply/bash-command.ts:350, 6d323ee73640)
• Exec defaults support the missing fields: ExecToolDefaults already exposes security, ask, pathPrepend, safeBins, strictInlineEval, safeBinTrustedDirs, safeBinProfiles, and approvalRunningNoticeMs, so the lower-level API can accept the policy fields that /bash currently drops. (src/agents/bash-tools.exec-types.ts:7, 6d323ee73640)
• Normal exec registration inherits broader policy: The main exec tool resolves global/per-agent exec config and forwards security, ask, PATH prepend, safe-bin policy, strict inline eval, approval running notice timing, timeout, and notification defaults to createExecTool, making /bash the outlier. (src/agents/pi-tools.ts:194, 6d323ee73640)
• createExecTool consumes the omitted defaults: createExecTool normalizes path prepending and safe-bin policy, resolves approvalRunningNoticeMs, and applies default security/ask, so the missing behavior is at the /bash caller boundary. (src/agents/bash-tools.exec.ts:1379, 6d323ee73640)
• Docs expose these as exec policy knobs: The exec docs list notifyOnExit, approvalRunningNoticeMs, security, ask, strictInlineEval, pathPrepend, safeBins, safeBinTrustedDirs, and safeBinProfiles as tools.exec configuration, while slash-command docs describe /bash as a host shell command gated by commands.bash and elevated allowlists. Public docs: docs/tools/exec.md. (docs/tools/exec.md:97, 6d323ee73640)
• No current /bash policy inheritance regression test: Current main has bash alias, gating, and stop tests, but no colocated test asserting that /bash forwards exec policy fields such as pathPrepend, safe-bin profiles, strict inline eval, or approval running notice timing. (src/auto-reply/reply, 6d323ee73640)

Remaining risk / open question:

• Merging the PR exactly as submitted would improve the bug but still leave policy drift for at least approvalRunningNoticeMs and strictInlineEval.
• The final fix must preserve the existing commands.bash and tools.elevated allowlist gates while honoring explicit operator exec policy such as security=full and ask=off.

Codex review notes: model gpt-5.5, reasoning high; reviewed against 6d323ee73640.

ProjectClownfish could not safely update this branch, so it opened a narrow replacement PR instead.

Replacement PR: #73982
Source PR: #50392
Contributor credit is preserved in the replacement PR body and changelog plan.