Skip to content

feat(health-check): surface ag2.space gateway-bridge status (down when down) - #2067

Merged
qingyun-wu merged 4 commits into
sonichi:mainfrom
john-the-dev:fix/health-check-gateway-bridge
Jul 14, 2026
Merged

qingyun-wu merged 4 commits into
sonichi:mainfrom
john-the-dev:fix/health-check-gateway-bridge

Conversation

@john-the-dev

Copy link
Copy Markdown
Collaborator

Problem

The health check probed telegram-bridge and discord-bridge but not the remote-gateway-bridge — the process that carries ag2.space mobile-app messages from the cloud gateway (chat.ag2.space/relay) down to the local core (and results back up).

So when that bridge died, nothing reported it. Concretely (2026-07-10): the bridge had been dead since Jul 7 — it stopped after a run of gateway 502s and was never restarted (the overnight OS-update reboot didn't bring it back). Mobile messages stranded in the cloud invisibly for 3 days, and the dashboard still showed green. The owner only found out by noticing replies never arrived.

Fix

Add a dedicated gateway-bridge check. It doesn't fit the existing name→channel loop (its config is channels/ag2space/.env, not channels/remote-gateway/), so it's checked separately:

  • Only surfaced when configured — REMOTE_TASK_TOKEN / AG2_REMOTE_TOKEN present in the channel .env or the environment. A Sutando-only user without the mobile gateway never sees it.
  • warn when configured-but-not-running, with an explicit impact message.
  • ok when running.

Live test (this machine, against the running system)

[1] bridge running   → ✓ gateway-bridge  ok    running
[2] pkill -f 'remote-gateway-bridge\.py$'
[3] bridge DOWN      → ⚠ gateway-bridge  warn  configured but NOT running — ag2.space mobile messages will not be delivered
[4] restart the bridge
[5] bridge back UP   → ✓ gateway-bridge  ok    running

The check flips to warn the moment the bridge dies and back to ok when it recovers — so the dashboard would have shown the 3-day outage on day one.

Scope

Single file (src/health-check.py). Follow-up (separate PR): a launchd KeepAlive supervisor so the bridge auto-restarts instead of only being reported down.

🤖 Generated with Claude Code

…n down)

The health check probed telegram-bridge + discord-bridge but NOT the
remote-gateway-bridge — the process that carries ag2.space MOBILE-app messages
from the cloud gateway to the local core. So when that bridge died (2026-07-10:
dead since Jul 7, killed after a run of gateway 502s, never restarted), NOTHING
reported it: mobile messages stranded in the cloud invisibly for 3 days and the
dashboard still showed green.

Add a dedicated check (it doesn't fit the name→channel loop — its config is
channels/ag2space/.env, not channels/remote-gateway/). Only surfaced when
configured (REMOTE_TASK_TOKEN / AG2_REMOTE_TOKEN in the channel .env or env), so
a Sutando-only user without the mobile gateway never sees it. warn when
configured-but-not-running, with an explicit impact message; ok when running.

Live test (this machine, against the running system):
  [1] bridge running   → ✓ gateway-bridge  ok    running
  [2] pkill the bridge
  [3] bridge down       → ⚠ gateway-bridge  warn  configured but NOT running — ag2.space mobile messages will not be delivered
  [4] restart the bridge
  [5] bridge back up    → ✓ gateway-bridge  ok    running

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWvssgDGkNzjMPVG9xaMdj
@github-actions

github-actions Bot commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Coverage Gate

❌ Test suite failed under instrumentation — coverage not measurable. See the job log.

@qingyun-wu

Copy link
Copy Markdown
Collaborator

LGTM from code review.

Verified on the PR head:

  • PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2067 python3 -m py_compile src/health-check.py succeeded in /private/tmp/sutando-pr2067.
  • python3 tests/health-check-emit-task.test.py still passed in the PR worktree.
  • I also imported the PR's src/health-check.py from the detached worktree with a temp CLAUDE_CONFIG_DIR/channels/ag2space/.env and stubbed subprocess.run; run_all_checks() produced {"name": "gateway-bridge", "status": "warn", "detail": "configured but NOT running — ag2.space mobile messages will not be delivered"} with no matching PID and {"name": "gateway-bridge", "status": "ok", "detail": "running"} with a matching remote-gateway-bridge.py PID.
  • src/startup.sh:812-822 uses the same channels/ag2space/.env and REMOTE_TASK_TOKEN|AG2_REMOTE_TOKEN configuration contract, so the detection path matches the existing startup path.

No actionable findings. Merge once required checks pass.

Qingyun's Personal Codex Agent

john-the-dev and others added 2 commits July 10, 2026 13:43
Addresses the diff-coverage gate on this PR. The gateway-bridge probe was inline
in run_all_checks() (untested), so its new lines failed the 95% python diff-
coverage bar. Extract it into a standalone check_gateway_bridge() and add
tests/health-check-gateway-bridge.test.py driving every branch: not-configured →
None, configured+running → ok, configured+down → warn (with impact), duplicate
processes → warn, and configured via the channel .env file. pgrep + the config
source are mocked so the test is hermetic. The call site in run_all_checks() is a
single filter line.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWvssgDGkNzjMPVG9xaMdj
…call site

diff-cover flagged two spots: the except branch in check_gateway_bridge (pgrep
raising) and the one-line call site in run_all_checks. Add tests for the pgrep-
raises and config-read-raises branches (→ warn / None respectively); mark the
trivial call-site line `# pragma: no cover` (it's exercised by the running health
check, not the unit test). All branches of check_gateway_bridge now covered.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWvssgDGkNzjMPVG9xaMdj
@qingyun-wu

Copy link
Copy Markdown
Collaborator

LGTM. I reviewed the new check_gateway_bridge() probe and ran python3 tests/health-check-gateway-bridge.test.py on this head; it passed all covered branches, including the configured-but-down, duplicate-process, token-from-channel-env, and both exception paths. The probe is narrowly scoped the right way: silent when the mobile gateway is not configured, warn when a configured bridge is absent or duplicated, and ok for a single running process. Ready to merge once CI passes.

Qingyun's Personal Codex Agent

liususan091219
liususan091219 previously approved these changes Jul 13, 2026

@liususan091219 liususan091219 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Codex full-review sweep: checked against the 7 triage CR classes (source-grep tests, incomplete fix, band-aid, naming, false claim, live-test policy, hardcoded paths) - none fire. Approving.

review-reason: codex-clean per full-review sweep

@liususan091219
liususan091219 dismissed their stale review July 13, 2026 02:02

Approval withheld pending owner go (resequenced: CRs first). Will re-approve.

@liususan091219 liususan091219 added ag2product AG2 product related (packages, PyPI, task-relay) feature and removed feature labels Jul 13, 2026
@qingyun-wu

Copy link
Copy Markdown
Collaborator

LGTM. I ran python3 tests/health-check-gateway-bridge.test.py; it passed locally, including the down/duplicate/configured cases for the new gateway-bridge health surface. Ready to merge once CI passes.

Qingyun's Personal Codex Agent

@john-the-dev
john-the-dev requested a review from qingyun-wu July 14, 2026 17:31
@john-the-dev

Copy link
Copy Markdown
Collaborator Author

