Skip to content

ADFA-365 - Project build Test for java and Base project tamplate - #3

Merged
romanlapa merged 3 commits into
stagefrom
feature/ADFA-365-java
Feb 10, 2025
Merged

romanlapa merged 3 commits into
stagefrom
feature/ADFA-365-java

Conversation

@YahorZCzygunki

Copy link
Copy Markdown
Contributor

No description provided.

- add Manifest file to the e2e tests to support an application class
- fixed problem with toolmanager
- finished test
@romanlapa
romanlapa merged commit cba8b78 into stage Feb 10, 2025
@hal-eisen-adfa
hal-eisen-adfa deleted the feature/ADFA-365-java branch July 4, 2025 16:40
fryanpan added a commit to fryanpan/CodeOnTheGo that referenced this pull request May 8, 2026
Smoke-tests the half of the plugin -> IdeTemplateService -> New Project
grid pipeline that lives on the plugin side: build a .cgt via the same
CgtTemplateBuilder surface FormsPlugin uses on activate / wizard finish,
then unzip and assert that templates.json, the per-template template.json,
the .peb-suffixed template files, and the static asset files all land at
the paths ZipTemplateReader / ZipRecipeExecutor look for.

Pins the Pebble syntax conversion ({{ APP_NAME }} -> ${{ APP_NAME }})
since the runtime Pebble engine is configured with the ${{ }} print
delimiter — a regression here would silently emit unrendered template
files into generated projects.

Pins sanitizePath rejecting absolute paths and `..` traversal — the
review checklist's appdevforall#3 (path traversal) is a recurring flag and this
keeps the plugin's contribution honest.

