Follow-up items from the review of #545 (observable blocker signal + bounded self-remediation, closes #251). None of these blocked the merge; they are hardening/tidy-up items surfaced during review. Filed so they are not lost.
1. Scope host-capturing egress detection to network-capable tools
post_tool_use_hook runs detect_egress_denial on every tool's output (agent/src/hooks.py, the egress_detected, host = detect_egress_denial(tool_response) call). A non-network tool — Read, Grep — that surfaces a file or log containing a string like Could not resolve host: foo.com will fire the observability event and, because a host was captured, latch the canonical terminal reason via _record_blocker_reason.
On a successful task this is harmless (the success path returns before the latch is read). The sharper edge: if the task later errors with no more specific reason, a stale/unrelated host can be promoted as the terminal cause, sending an operator to the DNS Firewall for nothing.
Suggested fix: gate host-capturing detection (or at least the terminal latch) to Bash/network tool calls, so file-content matches never poison the carry-path. The author already, correctly, accepts false positives for the live-stream event; this is only about the terminal reason.
2. Document / harden the process-global blocker latch for a future multi-task-per-process model
_LAST_BLOCKER_REASON in agent/src/hooks.py is a process-scalar, reset in pipeline.run_task (and relied on via the workflow-runner entry). Correct today under the one-task-per-Firecracker-MicroVM model (COMPUTE.md), and the author explicitly flagged the thread-safety nit.
Suggested fix: add a one-line note at the latch definition that correctness depends on one-task-per-process, and file/track the task-id-keyed refactor as the trigger if the execution model ever changes. No code change needed while the invariant holds.
3. auth_failure non-ARN remedy branch has no live producer
auth_failure and unknown_environmental are enum-reserved with no v1 detection site (documented in CEDAR_HITL_GATES §13.16). The auth_failure remedy's non-ARN "verify the credential is valid and has the required scopes" branch (error-classifier.ts) is therefore reachable only by the classifier unit test, not by any live producer — only the Secrets-Manager-ARN (IAM) branch fires in practice via SecretUnreadableError.
Suggested fix: when a runtime-credential-rejection detection site lands, wire it to emit the non-ARN auth_failure reason; until then, no action — flagging so the dead branch is intentional, not forgotten.
Filed as a follow-up to the review of #545. Refs #251.
Follow-up items from the review of #545 (observable blocker signal + bounded self-remediation, closes #251). None of these blocked the merge; they are hardening/tidy-up items surfaced during review. Filed so they are not lost.
1. Scope host-capturing egress detection to network-capable tools
post_tool_use_hookrunsdetect_egress_denialon every tool's output (agent/src/hooks.py, theegress_detected, host = detect_egress_denial(tool_response)call). A non-network tool —Read,Grep— that surfaces a file or log containing a string likeCould not resolve host: foo.comwill fire the observability event and, because a host was captured, latch the canonical terminal reason via_record_blocker_reason.On a successful task this is harmless (the success path returns before the latch is read). The sharper edge: if the task later errors with no more specific reason, a stale/unrelated host can be promoted as the terminal cause, sending an operator to the DNS Firewall for nothing.
Suggested fix: gate host-capturing detection (or at least the terminal latch) to
Bash/network tool calls, so file-content matches never poison the carry-path. The author already, correctly, accepts false positives for the live-stream event; this is only about the terminal reason.2. Document / harden the process-global blocker latch for a future multi-task-per-process model
_LAST_BLOCKER_REASONinagent/src/hooks.pyis a process-scalar, reset inpipeline.run_task(and relied on via the workflow-runner entry). Correct today under the one-task-per-Firecracker-MicroVM model (COMPUTE.md), and the author explicitly flagged the thread-safety nit.Suggested fix: add a one-line note at the latch definition that correctness depends on one-task-per-process, and file/track the task-id-keyed refactor as the trigger if the execution model ever changes. No code change needed while the invariant holds.
3.
auth_failurenon-ARN remedy branch has no live producerauth_failureandunknown_environmentalare enum-reserved with no v1 detection site (documented in CEDAR_HITL_GATES §13.16). Theauth_failureremedy's non-ARN "verify the credential is valid and has the required scopes" branch (error-classifier.ts) is therefore reachable only by the classifier unit test, not by any live producer — only the Secrets-Manager-ARN (IAM) branch fires in practice viaSecretUnreadableError.Suggested fix: when a runtime-credential-rejection detection site lands, wire it to emit the non-ARN
auth_failurereason; until then, no action — flagging so the dead branch is intentional, not forgotten.Filed as a follow-up to the review of #545. Refs #251.