Measure staleness from where trunk really is, not from the calendar

[juanmaguitar]

Why

The amber staleness signal answers "how old is this site's trunk snapshot", by the calendar. Age is a proxy for the thing every downstream failure is actually about — distance from trunk — and it misses in both directions: a three-day-old snapshot can be dozens of commits behind in a busy week, and a two-week-old one can be nearly current. A contributor who follows the advice faithfully still writes patches against a trunk that has moved, and one who ignores a spurious dot spends minutes updating for nothing.

The app never measured the distance because measuring it seemed to mean talking to the network. It does not: the remote will name the commit its trunk points at for the cost of one small request, no objects downloaded.

What changes

src/trunk-remote.js (new) asks the remote where refs/heads/trunk is, with git.listServerRefs — protocol v2 ls-refs, a refs lookup and not a fetch. No new dependency (it is the bundled isomorphic-git), no objects on disk, and the answer is one oid. It rejects rather than absorbing failures, so the caller can tell "this remote has no trunk" from "we could not ask".

The probe never blocks anything. site:status returns the last known answer as it stands and starts the refresh behind its reply, so opening a site never waits on the network. When the answer lands it is written to the site record and pushed on trunk:remote, because site:status is not on a timer — it is read on mount and after the long operations, so a stored-only answer would first be read on the next launch.

It probes at most once an hour per site, stamped before the attempt. That does two jobs: the several site:status calls a session makes (mount, then each install/build/update/apply/switch) collapse to one request, and a failure backs off for the full hour instead of retrying on every call. An hour is chosen because the signal it feeds is measured in days — an answer up to an hour old never changes the recommendation. A finished update records the oid it just fetched, so the signal clears immediately rather than an hour later.

The decision stays pure. trunkAgeInfo in src/renderer/update-plan.cjs takes the oid as plain data and returns a three-valued behind (moved / current / could-not-ask). The comment that used to state the opposite invariant — "never from a network probe" — is rewritten rather than left contradicting the code. The 14-day threshold survives as the offline fallback, which is what keeps the original promise: with no answer, the dot behaves exactly as it did before.

A new trunkUpdateAdvice owns every sentence the contributor reads about staleness, so the renderer holds no branching. It is careful about two things:

• It does not nag toward destruction. An update resets the working tree and takes an applied patch with it (src/main.js clears the record for exactly this reason). So an applied patch turns the recommendation off — the amber block and the next-action cue go away, and a neutral block states the fact and the cost instead. The dot's tooltip changes too, since it has no detail line under it to soften it.
• It says what updating actually fixes: the site. A ticket branch keeps the base it was born at, deliberately. Bringing one forward is Updating trunk leaves every ticket on the trunk it was born from #305's action, and the copy says so outright rather than letting the contributor infer otherwise.

And it surfaces where it is cheapest to act on: the "Trac ticket" card, before a ticket exists. Updating first means the ticket is not born behind, which removes the cheapest-to-prevent class of apply failures. Shown only when no ticket is linked and nothing is at risk.

Deliberately not in this PR: how many commits behind. The refs protocol carries oids, not distances — counting means downloading the objects between them, which is the fetch this avoids. "Trunk has moved" is what can be known for free, and it is already truer than a date.

How to test this

Platforms: any. The change touches no paths, spawning or line endings. Buildkite has a signed artifact for this branch — check it matches the head commit.

Starting state: a site with wordpress-develop cloned, installed and built, whose trunk snapshot is less than 14 days old (a site set up this week). That is the case the old signal could not see, so it is the one worth starting from. Have a Trac ticket number and a pull request that touches a file you are willing to edit.

1. Quit the app, then open it with the network on. Select the site. Within a few seconds — with no click and no reload — the panel shows "Trunk has moved since this snapshot" in an amber block, and an amber dot appears next to the site in the sidebar and beside the trunk as of … label. Before this PR the site looked perfectly fresh: too young for the calendar to flag. Hover the dot: the tooltip names trunk, not a number of days.
2. Read the amber block. It says updating brings this site's copy of WordPress up to date. It does not say or imply that it will update a ticket.
3. Scroll to "Trac ticket", with none linked. A short amber note offers Update to latest trunk and says updating first means the ticket is not born behind. Click it and let the update finish.
4. After the update. The amber block, the dot, and the note in the ticket card are all gone — immediately, not after a wait. The panel names today's trunk.
5. Link a ticket, apply a pull request, and let it succeed. Now force the stale case again: there is no way to move trunk backwards from the UI, so either wait for wordpress-develop to land a commit (usually minutes to hours) or, if you are impatient, edit remoteTrunkOid in the app's config.json (~/Library/Application Support/wordpress-contributor-toolkit/ on macOS, %APPDATA% on Windows) to any other 40-character hex string, and set remoteTrunkCheckedAt to a time within the last hour so the probe does not immediately correct you. Reopen the app.
6. The nag is gone; the fact is not. With a patch applied you get a grey, button-less block saying trunk has moved and that updating would remove the patch you applied — no amber, no "Update to latest trunk" button beside it, and the next-action ring does not point at updating. The dot's tooltip says the same. The update is still reachable from the header ⋮ → Update to latest trunk.
7. Revert the patch. The amber block and the recommendation come back on their own.
8. The offline case — the one that is easy to skip. Quit the app. Turn Wi-Fi off (or block github.com). Open the app and select the site. It opens at normal speed, with no spinner and no pause: nothing on the launch path waits on the network. A snapshot older than 14 days still shows the amber dot and the block, worded "This site's WordPress code is N days old" — the calendar fallback. A snapshot younger than that shows nothing. Nothing appears in the terminal panel about the network. Turn Wi-Fi back on and reselect the site: within a few seconds the wording switches to "Trunk has moved since this snapshot" if it has.
9. A captive portal, if you can stage one (a hotel or conference network that swallows connections rather than refusing them). Same as step 8: the app opens immediately and the probe gives up after 15 seconds without ever reaching the UI.

What must not have happened:

• The app never paused on launch or on selecting a site, in any of the steps — including step 8 with the network off. This is the regression that would be easiest to miss on a fast connection at a desk, and worst at a Contributor Day.
• No error about the network reached the terminal panel or a dialog while offline. Offline is a normal state; it is logged to the app log file (⋮ → the log file, look for trunk-remote) and nowhere else.
• The app never suggested updating while a patch was applied — not in the panel, not in the dot's tooltip, not through the next-action ring.
• Nothing claimed or implied that updating would bring an existing ticket forward.
• node_modules was not reinstalled or rebuilt except where step 3's update said it would be.
• The site's WordPress was not fetched, cloned or otherwise touched by the probe: only step 3 changed anything on disk.

What could not be tested by hand: rate-limiting from github.com — the request is one unauthenticated ls-refs per site per hour, which is far below any threshold reachable by trying. It is covered by the same path as offline (the probe rejects, the calendar takes over), which step 8 does exercise.

Risks and limitations

7 findings across two review passes · 5 fixed here · 2 deferred. The two deferrals are named below and neither is user-visible today.

• "Behind" is a yes/no, not a count. By design — see "What changes". If the count turns out to matter, it needs a different mechanism than a refs lookup.
• One request per site, not per remote. Every site uses the same URL, so a contributor with six sites makes six identical ls-refs calls an hour instead of one. Small (one tiny request each) but it is the "work should not scale with the site registry" rule applied to the network. Deferred: the fix is a per-URL memo or in-flight dedupe, and it has its own shape question. Related: two site:status calls racing on the same site can both find no stamp and both probe.
• A failed update can leave a false "trunk has moved" for up to an hour. updateToLatestTrunk writes the local ref before the checkout; if it throws after that, the site record is not updated and local trunk is briefly ahead of the last probed remote oid. Self-correcting at the next probe, and the site is already showing an "Update incomplete" banner in that state. Deferred.
• A local commit on trunk would read as "the remote moved". The app never makes one — contributor work goes to ticket branches (Working on a second ticket means rebuilding the world: tickets should be branches, not sites #108) — so this is a hand-edited repository only.
• The probe uses no authentication, so a fork of this app pointed at a private remote would simply always fall back to the calendar rather than failing loudly.

Related

Fixes #307. Part of #309. Adjacent to #305 (bringing a ticket forward), which this PR deliberately does not do and takes care not to promise.

---

Design decisions and alternatives considered

Why listServerRefs and not a shallow fetch. A fetch would give the count, and it would also download objects into the site, need a writable repository, and take long enough to need progress. ls-refs with prefix=refs/heads/trunk is one small request with no side effects, which is what makes it safe to run unattended in the background.

Why the answer is cached in electron-store and not in a module variable. electron-store is this repo's only persistence layer, and the invariant explicitly calls out state parked in a module-level variable that outlives a handler as drift. Two new keys, remoteTrunkOid and remoteTrunkCheckedAt, both absent-safe: an existing site record with neither behaves exactly as it does today, so no migration is needed.

Why the stamp is written before the probe, not after. It is the throttle and the offline back-off in one. Writing it after would leave a failing probe re-firing on every site:status, which is the worst behaviour for the machine most likely to be offline.

Why a push channel rather than making the renderer re-poll. The first review pass caught that site:status is not on a timer — it is called on mount and after long operations — so a store-only answer would have been read a session late, and the feature would have silently done nothing in the session that probed. Adding a poll to the renderer would put a timing decision in index.jsx, which the architecture rules keep out; a one-way webContents.send matches the existing ticket:carried-work and switch:progress shape.

Why uncommitted edits do not silence the advice, but an applied patch does. The issue asks the signal to stay quiet over "an applied patch or unsaved work". Those turned out to be different situations. An update removes an applied patch without asking. It does not do that to edits: startTrunkUpdate asks the narrow "is the worktree dirty" question first and opens a dialog that offers saving them (#234). Work that cannot be lost without a prompt is not a reason to withhold the signal, so it is named in the copy instead. That also keeps trunkUpdateAdvice away from a measure it would have got wrong — the value the panel holds is the branch-point one (#239), which counts parked work a force checkout survives, so using it would have gone quiet on trees that are perfectly safe to update. This is a conscious departure from the issue's wording, and the reasoning is in the module's own comment.

Why the pre-link prompt is a note in the ticket card and not a modal. It is worth a moment of attention, not an interruption — and it appears only when no ticket is linked and nothing is at risk, so it can never be the thing standing between a contributor and their work.

Review outcome (required — see AGENTS.md)

Two passes of .github/instructions/code-review.instructions.md, both dispatched to a fresh context.

First pass: 4 [fix here] · 3 [follow-up].

1. 🟡 [fix here] Architecture — the probe's answer never reached the UI in the session that fetched it, because site:status is not polled. Fixed: the trunk:remote push channel.
2. 🟡 [fix here] Architecture — the at-risk wording was unreachable: detail only rendered inside the block gated on recommendUpdate, which is false exactly when the at-risk sentences are non-empty. Green tests asserted copy the app could not show. Fixed: the neutral block.
3. 🟡 [fix here] Tests — no test file for the new module. Fixed: test/trunk-remote.test.cjs, with listServerRefs and timeoutMs injected (the house pattern) so the prefix over-match and the deadline are exercised without a network.
4. 🔵 [fix here] Architecture — a probe that succeeded with null left the previous oid in place, so a site could report itself behind for good. Fixed: the answer is always stored, null included.
5. 🔵 [follow-up] Performance — no deadline on the request. Fixed rather than deferred: a captive portal is this app's characteristic network, and it hangs rather than refusing. 15s via Promise.race.
6. 🔵 [follow-up] Architecture — hasChanges fed the wide branch-point measure.
7. 🔵 [follow-up] Architecture — a failed update can leave a false signal for up to an hour. Deferred — see Risks.

Second pass (reconciliation): 1–5 resolved, 7 deferral accepted, 6 reopened, plus 3 new.

• Finding 6 was reopened with the argument that the two measures answer different questions and the pre-existing deriveNextAction call is correct with the wide one. Fixed, differently: rather than plumb a second measure, uncommitted edits stopped being a suppressor at all — see the design note above.
• 🟡 [fix here] Tests — subscribeRemoteTrunk was missing from SUBSCRIPTIONS in test/preload-listeners.test.cjs, and trunk:remote is the first channel where every mounted SiteRow is a live subscriber — precisely the leak shape that file's header describes. Fixed: added to the list, which buys all four of its tests.
• 🔵 [fix here] Architecture — a probe in flight when the site is deleted would resurrect its siteMeta entry, leaving a phantom record for a directory that is gone. Fixed: both writes check the site is still registered.
• 🔵 [follow-up] Performance — one identical request per site rather than per remote. Deferred — see Risks.

Style notes from both passes (mixed apostrophe glyphs; a test leaning on process._getActiveHandles()) are fixed; the latter is now asserted through an injected clock.

npm test — 970 pass, 0 fail, on both .nvmrc's Node and Electron's bundled Node (npm run test:electron). npm run lint — clean. npm run build:once — clean.

Implementation notes

• src/trunk-remote.js (new, 126 lines) — remoteProbeDue (pure) and readRemoteTrunkOid. A stamp in the future reads as due, not as never-again: that is a clock that moved backwards, and the alternative silences the signal until the stamp's hour arrives. The refs row is matched on the full ref name, not taken as the first — prefix is a prefix, so refs/heads/trunk-experiment comes back under it too, and taking the first row would pin the site against a branch nobody works on.
• src/main.js — refreshRemoteTrunk (started, never awaited), REMOTE_TRUNK_CHANNEL, the throttle check in site:status, and the post-update record of the oid the fetch just resolved. The failure is logged to the app log and not streamed to the renderer: the "no silent catch" rule exists so a contributor is never left with a button that did nothing, and nobody pressed anything here.
• src/renderer/update-plan.cjs — trunkAgeInfo gains trunkOid/remoteTrunkOid and returns behind + source; trunkUpdateAdvice is new. No I/O, no DOM, as before.
• src/renderer/index.jsx — state for the two oids, the trunk:remote subscription, and five call sites reading the advice instead of formatting their own strings. No new branching in the component.
• src/preload.js — subscribeRemoteTrunk, a named function like every other bridge entry; no widening.
• test/ipc-wiring.test.cjs — the harness now stubs ./trunk-remote by default, with a readRemoteTrunkOid that rejects. Same rationale as replacing electron outright: site:status starts a network call behind its reply, so every one of the many site:status tests would otherwise reach github.com — slowly, flakily, and invisibly, since the call is deliberately not awaited. Tests that exercise the probe pass their own.

Screenshots or recording

Not captured. The visible surface is described precisely enough to check in "How to test this" — an amber block whose first line changes from "This site's WordPress code is N days old" to "Trunk has moved since this snapshot", the same block turning grey and losing its button when a patch is applied, and a new amber note in the "Trac ticket" card before one is linked. Reproducing the "behind" state for a screenshot needs wordpress-develop to have moved (or a hand-edited store), which is the same setup step a reviewer runs anyway.

[juanmaguitar]

Rebased into the stack and retargeted at #318, so the chain is now #313 → #317 → #318 → #319. Only two import lines collided — this change barely overlaps the others — but as a sibling it still could not be folded into a single build alongside them.

Suite on the stacked branches: 1028 pass, 0 fail; lint clean.

[juanmaguitar]

Parked for after 1.0, deliberately. The app is still proving whether it is useful at all, and 1.0 is being cut to what must not lie or lose work — this change replaces a heuristic that errs on the side of a cheap, idempotent click with a network probe, a throttle and a push channel. Good machinery, wrong phase.

The calendar dot stays as the 1.0 signal. The branch stays alive; if the app earns its keep, this is ready to pick back up. Removed from the v1.0-rc2 integration branch.

[juanmaguitar]

Closing as a deliberate post-1.0 deferral. For 1.0, the existing calendar signal is sufficient: a remote probe, throttling, timeouts and offline behavior add disproportionate machinery to an experimental app. The branch remains available if real use proves exact remote staleness worth that cost.