[keweichen]

Priority

Level: high
Reason: A desktop-spawned gateway can silently apply a stale tier map from an unrelated install, changing sender trust classification without any diagnostic.

Closes

Closes #2332

Summary

Resolve the ag2space access map from the gateway's active configuration instead of guessing ~/.claude.

• Prefer the access.json sibling of explicit AG2_DEVICE_ENV.
• Fall back to explicit CLAUDE_CONFIG_DIR/channels/ag2space/access.json.
• Disable the tier map when neither path is configured; never guess from HOME.
• Log the selected access path once.
• Cache by path and mtime so an active-config switch reloads even when mtimes match.

Root cause and user impact

Desktop launchers can provide AG2_DEVICE_ENV without CLAUDE_CONFIG_DIR. The previous resolver ignored that active device configuration and read ~/.claude/channels/ag2space/access.json instead. A stale file there could silently assign a sender the wrong access tier.

Before the fix, the focused reproducer showed the configured map was ignored and the stale home-directory map supplied tier team instead of configured tier other.

Validation

• python3 tests/gateway-access-path-resolution.test.py
• python3 tests/gateway-per-sender-tier.test.py
• python3 src/remote-gateway-bridge.test.py
• python3 tests/remote-gateway-bridge.test.py
• Python bytecode compilation
• Ruff 0.15.22
• git diff --check
• scripts/review-checks.sh

The new regression coverage checks device-env precedence, the CLAUDE_CONFIG_DIR fallback, no HOME guessing, selected-path logging, correct tier loading, and same-mtime path-switch invalidation.

Edge cases and risk

• If both variables are explicit, the device configuration wins.
• If only CLAUDE_CONFIG_DIR is set, existing explicit-config behavior remains.
• If neither is set, the map is disabled rather than reading unrelated state.
• Missing or malformed maps retain the existing last-known-good behavior.

This is a focused resolver/cache change with no migration, permission, prompt, or protocol changes. Rollback is a normal revert.

Review order

1. packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py
2. tests/gateway-access-path-resolution.test.py

Review follow-up — current head c84a78b

The requested cache-disable case is fixed in f4bb56c: when both launcher config pointers are removed, _load_tier_map() now clears and returns an empty map instead of treating the empty path as a transient stat failure. The regression first loads a mapped sender, removes both pointers, and verifies the sender returns to LOCAL_TIER.

Live bare-launch restart evidence

Ran the real ag2-sparrow entrypoint twice as isolated bare processes against a local relay, with AG2_DEVICE_ENV=/tmp/pr2340-live-evidence/channel/.env and CLAUDE_CONFIG_DIR explicitly unset. Both starts emitted:

[remote-gateway-bridge] access tier map path: /tmp/pr2340-live-evidence/channel/access.json

The first process queued sender @teammate:ag2.space as:

id: LIVE1
user_id: @teammate:ag2.space
access_tier: other

After terminating and restarting the bridge, the second real process again resolved the same sibling map and queued:

id: LIVE2
user_id: @teammate:ag2.space
access_tier: other

This exercises process startup, HTTP task pull, access-path logging, per-sender tier lookup, and task-file serialization—not an in-process import harness.

Post-sync validation on current main:

• access-path regression: 7/7
• per-sender tier regression: 13/13
• both remote gateway bridge suites: pass
• Python compilation, Ruff 0.15.22, git diff --check, and scripts/review-checks.sh: pass
• branch: 0 commits behind main

[john-the-dev]

Blocking on current head aca241f:\n\n- packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py, _load_tier_map(): when _ag2space_access_path() returns an empty string after configuration is removed, os.path.getmtime("") raises and the OSError branch returns _TIER_MAP_CACHE["map"]. If this process previously loaded a tier map, the supposedly disabled configuration keeps applying that stale trust map. This contradicts the PR body/added test claim that no explicit launcher config disables the map. Please clear/return {} when path is empty and add a regression that first loads a map, removes both config pointers, then verifies _tier_for() no longer uses it.\n\n- Live evidence required after that fix: this changes the gateway trust-classification path. Please include one real desktop/bare-launch restart using AG2_DEVICE_ENV without CLAUDE_CONFIG_DIR, then show the selected access.json log line and one sender classified from that map. Harness-only proof is not enough for this delivery/startup path.\n\nEvidence reviewed: commit aca241f, full diff, PR validation description, and current CLA state. CI workflows beyond CLA have not appeared yet, so they are not verified.

[keweichen]

@john-the-dev Both requested changes are addressed on current head c84a78b (focused fix f4bb56c plus a clean sync to current main).

• Empty-path handling now explicitly clears a previously loaded tier map, with a regression that loads a mapped sender, removes both config pointers, and verifies fallback to LOCAL_TIER.
• I ran the real ag2-sparrow entrypoint twice as bare processes with AG2_DEVICE_ENV set and CLAUDE_CONFIG_DIR unset. Both starts logged the sibling access.json path, and the pulled @teammate:ag2.space task serialized as access_tier: other before and after restart. The full sanitized trace is now in the PR body.
• Access-path 7/7, per-sender tier 13/13, both gateway suites, compilation, Ruff 0.15.22, diff integrity, and review checks all pass. The branch is 0 behind main.

[john-the-dev]

Current-head re-review on c84a78b.

The stale-tier-cache blocker is resolved: _load_tier_map() now treats removal of both explicit launcher pointers as a deliberate disable, clears the cached path/mtime/map, and returns the local tier. The new regression first loads a mapped sender, removes both pointers, and verifies the stale trust decision is not retained.

I also verified the requested live-path evidence in the PR body: the real bare ag2-sparrow entrypoint was started twice with only AG2_DEVICE_ENV; both starts logged the sibling access.json and serialized the mapped sender as access_tier: other before and after restart.

Focused verification on this head passed locally: access-path 7/7, per-sender tier 13/13, both remote-gateway suites, Python compilation, git diff --check, and scripts/review-checks.sh with the hardcoded-path scan clean. No remaining blocker from my prior review. The branch will still need a normal refresh/check rerun after today’s #2280 merge before merging.

Reviewed by John’s Codex.

[keweichen]

Refreshed this approved branch onto post-#2280 main (eab9525) at head a09ea40. The PR diff remains the same two gateway/access-path files and the branch is 0 behind.

Fresh rerun: access-path 7/7, per-sender tier 13/13, both remote-gateway suites, Python compilation, Ruff 0.15.22, git diff --check, and scripts/review-checks.sh all pass.

[github-actions]

Coverage Gate

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

Diff Coverage

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

No lines with coverage information in this diff.

[john-the-dev]

New blocker on exact head a09ea40 after tracing the path-switch failure mode. In _load_tier_map() lines 469-473, an OSError always returns the cached map even when the newly resolved path differs from the cached path. If install/config A loaded a map, then AG2_DEVICE_ENV switches to config B whose access.json is missing or unreadable, senders continue receiving A’s tier decisions. The empty-path fix does not cover this non-empty foreign-path case.

Please clear/fail to LOCAL_TIER when path != cache[path] and the new path cannot be read, while retaining last-known-good only for read failures on the SAME configured path. Add a regression that loads map A, switches to a different missing/unreadable path B, and verifies A is not reused. This is a trust-classification boundary, so my prior approval is withdrawn until that cross-config cache case is fixed and rerun.

[keweichen]

Addressed the requested cache-boundary fix in e06ae17. A switch from config path A to a missing or unreadable path B now clears the cached map and falls back to LOCAL_TIER; last-known-good retention remains scoped to failures on the same configured path. Added regression coverage for A -> missing B, B becoming readable, and same-path B removal. Validation: gateway access-path tests (10/10), per-sender tier tests (13/13), both remote-gateway bridge suites, py_compile, Ruff 0.15.22, and diff-check all pass. The branch is also synced with current main.

[liususan091219]

APPROVE on exact head e314e665 — re-reviewed the tier-cache isolation fix (e06ae17): same-path transient faults keep last-known-good (map floor never drops) but a config-PATH SWITCH with unreadable target now CLEARS instead of carrying trust maps across installs — the cross-install contamination hole closed, distinction verified in diff + the suite's new same-path/clear cases; 10/10 access-path checks green at this head, CI green. Original verification (at a09ea40): gateway access-map resolution verified: AG2_DEVICE_ENV-sibling first, CLAUDE_CONFIG_DIR second, deliberately NO ~/.claude guess (wrong-install trust-map hazard — same reviewed pattern as #2331's token fallback); key safety properties confirmed in code: map only DOWN-tiers named senders (no escalation path for unknowns, owner stays owner by absence), absent config = disabled, path changes logged (closes the no-diagnostic complaint), hot mtime reload; 7/7 access-path checks run green by reviewer at head incl. config-removal-clears-map.

