feat(aws-control): a Crews pane for the crews you deployed

[chenmingwei23]

What this adds

A Crews pane in AWS Control that lists the KiroCrew gateways you deployed into
your own AWS account as a service your customers reach: one CloudFormation stack per
crew, one ECS service inside it, behind the load balancer a shared base stack owns.
The pane is read-only. It creates nothing.

It also brings the deploy machinery that produces those crews into the tree: the
container image source, the two CloudFormation templates, the bundle curator, and
the deploy driver with its 218 gate tests.

Naming: the Agents page's user-facing copy says "agents", and only its code calls
them crews (CrewCard). These are remote, so the rail says Crews and the pane
title says Remote crews, with a blurb saying they run in the cloud on your bill
rather than on this machine.

What this does NOT do

Worth stating plainly, because the pane looks like a console and a console usually acts.

The UI is read-only. The backend adds exactly two routes, both GET, and between them
they issue three AWS calls: cloudformation describe-stacks, ecs describe-services, and
sts get-caller-identity. No mutating verb appears anywhere in backend/crews.py. The
only control in the pane is Refresh. There is no deploy button, no delete, no test
connection, and no way to talk to a crew from here.

Deploying is a CLI action, not a UI one. crew/scripts/smc-deploy.sh is what creates a
crew; the pane only reports what that script left behind.

No part of this has run end-to-end against a real AWS account. Not chatbot, not
persistent. The 218 gate tests all run under --dry-run, and every AWS-touching helper in
the driver takes a fixture branch before its real path -- secret_digest() opens with
if [ "$DRY_RUN" -eq 1 ]; then ... return 0; fi, and that same function held a defect
(documented below) that aborted a first real deploy before the secret was created. A bug
on the real path survived every green gate run, which is the honest measure of what these
gates cover.

The screenshots and the walkthrough clip are fixture data. The five crews in them
(billing-help, checkout-bot, winter-promo, ...) are invented by
website/scripts/lib/aws-control-crews-fixtures.mjs. They demonstrate that the pane renders
each state correctly. They are not evidence that a crew runs.

What a reviewer should therefore take from this pull request: the listing surface is
complete and tested, the deploy machinery is written and gated but unexercised against real
AWS, and a first real deploy is the outstanding validation.

Two memory modes, chatbot by default

Memory is a crew-stack parameter with two values.

chatbot keeps a conversation only while the task serving it is alive. No bucket
is configured, the backup sidecar does not run, and the task role carries no S3
statement at all. That last part is stronger than scoping the grant to the crew's
own prefix: a permission that does not exist cannot be reached by a future code
path, and it makes the mode auditable from the IAM role rather than from the
container's environment.

persistent restores the two authority files at boot and fetches one transcript
per turn. It is opt-in, not the thing you get by forgetting a flag, because it
carries the restore path and the S3 grant that chatbot does without.

A task replacement loses every conversation in chatbot mode, and that is not a rare
event: it happens on every deploy, every failed health check, and every platform
update. The UI says so in the mode's own label rather than in a footnote.

A property with no gate is a claim

restore emits one machine-readable line per boot, declared as an interface rather
than log prose (crew/runtime/container/backup/restore.py, SUMMARY_TOKEN). A
deploy gate reads it and refuses seven ways, and the mode decides which claim is
checked, because a gate that knew only one mode would refuse every deployment of the
other:

reading	verdict
line absent	refuse. Restore never ran, which is not the same as restoring zero
transcripts_restored > 0	refuse in both modes. The task holds what it did not serve
chatbot, state != disabled	refuse. A bucket is configured, so Memory and SMC_BACKUP_BUCKET disagree
chatbot, available > 0	refuse. Listing them needs the grant this mode withholds
persistent, state != ok	refuse. partial means an authority file was missing, so resume is broken
persistent, available == 0	refuse. Zero restored out of zero available proves nothing
a field repeated or zero-padded	refuse. Guessing which copy to believe is the wrong response

transcripts_available exists because the obvious assertion is vacuous alone: a crew
with no history would pass trivially. The counter is incremented at the write site
from layout.is_transcript, never hardcoded, so reintroducing a bulk restore moves
the number instead of leaving a constant for the gate to read.

Where things live, and one packaging trap

