Skip to content

fix(gateway): the bridge advertises the pool from the roster, not a frozen snapshot - #4161

Closed
qingyun-wu wants to merge 25 commits into
rescue/pool-uncommitted-2026-08-26from
feat/bridge-advertise-from-roster
Closed

qingyun-wu wants to merge 25 commits into
rescue/pool-uncommitted-2026-08-26from
feat/bridge-advertise-from-roster

Conversation

@qingyun-wu

Copy link
Copy Markdown
Collaborator

What

The AG2 Space gateway bridge advertised the worker pool from state/pool-status.json, a pool-lead snapshot frozen since the legacy installed-core pool stopped (ts 1788910739, live_cores: [core-1, core-2]), and PUT a profile card with no workers field. The broker replaces the profile document, so every bridge boot erased the worker map the picker draws from — observed on production tonight after the 20:28 core restart: the owner's binding disappeared from both clients until a worker re-advertised by hand.

The bridge now reads state/pool-advertisement.json, written by the pool side from the roster (sutando#4119 pool_advertise.write_advertisement): "workers" is POSTed verbatim, "profile_workers" rides on the card, and the file's mtime is part of the profile change key so a roster rewrite re-puts even an identical map. No file = no pool = no push. The retired snapshot path is gone from the bridge (the pool_status.py writer is untouched; retiring it is a separate change).

Stacked on the rescue line (#3604); base rescue/pool-uncommitted-2026-08-26.

Evidence

Control — the new test against the parent commit's bridge:

File ".../test_pool_advertisement_push.py", line 66, in test_boot_pushes_workers_and_a_card_carrying_them
    assert m._maybe_push_workers_snapshot() is True
AssertionError

and the retargeted tests/gateway-workers-snapshot-push.test.py against the parent: 8 failure(s).

At HEAD:

PASS test_boot_pushes_workers_and_a_card_carrying_them
PASS test_no_advertisement_file_pushes_a_card_without_workers
PASS test_a_file_without_the_keys_pushes_nothing_extra
PASS test_a_non_dict_workers_value_is_ignored
PASS test_a_later_mtime_repushes_both
PASS test_mtime_alone_repushes_the_card
ALL PASS test_pool_advertisement_push
### root workers-snapshot ###   0 failure(s)
### root agent-profile ###      0 failure(s)
### package suite (13 files) ### all PASS
### src/remote-gateway-bridge.test.py ### PASS — all checks green
### tools/test_no_drift.py ###  PASS — package in sync with src/
ruff: All checks passed!
review-checks: PASS (hardcoded-paths + root-artifacts + prose-cap clean)

Live path (post-restart round trip, local-homeserver lane, this commit, 2026-09-10 21:04 PT):

[remote-gateway-bridge] starting — gateway=http://localhost:9995/relay provider=ag2space tasks=<ws>/tasks (restored 0 in-flight)
[remote-gateway-bridge] workers-snapshot pushed
[remote-gateway-bridge] agent-profile pushed for @qingyun-oss.agent:ag2space.local

The production lane restart follows the owner's local-first instruction and is reported in the PR thread when done.

.github/workflows/ci.yml gains one line because the package tests are listed explicitly, not globbed; without it the new suite would run only in the tag-triggered publish workflow.

🤖 Generated with Claude Code

qingyun-ag2 and others added 25 commits August 31, 2026 15:01
…tribution

Delivered results get the stamp from the broker; a direct say bypassed it
and rendered plain. say now sends the stamp via the message op's existing
extra_content field — defaulting to core-$SUTANDO_CORE_ID, --worker '' opts
out. Proven live: event $rwVH5cFDdw2... carries {'id': 'core-1'}.
Completes direct-path attribution: both posting verbs now carry the
worker stamp from SUTANDO_CORE_ID.
…can self-identify (renders grey client-side)
Owner: "worker color" can read as skin color. Old COLOR name stays as a
one-release alias.
Names are what users see; numeric ids stay internal (owner 2026-08-31).
…te fix from PR branch

Gate keys on the OPERATION (any room-scoped write), not the mode flag,
and the incoherent flag pair is rejected outright. Found by air on
0df1059, independently reproduced by bassilkhilo-ag2 on 028e008.
…one-release alias

Owner: instances are workers. Stamp derivation emits worker-N; the
installer plist exports the new names beside the old; core-status's
follower guard and outbox_log honor either.
Affinity was lead-side only, so a dead lead dropped it entirely: acquire_work
degraded to a bare first-come rename over the pending list with no affinity
check, and all workers raced. Five Space Structure tasks went to core-3 tonight
while the room was pinned to core-2.

The fallback now reads `state/pool/affinity.json` before racing and skips a task
whose channel is pinned to a DIFFERENT instance that is still beating. A dead
owner is not deferred to, so a pin can never strand work — the property the
mutation test pins.

Only `pinned: true` counts; a bare sticky entry is a decayed handler, not an
owner's binding.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
affinity.json was re-read and re-parsed for every pending file — the same few KB
for an answer that cannot change inside one loop. Hoisted to a single read.

Measured on 50 pending files against a 60-entry table: 3.19 ms -> 0.85 ms.
Small either way at a 5-minute sweep, but the waste was structural.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous docstring claimed the answer 'cannot change mid-loop'. It can — an
owner can pin a room while a sweep runs. What the hoist actually buys is a
consistent view for every candidate in one sweep, with a mid-sweep pin landing
on the next one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rozen snapshot

On every boot the bridge re-POSTed state/pool-status.json (a pool-lead
snapshot frozen since the legacy pool stopped, naming core-1..core-4) and
PUT a profile card without a workers field; the broker replaces the
document, so each restart erased the worker map the picker draws from.

The bridge now reads state/pool-advertisement.json, written by the pool
side from the roster: "workers" is POSTed verbatim, "profile_workers"
rides on the card, and the file's mtime is part of the profile change key
so a roster rewrite re-puts even an identical map. No file = no pool = no
push; the retired snapshot path is gone.

@qingyun-wu qingyun-wu left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested at 9809862bcbb107938e16a76342d9fa0e9eb20d9a (self-author COMMENTED verdict).

The intended valid-file path works: six package cases, snapshot/profile suites, pin-affinity 6, progress-stamp 3, display-gate 14, room-relations 19 and room-say 20 checks pass; package drift check passes. With the parent bridge and current tests, the new package boot case fails and the snapshot suite reports eight failures.

Blocking: a failed advertisement read becomes a destructive profile update. Through the actual bridge methods with request capture:

valid file:             PUT includes workers
unchanged valid file:   no PUT
then corrupt JSON:     PUT omits workers
restore valid file:    PUT includes workers
then remove file:      PUT omits workers

The broker's replacement contract described in this PR makes those omissions erase the worker map. Preserve the last valid advertisement or defer the replacement when pool state is unknown; distinguish that from an explicit valid empty pool. Add valid→corrupt/missing/unreadable controls. The current malformed-file test only asserts that the field is absent, which pins the destructive request rather than preservation.

There is also an upgrade gap: an otherwise valid legacy pool-status.json alone now produces no workers POST, and this branch does not include the #4119 producer. Supply a staged producer/dependency or migration path before removing the old read contract.

Full current diff includes unrelated room display, attribution and follower-pin changes beyond the final gateway commit; split/rebase the advertised concern against the intended parent before landing. The body provides restart/profile-push logs but no received-task→delivered-reply witness; that remains required by CONTRIBUTING.md. Full scan passes with a rejecting forbidden-path control. Worst case is lost picker metadata/status during read failures or upgrades. No live network mutation or contributor branch change.

Reviewed by Qingyun’s Codex automation.

"host": {"host_id": host_id, "kind": "local"}}
# The broker REPLACES the profile document, so an omitted workers map
# deletes the pool the picker was drawing.
workers = _read_pool_advertisement()[1].get("profile_workers")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[P1] Do not erase worker metadata when the advertisement read fails

After one valid advertisement is pushed, corrupting or removing the file makes _read_pool_advertisement return an empty record. The card omits workers and the new cache key causes another PUT. Request capture through the production methods confirms valid→corrupt and valid→missing both send a replacement without workers, while the unchanged-valid control sends nothing. Under the replacement semantics documented here, this wipes the picker map. Carry unknown separately from an explicit empty pool and defer the replacement or retain last-good metadata on failed reads; test those transitions.

@qingyun-wu

Copy link
Copy Markdown
Collaborator Author

Superseded by #4162, the same reader re-authored on the pool stack (base #4115) per the owner — nothing new targets the rescue branch. The local-homeserver lane keeps running this commit until the replacement is deployed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants