Skip to content

Commit 25b1451

Browse files
fryanpanclaude
andcommitted
ADFA-4128: 0902 review round on quickbuild:core deploy/reload
Non-blocking items from Akash's 2 September round; the PR itself is approved. - Two sibling docs still stated a "restart closure" rule the policy does not implement. Both now say what DeployPolicy does - a declared restart-sensitive component restarts every code-bearing deploy - and the stale-helpers notice says which deploys can still reach it. #1718 (comment) - An ask is no longer reported as AWAITS_DEPLOY without checking that a deploy is actually still coming. Every path that declines to start a build leaves the work queued, so this changes no outcome today; it stops a future early return from dropping the tap silently instead. #1718 (comment) - installPriorityHold releases the hold it replaces, which is what its KDoc already promised. #1718 (comment) - Dropped an unused import ktlint's substring match cannot see. #1718 (comment) - RetainedPayloadStore's KDoc claims the last deploy's own parts rather than a cumulative set, which is what retain writes. #1718 (comment) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017o3nPrBbGi2XYkMUGavG2A
1 parent b72a117 commit 25b1451

6 files changed

Lines changed: 36 additions & 13 deletions

File tree

‎quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/ComponentInfo.kt‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,12 +27,12 @@ enum class ComponentKind {
2727
}
2828

2929
/**
30-
* The kinds whose live instance a loader swap cannot update, so a recompile inside their
31-
* restart closure forces a process restart ([DeployPolicy]).
30+
* The kinds whose live instance a loader swap cannot update, so an app that DECLARES one
31+
* restarts the process on every code-bearing deploy ([DeployPolicy]).
3232
*
3333
* One home for the set, because two rules key off it: the restart decision, and the
34-
* [org.appdevforall.cotg.quickbuild.domain.session.QuickBuildNotice.STALE_COMPONENT_HELPERS] warning that fires when one of these merely
35-
* EXISTS and the deploy hot-swapped instead. Both read it through [isRestartSensitive], which
34+
* [org.appdevforall.cotg.quickbuild.domain.session.QuickBuildNotice.STALE_COMPONENT_HELPERS] warning that fires when one of these
35+
* exists and the deploy hot-swapped anyway. Both read it through [isRestartSensitive], which
3636
* also applies the [COGO_INJECTED_COMPONENTS] exemption.
3737
*/
3838
val RESTART_SENSITIVE_KINDS: Set<ComponentKind> =

‎quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/reload/LiveReloadOrchestrator.kt‎

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,17 +219,17 @@ class LiveReloadOrchestrator(
219219
// names why, so only a forced blind rebuild repairs it.
220220
markBatchArrivalLocked()
221221
pendingForced = true
222-
outcome = LiveReloadRequestOutcome.AWAITS_DEPLOY
223222
maybeStartBuildLocked(events)
223+
outcome = deployAnsweringOutcomeLocked()
224224
}
225225

226226
!pending.isEmpty -> {
227227
// Accumulated work: build it now, routed by the classifier as any save
228228
// would be; the deploy answers the tap.
229229
markBatchArrivalLocked()
230230
pendingUserInitiated = true
231-
outcome = LiveReloadRequestOutcome.AWAITS_DEPLOY
232231
maybeStartBuildLocked(events)
232+
outcome = deployAnsweringOutcomeLocked()
233233
}
234234

235235
expectChanges -> {
@@ -254,6 +254,23 @@ class LiveReloadOrchestrator(
254254
return outcome
255255
}
256256

257+
/**
258+
* Whether a deploy is still coming for the ask just recorded, read after the build attempt.
259+
*
260+
* [maybeStartBuildLocked] can decline to start one - a build already in flight, a proxy app
261+
* rebuild absorbing the set, a route the live reload path hands off as an invalidation - and
262+
* the outcome was AWAITS_DEPLOY regardless, which promises the caller a deploy that may never
263+
* come. Every one of those declines leaves the work queued for a later build, so the promise
264+
* holds today; asserting it here is what keeps a future early return from silently dropping
265+
* the tap instead.
266+
*/
267+
private fun deployAnsweringOutcomeLocked(): LiveReloadRequestOutcome =
268+
if (inFlight != null || !pending.isEmpty || pendingForced) {
269+
LiveReloadRequestOutcome.AWAITS_DEPLOY
270+
} else {
271+
LiveReloadRequestOutcome.SWITCH_NOW
272+
}
273+
257274
/**
258275
* Disarms a tap still waiting for its save-all's watcher batch and says whether it was
259276
* waiting - the deadline half of the arm-on-batch tap protocol.

‎quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/domain/session/QuickBuildNotice.kt‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,11 @@ enum class QuickBuildNotice {
2727
* `Application`, so an instance of one keeps calling the PREVIOUS copies of the helper
2828
* classes this build recompiled until it restarts.
2929
*
30-
* Not a failure: the deploy worked and the recreated activity runs the new code. The restart
31-
* closure covers a component's own code and its supertypes, and a hit there restarts the
32-
* process; what it cannot see is a helper class the component merely calls.
30+
* Not a failure: the deploy worked and the recreated activity runs the new code. A declared
31+
* restart-sensitive component normally makes every code-bearing deploy a restart, so this
32+
* only fires where one did not - a baseline whose runtime cannot honour the restart request.
33+
* The stale copies are then the helper classes the live component calls, which no restart
34+
* decision inspects.
3335
*/
3436
STALE_COMPONENT_HELPERS,
3537

‎quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/DeployChannel.kt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import kotlinx.coroutines.async
88
import kotlinx.coroutines.coroutineScope
99
import kotlinx.coroutines.flow.first
1010
import kotlinx.coroutines.withTimeoutOrNull
11-
import org.appdevforall.cotg.quickbuild.service.telemetry.report
1211
import org.slf4j.LoggerFactory
1312
import java.io.File
1413

‎quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/ProxyAppConnections.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,9 @@ class ProxyAppConnections {
6161
* @param hold the hold to take on connect and drop on disconnect or session end
6262
*/
6363
fun installPriorityHold(hold: ProxyAppPriorityHold) {
64+
// The KDoc promises replacement, so the one being replaced has to be dropped: keeping it
65+
// would hold a gone proxy app out of the freezer for the rest of the process's life.
66+
if (priorityHold !== hold) priorityHold?.release()
6467
priorityHold = hold
6568
}
6669

‎quickbuild/core/src/main/java/org/appdevforall/cotg/quickbuild/service/deploy/RetainedPayloadStore.kt‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ import java.io.File
1010
* below the deployed generation can be answered by re-sending them at their original
1111
* generation (concurrency.md rules 3-4) instead of by a forced blind rebuild.
1212
*
13-
* Payloads are cumulative over their baseline, so the last-deployed set alone brings a
14-
* same-baseline app fully current. The bytes are copied because the executor's own artifacts
15-
* (the daemon's dex, the staged assets zip) are overwritten by the next build.
13+
* What is kept is the LAST deploy's own parts, not a cumulative union: a re-send therefore
14+
* brings a same-baseline app current only where that deploy's parts already covered it, and
15+
* whether every consumer needs more than that is the reconnect re-send's question, not this
16+
* store's. The bytes are copied because the executor's own artifacts (the daemon's dex, the
17+
* staged assets zip) are overwritten by the next build.
1618
*
1719
* Everything here is best-effort by contract: a failed [retain] or an unreadable [load] only
1820
* costs the caller its fallback - the forced catch-up build - never a build result. Call only

0 commit comments

Comments
 (0)