setup.cfg [options.package_data] globs builtin assets by fixed subdirectory
name: ui/, lib/, backend/, agents/, inject/, skills/, scripts/,
*.md. A new directory is not shipped, so everything works from a source checkout
and every file is missing on every pip and DMG install, with no error anywhere. This
is the accident the apple_speech/*.swift comment in that file describes. One glob
is added for crew/**, with a test that fails when the glob is removed, and the same
entry in MANIFEST.in for the sdist half.

The deploy driver ships as a shell script rather than being ported to python.
Precedent: deploy/skills/artifact-deploy/scripts/*.sh is a whole set of shipped
shell deploy scripts. Porting 2,000 lines would have discarded 218 gate tests.

Those gates run the driver under --dry-run, which is what they can prove and also
their limit: every AWS-touching helper takes a fixture branch first, so the gates
exercise the driver's logic and never its real path. An earlier draft of this
description said the driver was "proven against a real account". That was wrong and
is corrected below.

Defects found while building this

Each was reproduced before it was fixed, and each has a test that reddens when the
fix is reverted.

The account binding is asserted, not assumed. A profile is a name a child CLI
process resolves, so a profile repointed from account A to B would let a request for
A's crews report B's. That is a disclosure, not an error, so it answers 409
account_mismatch with its own copy rather than a generic error wall. Every listing
re-derives the account through the same profile first. This is the posture
storage.find_drive established for writes, applied to a read.

A boot loop, not a no-op. run_sidecar logs and returns when no bucket is
configured, and _wait_for_shutdown treats any child exiting as the end of the task.
An unconditional sidecar therefore meant the container would come up, lose its
sidecar within a second, shut down, and be replaced forever. Each piece behaved as
designed and nothing in the deploy would have said why. The supervisor tests ran with
backup_bucket=None and asserted the sidecar starts, so the suite had encoded the
loop as correct.

One punctuation mark defeated the isolation. The front proxies a turn and the
backend decides whether an id is legal, so the fetch ran first. id="dashboard:cust-1"
is not a legal backend id, but the fold turns it into dashboard_cust-1, which names
a real conversation: the old code downloaded that transcript and the backend then
rejected the turn, leaving the task holding a conversation it never served. The guard
is a shape test rather than a copy of the backend's grammar, which lives in a
dependency this process does not import.

The reader re-derived the writer's S3 key. The fetch had its own join of
backup_prefix and crew_name, identical to layout.object_prefix. It agreed and
was free to stop agreeing, and drift there is invisible: the fetch misses and a
returning customer is indistinguishable from a new one.

A health nobody measured. healthy was desired > 0 and running == desired, and
the list route never calls ECS, so every crew in a list came back False. That reads
as "they are all down" when it means "nobody looked". It is now None on a list, and
also None for a crew scaled to zero, because False there blames the crew for a
decision its owner made.

The seam guard's reverse direction had never run. It scanned for ParameterKey=,
the long-form CloudFormation syntax, while the driver uses the short Name=Value
form exclusively: the pattern matched zero times, so "driver passes nothing the
template fails to declare" was printed without ever having been tested. Repairing it
immediately found a real mismatch invisible until now: the driver passed
RestApiRootResourceId to the crew stack, which crew.yaml deliberately does not
declare and says so in a comment.

Two mode labels that were not one switch. "Keeps memory" opposite "Chatbot": one
states a behaviour, the other names a category, and the category says nothing about
memory, which is the whole distinction. Thirteen locales carried it.

Tested

• 162 python tests on the AWS Control surface, 234 vitest tests on the pane
• 218 gate tests and the seam guard, run from their new path
• The dry run reaches 13 gate PASS in both memory modes
• Mutation-verified: reintroducing the bulk restore, removing the id guard, locking
  on the raw id, ignoring the mode in the gate, granting S3 in chatbot mode,
  re-deriving the S3 key, dropping the account assertion, unanchoring the stack
  regex, starting the sidecar unconditionally, and removing the package_data glob
  each redden a named test
• Alignment across a card row is measured in a real engine, not asserted in jsdom
  (which reports every layout box as zero). The harness also fails when no row mixed
  a badged card with a bare one, so it cannot pass vacuously

Not done yet, deliberately

Neither memory mode has run on a real AWS account. Every number above is offline.

open_slots.json shrinks in persistent mode. Boot restores it, every transcript
it names is absent by design, and the backend treats an absent transcript as a
confident answer rather than an unreadable one, so the key is dropped from the next
flush and the sidecar uploads the reduced file. In chatbot mode this cannot happen,
because nothing is uploaded. It must be resolved before persistent mode is used, and
the choice is real: either the file is authoritative and this is data loss, or it is
not and the restore requirement should go.

No deploy from the UI. The pane reads. The driver is a shell script an owner runs.
Wiring it to a route needs its own gated service, because GATED_SERVICES exists
ahead of the first billable call and a crew is emphatically billable: a Fargate task
runs until it is stopped, behind an ALB, egressing through a NAT.

The conversation reader is a named stub. crew/conversations.py will read
transcripts out of S3 for the owner's view. Chatbot is the default and has no S3
conversations, so shipping it now would be a reader for something no default
deployment produces.

What it looks like

Captured against fixtured routes in an isolated instance, no AWS credentials. The
harness is website/scripts/capture-aws-control-crews.mjs and it asserts as it goes
rather than only saving files: it fails when two cards in a row put their fact grids
at different offsets, when no row mixed a badged card with a bare one (so it cannot
pass vacuously), and when any fact value is clipped to an ellipsis.

Two of these are empty states, and the easy mistake is to render both as "no crews".

The pane. Crews sits in the rail beside Files / Library / Backup / Access.
winter-promo is DELETE_IN_PROGRESS and is listed with a red border rather than
filtered out, because hiding a half-deleted crew is how it becomes a surprise on the
next bill. legacy-triage shows MODE Unknown rather than a mode it guessed: its
stack predates the parameter, and the backend returns an empty string so the UI can
say it does not know.

One crew opened. Stack, mode, service with running/desired, endpoint, the full
image digest, region. The URL stays /aws-control/crews, so this is view state with
a breadcrumb rather than a route, matching the drive.

No base stack, so no crew can exist yet. It says what is missing rather than
reporting an empty list.

The base is ready and the account holds none. Distinct copy from the frame above.

account_mismatch. The profile now signs in to a different AWS account, so listing
that account's crews would be a disclosure. Its own warning, not a generic error wall.

320px. One column, no card overflowing the viewport.

Walking the pane, one continuous take: click Crews, read the grid, open a crew,
return by the breadcrumb.

Walkthrough

A 19s silent clip of the pane, recorded against the real built SPA with every /api/**
call answered from fixtures, so it needs no gateway and no token and reproduces from a
clean checkout:

cd website && node --max-old-space-size=6144 ./node_modules/vite/bin/vite.js build
node scripts/record-aws-control-crews.mjs ../temp-screenshots/aws-control-crews

Same clip as mp4 (0.83 MB), if the GIF is too coarse to
read the digests.

It walks the grid, one crew opened, the detail Refresh, and then the three states that
are deliberately NOT one screen: base not deployed, base ready with no crews, and
account_mismatch. The mismatch banner is at ~13.5s. It ends at 320px wide.

vite build is invoked directly rather than through npm run build, which is
tsc -b && vite build: tsc -b fails on src/stories/** when the Storybook dev
dependency is not installed, and because of the && that means vite never runs and the
recording is made against a STALE dist. That happened on the first take. The stories
are leaves outside the SPA entry graph, so skipping the typecheck does not change the
bundle.

What the agent can reach under the unsandboxed default, measured

A reviewer flags build_backend_env's wholesale dict(os.environ) copy as putting
credentials in front of the agent. The copy is real and two credentials travel in it.
Neither is closed by stripping, and the consequence of each is bounded -- both facts
measured rather than argued.

KIRO_API_KEY is the model credential. The backend cannot answer a turn without it, so
under AllowUnsandboxedExec=true a prompt can read its own credential. There is no
version of this that keeps the model working and hides the key from it; that is the
sandbox decision, not a separate defect.

AWS_CONTAINER_CREDENTIALS_RELATIVE_URI is injected by Fargate and is NOT stripped, so a
shell can fetch the task role's temporary credentials. What that role can do:

• chatbot, the DEFAULT mode: nothing. crew.yaml's TaskRole is
  Policies: !If [PersistentMemory, [own-prefix-only ...], !Ref 'AWS::NoValue'], so in
  chatbot mode there is no policy at all. The driver's gate 7 asserts state=disabled and
  gate 13 proves the role carries no S3 action.
• persistent mode: s3:GetObject / s3:PutObject on crews/<crew>/* and a
  prefix-scoped s3:ListBucket. No s3:DeleteObject. That is the crew's own transcripts,
  which the container already reads and writes through its backup sidecar, so the
  credential grants no reach the crew did not already have.

Stripping the variable would be defence in depth and is deliberately not presented as a
fix, for the reason measured on the control secret above: everything in the image runs as
one user (USER crew), so a sibling process's /proc/<pid>/environ still carries it, and
neither del os.environ[...] nor os.unsetenv() scrubs that file.

The two real closes are the sandbox -- which on Fargate means the container refuses to
start, so --require-sandbox exists as the owner's opt-in -- and privilege separation
inside the image. Both are owner-facing decisions rather than defects, which is why they
are here in the description instead of being quietly changed.

The control secret IS readable by the agent, and that buys a 404

A reviewer raised this on crew.yaml's Secrets block, and the exposure half is
correct. SMC_CONTROL_SECRET is injected as a task-definition secret, the image runs
everything as one user (USER crew in the Dockerfile, no privilege separation between
supervisor, front and backend), and on Fargate the model runs unsandboxed with an
auto-approved shell. So a prompt can read another process's /proc/<pid>/environ and
recover it.

An earlier round already stripped it from the environment the backend hands the model
subprocess. That closed direct inheritance and nothing more: a sibling process owned by
the same user still has it.

Unsetting it after load is NOT the fix, measured rather than assumed. On Linux
/proc/<pid>/environ reads the block placed there at exec, and neither
del os.environ[...] nor os.unsetenv() scrubs it:

at start                     in /proc/self/environ: True
after del os.environ[...]    in /proc/self/environ: True
after os.unsetenv too        in /proc/self/environ: True
still in os.environ:         False

So an unset would read like a fix while changing nothing an attacker sees. It is not
being added for that reason.

What recovering the secret actually grants: a 404. The front's gateway route sends
everything that is not the single customer turn path through the control gate, and the
authorized branch returns control_not_implemented with status 404. The container front
implements no control operation at all -- the owner's control plane is off-box. The real
boundary is API Gateway AWS_IAM plus an internal-scheme ALB, which is why the contract
provides no ControlSecretValue and treats the header as a non-boundary in the first
place.

Closing it properly means privilege separation inside the image: run the front under a
different uid from the backend, so /proc/<front>/environ is not readable by the agent.
That is a Dockerfile and supervisor change, it is the right eventual shape, and it is not
in this pull request -- deliberately, because the thing it would protect currently answers
404.

The multi-tenancy boundary, and why it is a declaration rather than a mechanism

Reviewers raised this six times across seven rounds, from three different angles, and every one of them was right about the same thing: inside a persistent-memory crew there is no credential-enforced boundary between one customer and another. This section is the disposition, because the honest answer needed a decision rather than a patch.

What the isolation actually isolates. The task role is scoped to crews/<crew>/*. That separates one CREW from another, and the deploy gate proves it against lookalike prefixes like crews/<crew>2/ and crews/<crew>-evil/. It does not separate one CUSTOMER from another: every conversation the crew has ever served lives under that one prefix.

Three facts compose into the exposure. The prefix is shared. The container runs its backend with every tool auto-approved, because an unattended crew that stalls waiting for an approval nobody will see is a crew that answers nothing. And the conversation id arrives in the request rather than being derived from the caller. So a turn driven by one customer can name another customer's conversation.

What was fixed, and it is narrowing rather than closing. s3:DeleteObject is gone from the role. Nothing in the container could ever call it: the object store exposes put, get and list and no more, in all three implementations, and that is deliberate, because the restore path SKIPS objects it must not hold rather than removing them. In a backend that auto-approves every tool, a permission that exists is a permission a prompt can reach, so an ungranted destructive action is the difference between a customer-driven turn reading a transcript and erasing one. The isolation gate now asserts the absence in both modes, and that assertion was verified by granting delete back and watching it report allowed (expected denied).

What is now required rather than assumed. Memory=persistent refuses unless the owner also sets TrustDomain=single-principal. The refusal exists twice on purpose: a CloudFormation Rule, which a console deploy or a pipeline cannot skip, and a parse-time check in the driver, so the owner learns before step 0 instead of watching six steps of work reach CreateChangeSet. Setting it is the owner stating that every caller they authorise is one tenant, or that their own authentication layer scopes ids in front of the crew. Leaving it empty refuses the deployment rather than shipping on an assumption nobody made out loud. Memory=chatbot, the default, is unaffected: it grants no S3 action at all, so there is no shared prefix to reason about.

Why binding ids to the caller is not in this pull request. It is the right eventual fix and it is deliberately deferred, on an asymmetry: a refusal that is wrong fails closed, and a deployment does not happen. Identity binding that is wrong fails open, and one customer receives another's history.

This branch has direct evidence of how easily the second happens. A credential fence added earlier in this same review cycle was changed to judge the resolved target for two of its three checks and not the third, so a symlink to /proc/self/environ walked past all three and the deploy process's environment would have been inlined into a shipped prompt. That was written with the security intent explicit, on a branch already six review rounds deep. Adding a new mechanism in the customer data path at the end of a long review cycle, for a mode that has never run on a real AWS account, is the same conditions with a worse blast radius.

There is also nothing to bind to yet. AWS_IAM guards the CONTROL plane (base.yaml), the ALB is Scheme: internal, and the front reads no caller identity at all today, only the control secret header. Identity binding is a new path through the request, not a line to add, and which identity it should read depends on the authentication the owner puts in front, which this design leaves to them.

Why the reviewer's second suggested fix does not work on this integration. The
finding offers two remedies: apply the single-principal requirement to chatbot mode too,
or namespace ids with verified caller identity. The second one looked cheaper, so it was
costed first, and it does not hold here.

Namespacing needs the caller's verified ARN to reach the container. The only place that
identity exists is API Gateway, and the crew method is Type: HTTP_PROXY over a
VPC_LINK (crew.yaml). Injecting context.identity.userArn as a request header on a
proxy integration is not a boundary: proxy integrations forward client headers as they
arrive, and AWS documents parameter mappings as the capability of the CUSTOM (non-proxy)
integration -- "With the HTTP custom integration, also known as the non-proxy
integration, you have more control of which data to pass ... You do this using data
mappings." There is a reported case of a client value and an integration-configured
value both arriving at the backend rather than the latter replacing the former.

If that is the behaviour for headers too, a caller sends the header itself and chooses
whose conversation to resume, which is worse than the honest declaration this branch
ships today. Confirming the override semantics needs a live deploy of this exact
integration type, which has not been done, so the mechanism is not being built on an
assumption.

That leaves the first remedy, a resource policy on ControlApi pinning
aws:PrincipalArn. It is IAM-enforced and depends on no header semantics.
ControlApi is AWS::ApiGateway::RestApi with no resource policy today, so it is
implementable. It is not in this pull request because it has a consequence the owner
should choose rather than inherit: pinning the API to one principal locks out every
other principal in the account, including the common split where a pipeline deploys the
crew and a person invokes it. Defaulting it to the deploying principal would make that
lockout the default behaviour of the primary path.

Two alternatives considered and rejected, with reasons.

A sandbox would genuinely close it: the sandbox makes agentic command networking unreachable, so a shell could not reach the ECS credential endpoint and would hold no credentials to use. It does not apply here, and the code already says why -- Fargate does not permit an unprivileged user namespace. The driver therefore defaults AllowUnsandboxedExec=true, and --require-sandbox turns that into a refusal to boot rather than a sandbox. That default is a platform constraint, not an oversight, and it is stated here because until now it lived only in a comment.

A safety policy denying cross-conversation reads raises the cost of an accidental leak and is worth having, but it is not a boundary. The tool being policed is execute_bash with an arbitrary command, and pattern-matching that is a losing game: aws s3 cp, python -c, a curl at the credential endpoint, the same command base64-encoded. Something a prompt can talk its way around is not an isolation boundary, and describing it as one would be worse than the gap.

Rebutted: the crew GET routes do not audit a successful read

A review round asked for _audit on every outcome of both crew GET handlers, on the
grounds that a successful owner read records no permission decision. The routes follow
this file's own convention exactly, and changing only these two would make it
inconsistent in the other direction.

routes.py has two wrappers. _mutating(operation) audits success / refused /
error around a handler and is used 16 times, all mutations. _guarded(...) audits
DENIALS -- app disabled, restricted session -- and errors, and does not audit a
successful read. Both crew routes are registered with _guarded, so a denial and an
error are already recorded; the finding is about the success path only.

Counted rather than asserted: of the 15 _guarded GET handlers in the file, 14 have zero
_audit calls in their bodies.

_handle_accounts 0            _handle_drive_preview 0        _handle_library_list 0
_handle_backup_status 0       _handle_drive_search 0         _handle_profiles_available 0
_handle_costs 0              _handle_drive_status 0          _handle_reconnect_plan 0
_handle_crew_detail 0        _handle_drive_list 0            _handle_shares_list 0
_handle_crews 0              _handle_iam_policy 0            _handle_drive_download 1

The single exception says where the line is. _handle_drive_download audits because it
MINTS a presigned URL, and its own comment gives the reason: "Same decision class as a
share: the presigned URL works for anyone". What is recorded there is that a bearer
credential was granted, not that a read happened -- which is also why that one call is
asyncio.shielded, so the grant record survives a client disconnect.

The crew routes mint nothing. They make three read-only AWS calls
(cloudformation describe-stacks, ecs describe-services, sts get-caller-identity)
and return what they found. No mutating verb appears anywhere in backend/crews.py.

If successful reads should be audited, that is a repo-wide change across 14 existing
routes and belongs in its own pull request, where the volume it adds to the SEL log can
be weighed once rather than argued per route. Doing it for two routes here would leave
the file with one convention for crews and another for everything else, which is the
drift this branch has already paid for several times.

[chenmingwei23]

QA evidence

Captured against fixtured routes in an isolated instance, no AWS credentials. The
harness that produces these is website/scripts/capture-aws-control-crews.mjs, and
it asserts as it goes rather than only saving files.

Each state below is visibly distinct from the others, which is the point: two of them
are empty states and the easy mistake is to render both as "no crews".

frame	what it proves
crews.png	The rail carries Crews beside Files / Library / Backup / Access. Five cards, one per crew. winter-promo is DELETE_IN_PROGRESS and is listed with a red border rather than filtered out, because hiding a half-deleted crew is how it becomes a surprise on the next bill. legacy-triage shows MODE Unknown, not "Forgets on restart": its stack predates the parameter, and the backend returns an empty string so the UI can say it does not know.
crews-detail.png	One crew opened. Stack, mode, service with running/desired and a Serving badge, endpoint, the full image digest wrapping across two lines, region. The URL stays /aws-control/crews, so the detail is view state with a breadcrumb rather than a route, matching the drive.
crews-base.png	No base stack, so no crew can exist yet. It says the shared base is not deployed rather than reporting an empty list.
crews-empty.png	The base is ready and the account holds no crews. Distinct copy from the frame above.
crews-mismatch.png	account_mismatch. The profile now signs in to a different AWS account, so listing that account's crews would be a disclosure. Its own warning, not a generic error wall.
crews-narrow.png	320px. One column, no card overflowing the viewport.

Row alignment is measured in a real engine rather than asserted in jsdom, which
reports every layout box as zero. Both mixed rows (a badged card beside a bare one)
put their fact grids at the same offset, and the harness fails when no row mixed the
two, so it cannot pass vacuously. It also fails on any fact value clipped to an
ellipsis, checked against a 30-character crew name whose stack name is 38.

[github-actions]

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of ecc02869adaa1b1bdbc647619831181d7b7191c6 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

The screenshots are clean and the pane matches the product's established rail/card patterns (the card is a faithful sibling of the Agents page's CrewCard, whole-card clickable with the same hover affordance). Reconciliation shows the blind reader correctly identified every primary control — rail item, Refresh, breadcrumb, Copy, card-opens-detail — and dared to use them. What survives are copy gaps the reader explicitly hit, plus unscreenshotted states.

UX-Verdict: CONCERNS

Solid, product-consistent read-only pane; but both empty-state repairs are unclear — one offers no next step, the other says "ask your crew" for a crew.

Watch

• Base-missing empty state dead-ends: base_missing_body explains why the list is empty but names no way forward, and the blind reader said exactly that — "it gives me no button to fix it… I don't know how to make the 'shared base' exist from here." Every first-time user of this pane lands here before anything else (high frequency, friction, persists until they find the CLI). Fix: end it with the same pointer empty_body has, e.g. "Ask your agent to deploy the base and this page will update."
• "Ask your crew to deploy one" (empty_body) uses "crew" for the assistant on a page whose items are also crews; the reader flagged it verbatim: "'Ask your crew to deploy a crew' reads oddly." The blurb already disambiguates via "agents on the Agents page" — say "Ask your agent to deploy one."

Evidence gaps

• Detail-view states are never shown: crew_absent notice, the red "Not serving" badge, the idle "No tasks asked for" row, and the unknown-mode explanation (mode_unknown_why). One detail screenshot per state would close this.
• The new "Crews" row in the overview/narrow root list (CREW_PANES RootListRow group in AwsControlPage.tsx) appears in no screenshot; shot-05 shows only the pane behind it.

Suggestions

• crews.mismatch: "The key for this account…" — the reader called "the key" vague; say "The saved AWS credential for this account now signs in to a different AWS account…".

[UX-REVIEWED] ecc0286

[github-actions]

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of ecc02869adaa1b1bdbc647619831181d7b7191c6 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound, unusually honest design — but ~20k lines of billable-infra deploy machinery ships to users with zero real-AWS validation, and persistent mode is deployable with a known data-loss defect.

Watch

• The deploy driver ships in the PyPI package (setup.cfg glob crew/**) and the bundled SKILL.md actively promotes it ("one-command deploy it into the owner's own AWS account"), while the description states "No part of this has run end-to-end against a real AWS account" and documents a real-path bug (secret_digest aborting before secret creation) that survived every green gate run. An installer's first real deploy is the validation run, on their bill, provisioning ALB/NAT/Fargate. Do the first real deploy — or mark the skill/driver experimental — before this reaches a release.
• Persistent mode carries the acknowledged open_slots.json shrink ("either the file is authoritative and this is data loss…"), yet nothing refuses Memory=persistent; the TrustDomain opt-in gates tenancy, not this defect. An owner who opts in today gets silent conversation loss on flush.

Suggestions

• Refuse Memory=persistent at the same two seams TrustDomain uses (CloudFormation Rule + parse-time driver check) until the open_slots resolution lands — the fail-closed mechanism this PR already built is the right tool for its own known defect.

[DESIGN-REVIEWED] ecc0286

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of ecc02869adaa1b1bdbc647619831181d7b7191c6 — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

The shipped docstring still claims a "driver proven against a real account" — the exact sentence the description retracts as false — and a zero-consumer placeholder module rides along.

What this change ships

Intent: let an owner see, in AWS Control, the crews they deployed into their own AWS account — and land the deploy machinery that creates them. ADDITION.

1. New "Crews" rail group + read-only Remote crews pane (list/detail/refresh) — justified
2. Two GET routes with 409 account-mismatch refusal, reusing deploy.engine.run_aws — justified
3. Deploy machinery in-tree: driver, 2 CFN templates, curator, image source — declared unvalidated end-to-end
4. Memory chatbot/persistent stack parameter, chatbot default, no-S3-grant posture — justified
5. Restore summary line + seven-way deploy verify gate — justified
6. crew/** packaging glob (setup.cfg + MANIFEST.in), order-pinned by test — derived
7. share-my-crew SKILL.md, matching the existing app-skills convention — justified
8. crew/conversations.py placeholder + its pin test — zero consumers
9. Guard-test exemptions (agent-home, host-isolation, mcp-call-site, spawn-audit), each paid with a pin — derived
10. Riders: pycache fix in the syntax-regression test, dev deps, i18n ×13, screenshots per the documented temp-screenshots convention — undeclared but small

Watch

• crew/__init__.py:9-10 ships "147 gate tests and a driver proven against a real account"; the description says "No part of this has run end-to-end against a real AWS account" and calls that earlier claim "wrong". The correction reached the PR body, not the code — and both prose counts (147 here, 218 in the description) are the stale-count class AGENTS.md bans for denied rules.
• conversations.py: grepped aws_control.crew.conversations — 1 hit, its own placeholder test. A missing module already fails at the name just as loudly; the zero option costs nobody anything.
• PACKAGING-CONTRACT.md's "Tracks and ownership" section is build-time agent orchestration ("REPORTED in your final message"), shipped in the wheel; three new .md files also sit against AGENTS.md's no-new-markdown rule.
• crew/runtime/Dockerfile builds the wheel from a checkout via build_image.sh, while docker/Dockerfile:4-7 consumes the CI wheel precisely so "Docker bytes" cannot drift from pip bytes — two spellings of "install kiro-cli + the KiroCrew wheel" now diverge.

Subtractions

• Delete crew/conversations.py and test/test_aws_control_crew_conversations_placeholder.py — 0 consumers; absence fails louder than emptiness.
• Delete the "proven against a real account" clause and both gate-test counts from crew/__init__.py.
• Strip the "Tracks and ownership" section from PACKAGING-CONTRACT.md; keep only the layout/interface invariants.

[FIRST-PRINCIPLES-REVIEWED] ecc0286

[github-actions]

GPT 5.6 Review — ✅ human override accepted

Human judgment by @chenmingwei23 overrides the GPT 5.6 finding for ecc02869adaa1b1bdbc647619831181d7b7191c6; the recorded reason is authoritative for this commit.

This comment is updated in place on each push.

The model was not re-run because an authorized human decision supersedes it.

False positive or not applicable? A repository writer can comment:
/ai-review override gpt ecc02869adaa1b1bdbc647619831181d7b7191c6: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed ecc02869adaa1b1bdbc647619831181d7b7191c6 — this comment is updated in place on each push.

Review details

No blocking issues; one advisory finding.

FINDING — src/kiro_crew/apps/builtins/aws_control/crew/packaging/build.py:1347 — _copy_skill reads via _read_text (read_text universal-newlines CRLF→LF) and writes via _write_guarded (write_text→LF on Linux), so a CRLF-authored skill file is normalized in the staged copy while the reviewed pin was _tree_hash over raw source bytes (_sha(p.read_bytes())); _staged_tree_hash then hashes the LF staged bytes, staged != reviewed, and an unmodified skill is refused with ExportRefused("skills/{cid} changed while the bundle was being written") → Fix: copy skill files verbatim (dest.write_bytes(p.read_bytes())) after the text scan, or take the pin and the staged hash over the same normalization.

[OPUS-REVIEWED] ecc0286

Verdict parsed from the review's SHA-scoped output markers for commit ecc02869adaa1b1bdbc647619831181d7b7191c6.

False positive or not applicable? A repository writer can comment:
/ai-review override fable ecc02869adaa1b1bdbc647619831181d7b7191c6: <one-sentence reason>

[chenmingwei23]

Addressing the GPT 5.6 review

Four of the five blocking findings were real and are fixed in a70035c70. The fifth is rebutted below with the code that makes its premise false. Each fix has a test that fails when the fix is reverted, and I ran every revert rather than assuming.

front/app.py:176, a non-string id fetched another conversation. Fixed. This is the same defect class as the dashboard:cust-1 fold already fixed here, and the finding is a good catch because it went through a different door. That fix closed the punctuation door: a colon is not legal in a backend id, but the sanitizer folds it to an underscore and dashboard_cust-1 names a real conversation. {"id": 123} goes through the type door instead. str(123) is "123", a perfectly legal shape, so the guard passed it and dashboard_123 was fetched before the backend rejected the integer. A non-string id now yields no fetchable slot at all. The asymmetry that shaped the original guard is preserved: being wrong permissively about shape serves a redundant fetch, while being wrong restrictively would serve empty history and let the sidecar's whole-object put overwrite that customer's real transcript.

build.py:645, prompt expansion read a sensitive path. Fixed, with one correction to the finding. The fence is applied before the read now, using this repo's own is_sensitive_path rather than a second denylist. The correction: the report says the credential "is read and embedded in the deployment bundle", and embedding was already prevented, because scan_text runs on the text and raises ExportRefused on a match. So the reachable consequence was the read, not the publish. That distinction does not save the code, because a pattern scanner cannot be relied on to recognise every credential format (a kubeconfig's client-certificate-data is base64 and matches nothing), which is exactly why the fence belongs before the read instead of after it. The test asserts the read never happens, not merely that the export was refused.

smc-deploy.sh:1021, the base deploy passed an undeclared parameter. Fixed, and the gate that should have caught it is fixed too. The driver now passes LogRetentionDays, which base.yaml declares. The driver's own comment block had already documented this as item 9 and called it PRE-EXISTING and REPORTED; that item is deleted, because a comment describing a defect that no longer exists is worse than no comment.

The more useful half of this finding is why it survived a gate whose entire job is this class of defect. check_param_seam.sh asserts the driver and the templates agree about parameters in both directions, and it read crew.yaml and only crew.yaml. The base stack was never checked in either direction. It now covers base.yaml as well, and the new direction was proven by putting the defect back and watching it go red rather than by inspection:

FAIL  driver passes base parameters base.yaml does not declare: ['TranscriptRetentionDays']

That mattered because this gate's reverse direction had already been silently vacuous once: it scanned for ParameterKey= long-form syntax while the driver uses Name=Value exclusively, so the pattern matched zero times and printed a pass it had never tested. The gate count is 148, up from 147.

One judgment call worth surfacing rather than burying: RETENTION and LogRetentionDays do not mean the same thing. RETENTION is a transcript retention value; LogRetentionDays drives the LogGroup's RetentionInDays. Transcript retention has no enforcement in this template at all, which base.yaml notes next to ArtifactsBucket. The commit says so at the call site instead of letting the rename imply the two were always the same knob.

smc-deploy.sh:1595, the image builders were absent. Fixed, and the reason is a defect in my own migration map. build_image.sh and build_crew_image.sh are now in the tree. They existed all along in the source tree this work migrated from, at its root scripts/, and MIGRATION-MAP.md, whose stated rule is that a file with no row is a bug in the map, walked only deploy/. So the map had no row for either and the migration dropped both. Both now have rows explaining that origin, so the next reader can see why they were nearly lost. Their path assumptions were adapted to the new home (build context, Dockerfiles and vendor/ resolve under runtime/), and an absolute developer-home default was removed rather than rewritten: it is now a required argument, because a default that only works on one machine is worse than no default.

Both ship. setup.cfg's apps/builtins/*/crew/**/* glob covers them, which test/test_aws_control_crew_payload.py pins, and both carry the executable bit that the driver's [ -x ] check needs, matching the precedent in deploy/skills/artifact-deploy/scripts/.

