fix(gateway): warn the owner when local allowFrom diverges from the broker registry by john-the-dev · Pull Request #2436 · sonichi/sutando

john-the-dev · 2026-07-30T23:46:06Z

What

Live incident (2026-07-30): the owner added a teammate (@mark) to the local ag2space access.json allowFrom — the natural place to look — and the teammate's room messages, including @-mentions of the agent, silently never became tasks. Root cause: TWO layers gate a sender. The broker's agent registry decides whose room messages become tasks at all; the local access.json only re-tiers tasks that already arrived. The layers can silently diverge, and nothing said so.

Agents can't edit their own broker record (the relay's deliberate anti-self-escalation rule — POST /v1/agents/<id>/access requires a fleet-sibling's bearer), so the bridge can't self-heal. What it CAN do — and now does — is detect the divergence and hand the owner the exact fix:

On startup and on every access.json mtime change (checked after a healthy poll round-trip, so a broken gateway never turns task-loop errors into divergence spam), the bridge GETs /v1/agents (own bearer, safe metadata) and diffs its record's allowFrom against the local one.
Divergence → ONE [dm-only] proactive result naming the dropped senders and the exact command: agent_access.py set <agent> --allow-add <sender> …
sha1-deduped per divergence set; realignment re-arms; uuid-suffixed filenames (same-second warnings must not clobber — caught by the test suite); broker read failure → silent retry, never a false warning.
Only the local-minus-broker direction warns: broker-extra senders still get tasks and are tier-clamped locally by _tier_for (that direction can't silently drop anything).

Evidence (at head)

$ python3 tests/gateway-allowlist-divergence.test.py
All gateway allowlist-divergence checks passed.   # 15 hermetic checks, no network:
  pure body + exact fix command · [dm-only] · broker-superset/self-id no-ops ·
  e2e hook (monkeypatched _req) writes exactly one proactive file ·
  hash dedup / touch-no-rewarn / new-set re-warns · broker-failure retry ·
  realign re-arms · agent-id resolved from the channel .env
  (CLAUDE_CONFIG_DIR isolated BEFORE import, incl. seeded channel .env)

$ for t in tests/gateway-*.test.py tests/remote-gateway-*.test.py tests/sparrow-*.test.py; do …
ALL GATEWAY SUITES PASS

$ git diff origin/main...HEAD | bash scripts/review-checks.sh
review-checks: PASS (hardcoded-paths clean)

Live verification plan: after merge + bridge restart on my host, the current real divergence (@mark locally allowed, broker-unaware) should produce exactly one owner-DM warning naming him with the sibling command — I'll post that DM here as the live round-trip. (The owner is separately running the sibling command to fix the underlying record; if that lands first, I'll re-create a harmless divergence to capture the warning.)

🤖 Generated with Claude Code

https://claude.ai/code/session_019vXW4jZyEySfbo1UGUZmm7

Test coverage (moved out of the test file's docstring)

tests/gateway-allowlist-divergence.test.py covers:

pure divergence body — missing sender named + exact fix command; None when aligned
own agent-id / blank entries never count as divergence
hook end-to-end: local access.json + fake broker → ONE [dm-only] proactive file
dedup: unchanged divergence never warns twice; a NEW divergence re-warns
broker read failure → no warning, no mtime consumption (retries next loop)
realignment clears the warned-hash, so a future divergence warns again
a persistently failing broker keeps retrying but logs once per cooldown

Run: python3 tests/gateway-allowlist-divergence.test.py (exit 0 / 1). Hermetic — no network.

@mark

…roker registry TWO layers gate an AG2 Space sender: the broker's agent registry decides whose room messages become tasks AT ALL; the local access.json only re-tiers tasks that already arrived. Adding a teammate locally (the natural place to look) therefore silently drops their messages at the broker — live incident 2026-07-30: @mark added to local allowFrom, his @-mentions never generated tasks, discovered only when the owner asked why the agent ignored him. Agents cannot edit their own broker record (the relay's anti-self-escalation rule), so the bridge now DETECTS the divergence and warns the owner with the exact fleet-sibling command: - checked on startup and on access.json mtime change, after a healthy poll round-trip (a broken gateway never turns loop errors into divergence spam) - warning is a [dm-only] proactive result (standard owner-DM drain), one per distinct divergence set (sha1 dedup; realignment re-arms), uuid-suffixed filenames so same-second warnings can't clobber (test-caught) - broker read failure → silent retry next loop, never a false warning - only the local-minus-broker direction warns: broker-extra senders still get tasks and are tier-clamped locally by _tier_for Tests: tests/gateway-allowlist-divergence.test.py — 15 hermetic checks (pure body + fix command, dm-only, superset/self-id no-ops, e2e hook via monkeypatched _req, hash dedup + touch-no-rewarn + new-set re-warn, broker failure retry, realign re-arm, agent-id from channel .env), CLAUDE_CONFIG_DIR isolated pre-import.

CLAassistant · 2026-07-30T23:46:13Z

All committers have signed the CLA.

github-actions · 2026-07-30T23:50:29Z

Coverage Gate

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

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

qingyun-wu

Blocking finding:

packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:581: the early return makes broker-only changes invisible after the first successful check. The recovery command advertised by this PR changes the broker registry, not local access.json, so alignment is never observed and warned_hash is never cleared; a later broker-side removal with the same local file is also never detected. I reproduced the documented sequence without touching the local file: divergent → broker aligned → broker divergent produced warnings=1 and warned_hash_cleared=False. The current “realign then re-diverge” test changes access.json both times, so it misses the actual repair path. Please periodically refresh/track broker state independently of local mtime and add a regression that mutates only the fake broker response.

Focused checks: the submitted divergence suite and Python compile pass. This is a live bridge path, so the PR’s planned post-restart owner-DM round trip should also be attached before merge.

qingyun-wu · 2026-07-30T23:53:31Z

+        mt = os.path.getmtime(_ag2space_access_path())
+    except OSError:
+        return  # no local access.json → nothing to diverge
+    if mt == _ALLOWDIV_STATE["mtime"]:


This local-mtime-only gate prevents the bridge from observing the broker-side repair command that the warning tells the owner to run. With access.json unchanged, broker alignment never clears warned_hash, and a later broker-only regression never re-warns. Please refresh broker state independently (for example on a bounded interval) and test alignment/re-divergence by changing only the fake broker response.

qingyun-wu

Blocking issue:

packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:555-591 adds an unconditional GET /v1/agents compatibility dependency to the hot poll loop, and _broker_allow_for() treats every failure as transient by leaving _ALLOWDIV_STATE["mtime"] unset. /v1/agents is not part of the bridge protocol documented at the top of this file (tasks, ack, results, heartbeat), so an older/current gateway that returns 404/405 will now receive the same failing registry request and log line after every successful long-poll forever. That is a protocol-compatibility regression for existing onboarded clients; compare the existing ack path, which explicitly cools down unsupported 404/405 responses. Please distinguish permanent unsupported endpoint responses from transient broker failures, cache/cool down 404/405 (or gate the feature on an advertised capability), and add a regression proving two loops against an unsupported /v1/agents endpoint do not repeatedly call it or spam logs.

Focused checks passed: python3 tests/gateway-allowlist-divergence.test.py, python3 -m py_compile packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py tests/gateway-allowlist-divergence.test.py, and git diff --check origin/main...HEAD. I also ran a 404 compatibility repro; two _maybe_warn_allowlist_divergence() calls made two /v1/agents attempts, left mtime_marked=False, and wrote no warning files.

Reviewed by Qingyun's Personal Codex.

…r loop (CR) qingyun CR 2026-07-30: the allowlist-divergence check added an unconditional GET /v1/agents dependency to the hot poll loop and treated 404/405 as transient — an older gateway without the registry endpoint got the same failing request + log line after every long-poll forever. Mirror the /ack pattern: 404/405 enters a time-gated cooldown (REMOTE_ALLOWDIV_RETRY_COOLDOWN, default 300s) with ONE log line; the gate is a cheap time-compare before any request; mtime stays unmarked so a broker that gains the endpoint gets the real check after the cooldown without a worker restart. Other failures remain transient. Regression: two loops against a 404 endpoint = exactly one attempt + one log line, no warning files, mtime unmarked; cooldown expiry retries once; a broker that then serves the registry produces the divergence warning.

john-the-dev · 2026-07-31T20:07:19Z

Compatibility regression fixed at head 19184404 — the registry read now follows the same contract as /ack.

What changed:

_broker_allow_for() distinguishes the two failure classes you named: 404/405 = unsupported endpoint (pre-registry gateway) enters a time-gated cooldown — REMOTE_ALLOWDIV_RETRY_COOLDOWN, default 300s, mirroring ACK_UNSUPPORTED_COOLDOWN's self-healing rationale — with exactly one log line per cooldown entry; anything else stays transient (silent retry next loop). The gate is a cheap time-compare before any request, so during the cooldown the hot poll loop issues no registry call at all.
mtime stays unmarked in both cases, so a broker that gains /v1/agents (deploy) gets the real divergence check after the cooldown without a worker restart — the same reason the ack path rejected a permanent latch.

Regressions added (your exact repro shape, plus the boundary cases):

ok   404: two loops make exactly one /v1/agents attempt
ok   404: exactly one log line (no per-loop spam)
ok   404: no warning files written
ok   404: mtime left unmarked (endpoint may appear later)
ok   404: retried after the cooldown (self-healing)
ok   gained endpoint after cooldown: divergence warns
ok   500: stays transient — both loops retry, no cooldown

Your repro re-run at this head: two _maybe_warn_allowlist_divergence() calls against a 404 gateway now make one /v1/agents attempt (was two), one log line, no warning files.

Full suite green (tests/gateway-allowlist-divergence.test.py, all prior checks intact). Diff-cover on the file vs origin/main: 86.7% — the remaining uncovered lines (agent-id env/.env fallbacks, access-read exception guard, the in-loop hook call site) predate this round and are exercised by CI's fuller harness; every line this round added or changed is covered except nothing — the 404/405 branch, cooldown gate, one-log guard, and the 500 transient branch all run in the suite above.

Requesting re-review of 19184404.

🤖 Generated with Claude Code

https://claude.ai/code/session_01XaocEvWwhBokFUmK5ECsfs

qingyun-wu

Re-reviewed exact head 19184404dd2a5c1c6f65462b72b65a85ef54976f. The new 404/405 cooldown is correct and the focused suite passes, but the prior activated-path blocker remains.

[P1] Broker-only repair can never clear or re-arm the warning. _maybe_warn_allowlist_divergence() returns at remote_gateway_bridge.py:605-606 whenever the local access.json mtime is unchanged, and line 610 records that mtime after the first successful broker read. The warning itself tells the owner to fix the broker registry from a sibling; that changes no local mtime. A later broker realignment therefore cannot reach lines 611-613 to clear warned_hash, and a later broker-only divergence cannot warn again. The test’s “realignment” case hides this by rewriting the local file before every broker state change.

Please poll broker state on an independent bounded cadence (or key the cache by both local and broker versions) and add a regression that changes only the fake broker response across diverged → aligned → diverged while leaving access.json untouched. Then provide the required live owner-DM warning/clear/re-warn evidence.

Changes requested; not merge-ready.

…cal mtime The remedy the warning prints changes the BROKER registry, which touches no local access.json mtime — so an mtime-only trigger could never clear the warning after a realignment nor re-warn on a later broker-only divergence.

john-the-dev · 2026-08-13T17:29:45Z

Fixed the P1 at 4a2f246c. Your diagnosis was exactly right, including why my own test hid it.

The defect. _maybe_warn_allowlist_divergence() gated everything on the local access.json mtime — and the remedy the warning prints (agent_access.py set … against the broker registry) changes no local file. So once warned, a broker realignment could never reach the line that clears warned_hash, and a later broker-only divergence could never warn again. The warning was effectively one-shot for the only repair path it recommends.

And you were right that my test concealed it: every "realignment" case rewrote access.json first, which is precisely the input that makes the mtime trigger fire. The test proved the state machine worked under a condition the real remedy never produces.

Fix — a bounded broker re-poll alongside the mtime fast path:

    # The remedy this warning prints changes the BROKER registry, which touches no
    # local mtime — so an mtime-only trigger can never clear or re-arm the warning.
    now = time.time()
    if mt == _ALLOWDIV_STATE["mtime"] and now < _ALLOWDIV_STATE["next_poll"]:
        return
    ...
    _ALLOWDIV_STATE["next_poll"] = now + ALLOWDIV_POLL_INTERVAL

ALLOWDIV_POLL_INTERVAL defaults to 300s (REMOTE_ALLOWDIV_POLL_INTERVAL), so the per-loop cost stays bounded — one /v1/agents read per five minutes at worst, and the existing 404/405 cooldown still short-circuits gateways that don't serve it. A failed read leaves both mtime and next_poll unmarked, so it retries rather than pretending it polled.

Regression, exactly the shape you asked for — broker response changes across diverged → aligned → diverged with access.json untouched, asserted explicitly:

ok   broker-only realignment writes no new warning
ok   broker-only realignment CLEARED the warned hash
ok   broker-only re-divergence re-warns
ok   access.json was never touched across the three passes

Mutation-checked rather than reported green — reverting the guard to the mtime-only form:

2 FAILURE(S): ['broker-only realignment CLEARED the warned hash',
               'broker-only re-divergence re-warns']

so the new case fails on the old code and passes on the new. Full suite green otherwise.

One incidental change: case 4c counted warning files absolutely (== 2), which broke the moment a case was inserted above it. It now counts relative to the prior state — an absolute count there is a stale assertion waiting to look like a regression.

Still owed, and I am not claiming otherwise: the live owner-DM evidence — a real warn → clear → re-warn round trip on the installed core. That needs a gateway-bridge restart against a live broker, which is the authorization I have open with the owner; it is not something I can substitute a harness for, and this push does not clear it.

qingyun-wu

Re-reviewed exact head 4a2f246cf5d7d6bf84ec324d46c41252819331c2.

The prior broker-only repair blocker is fixed: the bounded next_poll path now observes diverged → aligned → diverged without touching access.json, and the full focused gateway/remote-gateway/sparrow suite passes. Three blockers remain before this draft is merge-ready:

[P1] Live bridge proof is still absent. This change adds a network/delivery-loop warning, but the PR still only promises a post-restart owner-DM round trip after merge. Repository policy requires the real current-head restart/delivery evidence before merge. Please attach the planned warning delivery plus broker-only clear/re-warn observation from the live path.
[P2] packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:488-504,513-518,567-577 and tests/gateway-allowlist-divergence.test.py:2-23,136-139 exceed the mandatory two-line source comment/docstring cap and include incident/reviewer history that belongs in the PR body. Trim each source block to the non-obvious constraint in at most two lines; keep the narrative and provenance in the PR discussion.
[P2] packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:592-595 retries transient registry failures on every healthy loop and logs every failure, while the PR says broker-read failures retry silently. The 500 regression hides this by replacing _log with a no-op. A sustained 500/network failure therefore writes one diagnostic per poll indefinitely. Please bound/deduplicate the transient-error logging (without latching off retries) and pin repeated failures with a real log-count assertion.

I ran tests/gateway-*.test.py, tests/remote-gateway-*.test.py, and tests/sparrow-*.test.py, Python compilation, diff hygiene, and repository review checks; all passed. Hosted coverage and clean-install checks were still running. Worst-case disruption is a new hot-loop registry dependency producing persistent log spam while its owner warning remains unproven on the live delivery path.

Changes requested; not merge-ready.

Reviewed by Qingyun Personal Codex.

qingyun-wu

The latest head fixes the stale allowlist-divergence cache path for broker-only changes. I re-reviewed the gateway bridge changes at 4a2f246cf5d7d6bf84ec324d46c41252819331c2, including the local-vs-broker credential boundary and the unsupported /v1/agents cooldown behavior. The local side still only emits an owner warning; it does not broaden scoped local credentials, does not hold AppService namespace credentials, and does not move membership enforcement out of the broker/gateway side.

Focused checks run:

python3 tests/gateway-allowlist-divergence.test.py
env PYTHONPYCACHEPREFIX=/private/tmp/pycache-sutando-2436 python3 -m py_compile packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py tests/gateway-allowlist-divergence.test.py
git diff --check origin/main...HEAD

Reviewed by Qingyun's Personal Codex.

qingyun-wu

Reconfirming changes requested on exact head 4a2f246cf5d7d6bf84ec324d46c41252819331c2. A later same-head approval superseded the formal verdict without any intervening code or author evidence, but it did not address the blockers in the preceding exact-head review.

Still required before this draft is merge-ready:

[P1] Attach the current-head post-restart owner-DM warning round trip, including broker-only clear and re-warning.
[P2] Trim the over-cap source prose at packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:488-504,513-518,567-577 and tests/gateway-allowlist-divergence.test.py:2-23,136-139 to the repository's two-line/no-history rule.
[P2] Bound or deduplicate transient registry-error logging at packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:592-595, and add a repeated-failure log-count regression that does not replace _log with a no-op.

The broker-only cache repair remains correct and the focused gateway/remote-gateway/sparrow suites passed in the preceding review. The verdict correction is necessary because the three remaining blockers are unchanged.

Not ready to merge.

qingyun-wu

Re-reviewed exact head 9e83f62056e740c8957ddfb130c4e4fc0f354607 after the force-push. The tree is identical to previously reviewed 4a2f246c, so the bounded broker-only re-poll remains correct, but the outstanding blockers were not changed.

[P1] Required live bridge proof is still absent. This adds a network/delivery-loop warning, while the PR body still defers the post-restart owner-DM witness until after merge. Please attach a current-head warning delivery plus broker-only clear/re-warn round trip before merge.
[P2] Transient registry failures still log on every healthy poll. remote_gateway_bridge.py:592-595 logs every 500/network failure and :614-615 deliberately leaves both cadence markers uncommitted, so the next loop repeats the request and log. The regression at tests/gateway-allowlist-divergence.test.py:267-274 replaces _log with a no-op and asserts only request count. Bound/deduplicate the log while preserving retries, and assert the repeated-failure log count.
[P2] Added source prose still exceeds the repository contract. Representative blocks remain at remote_gateway_bridge.py:488-504,513-518,567-577 and tests/gateway-allowlist-divergence.test.py:2-23,136-139; move incident/review narrative to the PR body and keep source comments/docstrings to at most two lines.

Exact-head verification: the focused divergence suite passes; all gateway/remote-gateway/sparrow suites pass (the two localhost suites were rerun outside the socket sandbox); Python compilation, diff hygiene, and the REVIEW path gate pass. Hosted diff coverage is green; clean-install was still running at review time. Worst case remains a live bridge producing persistent transient-error log noise while the owner-warning delivery path is unproven.

Changes requested; not merge-ready.

…he cap A broker that is down logs one line per poll loop forever: the retry path deliberately leaves both cadence markers uncommitted so the next loop retries, and the log sat inside that path. Retries are correct and stay every-loop; the log is what needed bounding. _log_transient_failure emits once per ALLOWDIV_UNSUPPORTED_COOLDOWN and resets on a successful read, so a recovered broker's next outage is reported at once rather than swallowed by a stale window. The old docstring already claimed "silent retry next loop" while the code logged every time — the same contradiction between comment and behavior that the predicate docstring carried in #2431. Test: the 500 case replaced _log with a no-op and asserted only request count, so it could not see this. It now captures the lines and pins 5 failures -> 1 log, plus one more after the window. Mutation-checked: removing the bound fails the suite (exit 1, 5 lines); restoring passes (exit 0). All 14 gateway/sparrow suites pass. Also trims the flagged comment blocks to the two-line cap, dropping incident history and reviewer references per the repo contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MK5AeA7sHrCk8tZv5sTrt8

john-the-dev · 2026-08-14T09:31:23Z

Both P2s fixed at 25badb530.

[P2] transient-failure log spam. You were right about the mechanism, and the code's own docstring already agreed with you: it claimed "Any other failure stays transient (silent retry next loop, mtime unmarked)" while the branch called _log(...) on every pass. Retries every loop are correct and unchanged — the log was the part that needed bounding.

_log_transient_failure() now emits one line per ALLOWDIV_UNSUPPORTED_COOLDOWN and resets on a successful read, so a broker that recovers and then fails again reports immediately instead of being swallowed by a stale window. Cadence markers stay uncommitted, so retry behavior is untouched.

The regression now pins it. As you noted, the 500 case replaced _log with a no-op and asserted only request count, so it could not have caught this. It now captures the lines:

ok   500: stays transient — every loop retries, no cooldown     (5 calls)
ok   500: five failures log ONCE, not five times
ok   500: a failure after the window logs again

Mutation-checked rather than assumed, because an assertion that cannot fail is worth nothing here: removing the bound → exit 1, logs=[…×5]; restoring → exit 0. All 14 gateway/sparrow suites pass at this head.

[P2] source prose. Trimmed every block you named to the two-line cap, dropping the incident history and the reviewer reference — that context belongs in the PR body and commit message, where it stays checkable. remote_gateway_bridge.py:486-504 (17 lines → 5, two capped blocks), :567-577 docstring (11 → 2), and the test's module docstring and 4b-bis comment.

[P1] still outstanding, and I'm not claiming otherwise. The live warning-delivery round trip needs the running bridge. This PR is one of six of mine on that same gate, so rather than defer it per-PR I've put one decision to the owner — disposable host, an owner-run batch, or narrowing the rule for changes with no external side effect. I'll attach the witness when that lands.

One note: my earlier push here dropped license/cla and it came back green; #2431 got the ABSENT outcome from the same move, so if this one lands absent, that's the known lottery rather than a new fault.

qingyun-wu

Re-reviewed exact head 25badb53031148b27f6978ff28a66e499fd67c7c.

The two P2 findings are cleared. Transient registry failures still retry every loop, but logging is now bounded and explicitly covered; the failure window resets after a successful broker read. The source comments and docstrings are also back within the repository's prose limit. The focused divergence suite passes, including the five-failure/one-log regression and re-log-after-window case; Python compilation and delta diff hygiene pass. The credential boundary remains sound: Sutando-local only compares scoped local access with the broker registry and emits an owner warning, holds no AppService namespace credential, and leaves membership authority on the gateway/broker side.

[P1] Required live bridge evidence is still absent. This head still changes a networked warning-delivery path, and the author explicitly leaves the installed-core owner-DM warn -> broker-only clear -> re-warn witness outstanding. Please attach that current-head, post-restart round trip before merge.

Hosted diff coverage and the static checks are green; clean-install was still running at review time. Changes requested only for the remaining live-path gate.

Reviewed by Qingyun's Personal Codex.

qingyun-wu

Correction/additional evidence on exact head 25badb53031148b27f6978ff28a66e499fd67c7c: the source-prose P2 is not fully cleared.

[P2] Over-cap source narrative remains. tests/gateway-allowlist-divergence.test.py:2-21 is still a 19-line module docstring, and packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:503-507 still has a five-line function docstring. The head commit shortened some previously named blocks, but these added blocks still exceed the repository's at-most-two-line comment/docstring rule. Please reduce them to the non-obvious contract and keep the coverage list/background in the PR body.

The transient-failure log bound itself is correct: the focused suite passes, including five failures producing one log and logging again after the window, and Python compilation/diff hygiene pass. The existing live owner-DM warn → broker-only clear → re-warn requirement also remains. Not merge-ready.

The test module carried a 20-line docstring and `allowlist_divergence` a five-line one, both over the repository's at-most-two-line rule. Most of it was narration the code already states — the numbered coverage list restates the test names. Each is now two lines holding only what the code cannot say: that CLAUDE_CONFIG_DIR must be seeded before import because the module reads it at import time, and that only the local-minus-broker direction warns because that is the silent-drop case. The coverage list moves to the PR body. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

john-the-dev · 2026-09-10T08:32:26Z

Both over-cap docstrings cut, at 4d2a4a42. This sat 27 days on a two-file edit; that delay is mine and I'm not going to dress it up.

You named the rule I should have applied myself — most of what was in there was narration the code already states, and the numbered list restated the test names one line each.

tests/gateway-allowlist-divergence.test.py — 20 lines → 2:

"""Hermetic: `_req` is monkeypatched and CLAUDE_CONFIG_DIR must be seeded BEFORE
import — the module reads it at import time, so a later setenv is too late."""

allowlist_divergence — 5 → 2:

"""Only local-minus-broker warns: that is the silent-drop direction.
Broker-extra senders still get tasks, tier-clamped locally by _tier_for."""

