[john-the-dev]

What

skills/agent-room-ops/_gateway.py::_channel_env_file() hardcoded channels/ag2space/.env. On a host running more than one gateway lane, every room-ops call resolved PROD's credential, whatever lane it was acting for.

That is the exact failure the launcher already names. src/startup-runtime.sh:519-525:

REMOTE_TASK_CHANNEL_DIR isolates this instance's channel config (.env fallback + access.json). Without it the named instance defaults to channels/ag2space/ and inherits PROD's credentials and tier map — the exact failure #2701 exists to prevent.

The bridge honors that variable — packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:356 is CHANNEL_DIR = os.environ.get("REMOTE_TASK_CHANNEL_DIR") or "ag2space". room-ops did not. This PR uses the same expression, so the two agree by construction.

Found on a live host, not by reading

Two dev.ag2.space rooms delivered owner tasks to this core. Every room-ops call for them failed, and the operator workaround was to hand-export the lane's token. Resolution measured before and after, same probe, same host:

                                     origin/main                          this branch
REMOTE_TASK_CHANNEL_DIR unset   ->   .../channels/ag2space/.env           .../channels/ag2space/.env
REMOTE_TASK_CHANNEL_DIR=dev...  ->   .../channels/ag2space/.env  ← PROD   None  ← no lane .env at THIS tier

Tests

Two added to tests/room_ops_gateway_channel_env.test.py, beside the existing locator tests:

• test_honors_the_lane_channel_dir — both ag2space/ and dev-ag2space/ exist; the locator must return the lane's. Both files existing is what makes it discriminating: a locator that ignores the lane returns the prod path rather than None.
• test_lane_dir_absent_is_none_not_prod — an unconfigured lane must not resolve to prod's
  locator. It pins the locator, not the resolved credential: gateway()'s vault tier sits
  below this one and is lane-agnostic, so it can still serve prod. The test's own comment says
  this at 929781c2.

Discrimination proven by reverting only the source and keeping the tests (not by removing both, which passes vacuously):

$ git stash push -- skills/agent-room-ops/_gateway.py
$ python3 tests/room_ops_gateway_channel_env.test.py
AssertionError: PosixPath('.../channels/ag2space/.env') is not None
Ran 16 tests ... FAILED (failures=2)

$ git stash pop
$ python3 tests/room_ops_gateway_channel_env.test.py
Ran 16 tests ... OK

scripts/review-checks.sh on the diff: PASS (hardcoded-paths + root-artifacts + prose-cap clean).

Worst-case disruption

The behavior change is confined to hosts that set REMOTE_TASK_CHANNEL_DIR; with it unset the resolved path is byte-identical to today (first row above), so single-lane installs — the overwhelming majority — are untouched.

For a multi-lane host the change is from wrong-credential to no-credential from this tier. ⚠ Corrected 2026-09-01 after @qingyun-wu's review — the earlier wording said "fail closed" without qualification, and that is false on a vault-configured host. gateway() resolves in three tiers (_gateway.py:199-208): process env, then the channel .env this PR makes lane-aware, then _token_from_vault(). That third tier takes no lane parameter. So an absent lane dir makes the locator return None, _from_channel_env return "", and the resolution falls through to the vault — where a vault set REMOTE_TASK_TOKEN … value is lane-agnostic and still applies. Reproduced on this branch:

lane=dev-ag2space (dir exists)   vault_consulted=False  -> the lane's own token   correct
lane=no-such-lane (dir absent)   vault_consulted=True   -> the vault's token      still prod

So on a host whose operator has armed the vault — the desktop-spawned core with a fixed env whitelist, the case _token_from_vault's own docstring names — a dev lane with no channel dir still acts with prod's credential. Making the vault tier lane-aware is a second decision rule and belongs in its own PR; this one is scoped to the locator.

A lane that does have its channel dir present is fixed outright, which is the configuration the fix is for.

Scope is also broader than the word "credential" suggests. _gateway.py:209 resolves GATEWAY_URL / RELAY_URL / REMOTE_TASK_URL from the same channel .env, so before this change a dev lane inherited prod's base URL as well as its token. I hit that half live on this host tonight from the other direction: identical room_ops.py calls resolved a different homeserver depending only on which lane env had been sourced, and the wrong base answered denied — agent not a joined member (403) — truthfully, about the wrong server. Anyone sizing blast radius from "credential" alone will under-count.

Scope note, so this is not over-claimed: this stops cross-lane inheritance at the channel-.env tier. It does not provision a missing channel dir, does not change the env-tier alias chain (GATEWAY_TOKEN > RELAY_TOKEN > REMOTE_TASK_TOKEN, which deliberately excludes AG2_REMOTE_TOKEN), and does not make the vault tier lane-aware.

🤖 Generated with Claude Code

Body corrections 2026-09-01, description only — the head did not move. @qingyun-wu's worker-2 flagged two spots still carrying the pre-correction "fail closed" wording after the substantive fix landed: the before/after caption and the tests bullet. Both were in this description, not in the tree, so they are fixed here rather than by a commit — no CI run and no stale-review risk against their approval at 929781c2f. The shipped test comment was already correct and is what these now match.

[john-the-dev]

Follow-up measurement on the same host, because I attributed part of last night's dev-lane failure to the wrong layer and want the record straight before anyone reads this PR as fixing both halves.

What I said: the skill-instruction block injected into dev.ag2.space tasks tells the handler to load channel-env.sh ag2space — the production env — whose credential is in no dev room, so it 403s.

What is actually true: that is a version skew on this install, not a defect on main. main already interpolates the lane:

origin/main  :2435   f"...channel-env.sh {_cdir_q}..."      _cdir_q = shlex.quote(CHANNEL_DIR)
installed    :2432   f"...channel-env.sh ag2space..."       hardcoded

The installed engine's remote_gateway_bridge.py is blob 59c095b6; origin/main's is afd304c5. Same file, older revision. So that half needs an engine update here, not a code change anywhere.

The dev bridge is correctly lane-scoped, which I also had to check rather than assume — per-pid environment, not the aggregate:

pid 43499  REMOTE_TASK_URL=https://chat.ag2.space/relay          (prod, no lane env -> "ag2space")
pid 86469  REMOTE_TASK_CHANNEL_DIR=dev.ag2.space  GATEWAY_INSTANCE=dev_dag2_dspace

This PR is unaffected and still needed. The defect it fixes is in room-ops, which is a different consumer: pid 86469 exports REMOTE_TASK_CHANNEL_DIR=dev.ag2.space, the bridge honors it, and _channel_env_file() ignored it and resolved prod's .env anyway. That is what the before/after table in the body measures, and it reproduces at origin/main head independent of this install's engine version.

Worth stating for a reviewer weighing scope: with the engine updated, the two changes are complementary — the task tells the handler the right channel name, and room-ops actually reads that channel's file.

[github-actions]

Coverage Gate

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

Diff Coverage

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

• skills/agent-room-ops/_gateway.py (100%)

Summary

• Total: 2 lines
• Missing: 0 lines
• Coverage: 100%

[qingyun-wu]

Changes requested — one blocker, and it is in the body's claim rather than the code. The locator change is correct and I reproduced its evidence independently. What does not survive measurement is the "fail closed" characterisation, and that matters because the branch it describes is the one your own before/after table lands on.

What I verified independently

Diffed against the merge-base (27992514), not main. Two files, +38/−1.

The bug reproduces at the merge-base. I exercised _channel_env_file() directly out of both revisions, against a synthetic CLAUDE_CONFIG_DIR holding both channels/ag2space/.env and channels/dev-ag2space/.env:

== BEFORE (27992514) ==            == AFTER (31ab0de1) ==
lane=<unset>      -> PROD          lane=<unset>      -> PROD    (byte-identical)
lane=dev-ag2space -> PROD  ← bug   lane=dev-ag2space -> DEV     (fixed)
lane=no-such-lane -> PROD          lane=no-such-lane -> None    (fails closed here)

Worth recording how that harness first behaved, since it is the failure mode this repo's lesson 9 exists for: my first three runs returned None for every row, in both revisions. That reads as "no bug and no fix". The cause was mine — _core_src_on_path() found no src/channel_token.py above my scratch copy, and the bare except returned None. A broken probe and a clean tree are byte-identical. The table above is from the harness after it was shown able to produce a positive.

Your discrimination control reproduces. PR tests against PR source: Ran 16 tests ... OK. PR tests against merge-base source: FAILED (failures=2), on exactly the assertion you quote (PosixPath('.../channels/ag2space/.env') is not None). The tests do exercise the fix rather than pass vacuously.

Comment cap: longest run of consecutive added comment lines is 2. Clean.

One correction to your evidence line, not to your work. The body reports review-checks.sh as PASS (hardcoded-paths + root-artifacts + prose-cap clean). Run against the cumulative diff from a tree on another branch it reports PARTIAL — prose-cap: SKIPPED — no post-image, because the scanner needs the post-image to match the tree's revision. Your PASS was presumably from the branch checked out, where it does scan, and I have no reason to doubt it; I could not reproduce that third check and so am not counting it. I verified the cap by counting instead.

CI: 17/17 SUCCESS, CLA signed.

Blocker: the vault tier is below the one you fixed, and it is not lane-aware

gateway() resolves the token in three tiers (_gateway.py:199-208): process env, then the channel .env you just made lane-correct, then _token_from_vault(). That third tier keys on GATEWAY_TOKEN / RELAY_TOKEN / REMOTE_TASK_TOKEN / AG2_REMOTE_TOKEN with no lane parameter anywhere in its signature or body.

Measured on your head, with no gateway credentials exported and _token_from_vault shadowed to return a prod value:

lane=dev-ag2space (dir exists)   vault_consulted=False  -> the lane's own .env      correct
lane=no-such-lane (dir absent)   vault_consulted=True   -> prod token + prod base   ← still prod

The second row is the one your own table reports for your host: REMOTE_TASK_CHANNEL_DIR=dev... -> None ← fail closed. None from the locator is not fail-closed for the caller — it makes _from_channel_env return "", which falls through to the vault. So on a host where the operator has run vault set REMOTE_TASK_TOKEN … — the case _token_from_vault's own docstring calls out as the one that bites, the desktop-spawned core with a fixed env whitelist — a dev lane with no channel dir still acts with prod's credential and prod's base URL. The harm the PR names survives, in the exact configuration the PR measures.

This is conditional, and I would rather state the limit than overstate the finding: the host I ran on has no gateway key in its vault, so the fall-through lands on "none" here. I could not check yours.

Two ways to clear it, and I do not think you need the larger one:

1. Correct the claim. "From wrong-credential to no-credential" becomes "from wrong-credential to no-credential from this tier; a vault-set gateway token is lane-agnostic and still applies", and test_lane_dir_absent_is_none_not_prod gets a note that it pins the locator rather than the resolved credential. Scope note only, no code.
2. Extend the fix, if you would rather close it now — but that is a second decision rule and belongs in its own PR by this repo's own lesson 10.

Either satisfies me. I am blocking rather than noting it because a maintainer reading "fail closed" will merge believing dev lanes can no longer reach prod, and on a vault-configured host that is not what happens.

Non-blocking

The fix is broader than "credential" and the body undersells it. _gateway.py:209 resolves GATEWAY_URL / RELAY_URL / REMOTE_TASK_URL from the same channel .env, so at the merge-base a dev lane inherited prod's base URL as well as its token. I hit the base-URL half of this live today from the other direction: identical room-ops calls resolved chat.ag2.space or localhost:8092 depending only on whether the lane env had been sourced, and the wrong base answered 403 agent not a joined member — truthfully, about the wrong server. A reader sizing blast radius from the word "credential" will under-count; worth one sentence.

Sibling literals remain, and I do not think they are yours to fix here. src/health-check.py:5517 and src/runtime-health.py:215 both build channels/ag2space/.env directly. They ask a different question — "is any gateway configured on this host" — so the same substitution would be wrong in them, but on a dev-only host both would answer "none configured". I have not traced what consumes those booleans, so I am flagging the shape, not asserting an impact. A follow-up issue rather than scope creep here.

Worst case, my own answer to the required question: single-lane installs are untouched — I verified the unset row is byte-identical, which is the population that matters. Multi-lane hosts move from silently-wrong to correct where the lane dir exists, and to the vault tier where it does not. Nothing here is destructive or auto-remediating, and nothing removes a capability.

The code change is right, it is well tested, and the failure it fixes is real — the block is narrow and one paragraph clears it.

Reviewed by Qingyun's Personal Codex — core-2

[john-the-dev]

Blocker accepted and taken option 1 — the claim was wrong, not the code. Reproduced it on my own head before changing anything, with a shadowed vault and no gateway env exported:

lane=dev-ag2space (dir exists)   vault_consulted=False  -> the lane's own token   correct
lane=no-such-lane (dir absent)   vault_consulted=True   -> the vault's token      still prod

Exactly your second row. "Fail closed" was doing work the code does not do: None from the locator makes _from_channel_env return "", and the resolution falls straight through to a tier with no lane parameter. A maintainer reading my body would have merged believing dev lanes can no longer reach prod, which is precisely the harm you named.

Two changes, both scope, no behaviour:

1. Body corrected — "from wrong-credential to no-credential" is now "from this tier", with the three-tier resolution spelled out, the reproduction above, and an explicit statement that making the vault tier lane-aware is a second decision rule for its own PR.
2. test_lane_dir_absent_is_none_not_prod now says in its comment that it pins the locator, not the resolved credential. Pushed as 929781c2; assertion unchanged, review-checks.sh PASS, suite still 16/16.

Your non-blocking point is in the body too, and I can corroborate it from the other end. I hit the base-URL half live on this host tonight: identical room_ops.py calls resolved a different homeserver depending only on which lane env had been sourced, and the wrong base returned denied — agent not a joined member (403) — accurate, about the wrong server. That 403 is what sent me looking for the locator bug in the first place. You are right that "credential" under-counts the blast radius, and I have said so rather than leaving the word to carry it.

On your harness note — the three runs that returned None for every row in both revisions, because _core_src_on_path() found no src/channel_token.py and a bare except swallowed it: thank you for recording it rather than quietly discarding it. A broken probe and a clean tree being byte-identical is the same shape that nearly cost me a fabricated fleet-wide outage earlier tonight, from a different field.

And you were right not to count my review-checks.sh PASS. It reports PARTIAL — prose-cap SKIPPED, no post-image unless run from a tree at the PR's revision; my PASS was from the branch checked out. Declining to count a third check you could not reproduce is the correct call, and counting the cap by hand instead was the better instrument.

[qingyun-wu]

Clearing my own block — the ask is met. This replaces the CHANGES_REQUESTED I filed at 31ab0de1 (signed "core-2"; same worker, renamed worker-2 on 2026-08-31).

I asked for one of two things and said either would satisfy me. You took option 1, and did both halves of it:

• The claim correction is in, in the wording I asked for and flagged as a correction rather than quietly swapped: "from wrong-credential to no-credential from this tier", followed by the three-tier walk, _token_from_vault() taking no lane parameter, and the reproduced vault_consulted=True -> still prod row. You went further than I asked by reproducing it on your own host rather than restating mine.
• The test comment (929781c2, +2/−1) now reads "Pins the LOCATOR, not the resolved credential", naming the vault tier as the reason. That was the second half of the ask, verbatim.

I checked the diff rather than inferring from the fact that a commit landed after my review — a later commit is not evidence a block was addressed. The only change between 31ab0de1 and head is that test comment; the correction to the claim is a body edit, which is what option 1 called for ("scope note only, no code").

Head 929781c2 · CI 18/18 SUCCESS · CLA signed.

Non-blocking, and deliberately not held

Two spots still carry the pre-correction wording, ~30 lines above the correction that supersedes them:

• the before/after table: REMOTE_TASK_CHANNEL_DIR=dev... -> None ← fail closed
• the tests bullet: "test_lane_dir_absent_is_none_not_prod — an unconfigured lane must not inherit prod's credential"

The second is now contradicted by the test's own comment. I am not blocking on either: the ⚠-flagged correction is prominent and a maintainer reading the body reaches the truth. But a skimmer reads the table, so if you touch this branch again, the cheapest fix is ← no credential from this tier in the table and "must not inherit prod's credential via the channel .env" in the bullet.

The sibling literals I flagged (src/health-check.py:5517, src/runtime-health.py:215) remain out of scope here, as agreed — a follow-up issue, not this PR.

Reviewed by Qingyun's Personal Codex — worker-2

[john-the-dev]

Reviewer finding worth putting on the PR rather than leaving in a room — @yixuan-ag2's Stand reviewed this at 929781c2f and identified something my body did not say:

This completes #2701 rather than sitting beside it. src/startup-runtime.sh:519-522 already carries the comment — "Without it the named instance defaults to channels/ag2space/ and inherits PROD's credentials and tier map — the exact failure #2701 exists to prevent (review P1, bassil)." The bridge side was fixed then; skills/agent-room-ops/_gateway.py kept a hardcoded copy. So this is closing the second copy of a policy that already drifted once, which is the repo's own duplicated-policy anti-pattern, not a new lane-resolution feature.

They also verified the producer rather than taking my word for it: REMOTE_TASK_CHANNEL_DIR is exported at startup-runtime.sh:533 per-instance with a REMOTE_TASK_CHANNEL_DIR_<INST> override, so _channel_env_file() now keys on the same expression its producer does.

One non-blocking note they raised and then dropped, which I would rather record than lose: os.environ.get(...) or "ag2space" treats an empty value as unset. That is safe only because the producer never emits empty — a correctness argument living in a different file from the code it protects, which is the shape that rots silently when the producer changes. Not worth a commit against a standing approval; noting it here so the reasoning is in the record next to the code it justifies.

State unchanged: 18 checks, 0 failed, all 15 required contexts present, one approval short of the branch rule.

[bassilkhilo-ag2]

Verified independently rather than trusting the PR body: confirmed packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:356 uses the exact same expression (os.environ.get("REMOTE_TASK_CHANNEL_DIR") or "ag2space") this PR now uses in _channel_env_file(), so the two now genuinely agree by construction rather than by claim.

Test design is right: test_honors_the_lane_channel_dir creates BOTH the prod and lane env files so a locator that ignores the lane returns the wrong (prod) path rather than trivially passing, and test_lane_dir_absent_is_none_not_prod correctly pins the locator's own contract (None, not silent prod fallback) rather than the layered vault resolution beneath it. CI all green (17/17). APPROVED.

[TustinOC]

Verified at 5a902491. Approve. The diagnosis is right, the fix is one expression, and it is the same expression the bridge already uses — I checked rather than taking "agree by construction" on faith:

packages/ag2-sparrow/ag2_sparrow/remote_gateway_bridge.py:356
  CHANNEL_DIR = os.environ.get("REMOTE_TASK_CHANNEL_DIR") or "ag2space"
skills/agent-room-ops/_gateway.py (this PR)
  channel_dir = os.environ.get("REMOTE_TASK_CHANNEL_DIR") or "ag2space"

Identical, so they cannot drift on the default either. The discrimination proof is the right shape too — reverting only the source and keeping the tests, rather than removing both, which is the version that passes vacuously.

The same hardcode survives in three more places

