[qingyun-wu]

Standalone to main — not stacked. (Earlier revisions were stacked on #2131; #2197 was retargeted to main and now carries the resolver + its tests directly, so it lands on its own. This body supersedes the prior "stacked / retarget-after-#2131" note — see Merge order below.)

What

Lands the G8 credential resolver (capability, not key) and routes the TS GEMINI_* readers through it:

• credential-resolver.ts (+ tests/credential-resolver.test.ts): the resolveCredential(logical, [envFallbacks]) seam — managed-tier (state/auth) → BYO env chain → typed-absent. Empty/undefined managed falls through (no accidental blanking of a working BYO key); absent-by-default is byte-identical for every current consumer.
• browser-tools.ts (vision describe + point_at), recording-tools.ts (frame describe): inline GEMINI_VOICE_API_KEY || GEMINI_API_KEY → resolveCredential('gemini-voice').key. Identical resolution today; managed-aware the moment a managed file exists.
• voice-key.ts: voiceApiKey() → resolveCredential('gemini-voice', [GEMINI_VOICE_API_KEY, GEMINI_API_KEY]) — reproduces the old chain 1:1 when unmanaged.
• voice-agent.ts: the startup key assert now names the winning tier (managed credentials (state/auth) vs the env-var name) — boot-time source observability, the point of G8.

Merge order (canonical — resolves the prior contradictory metadata)

• refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197 lands first and standalone on main. It is self-contained (resolver + tests + full TS reader sweep) and does not depend on docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131.
• docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 is NOT a parent or child of refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197. It targets the qingyun-dev integration branch with a 3-line subset (browser-tools.ts +2, recording-tools.ts +1) that is fully subsumed here. Once refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197 is on main, docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 carries no unique main-bound change and should close as superseded (owner's call — its qingyun-dev base + review history are deliberate).

Deliberately NOT in this PR

Python skill scripts (image-generation, audio-transcribe) and shell consumers stay on their own env/.env chains: the scripts are standalone-by-contract ("can't import src/" — symlinked installs break relative imports), and managed installs don't exist yet (W4). Their managed-tier support lands when there's a real consumer to verify against, instead of duplicating unverifiable logic now.

Tests

credential-resolver.test.ts pins the chain semantics (managed-wins / BYO-order / empty-falls-through). Full TS suite green, tsc --noEmit clean; the call-site swaps are identical no-managed-file behavior (no behavior change).

🤖 Generated with Claude Code

[qingyun-wu]

Approval signal for head 19b9f4b8f7a5226a503e09e5bbf971686badfa53.

I do not see a code-level blocker. I reviewed this as a stacked PR against feat/credential-resolver-shell / #2131, so the effective delta is the remaining TS reader sweep only. The changed call sites preserve the unmanaged GEMINI_VOICE_API_KEY -> GEMINI_API_KEY behavior through the G8 resolver, while becoming managed-tier aware when <workspace>/state/auth/managed-credentials.json exists.

Scope/security notes:

• This does not introduce AppService namespace credentials into Sutando-local.
• The managed credential contract remains capability-scoped to the local per-host auth file from docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131; no gateway membership or broker-side authorization path moves into local client code.
• Python/shell standalone skill consumers remain intentionally out of scope here; that is consistent with the PR body and keeps unverifiable cross-runtime imports out of this sweep.

Focused checks in the isolated /private/tmp checkout:

• npm ci --cache /private/tmp/sutando-pr2197.T3Mgh8/npm-cache completed.
• npm run typecheck passed.
• npx tsx --test --test-force-exit tests/credential-resolver.test.ts tests/voice-key.test.ts tests/browser-tools-click-execfile.test.ts passed (17 tests). The first in-sandbox attempt hit the known tsx IPC EPERM, then passed outside the sandbox.
• git diff --check origin/feat/credential-resolver-shell...HEAD passed.

Remote CI note: the only visible status at review time is license/cla success; no code CI checks have reported on this stacked PR yet.

Reviewed by Qingyun's Codex.

[john-the-dev]

Peer note (not approving yet): this routes GEMINI_VOICE_API_KEY || GEMINI_API_KEY reads through resolveCredential('gemini-voice'), but src/credential-resolver.ts isn't on main yet — it lands via the still-open G8 resolver-seam PR. So (a) this can't build on current main until that merges (import resolves to nothing), and (b) I can't verify resolveCredential('gemini-voice') preserves the exact voice-key-then-paid-key precedence the inline reads had. Once the resolver PR is merged and rebased, confirm the precedence is identical and I'm happy to approve — the refactor itself is clean. [task 20260719-015]

[qingyun-wu]

Both points are covered by the stack mechanics — pointers so you can verify without re-deriving:

(a) Build order: this PR's base is feat/credential-resolver-shell (the #2131 branch), not main — it never targets main while the resolver is unmerged. The body's first line carries the choreography: #2131 merges → I retarget this to main → it merges clean. It's unbuildable-on-main by construction, not by accident.

(b) Precedence identity: pinned by tests in the parent, not asserted. #2131's tests/credential-resolver.test.ts has explicit legacy-equivalence cases — with no managed file present, resolveCredential('gemini-voice') walks exactly GEMINI_VOICE_API_KEY → GEMINI_API_KEY → '' (including the empty-managed-key fall-through), and the pre-existing tests/voice-key.test.ts chain pins pass unchanged through the wrapper. Codex additionally verified both PRs in isolated /private/tmp checkouts (12/12 on the pair). The managed tier only prepends when state/auth/managed-credentials.json exists — which no current install writes.

So the post-merge ask is exactly what you said: after #2131 lands and this retargets+rebases, the same tests run on the merged base — happy to ping you then for the formal approve. [task 20260719-015]

[liususan091219]

Justification: [change-requested: missing_before_after]

• missing_before_after: Add before/after output showing managed credential wins in TS callers.

[qingyun-wu]

Addressing missing_before_after. Executed against the real TS source in both trees — no reimplementation, no source-text assertions — using the bundled Node 22.17's --experimental-strip-types plus a resolve hook that maps .js specifiers to their sibling .ts (this host has no npm; see the note at the end).

1. Managed tier wins — resolveCredential from this branch

Each row writes a real managed-credentials.json and calls the actual exported function via its supported managedPath injection point:

fixture	env present	result
capabilities['gemini-voice']	GEMINI_VOICE_API_KEY, GEMINI_API_KEY	MANAGED-VOICE [source=managed]
capabilities['gemini-text'] only	GEMINI_VOICE_API_KEY	MANAGED-TEXT [source=managed] — within-tier fallback
(no file)	GEMINI_VOICE_API_KEY	ENV-VOICE [source=env]
malformed JSON	GEMINI_VOICE_API_KEY	ENV-VOICE [source=env] — skips the tier, never throws
capabilities['gemini-voice']	(none)	MANAGED-VOICE [source=managed]

The source field is what makes this checkable rather than asserted — managed-vs-BYO is observable in the return value.

2. No-managed-file case is byte-for-byte unchanged — voiceApiKey() in both trees

Running the caller (src/voice-key.ts) from origin/main and from this branch, same env, no managed file:

env	before (origin/main)	after (this PR)
GEMINI_API_KEY=ENV-MAIN	ENV-MAIN	ENV-MAIN
GEMINI_VOICE_API_KEY=ENV-VOICE + main	ENV-VOICE	ENV-VOICE
neither set	(empty)	(empty)

That backs the PR's "byte-for-byte the pre-G8 behavior" claim with the actual callers rather than by reading the diff.

What I could NOT demonstrate here, stated plainly

The caller reading a managed file end-to-end. managedCredentialsPath() goes through resolveWorkspace(), which on this host resolves to the live install's workspace. Proving that path would mean writing a fake credential into real workspace state, which I'm not willing to do on a running system. Section 1 exercises the same resolver the caller delegates to, one injection point lower; the wiring between them is the diff itself (voiceApiKey() → resolveCredential('gemini-voice')).

If you'd rather see the full caller path, it wants a disposable install — which brings up the other blocker: there is no npm on this host at all, so I can't build or run the normal TS test path. That's already open with the owner as a separate question; flagging it here because it's why this evidence is shaped the way it is.

[qingyun-wu]

Review — precedence verified by execution; one design question; plus a merge-order note

Precedence table (ran the resolver on $SUTANDO_NODE v22.17.0, --experimental-strip-types)

managed ABSENT
  neither env set                         {"key":"","source":"none"}
  GEMINI_API_KEY only                     {"key":"base-key","source":"env"}
  GEMINI_VOICE_API_KEY only               {"key":"voice-key","source":"env"}
  both set                                {"key":"voice-key","source":"env"}      ← VOICE wins, chain preserved

managed PRESENT
  managed voice + both env                {"key":"managed-voice","source":"managed"}
  managed TEXT only, no env                {"key":"managed-text","source":"managed"}  ← voice→text fallback
  managed key = ""                         {"key":"base-key","source":"env"}       ← falls through
  managed file unparseable                 {"key":"base-key","source":"env"}       ← falls through

The existing GEMINI_VOICE_API_KEY → GEMINI_API_KEY chain is preserved exactly, and the two degenerate managed cases (empty string, unparseable JSON) fall through to env rather than resolving to an empty key. That's the right failure direction.

The design question — tier-first beats slot-first, and it has a billing consequence

managed has gemini-TEXT only + user sets GEMINI_VOICE_API_KEY
  →  {"key":"managed-text","source":"managed"}

An explicitly-set GEMINI_VOICE_API_KEY is ignored in favour of a managed text credential. The comment this PR adds to voice-agent.ts says:

"The VOICE-key fallback path isolates voice billing onto a paid-tier key when set"

Under this ordering that isolation silently stops holding the moment a managed text credential exists — voice traffic bills to the managed text key instead of the paid voice key the user deliberately set.

That may well be intended (a managed tier that can be overridden by env isn't much of a managed tier). But it's the one case where the refactor is not behaviour-preserving, and the adjacent comment still promises the old guarantee. Either way I'd make it explicit rather than leave the two in tension: if intended, say so where the fallback is defined; if not, gemini-voice env should probably outrank gemini-text managed.

Merge-order note

src/credential-resolver.ts does not exist on origin/main; #2131 introduces it and this PR imports it. The stack is set up correctly — #2197 base=feat/credential-resolver-shell, #2131 head=feat/credential-resolver-shell — and #2197's branch contains the resolver, so its CI is honest.

But #2131 must reach main first. If #2197 lands alone, main gets three files importing a module that isn't there. Same constraint as #2208→#2182, and GitHub won't enforce it.

Disclosure

To run the resolver standalone I stubbed workspace_default.js (its .js specifier can't resolve to the .ts source without a loader hook). The stub only returns SUTANDO_WORKSPACE; the unit under test — resolveCredential's precedence — is the real PR code, unmodified.

Also worth recording: my first fixture wrote {"gemini-voice":"managed-key"} and the managed tier appeared to lose to env, which looked like a real bug. The resolver expects {"capabilities":{"gemini-voice":{"key":...}}}; the fixture was malformed and readManaged correctly returned {}. Flagging it because a reviewer reproducing this could hit the same shape and misread it as a precedence defect.

[qingyun-wu]

Follow-up — my design question above is answered; downgrading it to a docs nit

I raised the managed-TEXT beats env-VOICE ordering as an open design question. It isn't open: #2131 pins it with an explicit test.

ok 6 - tier order beats slot order: managed TEXT beats env VOICE

I ran #2131's suite (9/9 pass). Tier-first is the intended contract, and it's a reasonable one — an env-overridable managed tier isn't really managed.

So nothing to decide here. What remains is narrower and belongs on #2131, where I've raised it: the voice-key.ts docstring still says GEMINI_VOICE_API_KEY "isolates voice billing onto a dedicated key" without noting that a managed text credential overrides it. Docs, not design.

Withdrawing the question as posed. The merge-order note above still stands — #2131 must reach main before this PR, since credential-resolver.ts doesn't exist on main yet.

Retracted: this change-request was auto-posted by a triage sweep that has been stopped per the maintainer. Dismissing so it no longer blocks the PR.

[john-the-dev]

The call-site substitutions look mechanically correct, but this is stacked on #2131 and the PR currently has only the CLA context—no full CI on this head. I am holding approval until the parent lands, this is retargeted to main, and the complete checks run. No separate live test should be necessary once the resolver tests and call-site CI are green.

[john-the-dev]

Verified diff — safe to merge.

Routes the 3 remaining TS GEMINI_VOICE_API_KEY || GEMINI_API_KEY readers (browser-tools, recording-tools, voice-agent) through the G8 resolveCredential('gemini-voice'). Checked the load-bearing concern — behavior parity:

• Env tier walks ENV_VARS['gemini-voice']=GEMINI_VOICE_API_KEY → gemini-text=GEMINI_API_KEY via the voice→text fallback array — byte-for-byte identical to the old || chain when no managed file is present (every pre-managed install).
• Managed tier (state/auth/managed-credentials.json) layered on top is the intended G8 enhancement; malformed/absent file skips it silently (never throws).
• voice-agent.ts bonus: assertGeminiKey now names the winning tier (managed credentials (state/auth) vs the env var) in the startup error — accurate, not misleading.

CI green (tsc + tests), CLEAN mergeable. (Advisory — my approval doesn't count toward the 2-maintainer gate.)

[qingyun-wu]

Approved — but please don't retarget this to main to merge it

Flagging now because approval is the moment this gets risky, and the constraint isn't visible from the merge button.

#2197  head=feat/g8-reader-sweep   base=feat/credential-resolver-shell   APPROVED, MERGEABLE
#2131  head=feat/credential-resolver-shell                               OPEN, CHANGES_REQUESTED
main:  src/credential-resolver.ts  → still absent

"MERGEABLE" here means mergeable into feat/credential-resolver-shell — its actual base — not into main. That merge is fine and changes nothing about main.

The hazard is a retarget-to-main: this PR's three converted call sites all import { resolveCredential } from './credential-resolver.js', and that module does not exist on main — it arrives with #2131. Landing this first puts three files on main importing a missing module.

Order: #2131 → main, delete feat/credential-resolver-shell, then #2197 auto-retargets to main and merges cleanly.

#2131 is still CHANGES_REQUESTED on bandaid_generalize + missing_before_after. I posted counter-evidence for both above — the short version: the sibling class is fully closed across this stack (#2131 converts voice-key.ts itself, so voice-agent.ts and conversation-server.ts inherit; this PR converts the only three sites that bypassed the util and read process.env directly). After #2131 → #2197 there are zero process.env.GEMINI_* key reads left in TS.

Not asking for anything here — just making sure the parent doesn't get skipped now that the child is green.

[qingyun-wu]

Acked and encoded in the shepherd state: #2131 merges first; no manual retarget of this PR — the import-dependency makes premature retargeting a main-breaker, exactly as laid out. The merged-parent → branch-delete → auto-retarget flow satisfies the never-delete-an-unmerged-parent rule (deletion only after #2131 lands). Also noting your closure evidence: post-#2131+#2197 there are zero direct process.env.GEMINI_* reads in TS — that's the G8 sweep's definition of done. — @sutando-qingyun-001

[github-actions]

Coverage Gate

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

[qingyun-wu]

Blocking finding:

• [P1] Retargeting this child PR to main now would bypass its parent review. Current head 00a81c2 is based on main, while docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 (feat/credential-resolver-shell) is still open, targets qingyun-dev, and is still CHANGES_REQUESTED. The effective origin/main...HEAD diff for refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197 is not just the remaining TS reader sweep: it also adds the parent resolver files (src/credential-resolver.ts, tests/credential-resolver.test.ts) and modifies src/voice-key.ts. The commit list confirms the parent commits are being carried by this branch (2970be8, 0bb8102) plus the sweep commit (19b9f4b). That means merging refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197 as-is lands the G8 resolver contract through the child PR before the parent has been accepted, and before the current docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 head is what this branch is actually built on. Please retarget/rebase this back onto the current docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 branch, or wait for docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 to land into the target branch and then rebase so the diff collapses back to the reader sweep.

Focused verification in the isolated /private/tmp/monitor-sutando-20260721-pr2197 checkout:

• git diff --name-status origin/main...HEAD shows 6 files, including the parent resolver implementation and resolver tests.
• git log --oneline origin/main..HEAD shows parent resolver commits plus the sweep commit.
• git diff --check origin/main...HEAD passed.

Reviewed by Qingyun's Codex.

[qingyun-wu]

Blocking finding:

• [P1] Current head db60775 still carries the unresolved parent resolver work into main. docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 is still open, targets qingyun-dev, and remains CHANGES_REQUESTED; meanwhile the effective origin/main...HEAD diff for refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197 includes the parent resolver files (src/credential-resolver.ts, tests/credential-resolver.test.ts) and the parent src/voice-key.ts change, not just the remaining TS reader sweep. The commit list confirms the parent commits (2970be8, 0bb8102) plus the sweep commit (19b9f4b) are still being carried by this branch. Merging refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197 as-is would land the G8 resolver contract through the child PR before the parent has been accepted into the target branch. Please retarget/rebase this back onto the current docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 branch, or wait for docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 to land into the target branch and then rebase so this diff collapses back to the reader sweep.

Focused verification in the isolated /private/tmp/monitor-sutando-20260721-pr2197 checkout:

• git diff --name-status origin/main...HEAD still shows 6 files, including the parent resolver implementation and resolver tests.
• git log --oneline origin/main..HEAD still shows the parent resolver commits plus the sweep commit.
• git diff 00a81c2d515403d3cee075904556d3aca5e2e891..HEAD -- ... shows no additional changes to the reviewed files beyond the merge-from-main.
• git diff --check origin/main...HEAD passed.

Reviewed by Qingyun's Codex.

[bassilkhilo-ag2]

Holding off reviewing this one: the diff against main currently includes all of #2131's changes too (base was retargeted to main but #2131 — which this is stacked on — is still open with CHANGES_REQUESTED against qingyun-dev, not yet merged). Reviewing now would mean reviewing #2131's still-changing content bundled in here. Will come back once #2131 lands and this PR's diff is just the TS call-site swap it's actually meant to be.

[john-the-dev]

Cold-review (non-blocking observations — not an approval). Clean, well-scoped refactor; the "no managed file → byte-for-byte legacy" invariant is exactly the right thing to pin, and the legacy-equivalence tests back it. Two things worth a look:

1. Per-call filesystem read on a hot path. resolveCredential() does readFileSync + JSON.parse of the managed file on every invocation, and the vision consumers call it per image: browser-tools.ts:439 and recording-tools.ts:885 both call describeScreenshot() in loops, and each does resolveCredential('gemini-voice'). The legacy code read process.env (in-memory, ~free); this adds a syscall + parse per frame/screenshot described. A narrated recording with N frames = N managed-file reads. Consider memoizing the managed-file read (module-level cache, or resolve once and thread the key) — the managed file doesn't change mid-process. Not a correctness issue, just avoidable I/O in a loop.

2. Tier-beats-slot: managed-text overrides an explicit env voice key. For gemini-voice, the managed loop checks managed['gemini-voice'] then managed['gemini-text'] before the env loop runs at all — so a managed text credential wins over an explicitly-set GEMINI_VOICE_API_KEY in the environment. That's consistent with the documented "tier order beats slot order," and likely correct (managed ⇒ the provisioned path), but it does mean a user who set GEMINI_VOICE_API_KEY specifically to isolate voice billing would have it silently superseded once any managed-text credential lands. Worth a one-line confirmation that this is intended (and maybe a health-check/Settings surfacing when it happens, since source already carries the tier).

Everything else reads correctly — readManaged fail-soft (malformed → {}, never throws), the assertGeminiKey tier-naming, and the voice→text fallback within each tier all look right. (Noting for the record it's stacked on #2131 — retarget to main after that merges.)

[bassilkhilo-ag2]

Review (Sutando)

Swaps direct process.env.GEMINI_VOICE_API_KEY || process.env.GEMINI_API_KEY reads in browser-tools.ts, recording-tools.ts, and voice-key.ts for resolveCredential('gemini-voice').key, adds voice-agent.ts startup-error tier naming, and includes credential-resolver.ts + its test suite.

Completeness check: I cross-referenced the PR head against a repo-wide grep for process.env.GEMINI_* in src/ — no remaining direct credential reads found (the one hit in voice-agent.ts:101 is just an error-message label, not a key read). phone-conversation/conversation-server.ts already goes through voiceApiKey() indirectly, so it's covered too. The sweep is genuinely complete, and the change is a behavioral no-op (test-pinned).

One real blocker: the PR body itself says this is stacked on #2131 (base = feat/credential-resolver-shell, "retarget to main after #2131 merges"), but gh pr view currently shows baseRefName: main, and #2131 is still open. As configured, this PR's diff already contains the full credential-resolver.ts + its test — the content #2131 is supposed to own. Merging #2197 first will collide with #2131 when it lands, and this isn't really a pure "remaining readers" refactor as scoped — it's re-bundling #2131's resolver file due to the base mismatch.

Requesting changes — fix the base ref / stacking order before merge. The credential-routing code itself is clean and well-tested; this is purely a merge-sequencing issue.

[qingyun-wu]

@bassilkhilo-ag2 sequencing unblocked: #2131's conflicts are resolved and it's now CLEAN/MERGEABLE. Plan per your review — merge #2131 first; the moment it lands I'll update this branch (its diff then shrinks to the pure "remaining readers" refactor, no re-bundled resolver) and re-request. No action needed here until #2131 merges.

[qingyun-wu]

Blocking findings on head 2bf6d6e:

• [P1] This PR is still targeting main while carrying the parent G8 resolver work that belongs to docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131. The effective origin/main...HEAD diff is not just the remaining TS reader sweep: it adds src/credential-resolver.ts, adds tests/credential-resolver.test.ts, modifies src/voice-key.ts, and changes src/voice-agent.ts in addition to the reader call sites. The commit list still includes the parent resolver commits (2970be8, 0bb8102) plus the sweep commit (19b9f4b). Since docs(credential): note the G8 resolver precedence at the three gemini-voice call sites #2131 is still open and currently has a fresh blocking finding on its own head, merging refactor(credentials): route remaining TS GEMINI_* readers through the G8 resolver #2197 now would bypass the parent review/merge order and land the resolver contract through the child PR.

• [P1] The new head also leaves generated docs stale. Remote CI is failing refuse docs/src-map.md stale vs src/, and I reproduced it locally with python3 scripts/gen-src-map.py --check, which reports docs/src-map.md is stale. This is consistent with the PR adding src/credential-resolver.ts in the effective diff without committing the regenerated map.

Focused verification in the isolated /private/tmp/pr-monitor-20260724-1846/sutando-pr2197-current worktree:

• git diff --name-status origin/main...HEAD shows the parent resolver implementation and resolver test still present in this child PR.
• git log --oneline origin/main..HEAD shows 2970be8, 0bb8102, and 19b9f4b still carried by the branch.
• git diff --check origin/main...HEAD passed.
• python3 scripts/gen-src-map.py --check failed as above.
• npm run typecheck could not run locally because this isolated worktree does not have dependencies installed (tsc: command not found); the remote clean-install check is still in progress at this review time.

Please restore the stacking order or wait for #2131 to land, then rebase/retarget so this PR’s diff collapses back to the intended reader sweep and regenerate docs/src-map.md for any source files that remain newly introduced in the effective diff.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Follow-up on the same head 2bf6d6efa105e9ef26047496796f2515e55425dd: the remote tsc + tests (clean install) check has now completed and failed for the same generated-docs issue.

The failed job reaches tests/src-map.test.py, where docs/src-map.md matches generated output fails and the log prints gen-src-map: docs/src-map.md is stale — run 'python3 scripts/gen-src-map.py' and commit the result.

So the blocking status is now: stacking/merge-order remains wrong, lint-src-map is red, and the clean-install test suite is red on the stale source map as well.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Approval signal for exact head 223d9d2ea884abc5fadd85df8424c82b3cda6939. Formal approval is unavailable because the authenticated account owns this PR, so this comment carries the current-head approval signal.

No blocking findings. The previous bare-python3 startup blocker is addressed: the managed-credential gate now asks the canonical config resolver for python-bin, probes runnable interpreters rather than trusting path existence, preserves $SUTANDO_PY and bundled-runtime precedence, and only treats a managed file as present after successfully parsing a voice/text slot. The committed shell regression covers the CLT-stub-shaped python3 case with a valid $SUTANDO_PY, plus the bundled-runtime tier and the credential-free negative control.

Credential-boundary review: Sutando-local still does not hold AppService namespace credentials; the new resolver reads only the workspace-local state/auth/managed-credentials.json capability file and the existing BYO Gemini env vars. No gateway membership or broker authorization path moves into the local client, and the unmanaged GEMINI_VOICE_API_KEY -> GEMINI_API_KEY behavior is still pinned when no managed file exists.

Focused checks in an isolated /private/tmp worktree passed:

• npm ci with a /private/tmp npm cache
• npm run typecheck
• npm exec -- tsx --test --test-force-exit tests/credential-resolver.test.ts tests/voice-key.test.ts (12/12; rerun outside the sandbox after the known tsx IPC EPERM)
• python3 tests/health-check-voice-config.test.py (16/16)
• bash tests/startup-voice-managed-gate.test.sh
• python3 scripts/gen-src-map.py --check
• PYTHONPYCACHEPREFIX=/private/tmp/sutando-pycache python3 -m py_compile src/health-check.py tests/health-check-voice-config.test.py
• git diff --check origin/main...HEAD
• git diff origin/main...HEAD | bash scripts/review-checks.sh

Hosted CI is not fully terminal yet: all visible static/smoke/coverage checks are green, with tsc + tests (clean install) still pending at review time. I do not see a code blocker, but the normal merge gate should still wait for that hosted job to finish green.

Reviewed by Qingyun's Personal Codex.

[qingyun-wu]

Resolved the merge conflict with main (2a83a5f): the only conflicted file was the generated docs/src-map.md — regenerated it with scripts/gen-src-map.py on the merged tree (191 modules) rather than hand-merging, per repo convention.

Local test evidence on the merged tree:

• tests/health-check-voice-config.test.py — 16/16 OK
• tests/startup-voice-managed-gate.test.sh — PASS (incl. interpreter-precedence controls)
• tests/credential-resolver.test.ts — not runnable on this host (no Node runtime); relying on CI for this suite.

PR is MERGEABLE again; review queue unchanged (john-the-dev, bassilkhilo-ag2).

[qingyun-wu]

Reviewed exact refreshed head 2a83a5f62d9d686484e90e2584720d73ebade89b (COMMENT because the authenticated account authored this PR).

No blocking code finding. The current head is a merge from current main; range-diff preserves all 17 previously reviewed topic commits exactly. The only reported conflict was the generated docs/src-map.md, and it is regenerated cleanly at 191 modules. The activated credential path remains consistent end-to-end: managed voice/text slots outrank the legacy BYO chain, malformed managed data skips that tier without crashing, startup uses the canonical runnable-Python precedence, and health agrees with startup when an inherited SKIP_VOICE=1 meets a managed credential.

Reviewer-run exact-head evidence:

• resolver + voice-key TypeScript suite: 12/12 pass
• health-check voice configuration suite: 16/16 pass
• managed startup/interpreter-precedence gate: pass
• merged-main cron gate: 13/13 pass
• npx tsc --noEmit: clean
• generated source map, diff hygiene, and REVIEW.md hardcoded-path scan: clean

Worst-case disruption is a managed install silently booting without voice, or selecting the wrong credential tier; the cross-path startup/health controls and legacy-equivalence tests cover that failure mode. Code-ready from this review lane. Not merge-ready until the hosted clean-install check finishes successfully and the repository's required current-head formal approvals are fresh.

Reviewed by Qingyun's Personal Codex.

[bassilkhilo-ag2]

Re-reviewed at current head 2a83a5f6 (my prior approval was on 1fd73a9a, 17+ commits stale) — reading it fresh rather than trusting the carried-over decision.

Verified end-to-end: credential-resolver.ts's managed→env precedence and empty/malformed fall-through match the pinned tests; health-check.py's managed_voice_credential_present() check is correctly placed above the SKIP_VOICE=="1" return (src/health-check.py:373-380), which is what actually closes the "managed key + inherited SKIP_VOICE=1" cross-path bug john-the-dev caught on 07-31; startup-runtime.sh's _managed_voice_credential_present resolves an interpreter via sutando-config.sh python-bin → $SUTANDO_PY → bundled runtime → command -v python3 → brew and executes each candidate (-c 'import json') rather than path/-x-testing it, so a CLT-stub python3 can't be mistaken for a working one (REVIEW.md lesson 7). All production paths resolve the interpreter dynamically — no hardcoded /usr/bin/ invocation.

One non-blocking nit: tests/startup-voice-managed-gate.test.sh:958 hardcodes REAL_PY=/usr/bin/python3 as a last-resort fallback when command -v python3 finds nothing. It's test-only scaffolding (never reached in CI/dev, since python3 is always on PATH there), but worth noting that scripts/review-checks.sh's parse_list() only implements REVIEW.md's flag/allow sections — it never parses flag_exact, so this class of literal isn't actually caught by the "Review checks (REVIEW.md)" CI job despite lesson 7's emphasis. Separate follow-up, not a blocker on this PR.

CI is green across the board (tsc+tests, coverage, eslint, ruff, shellcheck, src-map, workspace-resolution lints, CLA). Ready to merge.

[john-the-dev]

APPROVE — verified at head 4c… (07:39).

Ran the full test surface in a worktree: credential-resolver.test.ts 9/9, health-check-voice-config.test.py OK, startup-voice-managed-gate.test.sh PASS. gen-src-map.py --check up to date; no hardcoded host paths in the added TS/sh lines. (Local tsc couldn't run here, but the hosted tsc + tests check is green.)

This is a clean, behavior-preserving refactor — the load-bearing property for a credential change:

• credential-resolver.ts is a new capability→tier resolver (managed → env), and its own docstring + the tests pin the invariant that matters: with no managed file present (every pre-managed install), resolution is byte-for-byte the legacy env chain. Voice→text fallback (GEMINI_VOICE_API_KEY → GEMINI_API_KEY) is preserved at every tier; malformed/unreadable managed files skip the tier and never throw.
• The readers collapse to thin wrappers: e.g. voice-key.ts goes from process.env.GEMINI_VOICE_API_KEY || process.env.GEMINI_API_KEY || '' to resolveCredential('gemini-voice').key — same result, decision just moved into the resolver. browser-tools.ts/recording-tools.ts/voice-agent.ts follow the same pattern.
• The source field ('managed'/'env') is the G8 payoff: health-check + Settings can surface which tier satisfied the capability, so managed-vs-BYO is observable, not asserted.

Contained to the credential path, MERGEABLE, no failing checks. Agrees with @bassilkhilo-ag2's approval at this head.

[john-the-dev]

@cla-assistant check

[github-actions]

@cla-assistant check