Pulls plugin-api into the test classpath so the tests can reach
CgtTemplateBuilder directly without mocking PluginContext.
fryanpan added a commit to fryanpan/CodeOnTheGo that referenced this pull request May 8, 2026
Maps shipped 0 unit tests; both Forms and XKCD ship JVM unit tests as the
local style. Add a JUnit 4 scaffold with three test files covering the
load-bearing math + safety claims:

  - RegionCacheTest: regionId allowlist (path-traversal vectors per
    REVIEW2.md MB1 / theme appdevforall#3), delete idempotence, symlink-escape
    refusal, malformed/missing meta.json fallback, list-sort,
    bbox parse/drop semantics. 17 tests.
  - BboxTest: construction guards (anti-meridian rejection), boundary-
    inclusive contains(), width/height/area math at equator and 45°,
    aroundPoint() longitude scaling at high lat, lat clamp at poles,
    haversine sanity (SF↔NYC). 16 tests.
  - TileEstimatorTest: known-good slippy counts (z=0/1/2 worldwide,
    z=0..2 sum, mid-latitude box at z=14), zoom range guards, ~4×
    growth per zoom level, MB conversion. 9 tests.

The exercise surfaced a real correctness bug in TileEstimator.estimate:
the destructuring `val (xMin, yMax) = lonLatToTile(west, north, z)` /
`val (xMax, yMin) = lonLatToTile(east, south, z)` had y reversed —
slippy maps NORTH to LOW y, so NW→(xMin, yMin) and SE→(xMax, yMax). The
bug made `(yMax - yMin + 1)` always negative; `coerceAtLeast(0)` then
silently produced **zero tiles for every estimate**. Fix is the obvious
swap; tests assert the published z=0/1/2 worldwide counts (1, 4, 16).

Also adds:
  - `Bbox.contains(lat, lon)` (boundary-inclusive) so callers don't
    re-implement the lat/lon `in` check.
  - `Bbox.areaKm2()` for plan §5.2 size-estimate math.
  - `RegionCache.listFromRoot(root)` / `deleteFromRoot(root, id)` /
    `readDir(dir)` overloads taking explicit roots, so JVM tests can
    bypass `Environment.getExternalStorageDirectory()`. Production
    callers go through the no-arg variants unchanged.

Total: 42 tests, all passing. Build green for both `assembleDebug` and
`testDebugUnitTest`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jatezzz pushed a commit that referenced this pull request Jun 22, 2026
ADFA-365 - Project build Test for java and Base project tamplate
jatezzz added a commit that referenced this pull request Jul 17, 2026
Fan-out, adapter, and plugin-API fixes from the ProjectSearchExtension review:

- Guard plugin results against null elements (mapNotNull) and wrap the collection block in a logging try/catch so a bad element no longer kills the whole fan-out silently. (#1)
- buildRows buffers each section's rows and emits the header only when a group exists, skipping blank titles -- no more orphan/blank headers suppressing the empty state. (#2)
- Move the fan-out to lifecycleScope so a slow plugin can't pin a destroyed Activity/ViewModel or post to a dead Activity. (#3)
- Keep the search progress indicator up when the built-in search is empty but plugins are enabled, so a plugin-only match doesn't flash a terminal "no results" state. (#4)
- SearchListAdapter is now a ListAdapter + DiffUtil reused across emissions; the two-phase publish diffs instead of reallocating, preserving scroll and already-run highlights. (#5)
- Collapse the adapter constructors to one clicks ctor plus a thin map convenience ctor; drop the dead keys param and nullable-value scrub. (#6)
- Replace the blocking .get(10s) with a non-blocking withTimeoutOrNull + await so no commonPool worker is tied up. (#7)
- Trim pipe-split filter tokens at the source (endsWith(" kt") matched nothing in built-in search too); align ProjectSearchExtension KDoc with actual behavior and drop the non-ASCII em-dash. (#9, #10)
jatezzz added a commit that referenced this pull request Jul 20, 2026
Fan-out, adapter, and plugin-API fixes from the ProjectSearchExtension review:

- Guard plugin results against null elements (mapNotNull) and wrap the collection block in a logging try/catch so a bad element no longer kills the whole fan-out silently. (#1)
- buildRows buffers each section's rows and emits the header only when a group exists, skipping blank titles -- no more orphan/blank headers suppressing the empty state. (#2)
- Move the fan-out to lifecycleScope so a slow plugin can't pin a destroyed Activity/ViewModel or post to a dead Activity. (#3)
- Keep the search progress indicator up when the built-in search is empty but plugins are enabled, so a plugin-only match doesn't flash a terminal "no results" state. (#4)
- SearchListAdapter is now a ListAdapter + DiffUtil reused across emissions; the two-phase publish diffs instead of reallocating, preserving scroll and already-run highlights. (#5)
- Collapse the adapter constructors to one clicks ctor plus a thin map convenience ctor; drop the dead keys param and nullable-value scrub. (#6)
- Replace the blocking .get(10s) with a non-blocking withTimeoutOrNull + await so no commonPool worker is tied up. (#7)
- Trim pipe-split filter tokens at the source (endsWith(" kt") matched nothing in built-in search too); align ProjectSearchExtension KDoc with actual behavior and drop the non-ASCII em-dash. (#9, #10)
jatezzz added a commit that referenced this pull request Jul 20, 2026
…#1528)

* feat(search): add ProjectSearchExtension API for plugin result sections
Fan out to enabled plugins after built-in search and render their sections.

* style: apply spotless formatting to search extension files

* fix(search): address review findings on plugin search extension

- Bound plugin search fan-out with a 10s timeout and drop results from superseded searches via a generation counter in EditorViewModel
- Guard against Java plugins completing their future with null
- Flatten SearchListAdapter's nested RecyclerView into Row.Match rows so match rows recycle instead of inflating all at once
- Narrow catch(Throwable) to catch(Exception) per REVIEW.md
- Document plugin-api search contract types; coordinates are 0-based, end-exclusive
- Marshal plugin crash recording to the UI thread and drop the unread searchResults flow.

* fix(search): address review feedback on search results list

- Make the group row's LinearLayout the layout root so the click listener on binding.root receives taps (the clickable child was consuming them) and drop the redundant RelativeLayout wrapper
- Log highlight failures via SLF4J and keep the plain-text preview instead of overwriting the line with the matched text
- Skip files with empty match lists to avoid dangling group headers
- Hide the decorative file icon from accessibility services

* fix(search): address code-review findings on plugin search

Fan-out, adapter, and plugin-API fixes from the ProjectSearchExtension review:

- Guard plugin results against null elements (mapNotNull) and wrap the collection block in a logging try/catch so a bad element no longer kills the whole fan-out silently. (#1)
- buildRows buffers each section's rows and emits the header only when a group exists, skipping blank titles -- no more orphan/blank headers suppressing the empty state. (#2)
- Move the fan-out to lifecycleScope so a slow plugin can't pin a destroyed Activity/ViewModel or post to a dead Activity. (#3)
- Keep the search progress indicator up when the built-in search is empty but plugins are enabled, so a plugin-only match doesn't flash a terminal "no results" state. (#4)
- SearchListAdapter is now a ListAdapter + DiffUtil reused across emissions; the two-phase publish diffs instead of reallocating, preserving scroll and already-run highlights. (#5)
- Collapse the adapter constructors to one clicks ctor plus a thin map convenience ctor; drop the dead keys param and nullable-value scrub. (#6)
- Replace the blocking .get(10s) with a non-blocking withTimeoutOrNull + await so no commonPool worker is tied up. (#7)
- Trim pipe-split filter tokens at the source (endsWith(" kt") matched nothing in built-in search too); align ProjectSearchExtension KDoc with actual behavior and drop the non-ASCII em-dash. (#9, #10)

* style(search): expand when branches to block form for ktlint

Spotless ratchets from origin/stage; stage was reformatted after this branch diverged, so CI flagged the single-expression when branches in onUpdateWadbConnectionStatus and the multi-line FileOpResult.Success branch. Expand them to ktlint's canonical block form so the file is fully canonical and passes under any ratchet base.

* fix: reported violations for spotless
fryanpan added a commit that referenced this pull request Sep 5, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Sep 6, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Sep 8, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Sep 14, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Sep 16, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Sep 20, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Sep 24, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Oct 1, 2026
…completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2
fryanpan added a commit that referenced this pull request Oct 2, 2026
…pp (#1716)

* ADFA-4128: qb 04/12 runtime — Inside the proxy app: swaps code, resources and assets into the running process

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W

* ADFA-4128: qb 04 review fixes — reload failure attribution + surfacing

- Stale pendingReloadGeneration mis-blaming later crashes: the backgrounded
  apply now assigns the pending slot too (Generations.pendingAfterApply), and
  BootProbation.generationToBlame refuses a pending value the store has moved
  past. Covered by BootProbationTest.aPendingReloadTheStoreMovedPastIsNotBlamed
  (fails without the fix) and GenerationsTest.aBackgroundedApplyClearsThePendingSlotItAlreadyAcked.
- failReload swallowing every pre-apply failure: the newer-generation guard is
  now a three-way Generations.onReloadFailure — never-applied failures skip the
  rollback/quarantine but still reportCrash + banner; only a failure superseded
  by a newer live generation stays silent. Covered by
  GenerationsTest.aFailureTheStoreNeverAdoptedStillReports.
- Binder-thread setProviders + immediate provider close racing main-thread
  inflation: ResourceStore now performs the field swap, setProviders and the
  close of the replaced provider on the main thread (inline when already there,
  so the boot restore path still lands before first inflation; Looper FIFO keeps
  a posted swap ahead of the posted recreate). Pure threading with no JVM seam —
  justified in swapProvidersOnMain's doc; device-covered.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Kj9YeCDHGp9DU8LPtfWJ7W

* ADFA-4128 (4/11): address CodeRabbit review

- F1716-2 heal a half-finished asset merge on the next run
- F1716-5 stop answering a VirtualMachineError with another allocation
- F1716-7 take the asset length from the descriptor already open
- F1716-8 un-commit a resource provider swap that failed to install

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FstXxJ5cwWPcvmhZ9vJgJ7

* ADFA-4128 (4/11): address Akash's review

Fixes for every finding on the runtime module, plus two changes that came out of
reviewing them.

Crash banner. The copy said "New code crashed", which named the one event this
banner cannot observe: the CRASHED state is set only from failReload, so it is
always the reload machinery that failed, never the user's own code. It also
carried a stack summary it had no room for. It now reads

  Live reload crashed. App is on the last working version.
  For more info, see Build Output in Code on the Go.

and points at the pane where the full text already goes unchanged. At the
narrowest width measured on an A56 at 2x font scale that is five rendered lines,
four from 28 characters up; MAX_BANNER_LINES stays at 6, one line of slack,
because the line a tighter cap drops is the tail of the pointer - which leaves
the reader told to look somewhere without the name of the place.

Crash report. It walks up to three causes and prints each one's frames, not just
its toString. An Android lifecycle crash always arrives wrapped, so the top
frames are ActivityThread's every time and the line naming the developer's bug
sits in the cause; reporting the message alone named the exception without ever
placing it.

markGood retry. lastMarkedGoodGeneration was set before the write was attempted,
so a failed markGood was never retried and its latch blocked every later one for
the process lifetime, and the KDoc's justification was inverted. Clearing the
latch on a bare false is not safe either - markGood answers several situations
with one false, and persist runs before apply, so meta.json is briefly ahead of
the live generation on every deploy. markGoodCanSucceed separates a failed write
from a store that moved on, and only the failed write clears.

Payload overtake. onPayload is oneway, so a slower older deploy can be overtaken
while it reads its payload and then publish itself over the newer one, leaving
disk a generation behind the running process until the next cold boot adopts it.
PayloadStore.apply already refuses a generation that is not strictly newer, so
this could never reach the screen - only disk. PayloadPersistence now keeps the
highest generation this process has published and refuses anything older,
throwing StalePayloadException so the deploy path can tell a lost race from a
broken store and stay silent about it. The bar rises only after the publishing
rename, so a persist that threw part-way does not block its own retry.

That guard also separates the two cases a generation number alone conflates. A
restarted host counter - the project's state dir wiped while the app stays
installed - always arrives in a process that has published nothing, so the mark
is zero and the low generation is adopted as before. Both counter-restart tests
now build a fresh store object over the same directory, which is the only shape
that case has on a device.

Payload memory. The resource apk and the assets zip were read whole into memory
and written straight back out to files that are reopened as files afterwards, so
a cold deploy held two payload-sized arrays live for no benefit on the devices
least able to spare them. persist now takes both as streams and copies them
through a 16 KB buffer into the same temp-then-fsync-then-rename write. Only the
dex stays a byte array, because InMemoryDexClassLoader needs one. The 64 MB cap
is unchanged and now guards the streaming path; the parameter types are what
keep it that way.

Also from Akash: the manifest-merger comment, the KDoc corrections, and the test
helper that divided length by width - it modelled a renderer that breaks
mid-word, so it read the banner's six real lines as five and could not have
caught the overflow it existed for. It wraps on words now, and was watched
failing at the old cap before the cap moved.

Both new gates were watched red first: the payload cap with its check stubbed
out, the overtake refusal with its condition forced false. Only the intended
test failed each time. 248 tests green.

Banner inset. Photographing the new banner at 2x font scale showed it drawing
over the status bar: getRootWindowInsets() comes back null on the first render
after a config-change recreate, and the overlay took that as a 0 inset. A null
read now means "not measured yet" - the margin is left alone and re-read after
the next layout, once, by a listener that removes itself. The decision is a pure
static so it can be unit-tested; the deferred re-read firing is checked on a
device (A56: banner flush below the 101 px bar at 1.0 and 2.0).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xsc7AMGBVyEMfrwpZX87iC

* ADFA-4128 (4/11): drop the unused IQuickBuildHost.disconnect AIDL method

The runtime only ever disconnected by process death, which
ProxyAppConnections.onDisconnected already handles. Asked for in review on #1718.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xsc7AMGBVyEMfrwpZX87iC

* ADFA-4128: qb-04 review fixes - overlay observer capture, BUILD_FAILED clamp, streamed apk write, resource-attach latch, log overload

Akash's 08-31 review of #1716, items 1/2/3/5/6 (4 and 7 deferred to
followup tickets per the triage):

- StatusOverlay: capture the ViewTreeObserver at add time; on GIVE_UP the
  banner is detached and a re-fetch returns a floating observer, making the
  removal a silent no-op. isAlive() guard with a decor-observer fallback.
- OverlayState/CrashSummary: BUILD_FAILED detail clamped to a one-line
  budget (BUILD_FAILED_DETAIL_CHARS) and the Build Output pointer appended,
  same shape as CRASHED; budget arithmetic documented from both ends.
- LegacyResourceSwap: stream the resource apk to disk instead of buffering
  it in heap (small-heap API 28/29 devices); partial file deleted on a
  failed copy so the throw-means-nothing-written contract holds.
- ResourceStore: attachedAppResources latched only when the attach
  succeeded, so a swallowed failure is retried on the next deploy.
- QuickBuildClient: two-arg RuntimeLog.w keeps the stack trace.

Tests: clamp test verified red before the fix; partial-file-delete test
added; three edge tests updated for the pointer line. 255 green.

Also: plain-language pass over the comments added by these fixes

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01STCsdMzx9daNBcqMN424Ci

* ADFA-4128: 0902 review round on quickbuild:runtime

Ordering and lifetime fixes from Akash's 2 September round, plus the nitpick
sweep.

- The failed reload's decision and its rollback now happen under one lock
  (PayloadStore.restoreIfCurrent), so a deploy that applies between the two is
  no longer rolled back by a decision taken before it existed.
  #1716 (comment)
- A generation whose resource swap failed is marked before the failure is
  dispatched off-thread, and the posted recreate skips it. Moving failReload
  off the looper had left the recreate free to render the generation the
  rollback was undoing, and the mark also covers the inline swap, which fails
  before the recreate is posted at all.
  #1716 (comment)
  #1716 (comment)
- Provider swaps carry their generation and drop an overtaken one instead of
  installing it, so a slower deploy's swap landing last cannot put the older
  table back under the newer generation's label.
  #1716 (comment)
- The connect() handshake - the one non-oneway host call - runs off the
  binding callback's thread, so a cold CoGo no longer blocks the proxy app's
  main thread.
  #1716 (comment)
- Asset extraction is capped cumulatively at MAX_PAYLOAD_BYTES, matching every
  other payload path.
  #1716 (comment)
- The five fallback catches in the component factory rethrow fatal errors, so
  an OOM is grouped as itself rather than under the payload's throwable.
  #1716 (comment)
- writeAtomic no longer short-circuits on the delete: a first write, where
  there is nothing to delete, skipped the retry and leaked the temp file.
  #1716 (comment)
- PayloadStore's last throwable-concatenation site takes the two-arg log.
  #1716 (comment)
- LegacyResourceSwap's KDoc names the test that exists.
  #1716 (comment)
- The fail-reload dispatch test's KDoc claims what the test pins - the helper's
  contract - and says what it does not.
  #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* style: spotless reformat of QuickBuildClient, no functional change

The Eclipse formatter's member sorting moves connectToHost below its caller.
Standalone so it does not read as a behavioural change.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: hold a generation blamable until its first frame has drawn

Answers review thread 3926539435 (CRITICAL) on PR #1716.

onResume precedes the first traversal, so nothing that vouches for a payload
could legitimately happen there. The runtime acked the generation, cleared the
pending slot and wrote good.json from onActivityResumed, so a payload whose
activity resumed and then threw in measure, layout or draw reached the crash
guard with nothing pending and nothing unproven. generationToBlame returned -1,
quarantine then refused to name a generation already recorded good, and every
relaunch adopted it and died the same way with no in-app escape.

The ack, the pending-clear and the good-marking now move together into
onFirstFrameDrawn, released by a ViewTreeObserver.OnDrawListener whose
completion is posted rather than run inline: the listener fires at the start of
a draw pass, so only the posted message runs after the traversal that drew the
frame returns. An activity with no live view tree completes inline as before,
since waiting for a frame that will never arrive would strand the deploy
unacked.

The pending slot moves out of QuickBuildRuntime into FirstFrameGate, which is
plain Java and JVM-testable; FirstFrameGateTest pins that the generation stays
blamable across the undrawn window and is released only by a drawn frame.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: serialize and order the API 28/29 resource swap

Answers review threads 3926539450 and 3926539457 (both IMPORTANT) on PR #1716.

applyTableLegacy mutated the live AssetManager and flushed the Resources caches
straight off the binder thread the deploy arrived on, which is exactly what
swapProvidersOnMain's own KDoc says must not happen: addAssetPath re-tables the
AssetManager and flushCaches drops the drawable and typed-value caches, and
either can race an inflation already in progress. The path is live rather than
dead - CoGo's classifier routes resource edits with no SDK gate, so a res/ edit
on a 28/29 device lands here.

It also compared nothing before mounting, while both loader swaps drop an
overtaken generation against swappedGeneration. Two binder threads could
interleave so that the older table was the last one added, which wins the lookup:
the screen resolved gen N-1 while the store reported gen N, and legacyTableZip
then handed that apk to every activity created afterwards.

The write stays on the calling thread; only the mount is posted, under the
store's monitor, behind the same generation comparison its two siblings use. A
mount failure now travels back through the swap-failure listener instead of being
thrown synchronously, so the deploy still fails rather than acking a table that
never mounted.

Not JVM-tested: ResourceStore needs a Context, a Resources and a live main
Looper, and is one of the classes this module's coverage gate excludes as
device-only glue. Both siblings' identical guards are untested for the same
reason.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: a stale handshake failure must not tear down a live binding

Answers review thread 3926539468 (IMPORTANT) on PR #1716.

Moving the connect handshake onto its own thread left both failure branches
writing the shared host field unconditionally. The handshake can outlive its
binding: CoGo's service dies, onServiceDisconnected nulls the host, the framework
reconnects and a second handshake succeeds against a new proxy. The first
thread's connect() then fails and nulls that live host, unbinds a healthy channel
and schedules a rebind - and until the rebind lands, reportReloaded and
reportCrash only log "not connected", so every deploy in the window can end only
in the host's own timeout.

Both branches now go through abandonHandshake, which takes the monitor and
returns early unless the proxy that failed is still the live one. The test and
the teardown have to be one step because host is also written from the
framework's callback thread.

Not JVM-tested: QuickBuildClient is binder and ServiceConnection glue over a
Context, an IBinder and a main-thread Handler, and is one of the classes this
module's coverage gate excludes as device-only. The interleaving the reviewer
describes is a plausible reading of the code rather than one reproduced here;
what is verified is that the guard was absent and is now present.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: delete a temp file on every failure, not just a failed rename

Answers review thread 3926539487 (MINOR) on PR #1716.

AssetExtractor.writeFile deleted its .qb-tmp only from the rename fallback, so a
throw out of the copy - the payload cap, a write failure, a truncated entry -
left the partial file under current/assets/, which is the tree
DirectoryAssetsProvider resolves names against, and the app could open it by
name. Both write paths now delete the temp from one finally that covers the copy,
the close and the rename alike.

The reviewer cites two siblings as already correct. Only one is:
LegacyResourceSwap.writeResourceApk does delete its partial file when the copy
fails. PayloadPersistence.writeAtomic has the same gap being fixed here - its
finally only closed the stream, and its temp.delete() sat in the rename fallback
- so an oversize payload or a full disk left a .tmp in the store directory that
nothing sweeps. Both are fixed here.

Two tests, each verified to fail without the fix and for its own reason - a
leftover temp file, not an unexpected throw:
AssetExtractorFailurePathTest.aCopyThatFailsMidEntryLeavesNoTempFile drives a
truncated zip entry, and
PayloadPersistenceAtomicWriteTest.aStreamThatFailsMidCopyLeavesNoTempFile drives
a payload stream that dies mid-copy.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: drop the no-op test-heap override

Answers review thread 3926539494 (NITPICK) on PR #1716.

The root build already sets maxHeapSize = "1g" on every Test task in every
subproject (build.gradle.kts, the subprojects tasks.withType<Test> block), so
this module never saw Gradle's 512 MB default and the override changed nothing.
Its comment also compared against neither the real cap - Streams.MAX_PAYLOAD_BYTES
is 64 MB - nor the heap actually in force, so a reader trimming test memory later
would have trusted it twice over.

Verified against the root build before removing: the subprojects block does set
it, so dropping this leaves the same 1g in effect.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: style: spotless reformat, no functional change

The Eclipse Java formatter sorts members, so files this round touched came under
the ratchet and had their declarations reordered. Kept standalone so the
behavioural commits around it stay readable.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: hold a backgrounded deploy's ack until its resource swaps commit

Answers review threads 3926539472 (IMPORTANT) and, in part, 3926539480
(IMPORTANT) on PR #1716.

The earlier swap-failure fix reached only the foreground branch. applyTable posts
its swap to the main looper and returns, and the backgrounded branch then acked on
the binder thread while that swap was still queued. When the swap failed,
onSwapFailed ran later, rolled the store back to gen N-1, quarantined N and
reported the crash - after CoGo had already been told N reloaded. DeployChannel
resolves a deploy on the first report naming the generation, so the ack won: the
build was recorded as a successful reload with a timing number, the Crashed branch
never ran, and the session manager's separate collector still raised
RELOAD_CRASHED, so one save produced both signals. The comment on that branch says
the backgrounded case is the normal edit loop, so it is the branch a failing
resource swap usually takes.

SwapFailure becomes SwapOutcome and gains the success counterpart the reviewer
points at. Its contract is that exactly one of the two fires per applyTable or
applyAssets call that returns normally - including the calls that queue nothing
because this SDK level has no swap to make, since a deploy waiting on one of those
would wait forever. A swap dropped as overtaken reports committed rather than
failed: it returned normally, and the generation that overtook it owns the screen
and its own ack.

SwapAckGate counts a deploy's posted swaps down to the one ack it owes, and a
failure cancels it for good so a second swap landing afterwards cannot turn a
rolled-back deploy back into a success. A dex-only deploy - the commonest one -
posts nothing and still acks immediately, through noSwapPosted rather than
through committed, so a deploy with one swap in flight cannot mistake that call
for its swap's own commit.

The resumed check moves above the applies, because the commit callback is free to
fire before handlePayload returns and has to know which branch it is completing.
Arming the first-frame gate moves with it, which also closes a smaller hole: an
apply that threw used to leave an older generation's value in the pending slot.

The abandoned-generation half of 3926539480 comes with it: an applyAssets failure
left the table swap applyTable had already queued live under the rolled-back dex,
and nothing marked the generation abandoned, since only onSwapFailed did that. The
recreate then rendered gen N's table over gen N-1's classes while the banner said
the app was on the last working version. handlePayload's catch now marks it.

DEFERRED, deliberately: the other half of 3926539480 - having failReload restore
the provider set alongside the payload. A resource rollback is a new capability,
not a guard: ResourceStore keeps no per-generation provider history, the API 28/29
path cannot unmount an added asset path at all, and the store's own KDoc already
documents "a swap that already took is not undone" as the contract. Adding one
belongs in its own change with its own device verification, not folded into a
review fix. Marking the generation abandoned already stops the recreate, which is
what makes the banner honest.

SwapAckGateTest pins the rule and was verified to fail without it: with the gate
mutated to ack regardless of queued swaps, and with failed() made a no-op, three
of its seven tests go red on exactly the assertions they are named for. The call
site itself - handlePayload counting its swaps - needs a binder thread, a main
looper and a Context, so it is checked on device; QuickBuildRuntime and
ResourceStore are both in this module's device-only coverage exclusion list.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: name the unreported-relaunch-crash gap by its ticket, not by a planning-doc number

The backgrounded-deploy comment in QuickBuildRuntime deferred to "gap #91",
a number from quickbuild/docs/reliability-gaps.md that a reader of the
comment cannot follow. State the gap in words and cite ADFA-5466, filed for
it today; the gaps table carries the same key.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: pin the first-frame call site, and refuse an abandoned generation's swap

Two review gaps on the runtime, both about a generation that is no longer
supposed to be believed.

The first-frame gate had a test for the gate class but nothing for its caller,
so reverting the routing - completing the reload at onResume again, which is
the pre-fix behaviour - left every test green. onActivityResumed needs an
Activity, a Window and a live ViewTreeObserver, so the routing moves into a
package-private seam, completeOnResume, the same shape as startFailReloadThread.
The new test drives that seam and asserts what the resume must NOT do: with a
frame still coming it completes nothing, so the generation stays pending in the
gate and BootProbation still names it.

The second is a swap the store used to commit after the deploy that queued it
had already been rolled back. A swap is posted to main and commits after
applyPayload returns, so a deploy that throws in a later step - applyTable posts
before applyAssets can throw - had its rollback run with its own table swap
still queued. The store already drops an OVERTAKEN swap in all three swap
bodies; this adds the sibling case, an ABANDONED one, through the same guard.
Undoing a committed swap is not available: the store keeps single provider slots
and closes the previous provider after each swap, and the API 28/29 path cannot
unmount an added asset path at all, so refusing the commit is the whole remedy.
The runtime calls abandon() from both places it already gives up on a
generation.

The three swap bodies run on the main looper, so their call to the guard is not
pinned by a JVM test; what is pinned is the decision they take.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A

* ADFA-4128: say why the banner copy is inline English

The runtime AAR is injected into the user's app and carries no res/ of
its own, so the banner cannot use a string resource; REVIEW.md asks for
that opt-out to be stated, not inferred. MAX_BANNER_LINES is derived
from the literals' character counts, so its KDoc now says the
arithmetic assumes the English copy.

Review thread: #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: report a mixed state when the failed swap had already committed

failReload rolls the dex back, but a resource swap that committed before
the failure stays live: the store keeps single provider slots and the
API 28/29 path cannot unmount an asset path. That is the common
ordering, since applyTable posts and returns while applyAssets merges
on the binder thread. The banner then said "App is on the last working
version" while the screen served the failed generation's table under
the previous generation's classes.

Now the failure path reads ResourceStore.swappedGeneration() after the
generation has been abandoned (so a still-queued swap is refused rather
than committing later) and, when it equals the failed generation, shows
OverlayState.mixed() - "Restart the app - it is running mixed versions"
- and prefixes the report to CoGo so Build Output carries the restart
instruction in full. The decision lives in Generations.leavesMixedState
so it is JVM-tested; the wiring in failReloadNow is device-only.

Review thread: #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: run the boot resource restore off the main thread and report its failure

applyPendingBootResources is dispatched from onActivityPreCreated, inside
the first activity's creation on the main thread, and ran the whole
restore inline: the asset merge (a recursive delete plus an unzip) and,
on API 28/29, the relinked apk copy, both bounded only by the 64 MB
payload cap. Every cold start that adopts a persisted generation with
resources - every save after a restart deploy, and every process death
- paid that as launch jank or an ANR on the low-end devices the legacy
path exists for.

The extraction now runs on a qb-boot-restore thread and only the swap
is posted; the first activity inflates against the baseline table and
is recreated once the last swap lands, counted by the same SwapAckGate
a backgrounded deploy uses. markLiveGenerationGood waits for the
restore, since a frame drawn against the baseline proves the code half
only.

The restore also had the one remaining null outcome listener, so a
corrupt store file or a full disk inside the merge left the process on
this generation's code over the installed resources with one log line
and nothing else. It now shows the mixed banner and reports to CoGo
with a boot-specific first line; the report is best-effort, since CoGo
may not have connected yet.

Review threads:
#1716 (comment)
#1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: remove the first-frame draw listener from the captured observer

The listener was added to the observer captured before the draw and
removed from a re-fetched decor.getViewTreeObserver(). Once the decor
is detached - the activity destroyed between the draw and the posted
completion - that accessor returns a fresh floating observer that never
held the listener, so the removal was a silent no-op. Remove from the
captured observer while it is alive, falling back to the decor's when
the framework has merged it away, as StatusOverlay.reapplyInsetAfterLayout
already does.

Review thread: #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: write the client's host proxy under the monitor everywhere

abandonHandshake tests host and tears the binding down under the
monitor, but the four framework callbacks wrote host as plain volatile
writes, so the exclusion held only against other synchronized callers.
A disconnect-then-reconnect on the main thread could still land between
the handshake thread's read and its write and unbind a healthy binding.
onServiceConnected now writes under the monitor and the three null
writes go through a synchronized dropHost(); the callbacks themselves
stay unsynchronized so the main thread does not wait on a handshake
thread's binder round trip.

Review thread: #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: document the recreate-into-stopped case FirstFrameGate does not release

A recreate that succeeds but relaunches into the stopped state, whose
task is then swiped away, never resumes: no draw callback is installed
and nothing else releases the slot, so the deploy ends in CoGo's
timeout and the generation stays blamable until the next save. The
KDoc listed two no-frame fallbacks and not this one. It is recorded
rather than wired: recreate() destroys the armed activity on every
normal reload, so an onActivityDestroyed release would also need to
know a relaunch is still pending, which nothing tracks yet.

Review thread: #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: refuse asset payloads below API 30 instead of acking a merge nothing reads

Nothing below API 30 reads the merged asset dir: DirectoryAssetsProvider
needs a ResourcesLoader and LegacyResourceSwap mounts the resource apk
only. The legacy arm still ran the merge and reported the swap
committed, which is what settles a backgrounded deploy's ack, so a
reload the app could not show was acked. It now reports failed with the
reason before touching the fd or the Context, and the comment names the
host gate (QuickBuildModule's assetsLiveReloadable, the classifier)
that keeps it unreachable today.

The applyAssets KDoc also said a partial merge stays live until the next
deploy overwrites it; extractCumulative leaves MERGE_PENDING_MARKER and
the next merge clears the whole dir. Reworded to say so.

Review threads:
#1716 (comment)
#1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* style: spotless reformat, no functional change

Eclipse member sorting over the members the review-fix commits added;
no line inside any member changed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: decide what a frame proves on its draw pass, not when its completion runs

markLiveGenerationGood read bootRestoreInFlight when the posted first-frame completion
ran. The draw listener fires inside the traversal, an async message ahead of the sync
barrier, and the completion is posted behind it, so a boot restore's swap message could
land in between: the frame drew the baseline table, the swap committed and cleared the
flag, and the completion then recorded good a generation whose table never rendered -
unblamable if that table fails on the next boot.

frameCompletion samples the flag on the draw pass and hands the fixed verdict to
onFirstFrameDrawn / markLiveGenerationGood, which no longer re-read it. The seam is
static and Android-free so QuickBuildRuntimeFrameCompletionTest can pin the ordering.

Adversarial review 2026-09-04, finding #3 on 540eb96dd.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: say what a failed boot restore actually leaves behind

The catch in restoreBootResources claimed a failed restore leaves the process
wholly on the baseline table. abandon() only refuses a swap still queued; a
table swap that committed before applyAssets threw stays live, the app runs
mixed, and onBootRestoreFailed already reports it as mixed. The comment now
says that.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: serialize the cumulative asset merge against a second binder dispatch

Payloads arrive on a oneway binder callback, whose thread pool can dispatch two
at once - the interleaving PayloadPersistenceAtomicSetTest already pins for the
persist. extractCumulative merged into the one shared override dir with no lock,
so two merges could race entry-for-entry, and the pending marker cannot recover
that: the second merge clears it on the way out, leaving the dir holding two
generations with nothing left to notice.

The new test holds the extractor's monitor and asserts a concurrent merge cannot
finish, the same deterministic shape persistSerialisesOnTheStoreMonitor uses.
Without the synchronized keyword it fails with "a merge ran to completion while
the extractor monitor was held / expected to be false".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: stop two runtime tests passing for a reason they do not test

LegacyResourceSwapSweepTest.isBestEffortOverAnApkItCannotDelete assumed
setWritable(false) denies deletion. A root worker unlinks regardless, sweeps the
apk away, and fails the test for a reason it is not about. It now probes the
capability at stake - a sacrificial file in the same locked directory - and
skips when that deletes, rather than inferring privilege from a uid or from
user.name, which is not tied to the effective uid at all.

PayloadPersistenceAtomicSetTest.concurrentDeploysAlwaysLeaveOneWholeLoadable-
Generation read failure.get() straight after a timed-out join, so a worker still
inside persist could record its failure afterwards and the test would have
passed over it. Asserting the threads are not alive first closes that, and making
them daemons stops a hung persist outliving the Gradle worker.

Both proven by mutation: inverting the sweep test's guard reports it skipped with
"this worker deletes despite the directory mode", so the probe reads the real
filesystem capability; holding the store monitor across the joins leaves the
pre-fix test green with both workers still running, and red on "dex deploy thread
did not finish" with the assertions in place.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XkGof8cLt23LkxZ8MKzin2

* ADFA-4128: refuse an abandoned generation's swaps by generation, not by watermark

A cold start restores persisted gen 10 on qb-boot-restore while CoGo's
catch-up gen 11 arrives and fails; abandon(11) as a high-water mark then
refused gen 10's queued swap, and a refused swap reports committed, so the
restore logged success and recreated the activity over the baseline table
with no banner. Abandonment is now a set of generations, pruned as swaps
commit since the overtaken rule already covers everything below the
committed one. The dropped-swap log lines name the actual reason.

Answers #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: sweep the API 28/29 apk cache from the first write, off the main thread

onActivityCreated runs on the main thread inside the first activity's
creation, and the sweep it called there is a readdir plus one unlink per
apk the previous process wrote - the launch-path disk IO the boot restore
was just moved off main to avoid. The sweep now lives in ResourceStore,
under the lock every legacy write takes, and runs once before this process
writes its first relinked apk: that is still ahead of the first mount, it
runs on whichever off-main thread the write arrives on, and no latch is
needed to keep a deploy's write from racing it.

Answers #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: stash a persisted generation for boot restore only when it carries resources

loadPersisted stashed every adopted generation as pending boot resources,
so a dex-only one - the usual case, and what a restart deploy persists -
started a restore thread that swapped nothing, reported itself landed and
recreated the first activity for it on every cold start. Only a Loaded
with an arsc or an assets file is pending now.

Answers #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: give the boot restore's failure listener the deploy path's two guards

A persisted generation carrying a table and assets lands in two swaps. When
the first failed, the listener reported the mixed state but never abandoned
the generation, so the second swap committed anyway; and when both failed,
onBootRestoreFailed ran twice - two banners, two crash reports for one boot.
The listener now abandons the generation and reports only the first failure,
which SwapAckGate.failed() reports by settling the gate exactly once. The
deploy path's listener takes the same first-failure guard.

Answers #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: report a payload that fails before acceptance instead of rolling back

handlePayload's catch passed the pre-apply snapshot to failReload, but
that snapshot is still null for a failure before the acceptance check -
a dex read that throws, a malformed metadata document. failReload then
restored null whenever the live generation equalled the failed one, so a
replayed generation whose read failed went inert and quarantined the
generation the app was running. A pre-acceptance failure now takes a
report-only path: banner and crash report, no restore, no quarantine.

Answers #1716 (comment)

No JVM test: handlePayload needs ParcelFileDescriptor and SystemClock.
Reasoned from the code, not run on a device.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: keep unbind and rebind outside the client monitor, and drop the host on a null proxy

abandonHandshake was synchronized as a whole, so unbindService - a
synchronous binder transaction - ran under the monitor the framework's
main-thread callbacks take, which is the stall dropHost's KDoc says the
design avoids. Only the host test and the null write need the monitor.

onServiceConnected's null-proxy branch left host set while it unbound
and scheduled a rebind, and the rebind runnable returns early while a
host is set; the other failure paths drop the host first, so this one
does too.

Answers #1716 (comment)
and #1716 (comment)

Not run on a device; both are reasoned from the code.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: make the status banner a polite live region

The banner is the only in-app trace of a failed deploy and takes no
focus, so a screen reader never announced it.

Answers #1716 (comment)

Not yet checked with TalkBack on a device.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: clear a persisted payload the stamped baseline rejects

The store is keyed on the baseline dex alone, so a dex-identical
rebaseline left the superseded epoch's files on disk and the next
deploy's persist inherited that epoch's meta as its own history.
selectPersisted now clears the store when it rejects a payload.

Answers #1716 (comment)

PersistedSelectionTest.aRejectedPersistedPayloadIsClearedFromDisk fails
without the fix: "value of: load(...) expected: null but was:
PayloadPersistence$Loaded@226b143b".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: pass the stamp read failure to the logger

Answers #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: round 5 doc and comment fixes on quickbuild/runtime

- FirstFrameGate: cite ADFA-5524 for the unreleased-recreate case.
  #1716 (comment)
- PayloadStore.restore: documented as the test seam it is; production
  rollback goes through restoreIfCurrent.
  #1716 (comment)
- AssetExtractor.writeFile: temp-deletion note in the description,
  remaining and the return documented.
  #1716 (comment)
- ActivityTracker.onActivityCreated: the first activity attaches nothing;
  the boot restore's recreate delivers the restored table.
  #1716 (comment)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* style: spotless reformat, no functional change

Eclipse member ordering for the two Java files added in the round 5 fixes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Y6rFuAJin4QFFwwzUTFZ1K

* ADFA-4128: sample the drawn generation on the draw pass, not at completion

onFirstFrameDrawn read the store's live generation when the posted
completion ran. A deploy on a binder thread can persist, apply and arm a
newer generation between a frame's draw pass and that message, so the
completion acked the newer generation off a frame that drew the older one
and markLiveGenerationGood wrote good.json for a generation that had never
rendered - which quarantine() then refused to name.

frameCompletion now samples the generation on the draw pass alongside the
boot-restore flag, and drawn() and markLiveGenerationGood() take it instead
of re-reading the store. A newer generation armed in the gap stays pending
until a frame of its own.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0153YfwDnqn7ktHcVgXSNNe8

* ADFA-4128: keep the boot restore's payload files out of the orphan sweep

collectOrphans protected only the just-published names and good.json's. On
a cold boot the adopted generation has never drawn, so good.json does not
name it, and the restore opens its files one at a time (on API 28/29 the
whole relinked apk is copied before the assets zip is opened). A catch-up
deploy persisting in that window swept the not-yet-opened file, openReadOnly
threw, and a deploy that succeeded raised the mixed banner plus a crash
report.

PayloadStore now retains the boot-pending names in the persistence store
when it stashes them, the sweep treats them as referenced, and the runtime
releases the hold once the restore has landed or failed. The set is a
volatile replaced whole so the main-thread release never contends with a
persist for the store's monitor.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0153YfwDnqn7ktHcVgXSNNe8

* ADFA-4128: guard the two apply catches on the swap gate too

onSwapFailed returns unless gate.failed() settled the gate, but the catch
blocks in handlePayload and restoreBootResources called failed(), discarded
the result and reported regardless. A persisted generation carrying both a
table and assets could then report twice for one boot: the table swap fails
on main and reports, the restore thread throws in applyAssets, and the catch
reports again - a second mixed banner and a second crash report.

ackGate moves out of handlePayload's try so its catch can ask it, and both
catches now report only when they are the first to settle the gate.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0153YfwDnqn7ktHcVgXSNNe8

* ADFA-4128: put both apply catches through one tested report-once seam

The previous commit added the two guards but left them unreachable from a
JVM test: SwapAckGateTest pins SwapAckGate.failed()'s idempotence in
isolation, so deleting either `if` and reporting twice again kept the whole
suite green. The defect they fix is user-visible - a second mixed banner and
a duplicate crash report for one save - so it needs a test that goes red.

handlePayload's catch and restoreBootResources' catch now both call
reportApplyFailureOnce(gate, report), which owns the guard AND the report it
gates. That is what makes it testable: a bare boolean helper would still
leave the `if` at each call site, deletable with the suite green. The Runnable
seam follows completeOnResume, the same shape already in this file for the
same reason - the real call sites need a binder thread, a main looper and a
Context, so the branch is otherwise checkable only on a device.

Behaviour is unchanged. handlePayload's negative branch fell through to the
`finally` via `return`; it now falls through by not running the body, and the
boot restore's unconditional abandon stays ahead of the guard.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015WMhaGYg4sSCcAzQEdNtLa

* ADFA-4128: say what PersistedSelection gates instead of naming a test scenario

Akash on #1716: "the S7 fix" is our shorthand for a scenario in a test-plan
doc, so a later reader of this class has nothing to look it up in. The
sentence after it already says what the gate does, so the shorthand was
carrying no information the KDoc did not otherwise have.

The test's class comment used the same two phrases; it now names the class
under test and the failure the mutation reproduces.

Comments only. No behaviour change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XN91S41dghWHoXAoWhJAnm

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants