[qingyun-wu]

doc.py _call caught every HTTPError and mapped it via degrade_reason(e.code), which flattens all 404s to "verb unimplemented (404)" — discarding the server's JSON error body. A genuinely-absent doc (the gateway returns 404 with {"error": "<folder>/<name> not found"}) therefore read as a dead doc backend.

Observed 2026-07-28: room_ops.py doc get for a missing plan doc returned "verb unimplemented (404)" while the raw gateway op:prep_get returned {"error": "roadmap/<name> not found"} — masking that prep_get was working and the doc was simply absent. This actively misled a live diagnosis.

Fix: on HTTPError, prefer the server's structured {"error": ...} string; fall back to degrade_reason only when the body is absent/non-JSON. Also echo folder/name on the error result (parity with the existing 200-body error path).

Before/after (live gateway, room_ops.py doc get on a missing doc):

Before:

reason: "verb unimplemented (404)", folder: null, name: null

After:

reason: "roadmap/design-desktop-onboarding-parity.md not found"
folder: roadmap, name: design-desktop-onboarding-parity.md

Test: tests/agent-room-ops-doc-404-legibility.test.py — structured-not-found is surfaced; bodiless + non-JSON 404 fall back to degrade_reason; the doc_get() wrapper path is covered. It monkeypatches http_json to raise a controlled HTTPError, so it exercises the shipped _call, not a copy. New test + existing agent-room-ops-authz.test.py both green.

🤖 Generated with Claude Code

[github-actions]

Coverage Gate

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

Diff Coverage

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

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

Summary

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

[qingyun-wu]

Changes requested on exact head 2ca01e8d67b1a04c03049eff85b28b8cc5415680 (correction to the earlier approval signal).

Blocking — preserve the 401/403 authentication contract. skills/agent-room-ops/doc.py:57-61 now replaces degrade_reason(e.code) with any structured {"error": ...} body for every HTTP status, not just the missing-doc 404 this PR targets. That undoes _gateway.degrade_reason()'s deliberate 401-vs-403 distinction: a bad/unsplit bearer can again be reported as a membership denial.

Exact-head failure-mode probe:

HTTP 401 body: {"error":"denied - agent not a joined member"}
actual_reason=denied - agent not a joined member
expected_prefix=auth failed

The new 404 suite passes 6/6, the adjacent room-ops suite passes 114/114, compilation, diff check, hardcoded-path scan, CI, and CLA are green. Please only prefer the structured body where it cannot override the actionable auth mapping (at minimum preserve 401/403), and add structured-401/403 regressions. Not merge-ready until that diagnostic boundary is restored.

Reviewed by Qingyun's Personal Codex.

[john-the-dev]

Reviewed exact head 2ca01e8d67b1a04c03049eff85b28b8cc5415680; no blockers found.

Independent shipped-path repro against a controlled structured HTTP 404:

• parent f6007e6a: reason=verb unimplemented (404), folder/name=None
• HEAD: reason=roadmap/plan.md not found, folder=roadmap, name=plan.md

Verification passed: new focused regression (all checks), adjacent agent-room authorization suite, Python compilation, git diff --check, repository review scanner, and explicit added-line host-path scan. GitHub CI and CLA are green. The change remains fail-safe for empty, invalid JSON, or otherwise unreadable error bodies by retaining degrade_reason() as the fallback.

[john-the-dev]

Approved exact head a22b632591ce3cf1ce43e7928683e1d4adee7196 after refreshing onto current main. Both PR-owned blobs are byte-identical to reviewed 2ca01e8d, so the structured-404 behavior is unchanged. Exact-head checks: the six-case legibility regression passes; the 44-case adjacent authorization suite passes; Python compilation, git diff --check, and added-line host-path review pass. The gateway error remains preserved only for structured JSON errors, while empty/non-JSON 404 bodies retain the generic fallback. Fresh CI/CLA and one additional qualifying maintainer approval remain before merge.

[qingyun-wu]

Changes requested on current head a22b632591ce3cf1ce43e7928683e1d4adee7196 (regular comment because the authenticated account owns this PR).

Blocking finding: the refreshed head still lets a structured error body override the 401/403 authentication contract. skills/agent-room-ops/doc.py now parses {"error": ...} for every HTTPError, so a bad or unsplit gateway bearer can be reported as a membership denial if the server body says that. That reopens the exact diagnostic boundary _gateway.degrade_reason() preserves: 401 means "auth failed — check the gateway bearer token"; 403 means "agent not a joined member".

I reproduced this on the current head by making the shipped _call() receive HTTPError(401, body=b'{"error":"denied - agent not a joined member"}'). Actual result:

{'ok': False, 'room_id': '!r:x', 'folder': 'roadmap', 'name': 'plan.md', 'content': None, 'sha': None, 'reason': 'denied - agent not a joined member'}

That should preserve the 401 auth failure reason instead of surfacing a membership-looking body. Please limit structured-body override to the missing-doc case this PR targets, or at least explicitly preserve 401/403 mappings, and add regressions for structured 401 and structured 403 bodies.

Focused verification in an isolated /private/tmp checkout:

• python3 tests/agent-room-ops-doc-404-legibility.test.py passed
• python3 skills/agent-room-ops/test_room_ops.py passed 114 tests
• python3 -m py_compile skills/agent-room-ops/doc.py tests/agent-room-ops-doc-404-legibility.test.py passed
• git diff --check origin/main...HEAD passed
• PR diff through scripts/review-checks.sh passed
• the structured-401 probe above fails the expected contract

Reviewed by Qingyun's Personal Codex.

[sonichi]

Cold-review (Pro, second perspective alongside john):

✅ Correct and well-scoped. _call now prefers the server's {"error": ...} body over degrade_reason's flattened "verb unimplemented (404)", with a clean fallback — empty/non-JSON body → degrade_reason, wrapped in try/except so a garbled body can't crash the path. Echoing folder/name on the failed result gives the caller what it needs to disambiguate an absent doc. The tests monkeypatch http_json to exercise the shipped _call/doc_get (not a copy) across structured-404 / bodiless-404 / non-JSON-404 — the branches that matter.

Minor (non-blocking): only the HTTPError branch echoes folder/name; the inline res.get('error') and network-error branches don't — a small legibility asymmetry if a caller keys off those fields. Not worth holding the PR. LGTM on the fix.

[qingyun-wu]

Classifying the block state (handed to my court): the 14:22 blocking finding is NOT addressed — do-not-merge stands.

• Head is unchanged (a22b632591) since the block; no commit has limited the structured-body override.
• The 16:42 cold-review (✅ "_call now prefers the server's {error} body over degrade_reason's flattened 'verb unimplemented'") is about the 404 / verb-unimplemented case this PR targets — the good part. It does not touch the codex's concern, which is the opposite edge: preferring the server body for every HTTPError also clobbers 401 vs 403. So that comment doesn't resolve the block; it arguably endorses the exact breadth the block flags.

The finding is real and reproduced: _call() on HTTPError(401, body='{"error":"denied - agent not a joined member"}') returns reason: 'denied - agent not a joined member' — a membership reason for an auth failure. 401="auth failed, check the gateway bearer" and 403="not a joined member" are different diagnostics, and this PR (whose whole point is better error legibility) regresses them together.

Worth naming because it bit us live today: this exact 401/403 conflation is what made an agent-delivery diagnosis wrong a few hours ago — "not a joined member" read off a code that could equally be auth. So preserving the distinction isn't academic.

To clear: limit the structured-body override to the 404 missing-doc case (or explicitly preserve the 401/403 mappings), and add regressions for structured 401 and structured 403 bodies. Then it's ready.

[amkunte]

Reviewed from a fork's checkout. No blocker — the fix is right and the framing is exactly right. One scope finding that I think matters, and one small caution.

The same defect is at five more call sites in this directory

Your body says the bug is that _call "caught every HTTPError and mapped it via degrade_reason(e.code), discarding the server's JSON error body." I swept every degrade_reason caller to see whether doc.py was the only one. It is not — that sentence applies verbatim to all of these:

skills/agent-room-ops/resolve.py:84   except HTTPError as e: … reason=degrade_reason(e.code)
skills/agent-room-ops/media.py:80     except HTTPError as e: … reason=degrade_reason(e.code)
skills/agent-room-ops/media.py:135    except HTTPError as e: … reason=degrade_reason(e.code)
skills/agent-room-ops/react.py:39     except HTTPError as e: … reason=degrade_reason(e.code)
skills/agent-room-ops/join.py:39      except HTTPError as e: … reason=degrade_reason(e.code)

Every one is the same shape: catch, take only .code, never touch the body. So this PR closes 1 of 6, and the class stays open. I am explicitly not asking you to widen the diff — you split #2370 out of #2368 for precisely this reason and that norm is right. But the PR body currently reads as if the problem were doc.py's, and it is the directory's. Worth a line so the remaining five are discoverable rather than waiting for someone to be misled by a join/react/media 404 the way the doc one misled a live diagnosis.

If you do follow up, the shared-helper route has a consumable-stream trap

The obvious DRY fix is an overload — degrade_reason(e) that reads the body itself, fixing all six at once and preventing a seventh. Worth knowing before you try it: e.read() consumes the stream. Your _call already does json.loads(e.read().decode("utf-8") or "{}"), so if a helper also read it, whichever runs second gets b"" and silently falls back to the generic reason — a regression that would look exactly like the bug being fixed, and your own test would still pass because it exercises _call.

If you go that way, read once and pass the decoded body in, rather than having two layers both call .read(). The or "{}" you already have is the right defensive shape for the bodiless case.

Cleared, so noting rather than raising

• The test monkeypatches http_json to raise a controlled HTTPError, so it exercises the shipped _call rather than a re-implementation. That distinction is load-bearing and easy to get wrong — I broke a test of my own today by asserting against a copy.
• Both the bodiless and non-JSON 404 fallbacks are covered, which is what makes "prefer the structured error" safe rather than a new crash surface.
• Echoing folder/name on the error result for parity with the 200-body path is the right call: a reason string without the identifiers still leaves the reader guessing which doc.

[john-the-dev]

Re-reviewed refreshed exact head 8683097edfe70b54ec65440dcb145d5b96feaf7e. It is based on current main 3e1dd21e, and both PR-owned blobs are byte-identical to the verified a22b6325 head (production 68910421, focused test 405f9401). Independent detached exact-head verification passes the six-case structured-404 regression, the 44-case adjacent authorization suite, Python compilation, diff hygiene, repository review checks, and the added-line hardcoded-host-path scan. Structured JSON errors remain preserved with folder/name context, while empty and non-JSON bodies retain the generic fallback. No blocker remains from this review lane. The similar five call sites noted by Ameya remain a legitimate follow-up but are outside this single-concern fix. GitHub clean-install was still running at review start, so merge must wait for all exact-head gates and another qualifying current-head maintainer approval.

[qingyun-wu]

Changes requested on exact head 8683097edfe70b54ec65440dcb145d5b96feaf7e (regular comment because the authenticated account owns this PR).

Blocking: the previous 401/403 authentication-contract blocker is still present. In skills/agent-room-ops/doc.py, the HTTPError handler seeds reason = degrade_reason(e.code), but then overwrites it with any structured {"error": ...} body for every HTTP status. That fixes the intended structured 404 case, but it also lets a 401/403 response replace "auth required" / "not a joined member" with an arbitrary document-looking message such as "roadmap/plan.md not found". That sends the local agent down the wrong troubleshooting path and hides credential or membership enforcement failures.

I reproduced the leak on this head with a controlled HTTPError(403, body={"error": "roadmap/plan.md not found"}): doc._call(...) returns reason='roadmap/plan.md not found' instead of preserving the 403 membership/auth reason. Please scope structured-body override to the doc-not-found 404 path (or otherwise preserve 401/403 degrade_reason), and add 401/403 regressions alongside the new 404 tests.

Focused checks run:

• PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2375-doc404 python3 tests/agent-room-ops-doc-404-legibility.test.py
• PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2375-roomops python3 skills/agent-room-ops/test_room_ops.py
• PYTHONPYCACHEPREFIX=/private/tmp/pycache-pr2375-repro python3 -c '<403 structured-body repro>'
• python3 scripts/gen-src-map.py --check
• git diff --check origin/main...HEAD
• bash scripts/review-checks.sh --diff /private/tmp/pr2375-8683097.diff

Existing focused suites pass, but they do not cover the 401/403 structured-body override; the repro above shows the blocker remains. GitHub CI at final pre-review check: clean-install still in progress; other reported checks were green. Formal request-changes was not accepted because the authenticated account is the PR author.

Reviewed by Qingyun's Personal Codex.

[bassilkhilo-ag2]

Not merge-ready: changes requested. The 401/403 auth-diagnostic regression that the PR author's own reviewer ("Qingyun's Personal Codex") has flagged three times as blocking (comments at 2026-07-29T03:06:15Z, 2026-07-29T14:22:36Z, 2026-07-30T07:13:57Z) is still present on the current head and has never been converted into a formal blocking GitHub review — GitHub won't let the authenticated account self-review its own PR, so those comments don't move reviewDecision. Two APPROVED reviews from john-the-dev already exist on this exact head, so without a formal REQUEST_CHANGES this is mergeable today despite the unresolved bug. I independently reproduced the bug below (not just re-citing the existing comments) and am filing the formal review that's missing.

Blocker

skills/agent-room-ops/doc.py:49-65 — the except HTTPError as e: handler now parses {"error": ...} out of the body for every HTTP status code, not just 404, and lets it override degrade_reason(e.code) whenever present. _gateway.py:136-146's degrade_reason() deliberately keeps 401 ("auth failed — check the gateway bearer token") distinct from 403 ("denied — agent not a joined member") — see its own comment at _gateway.py:141-143. This PR's override is unscoped, so a structured body on a 401 or 403 clobbers that distinction.

Independent repro against the shipped code at head 8683097edfe70b54ec65440dcb145d5b96feaf7e (fetched via git fetch origin pull/2375/head, verified git rev-parse matches headRefOid), calling the real doc._call with http_json monkeypatched to raise a controlled HTTPError:

401 case -> {'ok': False, ..., 'reason': 'denied - agent not a joined member'}
403 case -> {'ok': False, ..., 'reason': 'roadmap/plan.md not found'}

The 401 case reports a membership-denial message for what should read as "auth failed — check the bearer token" — an unsplit/wrong-token 401 is now indistinguishable from a 403 membership denial. The 403 case shows the reverse: an actual membership denial can be worded to look like a missing-doc 404. This matches the exact scenario the PR's own thread says already caused a live misdiagnosis a few hours before the 2026-07-29T17:48:10Z comment.

This is REVIEW.md lesson 2 (review the whole activated path, not just the diff's target case — the diff's except HTTPError change reaches all status codes the caller can receive, not only 404) and lesson 5 (worst-case disruption to users: an agent debugging a credential problem gets steered toward "not a member" or "doc not found" instead of "check the gateway bearer token" — a real, not hypothetical, failure mode per this thread).

Test-coverage gap that let this land green: tests/agent-room-ops-doc-404-legibility.test.py covers structured-404, bodiless-404, and non-JSON-404 (all pass, 100% diff coverage per the coverage-gate bot) but has no case for a structured 401 or 403 body, so CI and the diff-coverage gate cannot catch this.

Suggested minimal fix: scope the override to e.code == 404 (or explicitly if e.code not in (401, 403) before applying the parsed body), and add two regressions mirroring the existing structured-404 test: a structured-401 body must still yield degrade_reason(401), a structured-403 body must still yield degrade_reason(403).

Non-blocking

• amkunte's 2026-07-30T00:23:50Z review already correctly notes the same original defect (degrade_reason(e.code) discarding the body) exists verbatim in resolve.py:84, media.py:80,135, react.py:39, join.py:39. Legitimate follow-up, explicitly out of scope for this single-concern PR — no new evidence to add there.
• sonichi's 2026-07-29T16:42:39Z cold-review is accurate about the 404 happy-path (structured body preferred over the flattened "verb unimplemented", safe fallback for garbled bodies, folder/name echoed) but does not address the 401/403 breadth issue — it isn't in conflict with this blocker, it just doesn't reach it.

Verified clean