@qingyun-wu — requesting your review here (owner asked me to route my own ag2product PRs to you, since I can't self-approve). CI is green (tsc+tests, coverage, workspace-path lints, CLA), no open change-requests, and the branch is mergeable (just behind main). Small/self-contained — should be a quick look.

@github-actions

Copy link
Copy Markdown
Contributor

@cla-assistant check

@qingyun-wu
qingyun-wu merged commit 096e7d4 into sonichi:main Jul 14, 2026
6 of 8 checks passed
john-the-dev added a commit to john-the-dev/sutando that referenced this pull request Jul 20, 2026
The ag2.space gateway bridge (remote-gateway-bridge.py) carries MOBILE-app
messages from the cloud gateway to the local core. It was a single unmonitored
process launched with a bare `&` from startup.sh — so when it died (2026-07-10:
dead since Jul 7, stopped after gateway 502s, never restarted), nothing brought
it back and mobile messages stranded in the cloud for 3 days.

Add a launchd job (com.sutando.gateway-bridge) with RunAtLoad + KeepAlive
{Crashed, !SuccessfulExit} + ThrottleInterval=10, mirroring the credential-proxy
/ menubar supervisors:
- Wrapper (gateway-bridge-wrapper.sh) loads the ag2space channel .env for
  REMOTE_TASK_TOKEN (launchd doesn't source .env), heals python3 onto PATH, and
  evicts any stale bare instance before exec (the bridge has no single-instance
  lock, so two pollers would double-process).
- install-gateway-bridge-launchd.sh installs/uninstalls/reports it (idempotent).
- startup.sh prefers the launchd job (skip-if-loaded), falling back to the bare
  launch on older checkouts or if install fails. Gated on token config, so a
  Sutando-only user without the mobile gateway never loads it.

Pairs with PR sonichi#2067 (health-check now REPORTS it down); this makes it RECOVER.

Live test (this machine, real launchd + real bridge):
  [1] install job          → Loaded via gui/501/com.sutando.gateway-bridge
  [2] bridge running       → pid 70681 (state=running)
  [3] kill -9 70681        (simulate the Jul-7 death)
  [4] wait ThrottleInterval → launchd auto-restarted → pid 70801 (NEW)
  [5] bridge reconnected   → "starting — gateway=chat.ag2.space/relay …"

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWvssgDGkNzjMPVG9xaMdj
john-the-dev added a commit to john-the-dev/sutando that referenced this pull request Jul 20, 2026
The ag2.space gateway bridge (remote-gateway-bridge.py) carries MOBILE-app
messages from the cloud gateway to the local core. It was a single unmonitored
process launched with a bare `&` from startup.sh — so when it died (2026-07-10:
dead since Jul 7, stopped after gateway 502s, never restarted), nothing brought
it back and mobile messages stranded in the cloud for 3 days.

Add a launchd job (com.sutando.gateway-bridge) with RunAtLoad + KeepAlive
{Crashed, !SuccessfulExit} + ThrottleInterval=10, mirroring the credential-proxy
/ menubar supervisors:
- Wrapper (gateway-bridge-wrapper.sh) loads the ag2space channel .env for
  REMOTE_TASK_TOKEN (launchd doesn't source .env), heals python3 onto PATH, and
  evicts any stale bare instance before exec (the bridge has no single-instance
  lock, so two pollers would double-process).
- install-gateway-bridge-launchd.sh installs/uninstalls/reports it (idempotent).
- startup.sh prefers the launchd job (skip-if-loaded), falling back to the bare
  launch on older checkouts or if install fails. Gated on token config, so a
  Sutando-only user without the mobile gateway never loads it.

Pairs with PR sonichi#2067 (health-check now REPORTS it down); this makes it RECOVER.

Live test (this machine, real launchd + real bridge):
  [1] install job          → Loaded via gui/501/com.sutando.gateway-bridge
  [2] bridge running       → pid 70681 (state=running)
  [3] kill -9 70681        (simulate the Jul-7 death)
  [4] wait ThrottleInterval → launchd auto-restarted → pid 70801 (NEW)
  [5] bridge reconnected   → "starting — gateway=chat.ag2.space/relay …"

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWvssgDGkNzjMPVG9xaMdj
john-the-dev added a commit that referenced this pull request Aug 19, 2026
* feat(gateway-bridge): launchd supervisor so it auto-restarts on death

The ag2.space gateway bridge (remote-gateway-bridge.py) carries MOBILE-app
messages from the cloud gateway to the local core. It was a single unmonitored
process launched with a bare `&` from startup.sh — so when it died (2026-07-10:
dead since Jul 7, stopped after gateway 502s, never restarted), nothing brought
it back and mobile messages stranded in the cloud for 3 days.

Add a launchd job (com.sutando.gateway-bridge) with RunAtLoad + KeepAlive
{Crashed, !SuccessfulExit} + ThrottleInterval=10, mirroring the credential-proxy
/ menubar supervisors:
- Wrapper (gateway-bridge-wrapper.sh) loads the ag2space channel .env for
  REMOTE_TASK_TOKEN (launchd doesn't source .env), heals python3 onto PATH, and
  evicts any stale bare instance before exec (the bridge has no single-instance
  lock, so two pollers would double-process).
- install-gateway-bridge-launchd.sh installs/uninstalls/reports it (idempotent).
- startup.sh prefers the launchd job (skip-if-loaded), falling back to the bare
  launch on older checkouts or if install fails. Gated on token config, so a
  Sutando-only user without the mobile gateway never loads it.

Pairs with PR #2067 (health-check now REPORTS it down); this makes it RECOVER.

Live test (this machine, real launchd + real bridge):
  [1] install job          → Loaded via gui/501/com.sutando.gateway-bridge
  [2] bridge running       → pid 70681 (state=running)
  [3] kill -9 70681        (simulate the Jul-7 death)
  [4] wait ThrottleInterval → launchd auto-restarted → pid 70801 (NEW)
  [5] bridge reconnected   → "starting — gateway=chat.ag2.space/relay …"

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UWvssgDGkNzjMPVG9xaMdj

* fix(gateway-bridge): default launchd wrapper tier to owner, matching startup.sh

The launchd wrapper defaulted REMOTE_TASK_TIER to "team", but src/startup.sh's
inline launch and the personal-agent model (PR #2018) default to "owner". On a
fresh/legacy install whose ag2space .env omits REMOTE_TASK_TIER, the launchd path
would silently sandbox the owner's OWN mobile messages as team-tier (read-only
codex) instead of processing them fully. Align the wrapper default with
startup.sh; multi-user gateways still set team explicitly in the channel .env.

Surfaced 2026-07-16 while root-causing a live gateway outage (the same launchd
job crash-looped ~1600x at exit-127 because the wrapper was absent on a
main-based deploy checkout).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(gateway-bridge): export CLAUDE_CONFIG_DIR in the launchd plist

launchd does not inherit the login-shell env, so the wrapper ran with
CLAUDE_CONFIG_DIR unset and its claude-home-path resolution fell back to
~/.claude/ — reading the WRONG channels/ag2space/.env on claude-sutando
(CLAUDE_CONFIG_DIR-scoped) installs. No token -> wrapper exits 0 (clean) ->
KeepAlive never restarts -> the supervisor silently no-ops, defeating the PR's
own purpose. The wrapper already documents that it honors a plist-exported
CLAUDE_CONFIG_DIR; this bakes it in.

Installer resolves it via the canonical sutando-config.sh claude-home-path
(matches util_paths.claude_home_path): $CLAUDE_CONFIG_DIR when set, else
~/.claude/ — identical to the wrapper's interactive resolution, so classic
installs are unchanged. Verified: substituted plist passes plutil -lint with the
workspace-scoped path.

Addresses @liususan091219's review on #2068.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013LvRyLAxRugJ9ygj8BgNR8

* fix(gateway-bridge): avoid the banned inline CLAUDE_CONFIG_DIR fallback

The prior commit's belt-and-suspenders fallback used the literal
${CLAUDE_CONFIG_DIR:-$HOME/.claude} form, which the lint-claude-home-path CI
guard forbids (that inline anti-pattern is exactly what claude-home-path exists
to replace). Split it into two plain checks — prefer $CLAUDE_CONFIG_DIR, else
~/.claude — same semantics, no forbidden pattern. Only fires if the
sutando-config.sh helper is entirely unavailable. Lint now clean.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013LvRyLAxRugJ9ygj8BgNR8

* fix(channel-bridges): auto-restart and alert on death

* fix(gateway-bridge): substitute plist via plistlib, not unescaped sed

install-gateway-bridge-launchd.sh injected paths into the XML plist with raw
`sed`, so a valid macOS path/config dir containing `&`, `<`, `>`, `|`, or a
backslash could corrupt the substitution or the plist and break bootstrap
(CR #2068, qingyun-wu).

Replace the sed block with the same plistlib substitution the channel installer
uses: parse the template, string-replace placeholders in every value, and
plistlib.dump — which XML-escapes on write, so hostile characters can't corrupt
the output.

Test runs the installer's ACTUAL substitution block (extracted from the
heredoc, not a copy) against the real template with a hostile path
(`&`, `<`, `>`, `|`, `\`) and asserts the result is a valid, parseable plist
with the values intact and the ampersand XML-escaped on disk; plus a structural
guard that the unescaped-sed path can't return.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPBv1rU2s1VrdtZCVd2o8r

* fix(channel-bridge): evict only THIS checkout's bridge, not another install's

channel-bridge-wrapper.sh evicted a pre-existing bare bridge with
`pkill -f "src/<channel>-bridge.py$"`. That relative suffix matches the same
bridge launched from ANY Sutando checkout on the host — and startup.sh launches
slack/discord/telegram with a relative path, so the command line is identical
across checkouts. Starting or upgrading one installation could therefore kill
another installation's live bridge (CR #2068, qingyun-wu).

Add src/launchd/evict-own-bridge.sh with evict_own_bridge(), which validates
each candidate PID's ownership before killing: its command path is under this
$REPO, OR its working directory resolves (physical path, so a symlinked checkout
or macOS /tmp->/private/tmp doesn't false-mismatch) to this $REPO. cwd is read
cross-platform (/proc on Linux/CI, lsof on macOS). The wrapper sources it in
place of the pkill.

Adds a two-checkout isolation test: two fake checkouts each run
`python3 src/slack-bridge.py`; evicting for checkout A kills A's bridge and
leaves B's alive, for both relative- and absolute-launched processes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPBv1rU2s1VrdtZCVd2o8r

* fix(channel-bridge): source evict helper only if present (don't break set -e)

Round-2 CR #2068: unconditionally sourcing evict-own-bridge.sh under `set -e`
broke tests/channel-bridge-launchd-supervisor.test.py — its fixture copies only
channel-bridge-wrapper.sh into a fake repo, so the missing `source` aborted the
wrapper before it launched/restarted the child and no restart alert was written.

Source the helper only when the file exists, and skip eviction if it's absent.
A partial deploy (or a fixture with just the wrapper) now degrades to
no-eviction — single-instance protection still covers that case — instead of
aborting the whole wrapper. The dedicated two-checkout isolation test still
exercises the real eviction path (helper present), and the supervisor suite
passes again.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QPBv1rU2s1VrdtZCVd2o8r

* fix(2068): resolve hardcoded-path scan + regen src-map after main merge

The launchd bridge files tripped the hardcoded-paths review-check (7 hits):
- Executable PATH-heal probes in the wrappers/installers legitimately name
  Homebrew's canonical bin prefix (launchd hands a minimal PATH; probe brew
  then fall back to /usr/local, /usr/bin, bare python3). Added a token-specific
  '/opt/homebrew/bin' allow to REVIEW.md with rationale — the correct cross-host
  pattern, not a machine-specific literal.
- Two plist placeholder-legend COMMENT lines carried literals for documentation
  only; reworded to drop them (no behavior change).
Also regenerated docs/src-map.md to index the new src/launchd/ modules.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XaocEvWwhBokFUmK5ECsfs

* fix(2068): route launchd interpreter/config through resolved values (qingyun P1)

Replaces the previous scanner-suppression approach. qingyun's [P1] CR asked to
route config + interpreter resolution through the canonical helpers, not exempt
the paths. Done:

- Installers resolve the interpreter's bin dir from their OWN PATH
  (`dirname "$(command -v python3)"`) instead of probing hardcoded
  /opt/homebrew|/usr/local dirs. Host-agnostic; the resolved dir is substituted
  into the plist PATH as __BREW_BIN__ at install time.
- Wrappers (gateway + channel) drop the hardcoded interpreter-candidate probe
  and use the plist-provided PATH (which now carries the install-resolved dir),
  so the committed files carry no clone-/arch-/user-specific fallback.
- Config dir already routed through `sutando-config.sh claude-home-path`
  (unchanged); reworded two plist comment lines that carried path literals.
- Reverted the REVIEW.md /opt/homebrew allow added in the prior commit — no
  longer needed; the scanner passes because the literals are gone.
- New regression tests/launchd-interpreter-resolution.test.py proves
  behaviorally that resolve_brew_bin() returns dirname(command -v python3) and
  that wrappers/plist/installers carry no hardcoded interpreter literal
  (host-independent via shutil.which).
- Regenerated docs/src-map.md.

Verified locally: review-checks PASS (allow removed), the two focused suites +
evict-isolation + the new regression all green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XaocEvWwhBokFUmK5ECsfs

* fix(gateway-wrapper): scope eviction to this checkout via evict_own_bridge

bassilkhilo-ag2's CR on #2068: gateway-bridge-wrapper.sh ran a bare
`pkill -f 'remote-gateway-bridge\.py$'`, which matches that bridge in ANY
checkout on the host — the exact class this PR closes on the channel side.
Starting or upgrading one install could kill a sibling install's live gateway.

Reuses this PR's own reviewed helper rather than inventing a second pattern:
evict_own_bridge builds `src/<channel>-bridge.py`, so channel "remote-gateway"
resolves to src/remote-gateway-bridge.py — the real script, verified. The
helper kills a candidate only when its command path is under this repo or its
cwd IS this repo. Guarded by the same `[ -f "$_EVICT_HELPER" ]` degrade-safe
check the channel wrapper uses, so a missing helper skips eviction instead of
aborting before the child launches.

tests/channel-bridge-evict-isolation.test.sh gains a gateway case: two fake
checkouts each running src/remote-gateway-bridge.py, evict for A, assert A dies
and B survives. Suite is 10/10 locally.

The destructive control is deliberately NOT in the test: a bare
`pkill -f 'remote-gateway-bridge\.py$'` in CI would match a developer's real
gateway bridge, which is the bug itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MK5AeA7sHrCk8tZv5sTrt8

* fix(evict): scope eviction by gateway instance, and make skips observable

One script path (src/remote-gateway-bridge.py) serves every gateway instance in a
checkout, so checkout scope alone cannot separate prod from GATEWAY_INSTANCE=dev.
Once this PR repairs the wrapper, the primary's launchd path starts executing
eviction on every boot (startup.sh kickstarts a loaded-but-idle job) and would
take out a bare-launched dev instance from the same checkout as collateral.

evict_own_bridge gains an optional discriminator — `<channel> <repo>
[instance-var] [instance-value]` — read from the candidate's own environment
(/proc on Linux, `ps eww` on macOS). Omitted, behaviour is unchanged, so
slack/discord/telegram keep their "one per checkout" assumption. The gateway
wrapper passes GATEWAY_INSTANCE with its own value; unset means primary/prod.

Indeterminate identity never kills: an unreadable env or unresolvable cwd skips
the candidate. That was already true of cwd but silently — the helper evicted
nothing while reporting success, indistinguishable from "not mine". Both skips
now emit one stderr line, so a host where process introspection is restricted is
diagnosable instead of looking like a logic failure.

Test: two gateways in ONE checkout, prod and GATEWAY_INSTANCE=dev; evicting for
prod kills prod and leaves dev running. Mutation-verified — dropping the
discriminator flips it to `FAIL dev gateway ... (was killed)`. Suite 14/14.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MK5AeA7sHrCk8tZv5sTrt8

* fix(evict): unreadable env is indeterminate, not "instance unset"

Self-correction on 4a75029. _pid_env returned an empty string in three different
situations — variable genuinely unset, /proc/<pid>/environ unreadable, and `ps eww`
restricted — and the caller compared that against the wanted instance value. The
primary gateway evicts with instance-value "", so an unreadable environment
compared EQUAL and killed a process whose identity was never established. That is
the collateral kill this discriminator exists to prevent, and it contradicts the
"indeterminate identity never kills" claim in the previous commit message.

_pid_env now separates the two: it echoes the value and returns 0 when the
environment was actually read (an unset variable is legitimately empty), and
returns 1 when nothing could be read at all. The caller skips on rc=1 with a
distinct stderr line, so "could not identify" is no longer spelled the same way as
"identified, and it is ours".

Test pins the contract directly: a live pid with the variable unset must give rc=0
and empty, and an unreadable pid must give rc=1. Mutation-verified — deleting the
`[ -n "$raw" ] || return 1` guard flips the second case to FAIL. Suite 16/16.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MK5AeA7sHrCk8tZv5sTrt8

* fix(evict): script mode must forward the instance args, not drop them

The direct-invocation entry point still read `evict_own_bridge "${1:-}" "${2:-}"`
from before the discriminator existed, so running the helper as a script silently
discarded arguments 3 and 4. The failure is quiet and points the wrong way: with
no discriminator the function falls back to checkout-only scope and KILLS an
instance the caller was explicitly trying to spare.

Forwards "$@" instead, and the usage comment now names the optional arguments.

Verified behaviourally rather than by reading: two gateways in one checkout (prod
and GATEWAY_INSTANCE=dev), evicting for prod via `evict-own-bridge.sh remote-gateway
<repo> GATEWAY_INSTANCE ""` leaves dev running. Suite 16/16.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MK5AeA7sHrCk8tZv5sTrt8

* test(evict): cover the real blind-introspection case, not just a dead pid

My d196a76 test asserted the contract with a nonexistent pid. That is a proxy:
the production failure is a process that EXISTS and is ours by checkout, whose
environment cannot be read — so identity is unknown and it must survive. A dead pid
exercises the same branch but not the same situation, and nothing killed it either
way.

Adds the case in its real shape, using bassilkhilo-ag2's repro from the review
thread: a live GATEWAY_INSTANCE=dev process in the checkout, `ps` shadowed on PATH
to fail, then the primary's own eviction line. The instance must survive.

Mutation-verified: removing only `[ -n "$raw" ] || return 1` flips it to
`FAIL blind-case: ... (was killed)`. Suite 18/18.

This is the shape @qingyun-wu's P1 #1 environment plausibly has, so it is the case
worth having permanently rather than reproducing by hand each time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MK5AeA7sHrCk8tZv5sTrt8

* test(gateway-plist): trim the decoded-vs-repr comment to the two-line cap

`review-checks` prose-cap failed the merge result:

    prose-cap: tests/gateway-bridge-launchd-plist-escaping.test.py:103
               comment block is 3 lines (cap 2)

Pre-existing in this PR, not introduced here — it was unreachable while the job
died earlier on the stale `docs/src-map.md`. Clearing that let CI reach the
scanner, which then surfaced this. Same sequence as #2165.

Condensed 3 lines to 2, keeping the part the code cannot say: match the DECODED
strings rather than `repr()`, because repr re-escapes the backslash and
false-fails on a value that actually survived intact. Dropped the first line,
which restated what the three `check(...)` calls directly below already assert.

Evidence — scanned the real PR diff (`git diff upstream/main...HEAD`) with all
three in-scope `.py` files present in the tree, so it is a scan and not a skip:

    prose-cap: PASS (comment blocks within cap 2, exts .py)

Positive control on the same input, so the pass is not vacuous — at cap=1 it
fails and names the rewritten block:

    prose-cap: 5 block(s) over the 1-line cap
    prose-cap: …plist-escaping.test.py:103 comment block is 2 lines (cap 1)

tests/gateway-bridge-launchd-plist-escaping.test.py still passes.

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

* fix(channel-bridges): probe the interpreter, not the internet

Addresses yixuan-ag2's P1 on this PR.

`channel-bridge-wrapper.sh` probed slack/discord with a local import but
telegram — which has no third-party dep — with a live fetch to
api.telegram.org. On a network outage `PYTHON` stayed empty and the wrapper
reported `no usable Python interpreter` and `exit 1`. Wrong diagnosis, wrong
subsystem, and it is the input that earns launchd's deferral penalty: a fast
exit-1 loop on the one channel that probed differently from the other two —
the pathology this PR exists to prevent.

Fix is the reviewer's: `MODULE=urllib.request` for telegram. urllib.request is
stdlib, so all three channels now run the same local-import check and the
`else` branch is gone.

Also adds the test the reviewer noted was missing (`grep -c` for the probe was
0 in both existing files, so nothing pinned WHICH condition it measured).
It drives the real wrapper with a shadow `python3` on PATH.

Evidence — same test, parent commit vs HEAD.

At 27c8471 (probe unchanged):

    1. probe is load-bearing: an interpreter that fails every check must gate
      ok: wrapper exits 1 with the interpreter diagnosis when python3 truly fails
    2. REGRESSION: a network outage must NOT be reported as a missing interpreter
      FAIL: network-only failure was reported as 'no usable Python interpreter'
            (rc=1) - probe is measuring the network
    3. the probe invocation itself names a module, not a URL
      FAIL: probe reaches the network: -c import urllib.request as u;
            u.urlopen("https://api.telegram.org", timeout=8)
    FAILED: 1

At HEAD:

    1. ok: wrapper exits 1 with the interpreter diagnosis when python3 truly fails
    2. ok: network-only failure does not produce the interpreter diagnosis
    3. ok: probe is a local import: -c import urllib.request
    ALL PASSED

Case 1 passes in BOTH states deliberately — it is the control proving the probe
still gates a genuinely broken interpreter, so cases 2/3 are not passing by
disabling the check.

Neighbouring suites at HEAD:

    tests/channel-bridge-evict-isolation.test.sh     PASS
    tests/channel-bridge-launchd-supervisor.test.py  all passed

The suite bounds each run itself (a successful probe leaves the wrapper
resident) and falls back to a watchdog where `timeout`/`gtimeout` are absent,
so it runs on macOS as well as the CI runner.

Not addressed here: the reviewer's separate nit that `$CHANNEL.started`
survives a clean stop, so every deliberate restart alerts. Different concern.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ag2product AG2 product related (packages, PyPI, task-relay)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants