Repository navigation
ADFA-365 - Project build Test for java and Base project tamplate - #3
Merged
Merged
Conversation
- add Manifest file to the e2e tests to support an application class - fixed problem with toolmanager - finished test
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
This was referenced Aug 27, 2026
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.