[keweichen]

Refreshed the branch onto current main (c71a637) after the 24-hour review grace window; new head is f1586f6. The 13 main-only commits did not overlap the PR files, and the PR diff remains limited to the gateway implementation and access-path regression. Fresh validation passes: access-path 10/10, per-sender tier 13/13, both remote-gateway suites, Python compilation, Ruff 0.15.22, and git diff --check.

[john-the-dev]

Re-reviewed exact head f1586f69c096830101003658bcf0c88072976707.

My prior cross-config trust-cache blocker is resolved. _load_tier_map() now distinguishes a foreign path from a transient fault on the same configured path: switching to a missing/unreadable path clears the previous install’s map and returns local tier, while a same-path transient failure retains only that path’s last-known-good map. The regression covers load A → missing B → empty, B appearing → load B, same-path disappearance → retain B, and removing both config pointers → clear.

Independent exact-head verification passed: access-path suite 10/10, per-sender tier suite 13/13, both remote-gateway bridge suites, Python compilation, git diff --check origin/main...HEAD, and the added-line hardcoded-host-path scan. All required GitHub CI and CLA checks are green. The PR body’s real bare-launch restart proof covers the unchanged startup path: AG2_DEVICE_ENV without CLAUDE_CONFIG_DIR selected/logged the sibling access map and serialized the mapped sender as other before and after restart.

No blocker remains from my review. Approved.

[john-the-dev]

Re-reviewed refreshed exact head 09b590822fbc253b71424b4ae1c7edb14c967bf6. This head is based on current main 3e1dd21e, and both PR-owned blobs are byte-identical to the previously verified f1586f69 head (production ac3ba1ee, focused test 1c233854). Independent detached exact-head verification passes access-path 10/10, per-sender tier 13/13, both remote-gateway bridge suites, Python compilation, diff hygiene, repository review checks, and the added-line hardcoded-host-path scan. Every GitHub CI/coverage/lint job is green and GitHub reports the head mergeable. The cross-config trust-cache fix remains intact: a missing foreign path clears the prior install's map, while same-path transient failure retains only that path's last-known-good map; explicit config removal clears it. The prior real bare-launch restart proof still applies because the startup-path blobs are unchanged. No blocker remains from my lane. One more qualifying formal approval must target this exact refreshed head before merge.

[keweichen]

Refreshed this approved branch after the 24-hour review grace window onto current main (37af60a); new head is c101a84 and the branch is 0 behind.

The merge was clean. Fresh focused validation passes: access-path 10/10, per-sender tier 13/13, all remote-gateway bridge/attachment/interaction/singleton suites, Python compilation, diff hygiene, and repository review checks. Ruff is not installed in this local checkout, so I am leaving that claim to the required GitHub Ruff gate now running.

[john-the-dev]

This head has two qualifying exact-head approvals, green CI/CLA, and concrete bare-process live restart evidence. Required-up-to-date protection is the only merge blocker.

I attempted GitHub's update-branch API, but the repository token cannot update this fork (403 user doesn't have permission to update head repository). @keweichen please refresh the branch onto current main, rerun CI, and request fresh exact-head approvals; it can be merged as soon as those gates are restored.

[keweichen]

Refreshed the branch onto current main (ecfe08a) as requested; new head is e86f591 and the branch is 0 behind. The merge was clean and did not change either PR-owned blob. Fresh validation passes: access-path 10/10, per-sender tier 13/13, all remote-gateway bridge/file-attach/interaction/singleton suites, Python compilation, diff hygiene, and repository review checks. @john-the-dev fresh exact-head review is requested.

[keweichen]

@cla-assistant check

[bassilkhilo-ag2]

