Skip to content

feat(kill-switch): admin/kill A2A verb, CancelAll, accepting-gate - #439

Merged
initializ-mk merged 3 commits into
mainfrom
feat/agent-kill-switch
Sep 4, 2026
Merged

feat(kill-switch): admin/kill A2A verb, CancelAll, accepting-gate#439
initializ-mk merged 3 commits into
mainfrom
feat/agent-kill-switch

Conversation

@initializ-mk

Copy link
Copy Markdown
Contributor

Phase 1 (forge) of the agent kill switch — the runtime primitive the platform drives to disable an agent and kill all its active sessions/tasks. Sync A2A only in this PR; async (orchestrator) and the Claude SDK runtime follow in their own repos/PRs.

What this adds

  • forge-core/runtimeCancellationRegistry.CancelAll(reason) int + CancelReasonKillSwitch. CancelAll cancels every in-flight invocation at once (snapshot cancel-funcs under the lock, invoke them outside it — same contention profile as Cancel; each invocation's deferred release() pops its own entry as executeTask unwinds).
  • forge-cli/runtime — the kill gate + admin/kill verb. A killed atomic.Bool on the Runner:
    • admin/kill JSON-RPC handler → sets killed, calls CancelAll(kill_switch), returns {killed, cancelled}. Each cancelled invocation emits its own invocation_cancelled audit event with reason=kill_switch (distinct from a per-task operator cancel).
    • tasks/send + tasks/sendSubscribe refuse new work once killed (clear error, not a dropped socket).

How the platform uses it (context, not in this PR)

agent-builder's admin-RBAC POST /agents/{id}/kill calls admin/kill over the in-cluster A2A channel (graceful cancel + audit), then scales the Deployment to zero regardless of the result so no new transaction is admitted even if this call timed out. A mirrored killed flag on the record/registry stops the orchestrator re-dispatching and the console from offering run/URLs.

Auth

admin/kill is behind the server-wide AuthMiddleware (only authenticated callers reach any handler). The primary access control is agent-builder's admin-RBAC on the /kill endpoint. TODO(hardening): additionally restrict admin/kill to the platform/agent-runtime identity via the verified role claim (the Identity.Claims role key needs settling first).

Tests / checks

  • New unit tests: CancelAll signals every in-flight invocation + propagates kill_switch via context.Cause; empty/second-kill returns 0; kill_switch is a valid reason.
  • go build + go vet clean (forge-core, forge-cli); golangci-lint run ./runtime/... = 0 issues both modules; go test ./runtime/ passes both modules; gofmt clean.

Idempotency

A second kill re-signals an empty registry (returns 0) and leaves killed set — safe for the platform to call optimistically.

Phase 1 of the agent kill switch — the forge-side primitive the platform
(agent-builder) drives to disable an agent and kill its active work.

- forge-core/runtime: add CancelReasonKillSwitch and
  CancellationRegistry.CancelAll(reason), which signals every in-flight
  invocation at once (snapshot-under-lock, cancel-outside-lock; each
  invocation's own release() pops its entry as executeTask unwinds).
- forge-cli/runtime: a `killed` atomic gate on the Runner. New admin/kill
  JSON-RPC handler flips the gate and calls CancelAll — every cancelled
  invocation emits its own invocation_cancelled audit event with
  reason=kill_switch. tasks/send and tasks/sendSubscribe refuse new work
  once killed. admin/kill is behind the server-wide AuthMiddleware; the
  primary access control is agent-builder's admin-RBAC /kill endpoint,
  which calls this then scales the workload to zero regardless of outcome.

Idempotent: a second kill re-signals an empty registry (0). Tests cover
CancelAll signalling + reason propagation + the empty/idempotent path.

@initializ-mk initializ-mk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against the branch source. Strong core primitive, but one gap that undercuts the feature's central guarantee (Finding 1) and one audit gap (Finding 2) I would want addressed before merge. All 10 CI checks green.

What is solid (verified)

  • CancelAll is correct — snapshots cancel-funcs under the lock and invokes them outside it (matches Cancel's contention profile, avoids re-entrancy), and uses the identical cause type &cancelledByOrchestrator{Reason: reason} as Cancel (cancellation.go:197 vs 175). So CancellationReasonFromCause unwraps it and each invocation's invocation_cancelled event carries reason=kill_switch via the already-working path. Release-pops-own-entry is right; tests cover the count, per-ctx reason, and the idempotent empty path. kill_switch added to IsValid() and tested.

Finding 1 — should-fix: the kill gate misses the REST ingress paths (2 of 4 sites)

The gate guards only the JSON-RPC tasks/send (1640) and tasks/sendSubscribe (1688). But registerRESTHandlers (live, registered at runner.go:1590) exposes two more new-work entry points with no killed check:

  • POST /tasks/send (runner.go:2251)
  • POST /tasks/sendSubscribe (runner.go:2304)

I read both handler heads — they decode the body and admit work unconditionally. So a killed agent still accepts new work over REST. This is in-scope for "sync A2A": the JSON-RPC handler's own comment (1638) notes it "goes through the same wiring as REST POST /tasks/send." Scale-to-zero is a backstop, but the primitive's stated contract ("refuse new work, clear error") is silently violated on half the ingress surface, and any direct/test use of admin/kill without the k8s race leaves REST fully open. One-liner at the top of each REST handler, symmetric with the JSON-RPC ones. (tasks/get / tasks/cancel / /tasks/{id}/decisions correctly stay open — they act on existing work.)

Finding 2 — should-fix: the kill action itself is not in the audit stream

admin/kill records the actor only via r.logger.Info (ops log). It captures caller identity — good — but that never reaches the tamper-evident audit NDJSON. If the agent is idle (cancelled=0), the kill produces no audit event at all — no record that a destructive admin action happened or who did it. Emit an admin_kill audit event via EmitFromContext (caller / reason / cancelled), independent of whether any invocation was in flight.

Finding 3 — note (PR-acknowledged): authorization

admin/kill is behind only AuthMiddleware, so any authenticated caller can trip it — killing every peer's sessions and flipping the accepting gate (a DoS / privilege gap). The TODO(kill-switch hardening) + reliance on agent-builder RBAC on a different endpoint is acceptable for a Phase-1 primitive only if the A2A surface exposing admin/kill is platform-only / in-cluster, not end-user-facing. If end users reach the same authenticated A2A server, the TODO should land before this is relied upon. Worth confirming the topology assumption.

Minor

  • The killed-refusal returns ErrCodeInternal on both handlers — semantically it is a deliberate unavailability, not an internal error; a more specific code reads better. Cosmetic.

Nice, clean core — CancelAll and the cause/reason propagation are exactly right. Findings 1 and 2 are the ones I would resolve before this ships as a relied-upon control.

// JSON-RPC path goes through the same audit + accumulator wiring as
// REST POST /tasks/send. See issue #87 / FWS-3.
srv.RegisterHandler("tasks/send", func(ctx context.Context, id any, rawParams json.RawMessage) *a2a.JSONRPCResponse {
if r.killed.Load() {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finding 1 (should-fix): this gate is applied here and on tasks/sendSubscribe (1688), but the two REST mirrors — POST /tasks/send (runner.go:2251) and POST /tasks/sendSubscribe (2304), both live via registerRESTHandlers — have no r.killed.Load() check. I read both: they decode the body and admit work unconditionally. So a killed agent still accepts new work over REST, silently defeating the accepting-gate contract on half the sync-A2A ingress surface. Add the same guard at the top of each REST handler. (I could not inline-anchor on 2251/2304 themselves — they are unchanged lines, not in this diff — hence the note here.)

if idn := auth.IdentityFromContext(ctx); idn != nil {
caller = idn.Email
}
r.logger.Info("admin/kill", map[string]any{

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Finding 2 (should-fix): the kill action is recorded only on the ops logger, so it never lands in the tamper-evident audit NDJSON. You already capture caller here — good — but if the agent is idle (cancelled=0) the kill emits NO audit event at all (no per-task invocation_cancelled either), leaving a destructive admin action with no forensic trail and the actor absent from the signed chain. Emit an admin_kill audit event via EmitFromContext (caller / reason / cancelled) unconditionally, independent of in-flight count.

Review findings on #439:

- Finding 1 (should-fix): the killed gate guarded only the JSON-RPC
  tasks/send + tasks/sendSubscribe; the REST mirrors POST /tasks/send and
  POST /tasks/sendSubscribe admitted work unconditionally, leaving half the
  sync-A2A ingress open on a killed agent. Guard both REST handlers (503).
- Finding 2 (should-fix): admin/kill recorded the actor only on the ops
  logger. Emit a new admin_kill audit event via EmitFromContext
  UNCONDITIONALLY (caller / reason / cancelled) so a destructive admin
  action always lands in the tamper-evident chain, even when the agent was
  idle (cancelled=0) and no invocation_cancelled fires.
- Minor: the killed refusal returned ErrCodeInternal; add a server-defined
  ErrCodeUnavailable (-32000) for deliberate unavailability and use it on
  both JSON-RPC gates (REST uses HTTP 503).

New test drives admin/kill then asserts all four ingress paths refuse work
(JSON-RPC Unavailable + REST 503); the NDJSON confirms admin_kill emits with
a correct seq even when idle. build/vet/golangci-lint(0)/gofmt/test all green.
@initializ-mk

Copy link
Copy Markdown
Contributor Author

Addressed in 76a5894:

  • Finding 1 (REST ingress gap) — fixed. Added the killed guard to both POST /tasks/send (runner.go:2251) and POST /tasks/sendSubscribe (2304) → HTTP 503 when killed, symmetric with the JSON-RPC gates. New test TestRunner_KillSwitch_RefusesNewWorkOnEveryIngress trips admin/kill then asserts all four ingress paths refuse work (JSON-RPC Unavailable + REST 503). tasks/get/tasks/cancel stay open by design (they act on existing work).
  • Finding 2 (kill not audited) — fixed. New admin_kill audit event (forge-core/runtime/audit.go), emitted via EmitFromContext unconditionally — the test NDJSON shows it landing with seq:2 and fields{caller,reason,cancelled} even on an idle agent (cancelled:0), so a destructive admin action always has a signed-chain record with the actor.
  • Minor (error code) — done. Added server-defined ErrCodeUnavailable = -32000 and used it on both JSON-RPC gates (REST uses 503).

On Finding 3 (authz) — you're right to push here, and I want to flag it honestly rather than wave it through: a deployed agent's A2A Service is ClusterIP, but agents can be exposed via public ingress (our own demo agent had a *.fndlint.net URL). Where that's true, admin/kill behind only AuthMiddleware means any authenticated caller can trip it — so the TODO(hardening) (restrict to the platform/agent-runtime role) should land before this is a relied-upon control on any publicly-exposed agent, not just be deferred. The blocker is that verifier.Identity exposes only Claims (no typed role), so I need the agent-runtime role-claim key settled first — I'll pick that up as a fast-follow. For now the primary gate remains agent-builder's admin-RBAC /kill endpoint. Want me to fold the role-check into this PR instead of a follow-up?

All checks green (build/vet/golangci-lint 0/gofmt/test, both modules).

@initializ-mk initializ-mk left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix verified — all three findings addressed cleanly. ✅ Traced 76a5894 against the branch source; all 9 CI checks green (incl. the Test job carrying the new integration test).

Finding 1 (REST gating) — resolved. Both POST /tasks/send (2261) and POST /tasks/sendSubscribe (2318) now check r.killed.Load() at the very top, before body decode, returning 503 Service Unavailable — symmetric with the JSON-RPC gate. All four sync-A2A ingress sites are now covered. The new TestRunner_KillSwitch_RefusesNewWorkOnEveryIngress is exactly the contract test this needed: it stands up a real Runner, trips admin/kill, then asserts JSON-RPC tasks/send → Unavailable and both REST endpoints → 503. (It exercises 3 of the 4 sites; the untested one, JSON-RPC sendSubscribe, shares the identical guard at 1688 — so all four are covered in code.)

Finding 2 (audit the kill) — resolved. New AuditAdminKill = "admin_kill" is emitted unconditionally via EmitFromContext(ctx, …) with caller / reason / cancelled, before the ops-log line. I confirmed auditLogger is a parameter of registerHandlers (runner.go:1633), so this is the real logger and the event rides the tamper-evident chain with correlation/tenancy/seq from ctx. An idle-agent kill (cancelled==0) now leaves a forensic record with the actor — the exact gap that was open.

Minor (error code) — resolved. ErrCodeUnavailable (-32000, in the JSON-RPC server-defined range) replaces the ErrCodeInternal misuse on the JSON-RPC refusals; REST uses HTTP 503. Semantically a deliberate refusal now, not an internal fault.

Finding 3 (authorization) remains the acknowledged TODO(kill-switch hardening) — appropriate to defer for this Phase-1 primitive, with the caveat noted earlier: land it before admin/kill is relied upon as a control if the A2A surface is end-user-reachable.

No new issues. Core mechanics (CancelAll cause/reason propagation, release-pops-entry, idempotency) were already correct and are unchanged. LGTM for the Phase-1 scope — nice, responsive iteration.

Match the <entity>_<verb-past> audit-event naming convention (cf.
agent_killed, egress_blocked); admin_kill was present-tense. Renames the
event string and the AuditAdminKilled constant.
@initializ-mk
initializ-mk merged commit 468f906 into main Sep 4, 2026
9 checks passed
initializ-mk added a commit that referenced this pull request Sep 4, 2026
Review findings on #439:

- Finding 1 (should-fix): the killed gate guarded only the JSON-RPC
  tasks/send + tasks/sendSubscribe; the REST mirrors POST /tasks/send and
  POST /tasks/sendSubscribe admitted work unconditionally, leaving half the
  sync-A2A ingress open on a killed agent. Guard both REST handlers (503).
- Finding 2 (should-fix): admin/kill recorded the actor only on the ops
  logger. Emit a new admin_kill audit event via EmitFromContext
  UNCONDITIONALLY (caller / reason / cancelled) so a destructive admin
  action always lands in the tamper-evident chain, even when the agent was
  idle (cancelled=0) and no invocation_cancelled fires.
- Minor: the killed refusal returned ErrCodeInternal; add a server-defined
  ErrCodeUnavailable (-32000) for deliberate unavailability and use it on
  both JSON-RPC gates (REST uses HTTP 503).

New test drives admin/kill then asserts all four ingress paths refuse work
(JSON-RPC Unavailable + REST 503); the NDJSON confirms admin_kill emits with
a correct seq even when idle. build/vet/golangci-lint(0)/gofmt/test all green.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant