`3df7d6764a` hardened `start_local_agent` so a pre-existing listener on 8080
could not be mistaken for the child it just spawned. That hardening never ran.
`main()`'s `--local` branch probed the port first and, on a successful TCP
connect, printed "Agent already running on localhost:8080" and skipped
`start_local_agent` entirely - the function is called from exactly one place,
the `else` of that same probe. So in the one scenario the hardening existed for,
a stranger owning 8080, the hardened code was unreachable and the tester chatted
with the stranger under a success banner, exit 0.
A bare TCP accept only establishes that *some* process is listening. It cannot
establish that the process is this example's agent. The fix removes the check
that made that inference:
- `main()` no longer probes 8080 on the start path at all. Adopting a listener
is now opt-in via `--use-running-agent`, and even then it is announced as
unverified ("did not start it and cannot verify it is an agent") rather than
as "Agent already running". The flag errors out when nothing is listening, and
argparse rejects it without `--local` instead of silently ignoring it.
- The port check moved into `start_local_agent`, before the "Starting local
agent" banner, where it now REFUSES on an occupied port instead of spawning a
child that cannot bind. Because `main()` no longer duplicates the probe, this
is the only port check on the start path, so it is genuinely reachable from
the shipped CLI - which is precisely what the previous attempt was not.
- With the pre-spawn refusal in place, the loop's `not port_already_busy` guard
became a provably-constant conjunct and was folded away. The durable half of
the earlier hardening, polling the child for liveness BEFORE looking at the
port, is unchanged and still reachable.
Same-pattern audit of the file found one more instance: `run_chat` printed
"[Completed in Xs]" purely because `invoke_agent` returned, which it also does
after an HTTP error. `invoke_agent` now returns a bool and the line reports
"[Failed in Xs]" when the exchange did not succeed. The request payload and the
streaming decoder are deliberately untouched (deferred).
Verified by driving the real `main()` via importlib against a foreign HTTP
server bound to 127.0.0.1:8080:
- pre-fix, `--local`: "Agent already running on localhost:8080" then
"Agent: I am a STRANGER on 8080, not the agentcore agent", exit 0.
- post-fix, `--local`: "Port 8080 is already accepting connections" plus how to
proceed, exit 1, nothing spawned.
- post-fix, `--local --use-running-agent`: talks to it, labelled unverified.
- post-fix, `--local --use-running-agent` with nothing listening: exit 1.
- ordinary path (port free, child really binds 8080): "Agent started
successfully", byte-identical to pre-fix output.
- child exits 1 with no listener: still caught in ~1s, not 30s.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The startup wait loop in start_local_agent checked the port before polling
the child, so any process already listening on 8080 satisfied the port check
on the very first iteration. The function printed "Agent started
successfully" and returned while the real child was dying, and the caller
then chatted with the impostor - the exact failure the fail-fast poll() was
added to surface.
Two changes, because reordering alone is not enough: on the first iteration a
doomed child has not exited yet, so a pre-existing listener would still be
mistaken for it.
- poll() now runs before the port check, so an already-exited child is always
reported with its exit code instead of being masked.
- Port ownership is snapshotted before the spawn (check_port_available()
returns True when the port is OCCUPIED, despite its name). When 8080 was
already busy, an open port is no longer accepted as proof this child is
serving; the child that cannot bind will exit and be reported with its real
exit code, and the timeout message names the port conflict.
Fully attributing a listener to a specific child would need a readiness
signal from the agent itself (identity endpoint or handshake); refusing to
trust a pre-existing listener is the smaller change that keeps the reported
outcome truthful.
Verified by driving the real function via importlib with a shimmed child:
- foreign listener on 8080 + child exits 1: was "Agent started successfully"
(returned a process that was dead 0.5s later), now "Agent exited with code 1
before port 8080 opened" and exit 1.
- port free + child really binds 8080: still "Agent started successfully" and
the live process is returned.
- port free + child exits 1: still caught in ~1s, no 30s timeout burn.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The AgentCore example states its Python-tooling and deploy contract in six
places — two READMEs, four script self-docs, terraform.tfvars.example, the
Terraform variable descriptions and .gitignore. There is only one contract, but
each of the last three review rounds corrected a single copy of it, so the
copies drifted apart and now contradict each other. This pass reconciles all of
them against measured behaviour instead of patching one more surface.
What the contract actually is, verified by running each command:
- test-agent.py imports boto3/requests/colorama, so it runs under uv with no
--project flag. `uv run` resolves the script path against the shell's cwd, not
the project root, so `--project ..` is redundant, not required: both forms load
infra-terraform/scripts/test-agent.py and both reach the same
`FileNotFoundError: 'terraform'`. The script's Usage block claimed the flag was
needed; it no longer does.
- deploy-frontend.py (Terraform) is standard-library only with a 3.8 floor, so uv
is optional. `uv run --no-project` and plain `python3` stop identically at
"terraform is not installed". Its "Requires: uv" line said otherwise.
- That same script cannot succeed at all. It requires a Terraform output named
feedback_api_url; no root or module outputs.tf declares one (only an SSM
parameter of that name). Fed the exact output set that outputs.tf does declare,
it exits 1 at "Missing required Terraform outputs: feedback_api_url" before any
build or upload. The README documented it as the working path for a Terraform
deployment; it now says what happens and points at infra-cdk. Repairing the
script or declaring the output is tracked separately.
- agents/ holds two uv projects plus agents/utils/, which both Dockerfiles COPY
in and which has no pyproject.toml or lockfile. "Each agent is its own uv
project" overstated the guarantee.
- docker mode needs Docker running but no separate build step: the apply's
docker_build_push provisioner builds and pushes ARM64 before the runtime
resource, which depends_on it. tfvars.example prescribed
apply -> build script -> apply, contradicting the build script's own header.
- .gitignore covered .venv/ but not venv/, the third and last surface of a guard
.dockerignore and the Terraform image-hash filter already cover. A real
UV_PROJECT_ENVIRONMENT=venv sync produced 2329 committable files (30MB); it is
now ignored, matching the other two.
Also corrected while auditing every command, path, prerequisite and tool version
in the same tree: the frontend is Vite, not Next.js; the CDK tester reads
config.yaml at the example root, not infra-cdk/config.yaml; the CDK frontend
deployer's floor is 3.8, not 3.11, and its usage hint named a path that does not
resolve from the example root; build-and-push-image.sh resolves region from
AWS_REGION/AWS_DEFAULT_REGION/aws-config with no us-east-1 fallback; up.sh
overwrites the STACK_NAME and MEMORY_ID that .env.example told you to fill in;
and backend_pattern's "available patterns" listed two agents this example does
not ship.
Deliberately untouched, tracked elsewhere: the missing docs/ directory and its
links, "Node.js 18+", the duplicated `cd infra-cdk` teardown, the
Memory-and-Gateway-only claim, the undeclared aws_region variable (still the one
remaining README/tfvars.example disagreement), the absent teardown section, the
duplicate deploy-frontend.sh, and every code-behaviour defect in the scripts.
Verified: py_compile on all four touched Python files, bash -n on all five shell
scripts, and every documented command run from the directory its text names.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
start_local_agent() launched the agent with stdout=PIPE and stderr=PIPE and
then never read either pipe. The child blocks the moment it fills a ~64KB
pipe buffer, so a chatty agent wedges before it can bind port 8080. Worse,
the 30-second startup-timeout branch called _agent_process.stderr.read() --
a blocking read to EOF -- on a child that was still alive, so the tester hung
forever instead of reporting the timeout.
This mattered more since the command became `uv run --locked --project ...`:
uv writes resolution and install progress to stderr before the agent starts,
and the message uv prints when uv.lock has drifted from pyproject.toml only
reached the developer through that same wedged branch.
Fix: do not pipe the child at all. stdout/stderr are inherited, so agent logs
and uv's errors stream straight to the developer's terminal (this is an
interactive tool), the child can never block on a full pipe, and no reader
threads are needed. The wait loop now also polls the child each second and
fails fast with its exit code when it dies early, instead of burning the full
30 seconds. The timeout branch delegates cleanup to stop_local_agent(), which
is now idempotent (clears the global first) so the timeout path, the SIGINT
handler and the atexit hook cannot double-stop or double-print, and kill() is
followed by wait() so the process is reaped.
Verification (standalone reproductions; this file has no test suite):
- Old code vs a child writing 8000 lines and staying alive: the child never
finished writing (deadlocked on a full pipe) and the parent hung in
stderr.read() until an external timeout killed it (exit 124).
- New code, real start_local_agent() driven against the same child: the child
wrote all 8000 lines, the parent hit its timeout, stopped the agent and
exited 1 after 30.1s.
- Real uv project with a stale uv.lock: the developer sees
"error: The lockfile at `uv.lock` needs to be updated, but `--locked` was
provided." live on the terminal, and the tester reports
"Agent exited with code 2 before port 8080 opened" after 1.0s.
- python3 -m py_compile passes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The deploy and local-dev scripts called bare `python3` and relied on the caller
already having PyYAML, requests, boto3 and colorama importable. `scripts/requirements.txt`
listed them but nothing installed it, and `uv run scripts/test-agent.py` — the
command the script's own docstring gives — failed because there was no project
for uv to resolve against.
Route every Python entry point through `uv run --project`, backed by the
example-root project added in the previous commit, and drop the orphaned
`requirements.txt`. Preflight now checks for `uv` rather than `python3`; uv
provisions the interpreter itself, so the hand-rolled Python 3.8 version assert
goes away with it.
`test-agent.py` starts a local agent inside that agent's own uv project with
`--locked`, so a locally run agent gets the same dependency set as its image
instead of an ad-hoc resolve.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Maxim