feat(aws-control): make a running backup a fact the server owns

[chenmingwei23]

Problem / Motivation

Starting a backup in AWS Control executed the whole thing inside the HTTP
request and returned its terminal record. So the fact that a backup of mine is
running existed in exactly one place: the runMut.isPending flag on the React
component that started it.

Unmount that component -- switch a tab, hit the breadcrumb, reload the page --
and the fact was destroyed while the upload kept going. Come back, and the row
said "Back up now" over a backup that was still streaming to S3. Two tabs
disagreed with each other, and a click in the second one started a second paid
upload of the same archive.

There was no backup run identifier and no server-side registry of in-flight
runs anywhere in that path.

Why it matters

A snapshot or sessions backup takes minutes and costs money to upload. The
owner is watching it, and the UI answered their only question -- "is it still
going?" -- by guessing from state it had already thrown away. Three concrete
outcomes: an idle-looking row over live work, a double upload from a second
click, and a gateway restart mid-backup leaving nothing that says the run ever
existed.

What changed (motivation -> approach -> change)

The run becomes a durable, server-owned record. POST /backup/{account}/run
now claims a Job SDK run and returns its id instead of blocking on the outcome;
BackupSection's rows follow it on the shared _jobs surface. A fresh mount
adopts a run it did not start, and sdk.reconcile() resolves a run left
non-terminal by a process that is gone to interrupted instead of serving it as
running.

The runner is a plain def, and that constraint shapes everything below it.
JobSDK._execute calls the runner and discards its return value, and
register() validates the kind but not the callable. An async def runner
would therefore hand back a coroutine nobody awaits: the body would never
execute, nothing would raise, and the record would settle on done for a backup
that never happened. test_the_runner_is_not_a_coroutine_function pins this.

That rules out three coroutines the old resolution path used:
accounts.resolve_account_profile, aws_consent.probe_identity, and
aws_consent.refuse_and_log. asyncio.run inside the worker is not the way
around it -- a second event loop in a worker thread is precisely the #4800
failure this package already documents at accounts.py:62 ("a module-global
asyncio primitive binds to the import-time loop and raises when acquired from
another"), which is why _snapshot_lock is a LoopBoundLock at all. So the
resolution path is sync end to end:

• the account comes from the run's own dedupe_key (see below);
• (profile, region) comes from a new sync
  accounts.resolve_account_profile_cached(), which reads the snapshot the loop
  already built and returns None when it is absent or past its TTL. The
  profile-selection policy is extracted into one _pick_profile() that both the
  async and sync readers call, so the decision of which credentials an
  operation runs under cannot exist in two drifting copies;
• the bucket is re-discovered per run by storage.find_drive (already sync).

Authorization does not move, and the generic entrance is gated more strictly
than the app's own. Once app.json carries "jobs": true, POST /_jobs/snapshot/start is reachable and does not run this app's _require_drive
pre-flight. That is not a hole: the gate has always been
backup._authorize_upload, which runs immediately before every put_file
(backup.py:266 and :446, one per runner, no other call site) and checks the
live sts:GetCallerIdentity against the target account, that the app is still
enabled, that the S3 grant matches this profile and region, and that the
recorded grant's own account field is this account -- a check the route's
_consent (which calls aws_consent.authorize) does not make. So the pre-flight
is now a fast-fail nicety that keeps specific, localisable 409s for an
unreconnected account, unconfirmed S3, or a missing drive; the authorization
argument rests on the worker.
test_a_run_started_off_the_generic_surface_is_still_consent_gated proves it
with no route guard involved.

dedupe_key carries the account, which is identity as well as dedupe.
Calling this out so a reviewer does not have to find it. P1 deliberately has no
params channel -- a runner receives its handle and nothing else -- and
run_id is minted inside start(), so the route cannot stash parameters
under the id before the worker launches. The account is the honest carrier on
its own merits: it is exactly this run's concurrency identity, since two
snapshot backups of one account must not both perform the paid upload. The
SDK's index is (kind, dedupe_key), so a snapshot and a sessions backup of the
same account still run independently. It is also the only field a runner can
read without touching a private attribute (get() is public; the key is
withheld from _public_view and never logged by the SDK). Errors deliberately
do not quote it back. Re-resolving profile/region/bucket per run rather than
carrying them is not a workaround either -- it is the rule this app already
documents for its nightly loop: the drive is tag-discovered per run rather than
trusted from memory.

The terminal ledger is untouched. The Job SDK records that a run existed and
how it ended; what a backup produced -- key, size, when -- stays in the app's
own state via _record_run, and GET /backup/{account} still serves it as
runs. This consumer already had its own result channel, so P1 cutting the
payload channels costs it nothing.

Frontend. BackupSection keeps its existing
['aws-control', 'backup', account] query and derives each row's busy state from
the account-scoped jobs block that query already returns. No shared hook and no
new module: an earlier revision added a useAppJob in website/src/app-sdk/, and
moving the read onto the app's own endpoint left it with zero consumers, so it was
deleted rather than kept (see below). The query polls at 3s only while a run is in
flight, and a start invalidates it immediately so the row turns over without
waiting out the gap.

A run is per-account, so the read is too.
The shared _jobs/active surface is app-scoped by construction -- one app, one
kind, every run -- and it withholds dedupe_key from its public view on purpose,
so a browser cannot tell which account a listed run belongs to. An app whose work
is per-account therefore needs an account-scoped read, and providing one is the
APP's job rather than the SDK's. GET /backup/{account} is already account-scoped
and already what the page reads for that account, so the in-flight run belongs in
its payload: _account_jobs filters on dedupe_key server-side, where the field is
available, and the key still never crosses to the client.

This is not a workaround for a missing SDK feature. Whether the SDK should grow a
scope concept is open (#7590),
and the sibling PR pins the withholding with
test_a_failed_run_does_not_echo_its_dedupe_key -- the SDK does not echo a
caller-supplied string, by design.

Reading the app-scoped surface instead was a real bug, not a hypothetical: with two
connected accounts, account A's running snapshot rendered account B's row as
"Backing up..." and disabled B's own button, so it blocked a legitimate action
rather than merely mis-rendering. The Sage audit predicted exactly this when it
established dedupe_key as write-only across the HTTP surface; this is that
prediction confirmed by the first real integration, and it is
recorded on #7590.

A failed run says so.
The durable record's most useful new information is that a run failed and why, and
the app's own runs ledger only gains an entry when an upload SUCCEEDS -- so a
failed run left it untouched and the row simply stopped spinning, which is itself a
false statement about what happened. The payload now carries lastFailed per kind
for this account, and the row renders the reason. runMut.isError gets a line too,
for a start the route refuses outright.

Two details that decide whether that helps or misleads. Only the NEWEST terminal run
may speak, and only when it did not succeed: reporting the first non-done run
instead would skip past a newer success, so a fail-then-retry showed "last run
failed" directly above the fresh success the ledger had just recorded -- the row
contradicting itself, and persisting until the failure aged out of the window. And
the error is clamped to 180 characters in the view, because the SDK stores up to
2000 and the row renders it in a 12px caption, where an expired-credential botocore
message would blow the line out. The full text stays on the run record.

_job_view is a separate projection on purpose, not an undeduplicated copy.
It answers a different question from job_routes._public_view, and the two already
differ in fields today rather than only in principle: _public_view serves
cancellable, cancelling and the full error, while this one omits both cancel
fields -- the backup runners declare no cancellability, so they would be permanently
false -- and clamps error for the caption that renders it. _public_view also
takes a required cancelling set read from the SDK's live table, which this
endpoint has no reason to compute.

Sharing one projection across two endpoints with different contracts is how a field
leaks into one because the other needed it, and the field at stake here is
dedupe_key -- whose leaking IS the defect this endpoint exists to fix.
test_the_account_never_reaches_the_client guards that on this side, which makes
this a deliberately separate contract with its own proof. Secondarily,
_public_view is private to P1, so importing it would stop P1 changing its own
projection without breaking a consumer it does not know it has.

Authorization precedes any remote work, discovery included.
The job runner resolved the drive with storage.find_drive before calling
_authorize_upload, and find_drive reaches the AWS tagging API -- so a run whose
consent had been withdrawn issued live requests on the owner's credentials while in
the act of refusing the work, and the error shape or timing would tell an
unauthorized caller whether a bucket exists. The gate takes no bucket, so nothing
forced it to wait; it now runs first. This is IN ADDITION to the pre-upload
re-check, not instead of it, and collapsing the two would trade a real check for
tidiness: they answer different questions at different times. The new one decides
whether we may touch AWS at all. The old one decides whether the bytes may leave --
which matters because an archive build runs for minutes inside a worker thread, and
consent can be withdrawn, the app disabled, or the profile repointed DURING that
build. Only a check immediately before put_file can see that. The test
asserts the recorded CALL SEQUENCE is exactly ["gate"] rather than asserting a
refusal, because "it raised" is compatible with having already probed. Reversing the
two lines reddens it with ['discovery', 'gate'].

The polling fix traded paid calls for an unreachable feature, and both belong in
the record. Making the remote half opt-in behind ?remote=1 removed hundreds of
AWS calls per session, but the disclosure that is the ONLY way to request remote
data was itself gated on data.remote -- so the control that enables the fetch
rendered only if the fetch had already happened, and the stored-archive list and
Restore were unreachable for the entire session. The disclosure now renders whenever
status data exists, with a test that it is reachable and openable on a first paint
with remote: null, which is the state the bug lived in. One existing assertion
changed with it, called out here rather than quietly edited to make new code pass: a
test required the disclosure to be HIDDEN when remoteError was set, and that
invariant is what produced the regression. Under opt-in a remoteError can only
exist because the control already requested the data, so "errored and hidden"
describes a state the running app cannot reach -- and worse, it hid the control at
exactly the moment the owner needed it to collapse and retry. The assertion now
requires the control to REMAIN available.

Refusals that name a cause, not just a failure.
The route answers aws_consent_required and drive_missing with codes precisely so
the UI can localise them, so the row maps each to its own cause-and-next-step line
through a literal-key map (the convention already used for KIND_LABEL_KEY here and
UPDATE_ERROR_KEYS in AboutPanel.tsx, which keeps every key visible to the
extractor and the parity gate). Collapsing them into one generic string left the
owner guessing which of several repairs to attempt. invalid_account is
deliberately NOT mapped: the account comes from the page rather than from anything
the owner types, so the start button is not where a malformed one surfaces, and a
bespoke string there would be UI for a state this control cannot produce.

The asymmetry with drive_missing is the reason rather than an exception to it.
Under the rail, DrivePaneGate mounts this section only when the drive exists, so
drive_missing at the click is a RACE -- the drive deleted between the pane
rendering and the press. Rare is not impossible, and a message for a reachable race
is not UI for an unreachable state, which is exactly what separates it from
invalid_account.

jobs_unavailable no longer says "right now" -- it means the job runtime was never
registered, so implying that waiting helps was the same false-hope defect in softer
form.

runId is returned for a reason, and the reason is not the app's own row.
The row follows server state, so it never reads runId. The POST returns it because
a reply that says started: true without saying WHAT started cannot be verified by
anyone -- and the capture harness uses exactly that to assert the run the client
follows is the run the POST created, before and after a re-mount. That assertion is
this PR's evidence that adoption is causal rather than coincidental.

"Try again" only where a retry can clear the refusal.
The start-refusal line would otherwise have promised a retry for every failure, and
of the codes this path answers only one rewards it. Enumerated from the route rather
than special-cased: aws_call_failed is a transient 502 from a live AWS call, and a
transport-level http_5xx is the same shape -- those get "Try again". Lapsed S3
consent, no drive yet, a malformed account and an absent job runtime all need the
owner to do something ELSE, so the generic line promises nothing and
jobs_unavailable says the service is unavailable. Defaulting to "try again" and
excepting one code was the wrong way round; the honest default is to promise nothing.

Telling the owner to take an action that cannot succeed is the UI asserting
something untrue, which is the class this change exists to close -- and it would have
been introduced BY this change rather than inherited, so fixing it finishes the line
rather than widening the diff. One test per branch: transient invites the retry, a
must-act-first refusal does not, and the runtime-absent case says so specifically.

The reason text is clamped for the caption at 180 characters and MARKED when cut,
because a sentence truncated with no sign of it reads as a complete thought that
happens to be ungrammatical. A short error is passed through untouched.

Polling does not cost AWS calls.
Because the page now polls this endpoint while a run is in flight, the remote
listing had to become opt-in. Its remote half tag-discovers the bucket on every
call and then lists the archive, so a minutes-long backup polled every 3s would
have fired hundreds of paid AWS round trips to learn jobs.active, a fact the
server already holds in memory. GET /backup/{account} now returns the cheap half
by default and the archive only for ?remote=1, which the page asks for exactly
when the stored-archive list is open -- the same condition it already gated the
display behind. The poll is otherwise a local read.

useAppJob is deleted rather than kept.
It was written for this one consumer against the app-scoped surface. Once the page
reads its own account-scoped payload the hook has no caller, and a shared frontend
module with no consumer is speculative generality on a surface that is hard to
withdraw later. Its test file goes with it. The page uses its existing
['aws-control', 'backup', account] query, which already fetched this data and now
polls only while a run is in flight.

Not fixed here, and stated rather than left to be found.
(kind, dedupe_key) dedupe only protects paths that go through the SDK. The nightly
loop in hooks.py still calls run_snapshot_backup directly, so nightly plus a
manual click can both upload for one account, and a nightly run is invisible to the
row for the same reason -- it has no SDK record to read. Migrating it is its own
change with its own design question (what a scheduled run should do when the gateway
was down at its hour), so it is filed as
#7783 rather than widened into
this PR.

A refused upload is auditable, not merely a failed run.
Moving the upload off the request path moved the authorization decision off the
audited path with it. routes._audit has already recorded successful by the
time the request returns a run id, and the Job SDK only records that the run
failed -- so a real authorization denial produced no SEL denial anywhere, and a
reader scanning for denials saw nothing. Every refusal in _authorize_upload now
goes through _refuse_upload, which emits sel().log_api_access carrying the
account and the specific reason, then raises so the run still ends failed.

Three choices inside that worth stating. It is emitted at the DECISION, not in the
Job SDK runner as first suggested, because the nightly loop in hooks.py reaches
the same gate without a job at all and a runner-level catch would leave that path
unaudited. It reuses this app's existing event shape rather than a second
convention for the same kind of decision -- importing routes was not an option,
since routes.py imports backup, so it goes through the shared sel() accessor.
And the five access decisions record denied while teardown records failed:
every refusal leaves a trace, because one covered path among several would make the
rest look like non-events, but a routine restart filed as denied would sit beside
a withdrawn consent and devalue every real denial. Both values come from the
vocabulary sel.py documents for that field.

Who triggered it is threaded, not guessed.
Covering the nightly path is precisely what made a hardcoded caller a lie: an
unattended run refused at 03:00 would have been recorded against the dashboard
owner, a person who was not there. That is the same defect as everywhere else here
-- a record asserting something nobody observed -- committed at the audit layer
instead of the record layer.

Flattening both paths to a neutral app:aws-control would fix the lie by throwing
away the truth on the interactive path, where the owner really did trigger the work.
So caller is threaded from each entry point instead: the Job SDK runner passes
CALLER_OWNER because a job exists only because an owner asked through an
owner-gated route, and the nightly loop passes CALLER_SCHEDULED. It is a REQUIRED
keyword all the way down to _authorize_upload, with no default, so a call site
added later cannot inherit whichever guess happened to be written first -- and the
guess written first here was the interactive one.

The capture harness, and a pre-existing defect in it.
website/scripts/capture-aws-control.mjs gains a backup fixture that behaves like
a server which CLAIMED a run -- the jobs block is mutable and the start handler
writes it -- plus the causal phase described below. It also honours ?remote=1, so
the harness exercises the same paid-call split the product ships.

Two things this section previously described are now gone, and a reviewer who read
the earlier version will look for them. Main's be2ee9474 (#7801) rebuilt the
drive as an in-app rail and rewrote this harness for the new IA, which removed both:

• The shared website/scripts/lib/aws-control-fixtures.mjs module is deleted, not
  re-argued. It was extracted so one module answered both the stills and the clip;
  main's rewrite carries its own inline fixtures and has no reason to import it, so
  the module has no consumer left. A shared module whose only caller went away gets
  deleted -- the same call as useAppJob earlier in this PR.
• The usage.sections fixture repair is dropped: main fixed the identical defect
  independently, and its own comment now records that sections is required by
  DriveUsage. Nothing is owed here; the correction simply belongs to main.

The causal phase is re-derived against the rail rather than ported. It clicks
Back up now, then leaves the pane and returns via the rail, and asserts the run
the row follows is byte-equal to the id the start POST returned. The restructure
made this evidence stronger rather than merely different: switching panes now
UNMOUNTS BackupSection outright, so "the indicator is not component state" is
demonstrated by a real unmount instead of a synthetic re-mount, and the pane tree
being keyed by the selected account reinforces the account-scoped premise this
change argues for. A fourth assertion checks the account never crosses to the
client, testing the account ids the fixture actually uses and the dedupe_key field
name -- an earlier version scanned for any twelve-digit run of characters, which the
32-character run id matched, so it flagged its own fixture and would have passed a
real leak just as happily. Both are mutation-checked under Tests.

Dependency and ordering

The P1 Job SDK this consumes has merged. Because the repo squash-merges, that was
confirmed by CONTENT rather than by SHA ancestry: origin/main carries
apps/job_sdk.py and apps/job_routes.py with STARTING and reconcile_all
resolving, and this branch is rebased onto it. merge-base --is-ancestor against
the old base correctly reports false, which is what a squash does to ancestry and
not a sign anything is missing.

A sibling PR, #7737
(fix(apps): three places the Job SDK asserted a fact it never verified), fixes
three SDK defects found by auditing the SDK against real consumers -- including
the one that bears directly on this change: register() accepts an async def
runner and the run records DONE while the body never runs.

This PR is not blocked on #7737, and the ordering is safe either way. The
async def hazard is neutralised here consumer-side: both runners are plain
def, and test_the_runner_is_not_a_coroutine_function fails if that ever
changes. If #7737 lands first this guard becomes belt-and-braces; if this lands
first the guard is what holds. Neither PR touches the other's files.

Tests

Every new test was confirmed to fail before the change and pass after, by
stashing only the source files and keeping the tests. Before: 20 failed, 73 passed, 10 errors. After the rebase onto main: 500 passed across the nine
related backend suites, and 151 passed across the six frontend files.

test/test_aws_control_backup_job.py (new, 28 cases):

• the route returns a runId and does not perform the backup in the request;
• the account is passed as the dedupe key, so a double click adopts the first run;
• an absent job runtime and an unregistered kind are 503 jobs_unavailable, and
  a refused claim is 503 backup_start_failed;
• the runner is not a coroutine function, refuses a kind it does not own, and
  resolves its work function at call time rather than capturing it;
• the runner backs up the account named by its dedupe_key, and re-discovers the
  drive rather than trusting a carried name;
• both keys the generic start route can produce fail with a clean recorded
  error and touch no AWS: absent (dedupe_key defaults to "") and a
  well-formed string that is not an account id;
• a failed run's error does not echo the dedupe key;
• an unreachable account fails with no fallback to another account's credentials;
• a run started off the generic surface is still consent-gated, and put_file is
  never reached;
• a record left non-terminal by a foreign origin becomes interrupted and
  drops out of list_active; reconcile is idempotent so the gateway's own
  post-enable-loop pass finds nothing left;
• on_startup registers both kinds, does so even when the nightly task is
  already live (a re-enable brings a fresh SDK with an empty runner table), and
  survives a context with no job runtime;
• app.json declares the jobs grant;
• the terminal ledger still records a successful run, and GET /backup/{account}
  still serves it.

website/src/apps/aws-control/DrivePage.test.tsx (63 cases, extended): adoption on
a fresh mount from the account-scoped payload, per-kind scoping with the sibling row
left usable, a failed run rendering its reason while the button stays enabled, and
an absent jobs entry reading as idle rather than unknown.

website/src/apps/aws-control/DrivePage.test.tsx: two new cases -- a backup
shows as running on a fresh mount with no click in this session, and only the
kind that is running is adopted, leaving the sibling row usable.

Four existing tests in test/test_aws_control_routes.py pinned the removed
blocking contract (ran: true, the 409/502 runner-error mapping) and were
rewritten to the new one, not deleted.

The capture harness asserts rather than just saving PNGs, so a stale bundle or a
changed surface fails the run instead of quietly photographing the old page: the
idle row is enabled, the adopted row is disabled and its label contains "Backing
up", the sibling row stays enabled, and nothing overlays the row. It exits
non-zero on any mismatch -- which is how the fixture defect above was found.
51 assertions pass, 0 mismatches.

Nine of those are the causal phase, and the load-bearing one was mutation-checked
rather than assumed: making the stub return an id different from the run it starts
turns causal run in flight is the one the POST returned and causal same run after the re-mount red and the harness exits 1. Without that check the assertion
could have compared a value against itself and passed while proving nothing.

TestEveryUploadRefusalIsAudited covers the audit fix per refusal path rather than
once -- five parametrised access decisions plus teardown -- because an audited
consent refusal beside a silent identity mismatch would make the mismatch look
like a non-event. Each case puts every other check in a passing state so it fails
exactly one, and asserts the outcome, the account and the specific reason reach the
record. A ratchet, test_no_refusal_path_bypasses_the_audited_helper, reads the
function's own source and fails on any raise that does not go through
_refuse_upload, so a path added later cannot be silent -- which is the failure
mode that made the original defect invisible. Mutation-checked: replacing one
audited refusal with a bare raise reddens both the ratchet and that path's case.

Gates: flake8, black --check, tsc --noEmit and eslint clean on the changed
files, and jscpd reports 0 clones (extending the existing harness rather than
adding a second one is what keeps it there -- the duplication threshold is 0%).

Manual verification

Captured, with one limit stated below rather than left for a reader to find.

The behaviour cannot be filmed against an unprovisioned account, and the reason is
mechanical rather than a matter of effort. Through the generic _jobs/{kind}/start
route a run is created for real, but the worker's first act after flipping to
RUNNING is resolve_account_profile_cached, an in-memory read that returns
None with no account snapshot and raises immediately. The whole
RUNNING-to-FAILED window is two atomic_writes and a dict lookup -- single-digit
milliseconds -- while the page polls at 3000ms, so no poll can catch it in
flight. A live film of the navigate-away-and-back beat needs a provisioned AWS
account with a tag-discovered drive and a backup still uploading across the
navigation.

The screenshots below take the other route: the stub holds _jobs/active in a
state a live unprovisioned gateway cannot hold still. That is legitimate here
because the thing under test is the UI following a server-owned record, and the
stub supplies exactly the server's half of that -- it does not fake a backup.

Screenshots / video

Two frames of the same backup rows, from the repo's mocked-API harness
(website/scripts/capture-aws-control.mjs), with no gateway, no credentials and
no AWS call anywhere in the capture. The only difference between them is what
/api/apps/aws-control/_jobs/active answers: nothing in the first, one running
snapshot run in the second.

The two frames are nearly identical, and the whole delta is one control. It is
the button at the right of the first row, Memory & workspace -- the row that
carries the snapshot kind. It reads Back up now and is enabled in the first
frame; it reads Backing up... and is disabled in the second.

The strongest thing in the pair is the row that did not change. Sessions archive offers Back up now, enabled, in BOTH frames. So the busy state is
scoped to one run and is not a page-level or section-level disable -- the host
indexes a run by (kind, dedupe_key), and the second frame is what that indexing
looks like from the outside. Without this, a reviewer would have to take the
scoping on faith.

The clip carries the one thing no still can: that a navigation happened.

backup-adoption-flow.mp4 -- 29s, silent, same mocked harness

Six beats, answered by the same fixtures as the stills: the rows idle; Back up
now clicked; the row reading Backing up...; the Backup section left entirely
for the drive root; the section re-entered; and the row still reading Backing
up... on that fresh mount.

The footage is of THIS diff's route as a measured fact rather than as a consequence
of the build steps having been correct: before filming, the recorder counts the
route strings in the served AwsControlPage-*.js chunk and requires
_jobs/active = 0 and /backup/ > 0. An earlier cut of this clip was recorded
against _jobs/active, which this diff deletes -- identical pixels, but evidence of
a path that is no longer here. Counting the shipped bundle is what makes the
difference checkable instead of asserted.

The click is causal, not staged. The stub mints the run and returns its id the way
_handle_backup_run does, and the run the row then follows is that same run. That
part is not visible in the frames -- a run id does not render -- so it is
asserted by the capture harness rather than claimed here: causal the run the client follows IS the one the POST returned reads the id out of the
GET /backup/{account} response the browser actually receives and compares it to
what the POST answered, before and after the re-mount, alongside a check that the
account does NOT cross to the client. What the clip carries is the flow; what the
harness carries is the identity. The alternative -- swapping a run in beside a click
that did not create it -- would have depicted a sequence rather than the code path,
so it was rejected.

What the media does and does not establish: the frames and the clip exercise the
hook and the row against a stubbed API. Nothing here is a real backup -- no AWS
account, no S3 call, nothing uploaded. A still also cannot show the spinner
turning; what it shows is the disabled control and its label.

One thing that looks wrong at a glance and is not: in the second frame Not
backed up yet still sits under a row reading Backing up.... That is the design
-- the Job SDK records that a run exists, while the app's own ledger records what a
run produced, and the ledger gains its entry only when the upload finishes.

End-to-end durability is not what these frames prove. A gateway restart resolving
an interrupted run, the runner's refusals, and the consent gate are covered by
test/test_aws_control_backup_job.py.

Related Issues

Spec: docs/system-specs/features/app-sdk-durable-jobs-and-view-state.md,
sections "Current SDK boundary" and "AWS Control backups". Sibling:
#7737 (not a blocker -- see
Dependency and ordering).

Any other suggestions on the work

Five things deliberately left out of this diff, each with its reasoning:

1. _jobs/active cannot be filtered by account. dedupe_key is withheld
   from _public_view, so the client cannot tell whose run it is. Driving two
   accounts' snapshot backups concurrently would show one account's run as
   running on the other's page. Not fixable client-side without serving
   dedupe_key, which is an SDK change and out of scope here. Narrow today: the
   nightly loop runs against the registry-default account only, and is not
   SDK-routed (see 2).
2. The nightly loop still calls run_snapshot_backup directly. Consequence:
   a nightly backup does not appear as a job in the UI, and nightly can overlap
   a manual click on the same account, which the dedupe key would otherwise
   prevent. Routing it through sdk.start is a few lines and worth doing; kept
   out of this PR to keep the diff to the surface the DoD names.
3. A failed run has no UI surface. Parity with the previous code, which
   rendered nothing for a failed runMut either. Showing it needs a new i18n key
   across 13 locales plus the parity and ratchet gates -- a small, separate
   change.
4. cancellable is left at its default of False. Neither backup runner
   polls handle.cancelled; the only stop signal they honour is the teardown
   event checked in _authorize_upload, which is not a cancel checkpoint. The
   SDK cannot verify the app's assertion, so claiming True would put a Cancel
   button in front of the owner that does nothing. The UI hides it instead.
5. No shared frontend surface is added. An earlier revision put a
   useAppJob hook in the app-sdk; moving the read onto the app's own
   account-scoped endpoint left it with no consumer, so it was deleted rather
   than published. A shared module is worth adding when a second consumer exists.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable) -- the spec on main already describes
  this consumer; no doc change needed in this PR
• No secrets, credentials, or internal references in the diff

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

UX-Verdict: PASS

Durable server-owned run state fixes the real UX lie — an idle row over a live backup — and refusals now name a next step.

Suggestions

• backup_start_unavailable ("Check that the job runtime is registered.") is mechanism vocabulary the owner cannot act on; replace the second sentence with a user action, e.g. "Reinstall or update the AWS Control app."
• backup_start_consent names the fix but not where: append the location ("Grant S3 access in the Access pane, then start the backup.") — the pane is one rail item away and the string currently makes the owner hunt.
• backup_failed ("Last run failed: {{reason}}") fronts "failed" over interrupted runs too, and {{reason}} is stored runner text — for a genuine exception that's raw botocore prose in a 12px caption. A neutral prefix ("Last run did not complete: …") or mapping the common expired-credential case to a plain line would keep the label honest.

[UX-REVIEWED] ae601aa

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

Server-owned run state is the correct root-cause fix; every design compromise (dedupe-key-as-identity, nightly gap, no cancel) is deliberate, tested, and tracked upstream.

[DESIGN-REVIEWED] ae601aa

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of ae601aaca79fcb7f3f35ffc0dd2204a8e7466abf and found no blocking issues.

This comment is updated in place on each push.

Review details

FINDING -- src/kiro_crew/apps/builtins/aws_control/backend/routes.py:1290 -- limit=20 truncates before account filtering, so 20 newer runs for another account hide this account’s latest failure -> Fix: filter by account before selecting the newest terminal run.
FINDING -- src/kiro_crew/apps/builtins/aws_control/backend/backup.py:565 -- “register() validates the kind and not the callable” contradicts the SDK’s callable validation and undriven-result rejection -> Fix: update the paragraph to match the SDK contract.
[GPT-REVIEWED] ae601aa

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