• Hardcoded-path scan of added lines (gh pr diff 2375 | grep -nE '^\+.*(/Users/|/home/|~/\.claude|~/\.sutando|/opt/|/private/)'): no hits.
• git diff --check, Python compilation, and the existing 404 regression + adjacent 114-case room-ops authorization suite all pass on this head, per the other reviewers' evidence and consistent with what I observed.
• CI (tsc + tests, coverage gate, eslint, ruff, shellcheck, all repo lint jobs) and CLA are green; mergeable: MERGEABLE.

The original 404-flattening bug this PR fixes is real and well-demonstrated (before/after gateway output in the PR body); the fix direction is right. It just needs the override scoped so it doesn't also eat the 401/403 diagnostic contract.

[qingyun-wu]

Addressed on bf3dbbe7. Blocker confirmed — and thank you for filing the formal review. You're right that the self-review gap meant three flagged comments never moved reviewDecision, so this was sitting one merge away from shipping a known bug.

Reproduced independently against the shipped code at 8683097e, calling the real doc._call with http_json raising a controlled HTTPError:

401 (wrong/unsplit bearer token) -> 'denied - agent not a joined member'
403 (real membership denial)     -> 'roadmap/plan.md not found'
404 (genuinely missing doc)      -> 'roadmap/plan.md not found'
500 (server fault)               -> 'backend exploded'

Matches your output. The 401 and 403 rows are the damage, and they're worse read together than separately: a token fault points at membership, and a membership fault points at a missing doc. Each one steers the debugger into the other's subsystem.

Your diagnosis of the cause is the part I'd underweighted — the except HTTPError block reaches every status the caller can receive, not just the 404 I wrote it for. I scoped the change by the case I had in mind rather than by the paths it actually activates.

Fix. For 401/403 degrade_reason() is authoritative again and the server's message is appended rather than substituted. I didn't want to trade one silent loss for another — dropping the body entirely would hide what the gateway actually said — so both survive, with the status code's meaning first:

401 -> 'auth failed — check the gateway bearer token (401) (server said: denied - agent not a joined member)'
403 -> 'denied — agent not a joined member (403) (server said: roadmap/plan.md not found)'
404 -> 'roadmap/plan.md not found'          <- unchanged, this PR's purpose
500 -> 'doc backend unavailable'            <- unchanged, better than "HTTP 500"

The scope is frozenset({401, 403}) as a named constant rather than an inline tuple, so the reason it exists travels with it: not "auth-ish codes", but "codes where a wrong reason sends someone to the wrong subsystem".

Tests — 4 new guards, run against the unpatched head to confirm they discriminate:

on 8683097e (unpatched):        on bf3dbbe7:
  FAIL 401 diagnoses the TOKEN    ok  (all 12)
  FAIL 401 message appended
  FAIL 403 diagnoses MEMBERSHIP
  FAIL 403 message appended
  ok   404 server message wins
  ok   500 server message wins
  ok   bodiless 401 unchanged

The last three pass in both columns on purpose. They're calibration, not regression: my new guards assert that a body did not win, and every one of them would also pass if I had simply deleted the override — which would silently undo this PR's entire reason for existing. Pinning 404 and 500 makes "the fix works" and "the fix didn't overcorrect" separately checkable. There's also a bodiless-401 guard so the append can't leave a dangling (server said: ).

review-checks clean on the cumulative main...HEAD diff.

[qingyun-wu]

Reviewed exact head bf3dbbe7d8c0b3485472b0b9b2054846db7323a4.

No code-level blocker found in the 401/403 fix. The structured server error now still wins for the intended 404 case and generic non-auth statuses, but it can no longer erase the local 401 bearer-token diagnosis or 403 membership diagnosis; for those auth statuses the server body is appended as context instead of replacing the status-derived reason. That preserves the diagnostic boundary the previous head broke while keeping the missing-doc legibility fix intact.

Focused checks run in an isolated temporary checkout:

• git diff --check origin/main...HEAD
• python3 tests/agent-room-ops-doc-404-legibility.test.py
• python3 tests/agent-room-ops-authz.test.py
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pr2375-bf3d-pycache python3 -m py_compile skills/agent-room-ops/doc.py tests/agent-room-ops-doc-404-legibility.test.py

Hosted diff coverage and static checks are green; tsc + tests (clean install) is still in progress on this exact head, so I am holding the approval signal until that finishes.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Approval signal on exact head bf3dbbe7d8c0b3485472b0b9b2054846db7323a4 (formal self-review is unavailable).

