aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

Follow-up: hardening items from #545 blocker-signal review

Open
#551 0 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

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._

Contributor guide

Open the contributing guide

Research direction

Start by tracing detect_egress_denial and _record_blocker_reason in agent/src/hooks.py, including the reset in pipeline.run_task, then read COMPUTE.md and the referenced CEDAR_HITL_GATES section. Review the auth_failure branch in error-classifier.ts and its classifier unit test; done means the host latch is scoped correctly, the process invariant is documented, and the intentionally unproduced branch is clearly tracked.

Written by the indexing model from the issue text.

Assessment

Tech stack
python, typescript
Domain
backend, observability
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.