[github-actions]

First Principles Review (Fable 5) — 🟡 CONCERNS

Premise-level review of ae601aaca79fcb7f3f35ffc0dd2204a8e7466abf — 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.

Verification is done. Key checks: temp-screenshots/ binaries follow a documented convention (.gitignore:97 — "committed deliverables", 639 tracked files); _job_view genuinely differs from job_routes._public_view:95 (4 fields plus a required cancelling set), so it is not a second spelling; the three frontend error codes are all reachable (routes.py:328, :595, plus the new 503); and the Job SDK (P1) pre-exists at base. The one depth gap: the nightly loop still runs backups outside the Job SDK.

First-Principles-Verdict: CONCERNS

The fix is real and cause-level, but the nightly backup is still a running backup the server does not own — the thesis's one counted unfixed sibling.

What this change ships

Intent: stop a running backup's existence from living only in the browser tab that started it — a FIX (double paid uploads, idle rows over live work) delivered by integrating the pre-existing Job SDK.

1. Starting a backup returns a run id immediately; the work runs server-side — justified
2. A reload, second tab, or fresh mount shows the run still going — justified
3. A second click adopts the running upload instead of paying for a second one — justified
4. A run orphaned by a dead gateway shows "interrupted", never "running" — justified
5. A failed backup now says why on the row (12 locales) — justified
6. Remote archive listing becomes opt-in (?remote=1) — changed default, derived from 3s-poll × paid AWS calls
7. Stored-backups disclosure stays visible after a remote error — rides along, derived from item 6
8. Upload refusals now leave an attributed SEL denial record — justified (gap created by moving work off the audited request path; SEL is a keep-listed control)
9. Manifest gains the jobs grant, opening the generic _jobs start surface — declared, gated in-worker with tests
10. Start failures answer 503 with machine codes — justified (AGENTS.md code-field rule)