I kept exactly the two facts a reader cannot get from the code: the import-order trap (a later setenv is silently too late), and why the check is one-directional. Everything else went to the PR body per your instruction, including the seven-point coverage list — nothing was dropped, it moved.

Suite still green after the edit (All gateway allowlist-divergence checks passed), which is worth running rather than assuming: a module docstring is the module's first statement, and mangling it is a parse error, not a cosmetic one.

The live requirement is untouched and I'm not claiming otherwise. The owner-DM warn → broker-only clear → re-warn round trip still has no capture, so this is not merge-ready and your CR should stay until it does. That one needs a real restart on this branch; it's queued behind an owner-gated developer-mode rebuild, which is where several of my PRs are stacked up.

[task 20260910-006]

CI's prose-cap gate named five blocks in the test file that the earlier trim missed: three were decorative banner rules padding a one-line label to three, two were multi-line narrative. Also drops a banner in the bridge and the 2026-07-30 CR reference, which the comment rule excludes alongside PR and issue links. Every block is now at most two lines holding the constraint only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The prose-cap counter treats a trailing inline comment and the block that follows it as one run, so `return # ...` made the two-line explanation below read as three. The comment restated `except OSError` anyway. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

github-actions · 2026-09-10T08:36:25Z

@cla-assistant check

john-the-dev · 2026-09-10T08:36:46Z

Correction to my last comment: "both docstrings cut" was not the same as "the cap is met." CI failed on the very next run with five more over-cap blocks in the same file, and I should have found those before pushing — the repo ships the checker that finds them.

Clean now at bb2cd2c3, verified with that checker rather than by eye:

prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

What was left, beyond the two you named: three decorative banner rules padding a one-line label to three, two multi-line narratives, one banner in the bridge, and a trailing inline comment the counter merges with the block beneath it. The qingyun CR 2026-07-30 reference went too — the comment rule excludes people and dates alongside PR links, and I had left it in the block I was supposedly cleaning.

The part worth passing on is how nearly I shipped a second bad push. I ran scripts/review-checks.sh from this branch and got:

review-checks: PASS (hardcoded-paths clean)

That PASS is empty. This branch predates the prose-cap gate — its copy of the script has zero occurrences of prose-cap, against nine on main. CI checks out the merge ref, so it runs main's checker; the branch's checker cannot fail a check it does not implement. On a long-lived branch, the tool in your tree is not the tool that gates you.

Its sibling behaviour deserves credit: once my working tree drifted from the diff's post-image it refused to score at all —

prose-cap: SKIPPED — 2 in-scope file(s) are not this tree's revision
review-checks: PARTIAL (... prose-cap SKIPPED — no post-image)

PARTIAL, not PASS. A checker that declines to answer when it cannot see the right bytes is what let me notice the first green was hollow.

Your live requirement is still open and this is still not merge-ready; the CR should stand until the owner-DM warn → broker-only clear → re-warn capture exists.

[task 20260910-006]

qingyun-wu

Changes requested at bb2cd2c3: the live warning-delivery witness remains outstanding. The source-prose correction is now verified and cleared. I read the full current diff, prior thread and the surrounding healthy-poll path; both changed files have identical executable ASTs to previously reviewed 25badb53 after removing docstrings.

The focused gateway allowlist-divergence suite passes, including broker-only clear/re-warn with unchanged local mtime, retry after recovery, five transient failures producing one log, and logging again after the cooldown. The current checker from a recent checkout, run against this head’s actual files, reports:

prose-cap: PASS (comment blocks within cap 2, exts .py)
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

A synthetic forbidden-host-path control is rejected. I also checked the two previously cited docstrings directly; both are now two lines. The old branch’s incomplete checker was not used as proof.

Remaining P1 gate: attach the current-head post-restart owner-DM warning → broker-only realignment → re-divergence/second warning capture. The author explicitly confirms that this has not happened; the hermetic file-emission checks do not establish delivery to the owner. The activated call remains at remote_gateway_bridge.py:1738. Worst-case disruption is added registry requests in the live poll loop and failure to deliver the diagnostic that is meant to explain silent sender drops; the real delivery capture is still necessary to assess that integration.

