fix(typescript): bound kill switch callbacks with a timeout

[Amrik Singh Khalsa (amriksingh0786)]

Summary

KillSwitch.kill() awaited termination and compensation callbacks in an unbounded loop, so a callback that never resolved blocked the kill path at exactly the moment it was needed. This races each callback against a configurable timeout and abandons it if it overruns.

Closes #3741.

Problem

Two defects in the same loop (src/kill-switch.ts:54-60).

A hung callback blocks forever. This is what #3741 reports. The Python port bounds this at DEFAULT_CALLBACK_TIMEOUT_SECONDS = 5.0 and explains why in a source comment: "A slow or hung callback must not freeze the kill flow, the whole point of a kill switch is responsiveness." The TypeScript port had no bound.

A throwing callback loses the record of the kill. Not in the issue. There was no try either, so a rejection propagated out of kill() and this.history.push(result) at :73 never ran. Measured against the current method:

kill() THREW:             handler blew up
history entries recorded: 0
still hanging after 300ms: true

Both are the same three lines and the same fix, so they are handled together here.

Changes

File	What changed
src/types.ts	callbackTimeoutMs?: number on KillSwitchConfig, documented as the per-callback budget
src/kill-switch.ts	New DEFAULT_CALLBACK_TIMEOUT_MS = 5000; both loops now go through a private runBounded that races the callback against a timer and catches rejection
tests/kill-switch.test.ts	Six cases: hung handler, hung compensation, later handlers still run after one hangs, sync throw, async reject, and a callback that completes inside the budget

A hung callback cannot be cancelled in JavaScript, so it is abandoned and left pending. Python abandons a daemon thread for the same reason, so the semantics match.

Two things worth review attention

callbacksExecuted semantics changed. It was handlers.length, which counted callbacks that were registered. It now counts callbacks that completed cleanly, so a hung or failed callback is visible in the result. Without this the timeout would be silent, which seemed worse than the hang. Existing assertions still pass, but this is a deliberate behavior change rather than an accident.

kill() no longer propagates callback errors. It records the kill and reports the callback as not executed, matching Python. Also a behavior change, and easy to split out if you would rather.

Relationship to #3750

#3750 adds terminated to KillSwitchResult and sets it to handlers.length > 0. This PR deliberately does not touch that field, to stay off its toes.

The two compose in either merge order. If this lands first, terminated can be derived from the per-callback outcome runBounded already returns rather than from whether a handler was registered, which is what the Python field means (kill_switch.py:201-235: True if the callback completed cleanly within the timeout, False if it timed out or raised). If #3750 lands first, I will rebase and wire terminated to the same outcome. Happy to do that as a follow-up or fold it in here, whichever you prefer.

Testing

npm run build   # tsc, clean
npm run lint    # eslint, clean
npm test        # 580 passed, 38 suites

The 8 kill-switch tests pass, including the 6 new ones. The suite emits a worker-teardown warning about leaking timers, but that reproduces on main with these changes stashed, and npx jest tests/kill-switch.test.ts --detectOpenHandles reports no open handles for this file. Timers here are cleared in a finally.

New tests use a 20ms callbackTimeoutMs rather than fake timers, so nothing waits five seconds.

[azure-pipelines]

Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

[github-actions]

Welcome to the Agent Governance Toolkit! Thanks for your first pull request.
Please ensure tests pass, code follows style (ruff check), and you have signed the CLA.
See our Contributing Guide.

[github-actions]

PR Review Summary

Check	Status	Details
🔍 Code Review	⚠️ Missing	No current-run comment
🛡️ Security Scan	⚠️ Missing	No current-run comment
🔄 Breaking Changes	⚠️ Missing	No current-run comment
📝 Docs Sync	⚠️ Missing	No current-run comment
🧪 Test Coverage	⚠️ Missing	No current-run comment

Verdict: ⚠️ AI review incomplete; ready for human review

AI review comments are untrusted advisory output. The summary reports workflow-generated completion status only, not model-authored pass/fail claims.

[Amrik Singh Khalsa (amriksingh0786)]

@microsoft-github-policy-service agree

[Amrik Singh Khalsa (amriksingh0786)]

@microsoft-github-policy-service agree