fix(approval): drop dead eager redaction unblocking CI on live-config

[exiao]

Problem

CI is red on live-config, blocking every open PR (#118, #119). The failure:

tests/tools/test_approval_plugin_hooks.py::TestSmartModeFiresHooks::
test_smart_observer_redaction_is_forced_when_config_disables_redaction[check_execute_code_guard-...]
assert force_values == [True, True]
E  assert [False, False, False, True, True] == [True, True]

Order-independent (fails in isolation), so it's a real base-branch bug, not flake. Introduced by the upstream merge (#117).

Root cause

check_execute_code_guard redacted three display copies eagerly at the top of the approval path:

display_command = _redact_for_approval(command)
display_code = _redact_for_approval(code)
display_description = _redact_for_approval(description)

_redact_for_approval calls redact_sensitive_text(text) with the default force=False. Those three unforced calls run before the smart-approval observer, which then makes its two force=True calls → [False,False,False,True,True].

The eager block is dead: all three display_* names are unconditionally reassigned lower down (display_command = redact_sensitive_text(command) …) before their first use, past the early-return gates. The sibling check_all_command_guards already redacts for display only after the smart branch, which is why only the execute-code parametrization failed.

Fix

Delete the dead eager-redact block (10 lines, including its duplicated comment; the identical comment survives on the real assignment block). Display/redaction behavior is unchanged: the copies are still produced identically at their real assignment site. The only observable effect is the redaction call sequence now matches the command-guard path.

Verification

• tests/tools/test_approval_plugin_hooks.py → 21 passed (was 1 failed).
• The removed vars are provably dead: no reference between the deleted assignments and their unconditional reassignment.
• Remaining reds in the broader approval suite (TestDetectDangerousRm rm-detection, prompt_toolkit missing-module, YAML-off) reproduce on unmodified live-config or are cross-test pollution — unrelated to this one-file diff.

Patch note: ~/.hermes/plans/hermes-patches/execute-code-guard-eager-redact.md

[gemini-code-assist]

Code Review

This pull request removes the redaction logic for display commands, code, and descriptions within the check_execute_code_guard function in tools/approval.py. I have no feedback to provide as there are no review comments.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[claude]

Claude finished @exiao's task in 1m 10s —— View job

---

Review: drop dead eager redaction in check_execute_code_guard

• Read the diff (git diff origin/live-config...HEAD)
• Verify the deleted block is truly dead (no use before reassignment)
• Check the sibling command-guard path for consistency
• Assess prompt-cache / role-alternation / footprint invariants
• Post review

Findings

The diff is correct and the dead-code claim holds. I traced every reference to the three display_* names inside check_execute_code_guard:

• The deleted eager block sat right after command = f"execute_code <<'PY'..." (approval.py:3602).
• Between the deletion point and the unconditional reassignment at approval.py:3653-3655 there is no read of display_command / display_code / display_description — only the is_approved early-return gate and the smart-mode block, neither of which touches them.
• The first actual use is at approval.py:3665 onward, all downstream of the reassignment.

So the removed assignments were provably dead — their values could never reach a consumer. Display/redaction output is produced entirely by the surviving redact_sensitive_text reassignments and is unchanged. The only observable effect is that three unforced redact_sensitive_text calls no longer run ahead of the smart-approval observer, so the forced-redaction call sequence now matches the check_all_command_guards path — which is exactly what the failing force_values == [True, True] assertion expects.

Invariant checks: No impact on prompt-cache stability, message-role alternation, or the tool footprint (no tool/env/config surface touched). Pure dead-code deletion in one function.

One note (non-blocking, not introduced here): the surviving copies use redact_sensitive_text directly, whereas the deleted block used _redact_for_approval (which additionally applies _APPROVAL_SECRET_RE). Since the eager copies never rendered, this changes nothing observable, and both the divergence and the reassignment lines predate this PR — out of scope here, just flagging in case tightening execute-code display redaction to match the sibling paths is ever wanted.

Test execution is blocked in this environment, so I relied on static tracing rather than re-running the suite; the reasoning above is conclusive on its own.

LGTM — 0 blocking, 0 minor.

[claude]

LGTM — verified the deleted eager-redact block in check_execute_code_guard is provably dead: the three display_* names have no read between the deletion point and their unconditional reassignment at approval.py:3653-3655, so removal is behavior-preserving for display output and only drops three unforced redact calls that ran ahead of the smart-approval observer, aligning the force sequence with check_all_command_guards. No prompt-cache, role-alternation, or footprint impact. 0 blocking.

[exiao]

Superseded by #124, which stacks this commit (bbfba66) on top of #122 so the live-config base goes green in one merge. Both failure classes pass together there (454 tests). Recommend merging #124 and closing this.

[exiao]

Superseded by #124, which combined #122 + #120's commit and merged into live-config (5ed8af1). Closing as redundant.