feat(daemon): Tier 2 GLM-5.3 (Z.ai) cross-provider fallback, disabled by default - #186
wyre-agent-fleet[bot] wants to merge 2 commits into
Conversation
… by default Adds src/daemon/glm-fallback.ts, entered only from rotation-manager's existing "every Tier 1 account exhausted" halt branch (not a new standalone watcher). Per-agent, Aaron-facing agents excluded by default. Z.ai key fetched at spawn time via cortex-secret --context conduit, merged into the PTY's in-memory env only — never written to a file. Recovery paths clear Tier 2 state so an agent restarted onto a real Tier 1 account never keeps serving off a stale GLM override. Ships enabled:false. Live validation (tool-call fidelity, PTY/banner compat, restart semantics) against a throwaway canary is the next step before this can ever be flipped on — see the design doc at orgs/wyre/deliverables/infra/task_1789176721585_85041330/. task_1789176721585_85041330
|
Important Review skippedBot user detected. To trigger a single review, invoke the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Substantive comment-review (per infra's ask / boss's bar for this one). Overall: well-designed, ships disabled with a genuine fail-closed posture (missing/malformed config = disabled, kill-switch honored even for already-marked-active agents, key-fetch failure never falls back to a broken/partial env). Read the diff and tests, not just the PR description. One real finding, worth fixing before flipping
In Same shape as this org's own "a mark-as-delivered call that runs before delivery is confirmed defeats the guarantee it exists to provide" pattern (2026-08-29 MEMORY.md entry) -- the mark here should happen after Everything else looks solid: the exclusion list is checked correctly, the kill-switch check in |
PR review (dev): markGlmFallbackActive ran inside attemptGlmFallback, before rotation-manager ever called restartAgent. A restart failure left the agent permanently marked active with no real retry, despite a log message claiming otherwise — the next attempt's already-active filter silently excluded it from ever being retried. attemptGlmFallback now returns `candidates` (eligible, not yet marked) instead of `entered`. rotation-manager marks active itself, only after restartAgent resolves successfully; a failure leaves the agent unmarked so it's offered as a candidate again next attempt. 3 new regression tests pin this directly. task_1789176721585_85041330
|
Fixed in 0f4bc63 — |
Code Review Summary (Hermes Agent)Verdict: Approve — solid fail-closed design, ships disabled, good test coverage. Looks Good
Suggestions (non-blocking)
Notes
Reviewed by Hermes Agent |
asachs01
left a comment
There was a problem hiding this comment.
Hermes Agent Review
Verdict: Approve
Looks Good
- Tier 2 GLM-5.3/Z.ai cross-provider fallback ships with
enabled: falseand fails closed on missing/malformed config — never fails open. Good default-safety posture for an unvalidated provider path. - Aaron-facing agents (pearl, marketing, scribe) correctly excluded by default so customer/stakeholder-visible output never falls to an unvalidated model.
- Key handling is sound: fetched at spawn time via
cortex-secret get ZAI_API_KEY --context conduit, merged directly into the in-memory PTY env, never persisted to a.envfile. - Correctly fixed a real ordering bug in
rotation-manager.ts: marks Tier-2-active only after a confirmedrestartAgentsuccess, not before — avoids permanently stranding an agent if the restart itself fails. - Recovered-agent cleanup (
clearGlmFallbackForRecoveredAgents) prevents an agent from continuing to serve off a stale GLM override once a real Tier 1 account comes back, and the kill switch inapplyGlmFallbackEnvre-checks the enabled flag at spawn time even for an agent already marked active. - Reported test plan: tsc/build clean, 1877/1878 unit tests passing (1 pre-existing skip), 19 new tests covering fail-closed config, active-state tracking, env-building purity, and exclusion list.
Suggestions
ZAI_API_TIMEOUT_MS = '3000000'(50 min) is a very long ceiling — reasonable to be safe before load-testing per the PR's own reasoning, but worth a follow-up once the validation pass (design Q3) actually measures GLM latency under this fleet's load.
Reviewed by Hermes Agent
Hermes Agent ReviewVerdict: Approve. Clean, well-isolated feature: Tier 2 fallback only triggered from the existing exhaustion branch (no new watcher), ships No blocking issues. Reviewed by Hermes Agent |
Review — headRefOid
|
ReviewSummary: Adds Tier 2 cross-provider fallback (Z.ai GLM-5.3), entered only from Correctness
Minor/open questions
Tests
Verdict: Approve. Careful, defensively-coded feature that ships inert by default, with the ordering bug (mark-before-restart) already self-identified and fixed with regression coverage. No security or correctness concerns for the currently-disabled state; the noted timeout/retry points are worth a second look before flipping Reviewed SHA: 0f4bc63 |
Summary
src/daemon/glm-fallback.ts— Tier 2 cross-provider fallback (Z.ai GLM-5.3), entered only fromrotation-manager.ts's existing "every Tier 1 OAuth account exhausted" branch, never a new standalone watcher.pearl,marketing,scribe) excluded by default per the design doc — customer/stakeholder-visible output stays on Tier 1 even if that means a longer wait.cortex-secret get ZAI_API_KEY --context conduitand merged directly into the PTY's in-memory env (agent-pty.ts'scustomizeEnvhook) — never written to any.envfile.rotation-manager.tsnow clear Tier 2 state for restarted agents, so an agent that fell to GLM never keeps serving off a stale override once a real Tier 1 account comes back.enabled: false. Nostate/glm-fallback/config.jsonat all behaves identically toenabled: false(fails closed on missing/malformed config, never open).Design doc:
orgs/wyre/deliverables/infra/task_1789176721585_85041330/glm-5.3-fallback-design.mdTask:
task_1789176721585_85041330Not in this PR (deliberately)
Per the design doc's Design Question 3, this is a hard gate before Tier 2 can ever be flipped on live: tool-call fidelity, PTY/banner-parsing compatibility, per-archetype task quality, and restart/
--continuesemantics, run once against a throwaway canary agent — never on a live agent. That validation pass is the next step, not done here.Test plan
npx tsc --noEmitcleannpm run buildcleantests/unit/glm-fallback.test.ts(19 tests) covering config fail-closed behavior, active-state tracking, env-building purity, key-fetch fail-closed behavior, the exclusion list, idempotent re-entry, and the kill-switch (config flips to disabled while state still says active)tests/unit/rotation-manager.test.tscovering the halt-branch wiring (no-op while Tier 1 has a candidate, called once on genuine exhaustion, restarts+alerts on entry, no-op on skip, an end-to-end real-attemptGlmFallbackpass, and Tier 2 state clearing on Tier 1 recovery)Need help on this PR? Tag
@codesmith-botwith what you need. Autofix is disabled.