Watch

• One unfixed sibling of the root cause: hooks._run_once still calls run_snapshot_backup directly (grep for call sites outside the runner: 1, hooks.py:88), so a nightly run is invisible to the row and undeduped against a manual click — the double-paid-upload harm survives on exactly that pairing. Routing nightly through sdk.start needs a caller channel P1 lacks (CALLER_OWNER is hardcoded in the runner), so this reads as accepted-and-deferred; it should be a named follow-up, not silent.
• _job_view was verified as a distinct contract, not a copy of job_routes._public_view:95 (4 differing fields, dedupe-key containment) — no action, recorded so the next reviewer doesn't re-litigate it.

[FIRST-PRINCIPLES-REVIEWED] ae601aa

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

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

Review details

Based on my verification: list_recent(kind, limit=20) in job_sdk.py:1227 is confirmed cross-account (no dedupe_key scoping, sorted newest-first, capped at 20), so Candidate 1's mechanism holds. Candidates 2 and 3 are UX message/retry-classification judgments the author explicitly reasoned about in-code; neither reaches the defect bar.

No findings that block the merge.

FINDING — src/kiro_crew/apps/builtins/aws_control/backend/routes.py:441 — lastFailed reads sdk.list_recent(kind, limit=20) then filters r.dedupe_key == account, but list_recent is cross-account, so with ≥20 newer runs of that kind on other accounts an account's genuine most-recent failed run falls outside the window → newest is None → failed = None, and its failure banner silently disappears while the app ledger (successes only) also shows nothing. → Fix: scope the recency scan to this account (scan/page until this account's newest run is found, or query account-scoped) rather than taking the first dedupe_key match inside a fixed cross-account 20-run window.