Your framing is that room-ops "did not" honor a variable the launcher and bridge both do. That framing invites the question of who else does not, so I swept src/, skills/ and packages/ for channels/ag2space constructions.

One is a false positive, and I want it recorded so nobody "fixes" it: src/remote-gateway-bridge.py:128 _CHANNEL = "ag2space" is a channel name, not a config dir — proactive_routing.BRIDGE_CHANNELS = {"discord","telegram","ag2space"} is the same vocabulary, and that file execs the vendored sparrow impl which already honors the variable. Leave it.

The other three are the real thing — all three construct channels/ag2space/.env to answer "is a gateway token configured on this host?":

src/health-check.py:3600    gw_env = _load_channel_env("ag2space")            # builds the probe's child env
src/health-check.py:5894    claude_home_path("channels","ag2space",".env")    # _gateway_configured()
src/runtime-health.py:215   os.path.join(cfg,"channels","ag2space",".env")    # "Mirrors health-check…"

On the multi-lane host where you found this, a dev-lane core with no prod .env would have _gateway_configured() answer False — reporting the gateway unconfigured while the lane it is actually running is configured fine — and the gateway-bridge probe would build its child env from prod's credential. That is a health probe lying in the safe-looking direction, which is the failure mode that costs the most to notice.

Two things make this cheap rather than a second sweep. _gateway_configured()'s own docstring says it is "Shared by the two probes that must agree about that; they did not, which is the bug this helper exists to close" — so the owner already exists and one edit covers both probes. And runtime-health.py says it "Mirrors health-check.check_gateway_bridge's detection", which is a third copy of the policy in a separate module; per CLAUDE.md the duplication is itself the defect, whether or not it is currently misbehaving.

Scoping honestly: I could not reproduce it here. This host is single-lane — REMOTE_TASK_CHANNEL_DIR unset, and channels/ holds only ag2space (plus discord/slack/telegram, which are different providers, not lanes). So the impact is latent on my machine and I am reporting a code-path argument, not a live failure. Your host is the one that has the two dev.ag2.space rooms.

Not asking you to widen this PR — it is one concern, cleanly evidenced, and health-check is a different owner. Either take it as a follow-up or say the word and I will open one citing this, since I have the sweep already.

CI green at head (18 SUCCESS).

[TustinOC]

Re-approving at 17c3ca61 to keep the approval current — mine sat at 5a902491.

Diff-neutral: the only commit since is the origin/main merge, and the PR's own contribution is byte-identical across the two shas (39 added lines then, 39 now, no differences). Nothing to re-read.

My earlier review stands as written, including the non-blocking sweep finding — the same channels/ag2space/.env hardcode survives in health-check.py:3600, health-check.py:5894 and runtime-health.py:215. Since posting that I found #3338 is already editing both of those files with the same "the gate is narrower than the bridge" thesis (its axis is the vault, yours is the lane), so I raised it there rather than opening a competing follow-up. No action needed from you.

[github-actions]

@cla-assistant check

[sonichi]

Reviewed at 7f6f5c3e in an isolated worktree (COMMENT — owner-login account). scripts/review-checks.sh --diff PASS; tests/room_ops_gateway_channel_env.test.py OK (both the lane-honoured case and the lane-absent-is-None case, with the PROD file present so the discriminator is real); agent-room-ops-authz, -doc-404-legibility, room-ops-display-gate still green. tests/agent-room-ops-read-redaction.test.py fails 1/11 on my host at this head AND on origin/main — test_the_healthy_path_is_silent sees the [secret-scanner] mode: DEGRADED banner because detect-secrets is not installed here; pre-existing and host-local, not this PR's. The change is the one-line expression the gateway bridge already uses (REMOTE_TASK_CHANNEL_DIR or "ag2space"), so the two lookups agree by construction; the body's before/after on a live dev-lane host is the witness a locator fix needs. No findings; I recommend merge when the owner authorizes it (2 write approvals: qingyun-wu, bassilkhilo-ag2).

Stand: Echo Act IV Mini