SafetyOverride.deactivate() skips its audit event when the grant already lapsed

[chenmingwei23]

What

SafetyOverride.deactivate() returns early when the grant is already inactive, and the early return is above its SEL audit. So when an operator switches back to normal mode on a grant that had already lapsed, the action is recorded nowhere: no safety_override:deactivate event is emitted.

def deactivate(self, source: str) -> None:
    with self._lock:
        if not self._active:
            return          # <-- skips the audit below
        ...
    self._log_sel(operation="safety_override:deactivate", outcome="disabled", ...)

Lazy expiry makes this the common case rather than an edge one: it clears _active while leaving the rest of the grant's state in place, so any deactivate arriving after the TTL has elapsed takes the early return.

Why

The audit stream is the only durable record of who changed the auto-approval posture and when. An auditor reconstructing "was auto-approval on at 03:00, and who turned it off?" sees the activate event and then nothing — the operator's explicit decision is invisible, and it is indistinguishable from the grant simply having been left to lapse.

That distinction is not academic. It was load-bearing in #2443: a guard there assumed deactivate() always zeroes _expires_at, which the early return makes false, and the result was that an automatic lease could re-enable auto-approval an operator had just turned off. The guard is fixed (it now requires the grant to be live), but the underlying observation stands — this method's early return makes an operator action leave no trace at all.

Additional Context

• src/kiro_crew/safety_override.py — deactivate(). Compare deactivate_scope(), which has the same shape but only logs when something was actually removed; the difference there is that no state change happened, whereas here an operator DID make a decision.
• Same family as SafetyOverride.renew() extends a grant before auditing it, and swallows audit failure #2453 (renew() commits before auditing and swallows audit failure). Both are audit-completeness gaps on the same object; worth deciding together whether the object's contract is "log every decision" or "log every state change", because the two disagree exactly on this case.
• Suggested shape: emit the event whenever deactivate() is called with a grant that exists in any form (including lapsed), with the pre-call state in resources so a no-change deactivate is distinguishable from one that actually revoked a live grant. Genuinely absent grants (never activated) can stay silent.
• Test shape: activate, force lapse, call is_active() to trip lazy expiry, then deactivate() and assert a safety_override:deactivate SEL event was emitted. Inject the failure at the SEL sink rather than at _log_sel so the assertion cannot pass vacuously.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97]: Confirmed against main (10bba9f). SafetyOverride.deactivate() at src/kiro_crew/safety_override.py:414 takes if not self._active: return inside the lock, above its safety_override:deactivate SEL write, so an operator switching back to normal mode on a lapsed grant produces no audit event at all — and because lazy expiry clears _active while leaving the rest of the grant's state in place, that is the common path rather than an edge case. The fix is to resolve the object's contract in favour of "log every operator decision" for this method: emit the event whenever deactivate() is called against a grant that exists in any form, carrying the pre-call state (was_active / was_permanent / remaining) in resources so a no-change deactivate stays distinguishable from one that revoked a live grant, while a never-activated instance stays silent. deactivate_scope() keeps its "log every state change" shape, since no operator decision is lost there. Starting an automated fix — this issue is now claimed by bolichen97's instance.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97] — PR opened: #2483

deactivate() now emits its safety_override:deactivate SEL event for any grant that exists in any form (live or lapsed), keyed on the _expires_at 0.0 sentinel that lazy expiry does not clear. Pre-call state is recorded in resources; never-activated instances stay silent. The contract split (deactivate = log every decision, deactivate_scope = log every state change) is decided explicitly in the PR description. Driving CI to green now.

[bolichen97]

🤖 Kiro Crew Auto-Pipeline [operator: bolichen97] — PR #2483 is review-ready: 50/50 checks green on 32bdb228a45de8c273d8cdcee3618477340a4e2f, readiness:passed. GPT 5.6's round-1 blocker (stale _active overstating liveness for a lapsed-but-unpolled grant) was fixed by deriving was_active from permanence or an unexpired deadline, with a pinning test. Opus 4.8 and Design Review passed with no findings. Awaiting human merge.