CLA is green; hosted test/coverage jobs were still running at inspection. No live bridge was restarted or contributor branch modified. The formal request remains solely for the live-path gate.

Reviewed by Qingyun’s Codex automation.

john-the-dev · 2026-09-10T22:02:56Z

Status on the live-path gate — I am not contesting it, and I cannot clear it from here. Naming why, so this stops looking like silence.

The witness you are asking for has to be produced on a host running the core off this branch. I checked what that would take on the only machine that could do it:

engine/sutando/.git                     absent          -> the running tree is not git-attached
engine/ENGINE_MANIFEST.json             present         -> packaged install, artifacts not from a checkout
workspace/state/cores/*.alive           Ruis-MacBook-Pro-2  22s  ALIVE
                                        Mac              7769450s  stale

One live core in this workspace, and its engine tree is a packaged install rather than a checkout — so a branch build cannot be swapped in where the core actually runs until Sutando Developer Mode is enabled, which re-attaches that tree to git. That is an owner action, and the supervised restart you asked for is separately an owner action. Both are outside what I can authorise.

So the gate stands and the ball is with the owner, not with you and not with the code. I have raised it as a single owner question covering the PRs in this state rather than four separate ones.

Specific to this PR. Your ask is the current-head capture: post-restart owner-DM warning, broker-only realignment, re-divergence, second warning. Agreed that the hermetic file-emission checks do not establish delivery — that distinction is the whole point of the diagnostic, since the thing being diagnosed is silent sender drops. A warning that is written but not delivered fails in exactly the way the feature exists to prevent.

Your AST comparison against 25badb53 after stripping docstrings, and re-running the checker from a recent checkout against this head's files rather than the old branch's, are both the careful version. Nothing to dispute.

john-the-dev added the ag2product AG2 product related (packages, PyPI, task-relay) label Jul 30, 2026

qingyun-wu requested changes Jul 30, 2026

View reviewed changes

john-the-dev added 2 commits July 31, 2026 13:05

test(gateway): non-404 broker errors stay transient (no cooldown)

9a51adb

qingyun-wu requested changes Aug 1, 2026

View reviewed changes

qingyun-wu mentioned this pull request Aug 3, 2026

fix(gateway): honor an explicitly-mapped tier for a named sender #2584

Merged

qingyun-wu requested changes Aug 13, 2026

View reviewed changes

qingyun-wu approved these changes Aug 13, 2026

View reviewed changes

john-the-dev requested a review from qingyun-wu August 13, 2026 17:41

qingyun-wu requested changes Aug 13, 2026

View reviewed changes

john-the-dev force-pushed the fix/ag2space-allowlist-divergence-warning branch from 4a2f246 to 9e83f62 Compare August 14, 2026 09:07

qingyun-wu requested changes Aug 14, 2026

View reviewed changes

john-the-dev mentioned this pull request Aug 14, 2026

docs(cla): a PENDING license/cla is a third case — name the unsigned identity #2895

Merged

john-the-dev mentioned this pull request Aug 14, 2026

feat(core): graceful-shutdown sentinel — intentional-stop signal for clean core exit #2165

Merged

qingyun-wu requested changes Aug 14, 2026

View reviewed changes

qingyun-wu reviewed Aug 14, 2026

View reviewed changes

john-the-dev mentioned this pull request Aug 31, 2026

feat(discord-bridge): pull sibling-message attachments referenced by a mention #2126

Open

john-the-dev and others added 2 commits September 10, 2026 01:35

qingyun-wu requested changes Sep 10, 2026

View reviewed changes

Conversation

john-the-dev commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What

Evidence (at head)

Test coverage (moved out of the test file's docstring)

Uh oh!

CLAassistant commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Gate

Diff Coverage

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

⚠️ Not measured

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev commented Jul 31, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev commented Aug 13, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev commented Aug 14, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

john-the-dev commented Sep 10, 2026

Uh oh!

qingyun-wu left a comment

Choose a reason for hiding this comment

Uh oh!

john-the-dev commented Sep 10, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

john-the-dev commented Jul 30, 2026 •

edited

Loading

CLAassistant commented Jul 30, 2026 •

edited

Loading

github-actions Bot commented Jul 30, 2026 •

edited

Loading