CrewsPage.tsx:338, rebutted. The absent state is not a dead end. The finding's premise is that a crew-detail 404 leaves the user with nowhere to go. A back button renders unconditionally at the top of CrewDetail, above every notice, labelled with the pane title:

<button onClick={onBack} data-testid="crew-detail-back">
  <ChevronLeft size={14} aria-hidden="true" />
  <span className="min-w-0 truncate">{i18nT('apps.awsControl.crews.title')}</span>
</button>

So the absent state shows that button, and then "This crew is not in this account any more. It may have finished deleting." Returning to the list is the hand-off, and it is the right one: the two sibling notices carry askAgent because a failed read and an account mismatch are both conditions an agent can investigate, while a crew that has finished deleting is not. Offering to ask an agent about a resource that no longer exists would send the user after something nothing can recover. Leaving askAgent off here is deliberate, and the asymmetry between the three notices is the point rather than an oversight.

build.py:496 (non-blocking), fixed, all three sites. bool("false") is True, so a value that arrived as the string "false" turned each of these on. All three are reachable and none was left alone: plan include from operator JSON, stream from arbitrary customer JSON, and the sandbox opt-in from a config overlay. Each now requires a real boolean and refuses anything else rather than coercing "false" to False, because a config that says "false" and means it is a config its author misunderstood, and a silent coercion hides that from them. The sandbox one is the strictest: only a real True opts in, and a non-boolean raises ConfigError at boot instead of defaulting either way, since the consequence of guessing there is unsandboxed execution.

Verified after all fixes: 148 gate tests, 13 gate PASS in both memory modes, 254 crew tests, 149 repo tests, brand and scrub clean.

[chenmingwei23]

/ai-review override gpt ecc0286: 14 of the 15 _guarded GET handlers in routes.py have zero _audit calls in their bodies, and the one exception audits because it mints a presigned URL, so these two read-only routes follow the file's convention and auditing only them would make it inconsistent.

[github-actions]

Human judgment recorded

@chenmingwei23 marked the gpt AI finding as false positive, not applicable, or explicitly accepted for ecc02869adaa1b1bdbc647619831181d7b7191c6.

14 of the 15 _guarded GET handlers in routes.py have zero _audit calls in their bodies, and the one exception audits because it mints a presigned URL, so these two read-only routes follow the file's convention and auditing only them would make it inconsistent.

This decision applies only to this commit. A new push requires a new judgment.

[iamwhatever]

Too many changes in one PR, need a close look