Independently verified: read the full diff in remote_gateway_bridge.py end-to-end (resolver, cache, _tier_for) — no other callers of _ag2space_access_path/_TIER_MAP_CACHE exist outside this file. Confirmed the bug on pre-fix main by reading the old resolver (CLAUDE_CONFIG_DIR or blind ~/.claude fallback, ignoring AG2_DEVICE_ENV).

Ran the actual regression proof both ways: checked out the PR's new tests/gateway-access-path-resolution.test.py against pre-fix main's remote_gateway_bridge.py — 8/10 checks FAIL (wrong path resolved, stale-home guess not blocked, cache doesn't invalidate on path switch). Same test against the PR head — 10/10 pass. Also ran gateway-per-sender-tier.test.py (13/13) and both bridge suites — all green, matching the PR's own validation log.

No hardcoded paths, fail-closed when neither AG2_DEVICE_ENV nor CLAUDE_CONFIG_DIR is set (consistent with src/startup.sh always exporting CLAUDE_CONFIG_DIR, so this is a narrow edge case, not a default-path regression for normal Sutando boots). CI is green and this matches the existing exact-head APPROVED decision.

Ready to merge.

[john-the-dev]

@cla-assistant check

[keweichen]

Refreshed the conflicting branch onto current main (dab9c99); new exact head is d0fdc7b and the branch is 0 behind. The conflict was the overlap with merged #2584 in the gateway tier-map cache. The resolution preserves #2584’s identity-based cache and stale-escalation revocation while adding this PR’s active-config path boundary: the cache key now includes both configured path and file identity, cross-path read failures clear prior-install tiers, same-path failures retain only the fail-closed subset, and removing launcher config clears the map. I strengthened the path-switch regression by forcing an identity collision across two configured files. Fresh validation passes: access-path 10/10, per-sender tier 27/27, both remote-gateway bridge suites, file-attach (11), interaction-type, singleton (6), Python compilation, diff hygiene, and repository review checks. @john-the-dev fresh exact-head review is requested.

[sonichi]

Heads-up, not a review: #2340 and #2591 conflict with each other, so whichever of the two merges second will need a rebase.

Both are CLEAN + APPROVED right now, which is what makes this easy to miss — GitHub computes mergeStateStatus against main, not against a sibling PR, so both report mergeable and the conflict only appears once the first one lands.

#2340 then #2591   CONFLICT  packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py
#2591 then #2340   CONFLICT  (same file — it is symmetric, not an ordering fix)

Measured against origin/main with both refs verified equal to their API heads first.

Scope, so this doesn't read as bigger than it is: I checked each of these against every other merge-ready PR — there are 21 — and this pair is the only conflict among them. The remaining 19 merge in any order (28 orderings tested, all clean), and 19 + either one of you is clean. So nothing here blocks either PR on its own; it's purely "second one rebases."

No action requested from me, and no opinion on which should land first — that's the maintainers' call.

[sonichi]

Cold review at head d0fdc7b7. No blockers found. Picked this up because your head moved 04:34Z and the newest review predates it by ~19h, so nobody had looked at the current state.

Verified by running, not reading — worktree pinned to a throwaway workspace first (asserted the resolution before executing, since these tests write config trees):

tests/gateway-access-path-resolution.test.py   exit=0    11 checks
tests/gateway-per-sender-tier.test.py          exit=0    28
tests/remote-gateway-bridge.test.py            exit=0   152

The regression covers the direction that matters. For a trust map the dangerous failure is fail-OPEN, and half your assertions are exactly that case:

• no launcher config never guesses stale ~/.claude → _ag2space_access_path() == ""
• missing map after path switch clears the previous install's tiers → LOCAL_TIER
• removing launcher config clears a previously loaded tier map → LOCAL_TIER
• path change invalidates an identity-colliding cache — the ABA case, stated outright
• same-path transient failure retains its own last-known-good map — the deliberate asymmetry

One suggestion, and it's that the body undersells the code. The summary says "cache by path and mtime so an active-config switch reloads even when mtimes match." The implementation is materially stronger than that sentence:

ident = (st.st_mtime_ns, st.st_size, st.st_ino)   # not float mtime
...
and not _has_above_local(_TIER_MAP_CACHE["map"])  # never serve an escalation from cache

with the threat model in-line — a float mtime "can be restored outright with os.utime, which would serve a REVOKED grant from cache — reproduced on #2584". A reviewer reading only the body would assume a plain mtime cache and might flag the ABA hazard you already closed. Worth a line in the summary; it's the difference between "cache invalidation" and "revoked escalations cannot survive a forged identity."

I went looking for exactly that hazard and found it anticipated with a comment and a test.

Not approving — my gh auth resolves to the repo owner's identity, so an approval from this account would misrepresent who reviewed. This is a comment review; the merge decision stays with the maintainers.

One process note against myself: I first classified your assertions with a keyword grep for negative-sounding words and got "0 fail-closed cases" — badly wrong, as the list above shows. A textual filter can't see what an assertion actually exercises. Reading them was the only thing that worked.

[keweichen]

Resolved the post-#2591 merge conflict in c316fea: kept the merged tier-map contract wording and this PR’s launcher-scoped access-path logging. Eight focused gateway/access/tier suites all pass; the resolution adds no new behavior.

[keweichen]

@cla-assistant check

[keweichen]

Repaired the current main conflict at 23d1b2f4 (fast-forwarded the existing PR branch; no force-push). The overlap was with #2701's provider-specific CHANNEL_DIR; the resolution keeps that routing while preserving this PR's active-launcher config boundary and no implicit home fallback.

Verified on the pushed head:

• python3 tests/gateway-access-path-resolution.test.py — 10/10
• python3 tests/gateway-channel-dir.test.py — 2/2
• bash tests/startup-gateway-channel-dir.test.sh — PASS
• python3 tests/gateway-per-sender-tier.test.py — 27/27
• python3 tests/remote-gateway-bridge.test.py — PASS
• python3 tests/remote-gateway-file-attach.test.py — 11/11
• python3 tests/remote-gateway-interaction-type.test.py — PASS
• python3 tests/remote-gateway-singleton.test.py — 6/6
• python3 tests/gateway-task-telemetry.test.py — PASS
• python3 -m py_compile packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py
• git diff --check origin/main...HEAD

Please re-review the exact current head after CI settles.

[keweichen]

Rebased #2340 onto current main (bcc84d45) and force-updated the existing PR branch with lease. New exact head: 730ed0b4 (3 PR commits, 0 behind; GitHub reports MERGEABLE).

The conflict was the tier-policy evolution on main. The resolution keeps the broker-attested guest/team/owner cap semantics and provider-specific CHANNEL_DIR, while applying this PR’s active-launcher path resolution and cache trust boundary: the cache is keyed by configured path plus (mtime_ns, size, inode); a cross-path read failure clears prior-install tiers; a same-path failure retains only the fail-closed subset; removing both launcher pointers clears the map.

Fresh exact-head validation:

• access-path regression: 10/10
• per-sender tier regression: 30/30
• channel-dir: 2/2; startup channel-dir: PASS
• both remote-gateway bridge suites: PASS
• file attachment: 11/11; interaction type: PASS; singleton: 6/6; task telemetry: PASS
• package/source drift, Python compilation, git diff --check, and repository review checks: PASS
• GitHub Ruff gate: PASS

CI is running on 730ed0b4. Fresh exact-head review requested once it settles.

[john-the-dev]

APPROVE at the rebased head 730ed0b4. Rui asked me to re-review; I verified the rebase rather than taking the "conflicts resolved" summary on trust, because a conflict-free rebase can still be a semantic break — and this one lands squarely on access-tier logic.

Ran both suites at 730ed0b4, in an isolated worktree with a temp workspace:

tests/gateway-access-path-resolution.test.py   PASS — 10 access-path checks
src/remote-gateway-bridge.test.py              PASS — all checks green   (no regression)

Board at head: MERGEABLE / CLEAN, 0 pending checks, 0 failures, CLA success.

The core change is right, and it is a real fix. Keying _TIER_MAP_CACHE on path (not just the mtime/size/inode identity) is what makes the trust boundary correct: an identity collision across two configs can no longer serve install A's cached grants to install B. The test that pins this — deliberately os.utime-ing the second access.json to the first one's mtime, then asserting the map still re-reads — is exactly the case the old cache got wrong, and it would have passed the old code only by luck.

I also checked the failure directions individually, since they are easy to get backwards: a same-path transient read failure retains last-known-good (correct — a mid-write file must not drop live caps), while a path switch clears it (correct — different config, different trust boundary). Those two want opposite behaviour and the diff gets both.

⚠ One non-blocking observation, on the "disabled" state. Dropping the ~/.claude fallback means that with neither AG2_DEVICE_ENV nor CLAUDE_CONFIG_DIR set, the map resolves to "" and _load_tier_map() returns {}. Because the map only ever caps, _tier_for then falls back to LOCAL_TIER — so a sender explicitly capped in access.json loses that cap. On main that same unconfigured launch would have found ~/.claude/channels/ag2space/access.json, which is the default layout.

I do not think this blocks, for three reasons I checked rather than assumed:

1. LOCAL_TIER defaults to owner only under the personal-agent model (startup.sh:1075, and remote-gateway-bridge.test.py:147 pins it) — where there is no local cap to lose anyway. A multi-user gateway sets REMOTE_TASK_TIER=team, so the fallback is team, not owner.
2. The supervised launcher always exports CLAUDE_CONFIG_DIR before services start (startup.sh:285), so the disabled state is not reachable on the normal path — only a bare manual launch.
3. This PR adds the stderr line that announces access tier map path: disabled (…), so the state is observable instead of silent. That is the right call.

Worth being explicit in a follow-up about whether a multi-user gateway should fail closed (force guest) when the map is disabled, rather than falling back to LOCAL_TIER. Bounded escalation (guest → team), announced, and off the supervised path — a follow-up, not a blocker here.

Merge status: the rebase invalidated the earlier approvals — john-the-dev, liususan091219 and bassilkhilo-ag2 all approved at e86f5919, and there are currently zero at-head approvals. Mine brings it to one. @liususan091219 / @bassilkhilo-ag2, a quick re-confirm at 730ed0b4 would let this merge; the delta from your review is the conflict resolution plus the new path-resolution test.

[john-the-dev]

@keweichen this one is ready to merge except that it's BEHIND main, and I can't update it from this side — the branch is on your fork with maintainerCanModify: false, so gh pr update-branch returns user doesn't have permission to update head repository.

Current state as of now: 3 approvals (me, @liususan091219, @bassilkhilo-ag2), reviewDecision: APPROVED, every check-run green (publish skipped), combined status success. The only thing between it and merge is the rebase.

If you rebase onto current main and CI stays green, I'll merge it. Alternatively, enabling "Allow edits by maintainers" on the PR lets us keep it current without a round trip each time main moves — this repo requires up-to-date branches, so every merge puts it BEHIND again.

[john-the-dev]

Verified at head a5bc1fea before merging.

The branch was rebased on 2026-08-11, which put every recorded approval at a commit that no longer exists (e86f5919 is unreachable — the fork force-pushed). That normally means the approvals no longer describe the branch, so I checked whether the rebase carried new work:

bbb32506 author=2026-07-27T17:21:48Z commit=2026-08-11T23:46:42Z
a575fa12 author=2026-07-27T18:51:09Z commit=2026-08-11T23:46:42Z
a5bc1fea author=2026-07-28T06:50:14Z commit=2026-08-11T23:46:42Z

Three authored commits, all from 27–28 Jul, all re-committed in one instant — a replay onto newer main with nothing new authored since @liususan091219 and @bassilkhilo-ag2 reviewed. Their approvals are content-current; only the base moved.

Tests at head, clean worktree at a5bc1fea:

ok   AG2_DEVICE_ENV resolves sibling access.json
ok   resolved access path is logged
ok   CLAUDE_CONFIG_DIR is the non-desktop fallback
ok   no launcher config never guesses stale ~/.claude
ok   tier map comes from active device config, not stale home
ok   path change invalidates an identity-colliding cache
ok   missing map after path switch clears the previous install's tiers
ok   new path loads after its map appears
ok   same-path transient failure retains its own last-known-good map
ok   removing launcher config clears a previously loaded tier map

PASS — 10 access-path checks

The two that matter most for an access-control change both hold: a stale home config cannot supply tiers, and a transient read failure retains last-known-good rather than dropping to an empty map. Updating the branch and merging.

[github-actions]

@cla-assistant check