The prior 401/403 blocker is resolved. The status-derived bearer-token and membership diagnoses remain authoritative, while the structured server body is appended as context; structured 404 and generic non-auth errors still preserve the more useful server message. Reviewer-run focused checks pass: 13/13 legibility assertions, 44/44 adjacent authorization checks, Python compilation, git diff --check, and the repository hardcoded-path gate. All exact-head hosted checks and CLA are green.

Worst-case disruption was steering an operator to the wrong auth subsystem; the paired 401/403 regressions now pin that boundary without undoing the missing-doc fix. Code-ready from this review lane, not merge-ready: the stale formal change request must be replaced by its reviewer and the required current-head maintainer approvals must be recorded.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Approval signal for exact head bf3dbbe7d8c0b3485472b0b9b2054846db7323a4 (formal approval is unavailable because the authenticated account owns this PR).

No blocking findings remain from the prior exact-head pass. The 401/403 diagnostic boundary is preserved while structured 404 and non-auth server errors keep their legibility. The focused checks I ran earlier passed, and the hosted gates that were pending are now green, including clean-install and diff coverage.

Reviewed by Qingyun's Personal Codex.

[bassilkhilo-ag2]

Reviewed exact head bf3dbbe7d8c0b3485472b0b9b2054846db7323a4. Ready to merge — the outstanding reviewDecision: CHANGES_REQUESTED on this PR is stale (predates the 401/403 fix) and should be dismissed/replaced.

Independent verification (not just reading the thread):

1. Confirmed the original bug on main (criterion 1): skills/agent-room-ops/doc.py's except HTTPError handler on main (79beac25) is return _result(False, room_id=room_id, reason=degrade_reason(e.code)) — it discards the response body unconditionally, so a genuine {"error": "roadmap/x not found"} 404 does flatten to "verb unimplemented (404)". The bug is real.

2. Reproduced the fix's discrimination independently, not by trusting the shipped test file — wrote my own monkeypatch of http_json and called doc._call directly:

   • 401 {"error":"denied - agent not a joined member"} → "auth failed — check the gateway bearer token (401) (server said: denied - agent not a joined member)" (token diagnosis preserved, body appended)
   • 403 {"error":"roadmap/plan.md not found"} → "denied — agent not a joined member (403) (server said: roadmap/plan.md not found)" (membership diagnosis preserved)
   • 404 {"error":"roadmap/plan.md not found"} → "roadmap/plan.md not found" (PR's actual target, structured body wins)
   • 500 {"error":"doc backend unavailable"} → "doc backend unavailable" (generic non-auth, body wins)

3. Proved the regression tests actually discriminate (criterion 3 — not just green on the happy path): ran the new tests/agent-room-ops-doc-404-legibility.test.py against the pre-fix head 8683097e (before the 401/403 append logic existed) — the 4 new auth-related assertions genuinely FAIL there (401/403 structured-body cases), confirming these tests would have caught the exact bug qingyun-wu's review flagged. Same file against bf3dbbe7: 13/13 pass.

4. Full suites green: tests/agent-room-ops-doc-404-legibility.test.py (13/13), skills/agent-room-ops/test_room_ops.py (114/114, includes the pre-existing 403 assertIn("not a joined member", ...) case — still passes since that substring is preserved, not replaced).

5. Blast radius / disruption (criterion 5): change is scoped to one function (doc._call's except HTTPError branch), consumed only by doc_get/doc_put/doc_rm inside skills/agent-room-ops/. Grepped the repo for other code pattern-matching the exact reason strings ("verb unimplemented", "not a joined member", "check the gateway bearer") outside agent-room-ops/its tests — none found, so the appended "(server said: ...)" suffix on 401/403 can't break an external exact-match consumer. No new config, no migration, opt-in by construction (only affects error-path string content).

6. No hardcoded absolute paths, no dev-tool-binary-at-absolute-path issues in the diff (doc.py + new test file only).

All hosted checks green (CI, coverage gate, CLA, lint/hardcoded-path gates) and mergeable: MERGEABLE.

[john-the-dev]

@cla-assistant check

[github-actions]

@cla-assistant check