[OPUS-REVIEWED] ae601aa

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

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

[buluoray]

Verdict: 0 blocking, 2 non-blocking. The ownership move from client to server holds up under scrutiny — the run becomes a durable, server-owned record and every failure/restart/concurrency path is covered by the pre-existing Job SDK, which is genuinely at base.

What I verified

Read against head ae601aaca79fcb7f3f35ffc0dd2204a8e7466abf (SDK read at base 4098c32, where apps/job_sdk.py already lives — P1 is merged, not stacked):

• Restart recovery (a): an orphaned run does NOT wedge the feature. list_active (job_sdk.py:1221) returns any non-terminal record from disk with no live/origin filter, so recovery depends on reconcile. It is wired twice: the app's own _register_job_runners calls sdk.reconcile() at on_startup (hooks.py), and the gateway calls reconcile_all() after the whole enable loop (hooks_integration.py:597). A dead process's running record is flipped to interrupted. Pinned by test_on_startup_resolves_an_orphan_before_the_ui_can_adopt_it and test_a_run_left_running_by_a_dead_process_is_resolved (assert INTERRUPTED + list_active()==[]).
• Concurrency exclusivity (b): genuinely atomic, not a read-then-write race. JobSDK.start (job_sdk.py:926) does the _keys.get(key) check and the _keys[key]=run_id claim inside one self._lock critical section with no I/O; a second start adopts and returns the existing id.
• Flag cleared on failure/exception (c): _execute's finally (job_sdk.py:~1180) always calls _write_terminal and pops the dedupe key from _keys — success, failure, exception, and undriven-coroutine paths all clear it. A lost terminal write is caught by reconcile.
• Durability (d): file-per-run via atomic_write (job_sdk.py:680); the PR body's "durable, server-owned record" claim matches the code.
• Authorization not weakened: _authorize_upload (backup.py) runs in-worker before drive discovery and again before put_file, re-checking live account, app-enabled, S3 grant, and that the grant names this account. test_a_run_started_off_the_generic_surface_is_still_consent_gated drives the real gate with consent withdrawn and asserts put_file.assert_not_called().
• Account never crosses to client: _job_view (routes.py) omits dedupe_key; test_the_account_never_reaches_the_client guards it.
• Frontend/i18n (f): the six new backup_* strings go through en.json + 11 locales; CI Automated Rule Check / Inclusive Language / De-Amazon / Frontend Lint all green. Nightly path correctly attributes caller=CALLER_SCHEDULED.

Findings

1. routes.py:1290 (non-blocking) — cross-account truncation can hide a lastFailed banner. _account_jobs reads sdk.list_recent(kind, limit=20) then filters r.dedupe_key == account, but list_recent (job_sdk.py:1227) is cross-account. With ≥20 more-recent runs of the same kind on other accounts, this account's genuine most-recent failed run falls outside the window → newest is None → the failure caption silently disappears. Consequence: display-only degradation of a new secondary surface — the run record itself persists and active-run tracking is unaffected — which is why this is non-blocking (and why GPT 5.6 and Opus 4.8 independently rated it so). Suggestion: scope the recency scan to the account (page until this account's newest run is found, or query account-scoped) rather than a fixed cross-account 20-run window.
2. hooks.py (nightly loop, non-blocking) — nightly run bypasses the SDK. _run_once calls run_snapshot_backup directly rather than sdk.start, so a nightly run is invisible to the row and undeduped against a manual click; on the exact nightly+manual pairing the double-paid-upload harm this PR targets survives. Routing nightly through sdk.start needs a caller channel P1 lacks (CALLER_OWNER hardcoded in the runner), so this reads as accepted-and-deferred. Suggestion: track it as a named follow-up issue rather than leaving it silent (matches the First Principles CONCERNS).

What I could not verify

• End-to-end gateway restart on a live process — verified through reconcile logic + unit tests only, not a running gateway.
• The 12px caption rendering of {{reason}} / raw botocore text (UX Review advisory suggestions) — cosmetic, not visually verified.
• Whether AGENTS.md requires a docs/system-specs/ update for the new app-scoped jobs/lastFailed endpoint shape; I did not re-read AGENTS.md line-by-line, but the SDK spec pre-exists and the PR Scope + Detect-changed-surface CI gates are green.