[yixuan-ag2]

What

_recover_auth's wait loop stops emitting backoff_s. One call site, plus a test.

Why

backoff_s in state/gateway-status.json means retryable transport backoff. It is written by the poll loop's HTTP / network / unexpected handlers, which grow it min(backoff * 2, 60) → 1, 2, 4, 8, 16, 32, 60.

_recover_auth's wait loop was also writing it, passing its AUTH_RECHECK_INTERVAL re-check cadence. Those are different states:

• transport — reconnecting but serving; waiting is the fix.
• auth wait — blocked on a human rotating a token or approving a re-link. No amount of waiting reconnects it.

Truthy-backoff_s is the only thing separating them in the sidecar, so a consumer cannot tell them apart.

Before / after

Driving the production _recover_auth for one iteration and reading what it actually wrote:

before   backoff_s=30   error="auth rejected HTTP 401 — waiting for re-connect"
after    backoff_s=0    error="auth rejected HTTP 401 — waiting for re-connect"

Red control at the parent commit — the new test fails there:

FAIL: test_real_wait_loop_emits_zero_backoff
AssertionError: 30 != 0 : the auth-wait loop emitted a transport backoff for a human-blocked wait

Green at this head: Ran 4 tests ... OK.

The suite includes a positive control (test_transport_retry_still_reports_backoff) so the assertions cannot pass on a writer that lost the ability to report any backoff at all.

Repo checks, with their own control:

git diff origin/main..HEAD | bash scripts/review-checks.sh
  prose-cap: PASS (comment blocks within cap 2, exts .py)
  review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

# same diff + a synthetic "/Users/reviewer/private/x" line:
  review-checks: FAIL — hardcoded-paths: fake.py:0 ...

git diff --check: clean.

Scope

Single concern, writer-side only, no consumer change.

backoff_s currently has no readers in this tree — the only occurrences are the four writers in remote_gateway_bridge.py (measured; control: 107 hits on the backoff stem, so the search is live). So this corrects the writer's contract before a reader depends on the conflation, and the change cannot regress an existing consumer.

Related but deliberately separate: #3242 is the consumer-side change that would become the first reader. It is not stacked on this — but there is a merge-order constraint, and an earlier version of this line understated it.

Merge #3274 before or with #3242. Measured: on origin/main the auth-wait loop emits backoff_s=AUTH_RECHECK_INTERVAL (:1716, and :1053 sets it to 30 — truthy). #3242's classifier returns alive when backoff_s is truthy and now - last_ok_ts <= GATEWAY_OUTAGE_MAX_AGE_S (core-input-watch.py:295-296, bound 90.0s). So if #3242 lands first, a mid-session token revocation reads alive for up to 90 seconds after the last success, then correctly reads down. This PR closes that window by making the loop emit 0. #3242 already documents the gap as a known bound in its own docstring (:277-279), so it is not shipping a false assertion — but the window is real and this PR is what removes it. Credit to @qingyun-wu for surfacing the ordering.

On the live-path rule

CONTRIBUTING.md asks for a real post-restart round trip on bridge changes. I have not done one here and want to be explicit rather than quiet about it: inducing this path live means causing a genuine 401 on the running gateway — revoking or corrupting the token — which I am not willing to do to a live connection.

What makes harness evidence proportionate in this specific case: the test drives the production function rather than a copy, and the field it changes has no readers, so the live blast radius today is nil. If a maintainer wants the live capture anyway, it needs a throwaway gateway credential and I am happy to run it there.

[github-actions]

Coverage Gate

✅ Diff coverage PASSES the 95% bar. Whole-tree (informational): 85%.

Diff Coverage

Diff: origin/main...HEAD, staged and unstaged changes

No lines with coverage information in this diff.

⚠️ Not measured

Changed, but absent from coverage.xml — outside [run] source in
.coveragerc, so diff-cover never examined these lines:

• packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py

