#19595: fix: elevatedDefault should default to off when tools.elevated.enabled is true

## Summary Describe the problem and fix in 2–5 bullets: - Problem: when `tools.elevated.enabled: true` is set and `agents.defaults.elevatedDefault` is not set, elevated mode implicitly resolved to `"on"`. - Why it matters: this silently routed `exec` calls into elevated approval flow and could lead to approval timeout behavior instead of normal command execution. - What changed: implicit elevated fallback was changed from `"on"` to `"off"`; explicit `elevatedDefault` values are still honored; regression tests were added for config->runtime and exec approval behavior. - What did NOT change (scope boundary): no changes to group-chat logic, non-exec tools, or other channel-specific behavior. ## Change Type (select all) - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Docs - [ ] Security hardening - [ ] Chore/infra ## Scope (select all touched areas) - [x] Gateway / orchestration - [x] Skills / tool execution - [ ] Auth / tokens - [ ] Memory / storage - [ ] Integrations - [ ] API / contracts - [ ] UI / DX - [ ] CI/CD / infra ## Linked Issue/PR - Closes #19574 - Related # ## User-visible / Behavior Changes - With `tools.elevated.enabled: true` and no explicit `agents.defaults.elevatedDefault`, runtime elevated default is now `off` (not `on`). - With explicit `agents.defaults.elevatedDefault: "on"`, behavior is unchanged and remains `on`. ## Security Impact (required) - New permissions/capabilities? (`Yes/No`): No - Secrets/tokens handling changed? (`Yes/No`): No - New/changed network calls? (`Yes/No`): No - Command/tool execution surface changed? (`Yes/No`): Yes - Data access scope changed? (`Yes/No`): No - If any `Yes`, explain risk + mitigation: - Risk: changed default execution path for `exec` when elevated is enabled. - Mitigation: change is limited to implicit default only; explicit `elevatedDefault` still takes precedence; tests cover implicit off + explicit on + approval-flow regression. ## Repro + Verification ### Environment - OS: macOS - Runtime/container: local Node test runner - Model/provider: N/A - Integration/channel (if any): auto-reply config + exec tool tests - Relevant config (redacted): - `tools.elevated.enabled: true` - `tools.elevated.allowFrom.whatsapp: ["+1004"]` - `agents.defaults.elevatedDefault` unset or set to `"on"` ### Steps 1. Configure `tools.elevated.enabled: true` and allowlist sender; leave `agents.defaults.elevatedDefault` unset. 2. Trigger reply flow and inspect `bashElevated.defaultLevel` passed to runtime. 3. Run exec without explicit `elevated` and verify approval flow is not triggered by default. ### Expected - Unset `elevatedDefault` resolves to `off`. - Explicit `elevatedDefault: "on"` is respected. - Exec does not enter elevated approval flow unless explicitly requested/configured. ### Actual - Verified with added tests; behavior matches expected. ## Evidence Attach at least one: - [x] Failing test/log before + passing after - [x] Trace/log snippets - [ ] Screenshot/recording - [ ] Perf numbers (if relevant) Evidence summary: - Added tests: - `src/auto-reply/reply.elevated-defaults.test.ts` - `src/agents/bash-tools.exec.elevated-default.test.ts` - Updated related e2e coverage: - `src/auto-reply/reply.directive.directive-behavior.ignores-inline-model-uses-default-model.e2e.test.ts` - `src/agents/bash-tools.exec.approval-id.e2e.test.ts` ## Human Verification (required) What you personally verified (not just CI), and how: - Verified scenarios: - Implicit elevated default resolves to `off` with `tools.elevated.enabled: true`. - Explicit `agents.defaults.elevatedDefault: "on"` remains `on`. - Exec with elevated enabled + default off does not request approval by default. - Edge cases checked: - Explicit override precedence over implicit fallback. - Directive persistence/change tracking paths still compare against `off` baseline. - What you did **not** verify: - Did not change or rework Slack threading behavior or other channel-specific logic. ## Compatibility / Migration - Backward compatible? (`Yes/No`): Yes - Config/env changes? (`Yes/No`): No - Migration needed? (`Yes/No`): No - If yes, exact upgrade steps: ## Failure Recovery (if this breaks) - How to disable/revert this change quickly: - Revert commit `47e6002d5`. - Or explicitly set `agents.defaults.elevatedDefault: "on"` if old behavior is required. - Files/config to restore: - `src/auto-reply/reply/get-reply-directives.ts` - `src/auto-reply/reply/directive-handling.persist.ts` - `src/auto-reply/reply/directive-handling.impl.ts` - Known bad symptoms reviewers should watch for: - Elevated unexpectedly showing as `on` when no default is configured. - Exec prompting for approval without explicit elevated intent. ## Risks and Mitigations - Risk: implicit default change affects users who were accidentally relying on elevated-by-default behavior. - Mitigation: explicit `agents.defaults.elevatedDefault: "on"` remains supported and unchanged. ## AI Usage Disclosure - This PR was implemented with AI assistance (Codex/GPT-5) for code edits and test scaffolding. - A human reviewed the diff, verified behavior locally, and validated test outcomes before submission. <!-- greptile_comment --> <h3>Greptile Summary</h3> Changed implicit `elevatedDefault` from `"on"` to `"off"` when `tools.elevated.enabled: true` is set without explicit `agents.defaults.elevatedDefault`. **Key changes:** - Modified fallback in three locations: `get-reply-directives.ts:356`, `directive-handling.persist.ts:73`, and `directive-handling.impl.ts:273` - Changed from conditional fallback `elevatedAllowed ? "on" : "off"` to unconditional `"off"` - Explicit `elevatedDefault` values are still honored - Added comprehensive regression tests covering config resolution and exec approval flow **Impact:** - Prevents silent routing into elevated approval flow for users with elevated enabled but no explicit default - More predictable behavior: elevated mode requires explicit opt-in via config or directive - Backward compatible: users who want `"on"` can explicitly set `agents.defaults.elevatedDefault: "on"` <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - The change is well-scoped, thoroughly tested, and addresses a clear bug. The fix changes only the implicit fallback behavior (3 lines) while preserving explicit configuration. Comprehensive test coverage includes both unit tests for config resolution and integration tests for exec approval flow. The change is backward compatible since users relying on the old behavior can explicitly set `elevatedDefault: "on"`. - No files require special attention _{Last reviewed commit: 47e6002} <!-- greptile_other_comments_section --> _{(5/5) You can turn off certain types of comments like style [here](https://app.greptile.com/review/github)!} <!-- /greptile_comment -->