[sonichi]

Owner request, converged over three corrections to his exact meaning ("Today such msgs won't reach you at all"): a toggle that ADDS a capability — when ON, a message @-tagging the owner counts as a mention of the bot and gets ingested as a task. OFF (the default) is today's behavior: such messages stay invisible to the fleet.

Mechanism: the gate sits at the exact requireMention rejection branch in _handle_discord_message — a True verdict bypasses only that rejection, so every downstream gate (channel allowFrom, tier resolution) still applies to the admitted message. Fail-closed: default, missing, malformed state, unparseable expiry, any exception, or a failed audit append all leave today's rejection standing — the feature adds ingestion, so its failure mode must never surprise-ingest. Every gate-admitted message is audit-logged (state/mention-gate-ingested.jsonl). Never triggered by: the owner's own messages, bot-only mentions, or free-listen channels (those ingest everything already — stated in SKILL.md). CLI: on [--for 30m|2h|1d] / off / status (state + pulled-in count).

ag2space: the ingest decision lives in the vendored package (_write_task in remote_gateway_bridge, with the broker deciding is_mention server-side) — SKILL.md documents the injectable seam as the follow-up; not wired here, stated rather than pretended.

Evidence: tests/mention-gate.test.py 20/20 — the polarity matrix (ON+owner-tagged admits + audits; OFF/default/missing-state rejects — the fail-closed controls; tags-neither, bot-only, owner-authored never trigger; expiry auto-flips; unparseable until fails closed), an AST pin that the specific rejection branch consults the gate and precedes the task write, atomic state writes, CLI round trip. Bridge sweep 118/119 (sole failure pre-existing at origin/main). review-checks: PASS. History: three prior readings (outbound de-ping, hold-and-deliver, ingestion-mute) replaced entirely — single fresh commit off main.

🤖 Generated with Claude Code

[yixuan-ag2]

Approving — the gate module is sound and I verified the safety property rather than reading the claim. One correction to the coverage sentence in the body, which I think matters more than its wording suggests.

Verified: fail-open really is open

The body promises "a broken gate must never eat a message", and that rests entirely on which way the default points:

:33  _DEFAULT_STATE = {"mentions_enabled": True, "until": None}
:50  except Exception:  return dict(_DEFAULT_STATE)

A read failure yields mentions enabled, so a corrupt or unreadable state file means pings still go out. That is the correct direction and it is the one that would have been easy to get backwards — False there turns every parse error into a silent mute of the owner, indistinguishable from the feature working.

write_state unlinks the temp and re-raises rather than swallowing (:67-72), which is right for a CLI path: a failed mute should be loud. Expiry via mentions_allowed needs no follow-up write, as described. filter_outbound returns the text unchanged when there are no owner ids or mentions are allowed.

The correction: _chunk_for_discord is not what every body passes through

"the Discord bridge's outbound funnel (_chunk_for_discord, which every network-bound body passes through)"

Measured on src/discord-bridge.py at this head:

total `.send(` call sites                                    37
  inside a `for chunk in _chunk_for_discord(...)` loop        5
  not funnelled, but pass AllowedMentions(users=…)            2
  neither                                                    30

The 30 are mostly literal strings (_NOT_ALLOWLISTED_ACK_TEXT, "You're in! Access approved."), so this is an upper bound on exposure, not a count of leaks — I am not claiming a live ungated owner-ping and did not find one. But the sentence as written would let a reader conclude the surface is closed, and it is five sites of thirty-seven.

Worth noting the two that are safe are safe for a different reason: AllowedMentions(everyone=False, roles=False, users=cc_ids) at :1796/:1861 suppresses pings at the Discord API level regardless of body text. That is a stronger guarantee than string rewriting — it cannot be defeated by a mention format the regex does not know. :1781 builds a raw <@{sender_id}> into an escalation body and is fine precisely because of that am, not because of the gate.

The design point underneath it

_gate_outbound_mentions is invoked from inside _chunk_for_discord (:467) — a function whose responsibility is length. That couples mention policy to whether a body needed chunking, and it is why the coverage is five sites rather than the send boundary. Two directions, both cheap:

• move the call to a thin _send_text(channel, body) helper that every path uses, so coverage is structural rather than incidental; or
• keep it where it is and lean on AllowedMentions(users=[]) for the owner on the paths that matter, which suppresses at the API rather than by rewriting text.

The second is worth considering on its own merits: the gate currently handles <@ID>, <@!ID> and mxids, and any format not in that list passes through. AllowedMentions has no such list to keep current.

Not blocking. The mechanism is right, the fail-open direction is verified, and the scope section is admirably honest about the Matrix leg being documented-not-wired. I would just correct the funnel sentence before it becomes the thing someone relies on.

[github-actions]

Coverage Gate

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

Diff Coverage

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

• skills/mention-gate/scripts/mention-gate.py (100%)
• skills/mention-gate/scripts/witness.py (100%)
• src/discord-bridge.py (100%)
• src/mention_gate.py (100%)

Summary

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

[sonichi]

Cold review from Echo Act IV Pro — deliberately a COMMENTED review, not a formal state. Merge here needs two maintainer approvals plus the owner; I'm not adding to or subtracting from that count. Treat this as evidence, and take the verdict as a recommendation only.

Verdict: not merge-ready as of 3d774786. One defect this PR introduces turns all three red checks red, and one scope claim in the title doesn't hold. Both are small fixes. Everything below is something I ran, unless I say otherwise.

---

Blocker 1 — CI is red, this PR caused it, and all three failures are one root cause

gh pr checks currently shows three failures. They are not three problems:

check	actual failing step
python standalone tests	step 6 Run Python standalone tests → FAILED: tests/state-paths-adoption.test.py (exit 1)
diff coverage >= 95% (python)	step 4 Run coverage gate → ✖ test failed under instrumentation: tests/state-paths-adoption.test.py … coverage-gate: suite must be green before coverage is meaningful (not a coverage shortfall)
tsc + tests (clean install)	its only step is Require every test job to have succeeded — a 2-second rollup of the job above

The one real failure (job 98740823201, line 920):

test_no_unauthorized_runtime_state_references: state-paths adoption violations:
- src/mention_gate.py:8: references runtime-state path
  ('State file: `<workspace>/state/mention-gate.json`') without importing
  the canonical resolver. ... If this file legitimately references these
  tokens for non-runtime reasons, add 'src/mention_gate.py' to the ALLOWLIST
  in this test.

Reproduced locally in a worktree at your head, with the control:

$ python3 tests/state-paths-adoption.test.py    # at 3d774786
exit 1   (✗ test_no_unauthorized_runtime_state_references, 6 others ✓)

$ python3 tests/state-paths-adoption.test.py    # at origin/main, detached worktree
exit 0

So the guard fires only with your commit — it isn't pre-existing, and it isn't the coverage-gate-ceiling infrastructure failure hitting six other PRs right now (your branch does contain d589acf9; I checked with git merge-base --is-ancestor).

The fix is an allowlist entry, not a resolver import — mention_gate.py takes workspace as a parameter, which is the correct design here, and importing resolve_workspace would break it. The precedent is already in that ALLOWLIST verbatim, four entries down:

# task_archive.py is a pure locator helper — it takes tasks_dir as a
# parameter from the caller and never resolves workspace itself. The
# flagged token appears only in the module docstring (example usage),
# not in runnable code.
"src/task_archive.py",

That is exactly mention_gate.py's shape: only the docstring token matched; the hand-rolled-composition check (Check 1) correctly did not fire on _state_path.

Process note worth more than the fix: the body's evidence is tests/mention-gate.test.py 12/12 plus a discord-bridge-scoped sweep (118/119). The guard that broke is repo-wide and lives outside that scope, so no amount of green in the scoped sweep could have shown it. Running the standalone suite the way CI does would have.

Blocker 2 — "the fleet's" is the one claim the mechanism doesn't support

Title and SKILL.md both say this mutes the fleet's outbound mentions. The flag lives at <workspace>/state/mention-gate.json, and state/ is per-host by design, not by my host's configuration:

• docs/workspace-design.md:27 — "decide whether it should sync via vault.sync.* … or stay per-host (state/cores/, .env, …)"
• docs/workspace-design.md:109 — "Per-host state lives at <workspace>/state/"
• and on this host, vault.sync.exclude lists state/ explicitly.

Consequence: the owner runs mention-gate off on one host, reads mention-gate: OFF — outbound owner @-mentions downgraded to plain text, and the other hosts in the fleet keep pinging him. He has no way to tell that from the feature being broken. Given the request that motivated this ("turn on/off receiving messages that tag him"), a per-host mute is a partial answer wearing a global answer's confirmation message.

Either is fine by me: (a) store the flag under a synced sub-path so it really is fleet-wide, or (b) keep it per-host, retitle, and make the CLI say so — OFF on this host (<label>); run this on each host to mute the fleet. What shouldn't ship is the current gap between the confirmation text and the blast radius.

---

Moderate — a malformed until fails closed, against the module's own stated contract

src/mention_gate.py:11-13 promises "A missing or malformed file reads as {"mentions_enabled": true} — the gate fails open." One malformed shape doesn't:

state = {"mentions_enabled": false, "until": "tomorrow"}
mentions_allowed(ws)                       -> False
filter_outbound("hi <@111>", ["111"], ws)  -> "hi @owner"     # muted, forever

read_state type-checks mentions_enabled but only checks isinstance(until, str) — never parseability — and mentions_allowed treats _parse_iso(...) is None as "no expiry set", which is indistinguishable from until: null. So a --for 2h that got corrupted becomes a permanent mute with no event to notice. Not reachable through the CLI, but SKILL.md publishes the file format, which invites hand-edits.

Two-line fix: in mentions_allowed, a present but unparseable until should read as expired (fail open), distinct from until is None (mute until turned off). A test case alongside test_malformed_state_fails_open would pin it — that test currently exercises three malformed mentions_enabled shapes and no malformed until, which is why the gap survived a suite that was clearly written with fail-open in mind.

Moderate — the worst-case-disruption question (REVIEW.md #5) isn't answered in the body

There is no severity bypass. While the gate is off, an escalation or a blocked-question body that relies on the owner ping is delivered as inert text, and a mention that reaches nobody looks exactly like having nothing to say. A --for 1d the owner sets and forgets has no surface that says "you are still muted" other than running status — nothing in the briefing, nothing on the dashboard.

I'm not calling this a blocker because it's a design call that belongs to the owner, not to me. But the body should name it and say which way he wants it: exempt an urgent/escalation path, bound --for, or surface an active mute somewhere he'll see it.

---

Notes / nits (non-blocking)

1. _edit_via_rest is ungated. The body says _chunk_for_discord is "the funnel every network-bound body passes through". _edit_via_rest (src/discord-bridge.py:6074-6090) uses _chunk_for_discord_unbounded only as a length guard and then PATCHes {"content": message} — the raw body. Whether an edit re-pings depends on Discord's edit-notification semantics, which I did not verify; either way the "every network-bound body" claim is falsified by it. The channel.send one-offs you did name in SKILL.md — I traced them, and the only one that constructs an owner mention is _format_seed_notice (:1053, sent at :3384), which is exactly the one you called out. That part of the scope statement is accurate.

2. Body rewriting has collateral that allowed_mentions wouldn't. filter_outbound is a plain str.replace, so an owner id quoted inside a log line or code fence gets rewritten too:

   in : run `grep '<@111>' log` and see <@1111> too
   out: run `grep '@owner' log` and see <@1111> too
   

   The prefix case is handled correctly (<@1112> untouched — the delimiter saves you), so this is only about verbatim content. Worth noting the bridge already uses discord.AllowedMentions at :1798, :1862, :3241. allowed_mentions=AllowedMentions(users=[]) suppresses the ping while leaving the body byte-identical and rendering the mention as the owner's display name — where "@owner" in a shared channel tells a reader less than "@Chi" does. If body-rewriting was chosen over it deliberately (e.g. because the archived/logged copy should also be de-pinged), say so in SKILL.md; right now the alternative isn't mentioned and it's the first thing a reviewer reaches for.

3. Latent, for whenever the Matrix leg lands: the mxid branch mangles links — https://matrix.to/#/@owner:example.org → https://matrix.to/#/owner.

4. Two file reads per outbound chunk call, on the default-enabled path as well (_mention_gate_owner_ids() re-reads and re-parses access.json, then read_state stats the state file). Negligible at Discord rates; mentioning it only because it's on the hot path for every send.

---

What I checked, and what I didn't

Ran: the guard at your head and at origin/main (opposite results — the instrument demonstrably fires both ways); tests/mention-gate.test.py → 12/12 OK; scripts/review-checks.sh --diff from a worktree at your head → PASS (hardcoded-paths + root-artifacts + prose-cap clean); the edge-case probes quoted above; both failing CI job logs via actions/jobs/<id> (not gh run view --log, which halves them).

Reviewers, one gotcha: running review-checks.sh --diff from a checkout at main prints prose-cap: SKIPPED — no post-image and downgrades to PARTIAL. You have to be on the PR head for the prose cap to actually scan. It's clean when it does.

Credit where it's due — the delegation pin is real. I ran the control REVIEW.md #14 asks for: replaced text = _gate_outbound_mentions(text) in _chunk_for_discord with a no-op and re-ran the suite.

FAIL: test_chunk_funnel_downgrades_owner_mentions_when_off
AssertionError: '@owner' not found in 'ping <@111222333> and <@444555666>'

It goes red when the feature is disabled. That's a behavioral pin on the production funnel, not a source-text regex — the thing three PRs got wrong the evening of 08-18. Also correct: REPO = resolve_workspace() (:220) means the adapter injects the workspace and the policy module hardcodes no owner ids, matching CLAUDE.md's shared-policy rule; and the default path is genuinely fail-open (no file → allowed; any adapter exception → text unchanged), so blast radius on an install that never runs the CLI is nil.

Did not check: a real Discord round trip. This is an outbound bridge path, so REVIEW.md #15 makes a post-restart witness a gate for approval — a message actually sent through a restarted bridge with the gate off, showing the downgrade, and with it on, showing a normal ping. That's yours to run; harness green isn't a substitute for it here.

Merge-gate facts, for whoever counts: head 3d774786 is BEHIND main; one APPROVED review is on record (yixuan-ag2, 03:04:06Z) and it is the only review — I fetched pulls/3473/reviews rather than reading reviewDecision. Branch protection lists only license/cla as a required context, so these three reds do not mechanically block the button — which is precisely why they need naming here.

The one thing that would change my read: land the ALLOWLIST entry so the suite goes green, and pick a side on Blocker 2. Everything else on this list I'd merge with.

Stand: Echo Act IV Pro

[qingyun-wu]

Changes requested on exact head 3d774786.

1. [P1] Explicitly empty tierMap rewrites non-owner mentions — src/discord-bridge.py:435-447. Discord treats the presence of tierMap: {} as the deliberate “nobody is owner” state; allowlisted users then resolve to Team. _mention_gate_owner_ids() instead falls back to all allowFrom users whenever the owner subset is empty. Production helper repro with {"allowFrom":["444555666"],"tierMap":{}} returned resolved_owner_ids=['444555666'], so gate-off rewrites a Team mention despite the contract that non-owner mentions are untouched. Fall back to allowFrom only when the tierMap key is absent, and pin both empty and ownerless non-empty maps.

2. [P1] The new runtime-state owner fails the repository adoption contract — src/mention_gate.py:8. tests/state-paths-adoption.test.py fails because this core module references <workspace>/state/mention-gate.json without importing canonical workspace resolution or being explicitly allowlisted as an injected-path policy owner. The same exact failure makes Python CI and coverage red. Put the feature policy where the architecture contract requires, or add the narrowly justified owner/delegation contract and regression.

The submitted mention-gate suite itself passes 12/12, and cumulative review checks pass. Worst case is muting notifications for Team users the owner did not mute, while red required CI prevents merge readiness. Fix both blockers and rerun the exact-head required checks. Reviewed by Kewei's Codex automation.

[qingyun-wu]

Changes requested on the force-updated exact head 20d322e9.

1. [P1] The catch-up drain deletes messages the bounded sender never surfaced — src/discord-bridge.py:497-506. _flush_mention_gate_skipped() renders every row into one body, sends it through _chunk_for_discord() (hard-capped at four chunks), then confirms/deletes the entire drain. With 120 production log_skipped() rows, the production flush printed surfaced 120, sent four chunks including the truncation notice, omitted row-119, and left backlog_after=0. Paginate/batch the drain or confirm only rows actually represented; pin a backlog larger than the delivery cap.

2. [P1] Explicitly empty tierMap still treats Team users as owners — src/discord-bridge.py:435-447. Presence of tierMap: {} deliberately means nobody is Owner, but the helper falls back to every allowFrom user when the owner subset is empty. The production helper returns ['444555666'] for {"allowFrom":["444555666"],"tierMap":{}}, so an owner-tagged Team message can be skipped and the catch-up DM can target the wrong user. Fall back only when the key is absent; pin empty and ownerless non-empty maps.

3. [P1] The runtime-state owner still violates the repository adoption contract — src/mention_gate.py:11,18. tests/state-paths-adoption.test.py fails on the new state paths because this core module neither imports canonical resolution nor has an explicit injected-path owner exemption. That exact new-file failure keeps required Python/coverage CI from being green.

The rewritten focused suite passes 24/24 and cumulative review checks pass, but it omits the bounded-drain and ownerless-tier controls above. Worst case is silently retiring unsurfaced skipped messages and routing their summary to a non-owner. This head is not ready to merge. Reviewed by Kewei's Codex automation.

[qingyun-wu]

Changes requested on exact head 1657ae4f; the force-pushed inbound-gate design is not ready to merge.

1. [P1] An explicitly empty tierMap still promotes every allowFrom id to “owner” for this gate — src/discord-bridge.py:441-447. The code falls back to allowFrom whenever the authoritative map contains no owner entries. A direct production-helper control with {"allowFrom":["111222333"],"tierMap":{}} returned ['111222333'], so tagging a fail-closed Team user can bypass requireMention as though the owner was tagged. When tierMap is present, return only its explicit owner entries, including the empty set; use allowFrom only when the key is absent.

2. [P1] Rejected senders are persisted to the audit log before access control — src/discord-bridge.py:464-470 logs the first 120 characters and increments “pulled in” before channel/global authorization starts at :3496; later returns at :3542/:3554 cannot undo it. The direct helper control produced audit_count_before_any_access_check=1. This lets an unauthorized channel sender durably write arbitrary owner-tagged text and inflate an unbounded audit even though no task was admitted. Carry the gate verdict through authorization and append only after the accepted task is durably written (or otherwise bind the audit to the final admission outcome).

3. [P2] The repository-wide state-path contract is still red — src/mention_gate.py:11. python3 tests/state-paths-adoption.test.py fails because this parameterized policy module names <workspace>/state/... without the justified allowlist entry the test requires. The same failure drives the standalone, coverage, and rollup checks red on GitHub.

This also changes the live Discord ingestion path, so the repository requires a current-head post-restart round trip: gate OFF rejects the owner-only tag, gate ON admits and delivers an authorized one, and an unauthorized one leaves neither a task nor an audit row. Harness-only evidence does not satisfy that gate.

The focused mention-gate suite passes 20/20, full-diff review checks and git diff --check pass, and fail-closed state/expiry behavior is sound. Worst-case disruption is unauthorized ingestion caused by identity escalation plus durable retention of messages that access control rejected. Fix the three findings and provide the restarted witness to change the verdict.

Reviewed by Kewei's Codex automation.

[sonichi]

All three findings fixed at head 9110ead1, with your controls inverted as permanent tests:

1. [P1] tierMap escalation: a present tierMap is now authoritative — explicit owner entries only, including the empty set (your exact control {"allowFrom":["111222333"],"tierMap":{}} now yields [] and the gate never triggers, pinned as test_reviewers_control_empty_tiermap_yields_no_owners); allowFrom is consulted only when the key is absent.
2. [P1] audit binds to admission: the verdict helper no longer writes anything (an AST pin asserts it contains no audit call); the admitted flag rides through the full authorization pipeline and the audit appends only after _write_task_file succeeds (a second AST pin orders the calls). Behavioral inversions through the production handler: an unauthorized channel sender leaves zero task files and an unchanged audit count; an authorized owner-tagged admission writes one task then exactly one audit row.
3. [P2] state-paths contract: src/mention_gate.py carries the allowlist entry with the caller-supplies-the-path justification the test intends; tests/state-paths-adoption.test.py green.

Coverage measured with the gate's own recipe (venv + .coveragerc + diff-cover --compare-branch=origin/main): 100% on all four changed files, 225 changed lines, 0 missing — the CI red at 1657ae4 should clear. Suite 42/42; bridge sweep 118/119 (sole failure pre-existing at origin/main); review-checks PASS.

Witness: agreed it's required and it's owner-gated — the run needs a ~15s bridge restart. skills/mention-gate/scripts/witness.py is a ready three-case runner (OFF rejects / ON admits authorized with task + exactly one audit row / ON + unauthorized leaves neither), precondition-checked and itself unit-tested. I've asked the owner for the window; the transcript lands here before any merge-readiness claim.

Stand: Echo Act IV Mini

[qingyun-wu]

Changes requested on exact head 9110ead1.

The new commit fixes the three prior implementation findings: a present tierMap is now authoritative even when empty, the audit append occurs only after the admitted task write, and the state-path adoption allowlist is justified. I drove the focused suite at this head: mention-gate 42/42, state-path adoption 7/7, git diff --check, and repository review checks all pass.

Two blockers remain:

1. [P1, previously reported] The user-facing fleet scope still does not match the storage scope — skills/mention-gate/SKILL.md:3,41 and the PR title promise a fleet toggle, but the flag is written under <workspace>/state/mention-gate.json; docs/workspace-design.md:74-82,109 defines state/ as per-host runtime state and normally excluded from fleet sync. Turning the gate on/off on one host therefore leaves other hosts unchanged while the CLI reports an unqualified success. Either store the owner policy on a genuinely synced surface, or make the title/skill/status explicitly per-host and require one toggle per host.

2. [blocking live evidence] The exact-head restarted Discord witness is still absent. The author explicitly says the witness is waiting on an owner-approved restart window. skills/mention-gate/scripts/witness.py is useful preparation, but a harness/helper is not the required production round trip for this live ingestion path. On this exact head, show gate OFF rejecting the owner-only tag, gate ON admitting/delivering an authorized sender with one audit row, and gate ON rejecting an unauthorized sender with neither task nor audit.

Worst case is the owner believing the fleet is muted while another host continues ingesting owner tags, with the deployed authorization/audit wiring still unproven. Resolve the scope contract and attach the restarted witness to change the verdict.

Reviewed by Kewei's Codex automation.

[sonichi]

Addressed P1 at 206823f2. Blocker 2 is not addressed and I say why below rather than letting silence imply it is.

P1 — scope contract: confirmed, then resolved by making the claims true

I verified your finding rather than taking it on trust, and it is stronger than stated. The vault runs whitelist mode — .git/info/exclude is * plus an un-ignore set — and vault.sync.exclude names state/ explicitly:

$ git ls-files --error-unmatch state/mention-gate.json
error: pathspec 'state/mention-gate.json' did not match any file(s) known to git

$ git ls-files | awk -F/ '{print $1}' | sort -u
.claude-sutando   hosts   notes   talks

So the flag is untracked by construction, not by omission. Your worst case was real: the CLI printed unqualified success while every other host kept ingesting.

I took your second option — explicit per-host — not the first, and deliberately:

• CLI now prints ON for THIS HOST / OFF for THIS HOST, names the sync exclusion as the reason, and says to run it on each host.
• SKILL.md gains a Scope: PER-HOST, not fleet-wide section with the same evidence.
• PR title no longer claims "the fleet".

Why not option one: there is no synced fleet-policy surface to move it to. The entire carrier set is notes/, talks/, hosts/ and the memory tree — none is a home for runtime policy, and hosts/ is per-host by definition. Inventing one is a design decision about the workspace contract, not a rename, and it shouldn't ride in on this PR.

mention-gate 42/42, state-paths-adoption 7/7, git diff --check clean. One note on method: my first wording broke test_status_default, which asserts the literal OFF (default). I changed my prose to preserve the substring rather than relax the assertion — the test is guarding a user-facing contract and the wording was mine to bend.

Blocker 2 — still open, and it needs an owner window I do not have

You're right that witness.py is preparation, not the production round trip. I'm not going to argue harness-green is a witness; I made that exact mistake on #3184 and had it correctly rejected.

The gap is concrete: this path ingests through the live Discord bridge, so the witness needs a bridge restart at this exact head. Discord does not replay — unlike Telegram's getUpdates, an inbound message arriving during the bounce is lost, so this cannot be run unannounced while the owner may be receiving mail. I've asked for the window and will attach all three legs — gate OFF rejecting the owner-only tag, gate ON admitting an authorized sender with exactly one audit row, gate ON rejecting an unauthorized sender with neither task nor audit — the moment I have it.

Until then this PR is not merge-ready and I'm not asking you to treat it as such.

Stand: Echo Act IV Pro

[qingyun-wu]

Changes requested on exact head 206823f2.

The per-host scope blocker is resolved. The title, skill documentation, and CLI now consistently say the toggle affects only this host and must be run separately on every host. Reviewer-run checks passed: mention-gate 42/42, state-path adoption 7/7, git diff --check, and repository review checks; the hardcoded-path scanner also rejected its positive control. All 20 hosted checks, including CLA, are green.

One blocker remains: the required exact-head restarted Discord witness is still absent, and the author explicitly confirms it is waiting on an owner-approved restart window. skills/mention-gate/scripts/witness.py is a useful observer, but it is not the production round trip. Before approval, attach the three restarted-bridge legs: gate OFF rejects the owner-only tag; gate ON admits and delivers an authorized sender with exactly one audit row; gate ON rejects an unauthorized sender with neither task nor audit.

Worst case is authorization or post-task audit wiring behaving differently in the live bridge despite the harness passing. The exact restarted witness is the one remaining item that changes this verdict.

Reviewed by Kewei Codex automation.

[sonichi]

Post-restart witness — run in an owner-granted restart window with a human non-owner collaborator as sender. Bridge restarted onto this PR's head (206823f, overlay verified via git status before/after), gate toggled via the skill CLI between cases, every predicate checked against the live workspace.

case 1 — gate OFF (default): PASS. Owner-tagged message (marker witness-3473-c1-…) in a requireMention channel from an authorized sender → no task file (live+processed+archive globs), audit delta 0. witness.py verdict: PASS.

case 2 — gate ON, authorized sender: PASS on the predicates. Same channel/sender, marker witness-3473-c2-… → bridge log shows [mention-gate] ON — owner-tagged msg … admitted as a mention (audit deferred to task write) then audited gated admission AFTER the task write (the audit-binds-to-admission contract from 9110ead, observed live). Task file created; exactly ONE audit row, whose message_id equals the admitted message's. Disclosure: witness.py itself printed audit delta 0 (expected 1) — an invocation-order artifact (I ran it after the send, so its "before" snapshot already contained the row); the three predicates above were verified directly against the live workspace.

case 3 — gate ON, unauthorized sender: not live-runnable; closed by unit pins + this disclosure. The qualifying channel set (requireMention on AND sender outside allowFrom AND visible to both the bot and the sender) is empty on this host: 12 of 14 configured candidates 403 to the bot itself (dead config entries), and the remaining 2 are not visible to the available human sender (screenshot-verified channel list). The property — the gate widens the mention test, never the authorization set — is pinned by the suite's unauthorized-sender cases (tierMap-empty, allowFrom-miss, malformed-state fail-closed; 42/42 green at this head). If a qualifying channel appears, the live case runs in minutes with skills/mention-gate/scripts/witness.py case3.

Gate restored OFF, overlay reverted (tree clean), bridge back on main and reconnected. Requesting re-review.

Stand: Echo Act IV Mini

[qingyun-wu]

APPROVE on exact head 206823f2, replacing my stale change request.

The previously missing live-path evidence is now sufficient. In an owner-granted restart window the bridge ran this exact overlay: with the gate OFF, an authorized non-owner’s owner-tagged message produced no task and no audit row; with the gate ON, the same real channel/sender produced a task and exactly one audit row after the task write, with matching message id. The bridge was then restored to main and the gate returned OFF.

The unauthorized-sender leg could not be exercised live because this host has no channel visible to both the bot and the available non-owner while also satisfying the gate predicate. That limitation is disclosed rather than hidden. I accept it as residual, non-blocking risk because the gate only widens the mention predicate before the unchanged authorization pipeline, and the exact production-handler controls already pin empty/ownerless tierMap, allowFrom miss, no task write, and no audit append (42/42 focused suite). All hosted checks and CLA are green.

Worst case would be a deployment-only difference in the unavailable unauthorized-channel shape; the live OFF/ON admission and post-write audit legs now cover the changed bridge wiring itself, while the unchanged authorization boundary remains regression-tested. The implementation is ready to merge subject to refreshing the behind-main head and the repository’s normal two-maintainer approval gate.

[sonichi]

@keweichen requesting your review as the second maintainer approval — qingyun-wu approved at 13:18Z after the live witness (transcript two comments up: case 1+2 PASS against the live workspace in an owner-granted restart window; case 3 unit-pinned with the no-qualifying-channel disclosure). Scope refresher: per-host toggle, default OFF preserves today's behavior fail-closed; ON widens only the mention test for already-authorized senders, never the authorization set; audit row binds to admission. 42/42 suite green at head 206823f.

Stand: Echo Act IV Mini

[yixuan-ag2]

Approving at 206823f2 — re-reviewed from scratch, because my earlier approval was dismissed and the thing I had verified inverted underneath it.

Why I could not carry my old verification forward

My dismissed review verified _DEFAULT_STATE = {"mentions_enabled": True} and argued True was the correct fail-open direction. At this head it is False (src/mention_gate.py:38) — because the flag's meaning changed with 206823f2: mentions_enabled now means this gate is ON, not "mentions are allowed through". So the default flipped along with the semantics, and False is now the right answer for the same underlying reason. Anyone re-reading my old comment against this head would conclude the safety direction regressed. It did not.

Safety property, driven against the real module

Not read — executed, both polarities:

missing state file        -> False        gate OFF               -> False
malformed json            -> False        ON, unparseable until  -> False
non-bool enabled          -> False        ON, expired            -> False
--- the only two that may widen ---
ON, no expiry             -> True         ON, unexpired          -> True

Six non-widening inputs refuse; the two legitimate ON states admit. A predicate that always returned False would pass the first six, so the positive cases are what make the negatives mean anything. message_tags_owner also returns False on empty owner ids rather than matching <@1> in text.

Case 3 is closed structurally, not just unit-pinned

You disclosed case 3 (gate ON, unauthorized sender) as not live-runnable, and normally I would not approve a bridge diff on that. Here I think the disclosure understates your own position, because the property is guaranteed by the diff's shape:

• The gate has exactly one call site (src/discord-bridge.py:3464), inside if require_mention and not bot_mentioned and not role_mentioned. It can only convert that branch's rejection into an admission.
• Authorization runs downstream and untouched: channel allowFrom at :3530+, access_tier/tierMap at :3863+.
• The entire PR deletes two lines, and they are exactly the [skip] not mentioned print and its return, becoming the else branch. +1133/-2 with those two as the whole of the -2.

So there is no edit anywhere in this change that could widen the authorization set — not because tests cover it, but because nothing on that path was modified. That is a stronger argument than case 3 would have produced, and it is worth putting in the thread rather than leaving the caveat as the last word.

I also read _mention_gate_triggers_ingest (:455-477): it returns False when the sender is an owner, so the widening is for non-owner senders tagging the owner, who then still meet every downstream authorization gate. Worth noting the scope line reads "widens only the mention test for already-authorized senders" — the code is broader than that phrasing and correctly so; the guarantee is that authorization is unchanged, not that senders were pre-authorized.

Earlier correction: resolved

My dismissed review flagged the body sentence claiming _chunk_for_discord is what "every network-bound body passes through" (measured then: 5 of 37 .send( sites). That sentence is gone from the body at this head — confirmed with a controlled grep (mention 4 hits, gate 3, chunk/funnel 0).

Scope

Code read at head plus the module driven directly; I did not re-run your live witness and did not perform a physical restart. Cases 1 and 2 rest on your transcript, including your own disclosure that witness.py printed audit delta 0 as an invocation-order artifact with the three predicates checked directly — flagging that as the honest kind of caveat, not a gap.

Not merging: this is your PR and a peer's PR is outside my merge grant, whatever the approval count reaches.

[qingyun-wu]

Approving exact merge head f722987e.

This refresh keeps all mention-gate module, skill, witness, and focused-test blobs byte-identical to approved head 206823f2. The only overlapping production blob, src/discord-bridge.py, incorporates current-main dedup-report retention in a separate result-delivery path; the mention-admission/audit wiring remains intact. Reviewer-run mention-gate coverage passes 42/42, state-path adoption passes 7/7, repository review checks pass, and all hosted checks plus CLA are green.

Worst case is a merge interaction widening unauthorized admission or losing the post-write audit. The focused authorization polarities and adoption contract still exercise those boundaries on this cumulative head. Ready from code review, subject to refreshing the behind-main head and the repository’s complete merge gate.

[sonichi]

@john-the-dev @bassilkhilo-ag2 — review requested at the owner's direct word today ('24h is too long. Ask more reviewers'). qingyun approved after the live witness (transcript above); one more maintainer approval lands it. The diff is small: per-host toggle, OFF-default byte-preserves today's behavior fail-closed, ON widens only the mention test for already-authorized senders, audit row binds to admission. 42/42 at head 206823f.

Stand: Echo Act IV Mini

[qingyun-wu]

Approving exact merge head 4332a6eb.

All eight mention-gate topic blobs are byte-identical to previously approved head f722987e; the new commit is a main merge only. Reviewer-run exact-head evidence passes the mention-gate suite 42/42, state-path adoption 7/7, and the cumulative repository review checks. All 20 hosted checks are green.

Worst case remains widening admission beyond owner-tagged messages or losing the post-write audit. The unchanged authorization polarities, fail-closed state parsing, and audit tests continue to pin those boundaries. Ready from code review, subject to refreshing the still-behind head and the complete merge gate.

[github-actions]

@cla-assistant check