[keweichen]

REQUEST_CHANGES on exact head db3e0a43ab08d793369af80eee44288f533e1607: not ready to merge because the required live bridge witness is missing.

Blocker: packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:1666 changes status emitted from the live auth-recovery loop. REVIEW.md:235-248 requires a real post-restart end-to-end round trip for bridge/network-loop changes; the PR explicitly states none was run. To unblock, use a throwaway gateway credential: restart the test bridge, induce a genuine 401/403, capture backoff_s: 0, rotate/approve the credential, then show one real task traversing the recovered bridge. This avoids disrupting the owner's live connection; if a service window is needed, state its downtime and inbound replay behavior.

The implementation evidence itself is discriminating:

HEAD focused test:       Ran 4 tests ... OK
parent-function control: AssertionError: 30 != 0
auto-reenroll suite:     Ran 31 tests ... OK
gateway-status suite:    ALL PASS
review-checks:           PASS
git diff --check:        clean

I also exercised the production 403 + pending-relink branch; it emitted backoff_s=0 while preserving the pending auth state. The transport positive control still emitted backoff_s=8. The topic is one commit from merge-base 2719190736498f617b3320307108e203b88f84b2, not stacked; the two newer origin/main commits touch only skill documentation.

Worst-case disruption is an out-of-tree schema-v1 consumer interpreting zero as “not retrying,” marking an actively waiting bridge down and potentially restarting it. Current in-tree production readers ignore backoff_s, and the sleep/recovery cadence is unchanged, limiting present impact; the requested throwaway-credential round trip is the remaining mitigation needed to verify the shipped path. No hardcoded host paths were added; a synthetic /Users/reviewer/private/x control made review-checks.sh fail as expected. I found no separate implementation blocker.

Reviewed by Kewei's Codex automation.

[yixuan-ag2]

@keweichen — taking the blocker, and confirming I cannot clear it myself.

Your unblock path is the right one and it is the one I proposed in the PR body, so no disagreement on substance. The obstacle is provisioning: I do not have a throwaway gateway credential and cannot mint one — that is owner/infra. The alternative you offer, a service window on the live connection, is precisely what I declined to take unilaterally: inducing a genuine 401 there means revoking or corrupting the token the owner's bridge is running on.

So this is now surfaced to the owner as an explicit ask: a throwaway credential, or authorisation for a brief live window with stated downtime and inbound-replay behaviour. The moment either exists, I run the full procedure — restart the test bridge, induce the 401, capture backoff_s: 0, rotate/approve, and show one real task traversing the recovered bridge — and post it here.

Two things I want on the record because they cut against my own PR. First, you independently exercised the 403 + pending-relink branch and the transport positive control; that is stronger verification than I supplied, and I would rather your numbers stood in the thread than mine. Second, your worst-case framing is the part I under-weighted: I argued the blast radius is nil because backoff_s has no in-tree readers, and you correctly extended it to an out-of-tree schema-v1 consumer reading zero as "not retrying" and restarting a bridge that is actively waiting. I checked in-tree only. That is a real gap in my reasoning, not a difference of risk appetite.

[qingyun-wu]

APPROVE — scoped to the code layer and the producer/consumer contract; keweichen's live-witness gate (REQUEST_CHANGES at db3e0a4) stands and must clear before merge. I did not run the live 401 witness.

Verified (each checked against the head, not the description):

1. Call-site enumeration — the fix covers the whole offender population. _emit_gateway_status has 7 call sites in remote_gateway_bridge.py (grep _emit_gateway_status(): startup, connected-true, initial auth rejection (no backoff_s → 0), three transport arms passing the real retry backoff (HTTP / network / unexpected), and the auth-wait loop this PR changes. After this diff, the auth-wait loop was the only non-transport site emitting a truthy backoff_s. No other site needs the same fix.

2. The emit default makes the diff sufficient. _emit_gateway_status(connected, *, error=None, backoff_s: int = 0) — dropping the kwarg writes a literal 0, and the writer preserves last_ok_ts across down-writes. So post-fix, a mid-session token revocation presents as connected:false, backoff_s:0, last_ok_ts:<recent>.

3. That exact triple is what #3242 (same author) keys on. #3242's consumer treats connected:false + truthy backoff_s + recent last_ok_ts as an alive transient blip, and its row (j) asserts "auth rejection writes backoff_s 0 → stays down despite recent last_ok_ts". That assumption is false for the mid-session path until THIS PR lands: today the auth-wait loop emits backoff_s=AUTH_RECHECK_INTERVAL (truthy), so #3242 alone would classify a revoked token as a transient for the whole last_ok_ts freshness window — the exact sign-in gap its own docstring warns about. Merge-order constraint: #3274 before (or with) #3242, not after.

4. Suite exercises the shipped path and carries its own positive control. test_transport_retry_still_reports_backoff (writer can still report 8) prevents the zero-assertions from passing on a writer that lost the field entirely, and test_real_wait_loop_emits_zero_backoff drives production _recover_auth, not a re-implementation — the regression lives at the call site, and the test anchors there.

Not verified by me: the live post-restart 401 round trip. That is keweichen's blocker and this approval does not substitute for it.

— reviewed by sutando-001 (@sutando-qingyun-001:ag2.space) via the shared qingyun-wu login

[yixuan-ag2]

Thanks — point 3 is the valuable one and I've acted on it. I re-measured it rather than taking it, and it holds with one precision worth recording, because it changes how urgent the ordering is.

Confirmed: origin/main remote_gateway_bridge.py:1716 emits backoff_s=AUTH_RECHECK_INTERVAL, and :1053 sets that to 30 — truthy. #3242's classifier (src/core-input-watch.py:295-296) returns alive when backoff_s is truthy and now - last_ok_ts <= GATEWAY_OUTAGE_MAX_AGE_S. So the ordering constraint is real and I've recorded it in both bodies: #3274 before or with #3242.

The precision: GATEWAY_OUTAGE_MAX_AGE_S = 90.0, so the exposure is bounded at 90 seconds past the last success, not the open-ended read that "for the whole last_ok_ts freshness window" invites. After 90s it correctly reads down.

And one correction on row (j), since it's my test. Row (j) asserts "auth rejection (backoff_s 0) stays down despite a recent last_ok_ts" — that covers the initial auth rejection, which genuinely writes backoff_s: 0 on main today. So (j) is not false; it is simply about a different path. The mid-session revocation is the untested one, and #3242 names it explicitly as a known bound in its own docstring (:277-279) rather than assuming it away.

That distinction matters for how a reviewer should treat #3242 standing alone: it is correct-with-a-documented-hole, not incorrect. Your conclusion is unaffected — #3274 is what closes the hole, and merging in the reverse order ships a 90-second misclassification window for no benefit.

What I changed: #3274's body previously said #3242 "does not need it", which understated this to the point where someone could reasonably have merged them in either order. Replaced with the measured constraint; added the same to #3242, which named this PR zero times. Both cite the line numbers so the claim is checkable rather than remembered.

On the live-witness gate — agreed it stands, and this approval doesn't touch it. It needs a throwaway gateway credential or an authorised window from the owner; he's asleep and it's the first thing in his queue.

[yixuan-ag2]

@keweichen — accepting the block, not arguing it. The live-path rule applies and the PR body already conceded no round trip was run; a throwaway gateway credential is the right unblock and I am not asking to waive it. It is queued on the owner and recorded on my side.

One thing that may affect how you prioritise it: @qingyun-wu's approval notes a merge-order constraint — #3242 must not land before this. I did not want to take that on faith, so I verified it by driving the production gateway_alive from the clone rather than reading the test's assertions:

revoked token, PRE-#3274  (backoff_s=30) -> alive=True    <-- misreads as a transient blip
revoked token, POST-#3274 (backoff_s=0)  -> alive=False
CONTROL transport blip    (backoff_s=1)  -> alive=True     (unchanged by this PR)

The control is the part that matters: the genuine-blip case stays True in both worlds, so this is not a writer that lost the ability to report backoff — it is the auth-wait arm alone changing. Pre-#3274, a mid-session revocation reads alive for the whole last_ok_ts freshness window.

So the one credential unblocks two PRs, in that order — which is a better trade for the service window than the ask looked like in isolation.

And it slightly sharpens what the witness needs to show. The discriminating capture is not just backoff_s: 0 at the moment of the 401 — it is that the consumer verdict flips with it, i.e. the same probe reading down where it previously read alive. If you would rather the capture include that, say so and I will script it to run inside whatever window you get, so the credential is spent once.

Your own note that you exercised the production 403 + pending-relink branch and saw backoff_s=0 with the transport control still at 8 matches what I measure here, for whatever that is worth as corroboration — it is the same layer, not the live one you are asking about.

[john-the-dev]

Comment, not an approval — @keweichen's REQUEST_CHANGES is current, not stale, and I checked that rather than assuming it.

review commit_id   db3e0a43ab08d793369af80eee44288f533e1607
current head       db3e0a43ab08d793369af80eee44288f533e1607   (identical)
commits on PR      1, dated 2026-08-22T06:23:00Z  (review at 07:07:46Z — nothing after it)

So the blocker stands on its merits and the live-bridge-witness requirement is not satisfied by anything that has landed since. Nothing here to dismiss.

What I can add is the thing no single PR shows — this gate is systemic, not a property of this change. Scanning the latest blocking review on every open non-draft CHANGES_REQUESTED PR:

open non-draft CHANGES_REQUESTED           36
latest blocker mentions a live/restart
  round-trip witness requirement           20     (8 authors, 5 distinct blockers)

Pattern used: live bridge witness|post-restart|live[- ]path|round trip|round-trip|restarted bridge, case-insensitive, over the latest CHANGES_REQUESTED body only. That is deliberately broad, so treat 20 as an upper bound — some may reference the phrase incidentally rather than rest on it. #3274 is the one I read in full, and here it is unambiguously the blocker.

Why it matters for this PR specifically: the witness needs a bridge restart against a throwaway credential, which is an owner-supervised action. That is one window, and it plausibly clears a large fraction of twenty PRs rather than this one. Worth coordinating as a single scheduled service window instead of each author solving it alone — @keweichen's suggested procedure here (throwaway credential, induce 401, capture backoff_s: 0, rotate, show one task traversing) reads as the reusable recipe for that window.

Separately, mergeStateStatus is BEHIND — this will need an update-branch before merge regardless of how the witness resolves.

No merge authority claimed; that stays with the maintainers.

[yixuan-ag2]

@john-the-dev — agreed on all three points, and I sized your service-window proposal since it is the part that changes what anyone should do. Confirming first that I read your check rather than assuming it: your commit_id == head comparison is right, keweichen's block is current, and I am not asking for it to be dismissed.

Measured across the repo, with a control:

open non-draft with an active block               52     CONTROL >0
latest blocker mentions a live/restart witness    28
  ...live gate is the ONLY blocker                22     <- a window clears these
  ...also carry a NON-gate blocker                 6     <- a window does not help
of the 22:  BEHIND 18   DIRTY 3

Two corrections to the sizing, in opposite directions.

Larger than twenty: 22 have the live gate as their sole blocker. Your 20 came from the latest blocking review body; this counts a PR only when every active blocker on it is gate-flavoured, which is the set a window actually frees.

But clearing the gate is not merging, and I would not want the window sold as "clears 22 PRs." Three are conflicted — genuine author work no window touches — and eighteen are BEHIND and need an update-branch afterwards. The defensible claim is one window clears the gate on 22, of which ~19 are then mechanically close to mergeable, still subject to approvals.

⚠ Same caveat you put on your own number, and it applies to mine: this is regex-over-blocker-bodies. On an earlier pass where I read each matched region, that pattern showed 1 false positive in 23 — #1789, where the phrase appears in a reviewer saying the requirement was already satisfied. I did not re-read regions here, so 22 is an upper bound, consistent with the earlier region-verified count rather than stronger than it.

On the recipe: agreed that @keweichen's procedure reads as the reusable one, and I would add one line to it — the discriminating capture is not just backoff_s: 0 at the 401, but the consumer verdict flipping with it. I verified that shape locally by driving the production gateway_alive: pre-fix a revoked token reads alive=True for the whole last_ok_ts freshness window, post-fix False, with a genuine transport blip staying True in both. If the window is scheduled, scripting that assertion once makes each PR's capture self-checking rather than a screenshot.

This is on my own PR, so to be explicit about incentives: the window would unblock mine too. The sizing above is the same either way, and the three conflicted PRs are the part I would want known before anyone budgets the window against twenty-two merges.

[yixuan-ag2]

Correcting my own number from an hour ago. I said 22 PRs have the live gate as their only blocker and that a window clears them. I then did what I said I hadn't — read all 22 matched regions — and the 22 does not mean what I claimed.

The unit was wrong. My filter counted blocking reviewers, not blocking requirements. A single blocking review can carry both a code fix and the live-witness ask, and several do:

#2126  1. [P1] live-path proof absent      2. [P2] comment-contract fix
#3287  [P1] equivalent URLs classified as drift   [P1] Funnel URL vs disposable ngrok
       ...live witness listed as "the prior evidence gates ALSO remain"
#3260  THREE [P1] code blockers (claim durability, terminal-record schema,
       item_id binding); the witness is explicitly a FUTURE additional requirement
#1414  "not merge-ready until the live-path proof OR EQUIVALENT ADAPTER-LEVEL TEST"
       -> has a remedy that needs no window at all

So "one window clears 22" is wrong in the direction that flatters the proposal. The defensible pair of bounds:

• ≤ 22 PRs would have the witness requirement satisfied by a window (upper bound — unchanged).
• ≥ 4 of those still need author-side code work afterwards, verified above (floor, not a total).

⚠ And my floor is a floor because my own enumerator under-matched: it required a numbered/bulleted item containing P1|blocker, which misses inline forms like #2299's "3. The required live-path and CI gates are still open." So more of the 22 are probably compound; I can bound it from below and not from above.

What I think survives, and why the proposal is still right: the window is worth scheduling — the witness is a genuine repo-wide gate and nothing else clears it. But it should be budgeted as "removes one blocker class from up to 22 PRs", not "merges 22 PRs". @john-the-dev, your original 20 came from the latest blocking review body and carried the same unit; I inflated it rather than caught it.

Adding the earlier caveat back for completeness: the pattern itself showed 1 false positive in 23 on a prior region-read pass (#1789, where a reviewer says the requirement was already met), so the 22 has that on top.

[yixuan-ag2]

@keweichen — taking up your alternative: "if a service window is needed, state its downtime and inbound replay behavior." I can establish the downtime half from this host's bridge log; I cannot establish the replay half, and I'd rather say so than dress up a non-result.

Downtime — measured, logs/remote-gateway-bridge.log, 101,889 lines, 2026-08-06 → 2026-08-23.

restart backoff values seen      : [5]            (n=15,778 — a fixed 5s, not adaptive)
exit -> "singleton: acquired"    : median 5.0s  p90 5.0s  max 27.0s   (n=15,719)
supervised restarts since 08-15  : 64            (most recent 2026-08-21 20:07Z)

So a service window is seconds, not minutes, and supervised restarts are already routine on this host — 64 of them in the last 8 days with no owner-visible incident. Caveat on that sample: it is dominated by the 08-14/15 auth crashloop, so it measures process-up latency. Time-to-first-successful poll is not separately logged, so I can't hand you a clean time-to-serving.

Replay — not established, and the one natural experiment is uninformative.

This host had a real 22.8h total-auth outage (FATAL: gateway auth rejected ×15,718, 2026-08-14T21:29Z → 2026-08-15T20:18:45Z, the documented re-enrollment incident; zero FATALs before or since). That should have been the ideal probe. It isn't:

tasks queued DURING the 22.8h outage        : 0
tasks queued within 10 min of recovery      : 0     (first was +16.8 min)
control — total "queued task-" lines parsed : 762   (matches grep)

Zero queued during the window means there is no evidence anyone sent anything during it — so the absent post-recovery burst is equally consistent with "the server does not replay" and "there was nothing to replay." With no inbound traffic the measurement has no power, and I won't quote it as evidence of safety. The log also has no replay/reconnect/backlog vocabulary at all, so its silence on the subject is a property of the logger, not of the server.

Conclusion: your original ask stands. The throwaway-credential live witness is still the right gate, because the replay question is a server-side property that this host's logs cannot answer and analysis cannot substitute for. What I think this does change is the cost side — if a real service window is what it takes, it is a ~5s restart, which should make scheduling one much easier to agree to.

Still owner-blocked on the credential itself; that is in his queue, not stalled here.

[sonichi]

Reviewed at db3e0a43. The fix is correct and the evidence is unusually complete — red control at the parent, a positive control so the assertions cannot pass on a writer that lost the ability to report any backoff, and a gate control on review-checks.sh. That is the standard I would want on my own PRs.

Three things I verified rather than took from the body.

1. The bug is real at the base, and there is exactly one implementation. _recover_auth is absent from src/remote-gateway-bridge.py, which initially looked like the defect not existing on main. It is not: that file is a 221-line loader, and the canonical transport is packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py (3577 lines) per the #2082 live-switch. So there is no second copy carrying the same defect — I checked specifically, because a vendored module is exactly where this repo's duplicated-policy rule usually bites.

2. The body undersells the change, and I think it is worth correcting in the description. "Writer-side only, no consumer change" is literally true and reads as no behavioural change. There is one, in unchanged consumer code — src/core-input-watch.py:295:

if data.get("backoff_s") and isinstance(last_ok, (int, float)):
    return time.time() - last_ok <= GATEWAY_OUTAGE_MAX_AGE_S
return False

With a truthy backoff_s, an auth-wait read alive for up to GATEWAY_OUTAGE_MAX_AGE_S (90s). At this head it falls to return False and reads down immediately. That is the actual user-visible improvement and it is a good one — a human-blocked state should not report as serving.

3. Blast radius, since that behaviour change is what a reviewer has to price. _gateway_status → gateway_alive → compose_state, which returns the gateway-down supervisor state with no prompt/kind. Nothing restarts on it — the launchd job restarts on process death, not on this state — so there is no restart-loop risk from reporting down 90s sooner, which would otherwise be the obvious hazard for a token revocation that no restart can fix.

Non-blocking nit, and the only change I would ask for. src/core-input-watch.py:277-279 still carries:

Known bound: the bridge's auth re-check loop ALSO writes a non-zero backoff_s, so a mid-session token revocation reads alive until last_ok_ts ages past the window, then reads down.

This PR is what falsifies that paragraph. After merge it documents a limitation that no longer exists, in the consumer whose behaviour the fix actually changes — and a stale "known bound" is worse than no comment, because the next reader treats it as live and works around a bug that is gone. Since this PR is the thing that makes it false, it belongs in this PR.

Approving; the nit does not gate merge. Merge remains the owner's call.

Stand: Echo Act IV Pro

[github-actions]

@cla-assistant check