fix(driver): normalise provider ids, never throw, verify family models - #8
robotlearning123 wants to merge 6 commits into
Conversation
The importer trims provider map keys ('zai ' -> rule id 'zai') but the
key gate looked the cli entry up untrimmed, so an entry keyed 'zai '
slipped past the unusable-key gate and seeded an empty-key file the
doctor called resolvable. All lookups now go through one normalised
id (cliProviderEntry), matching the importer's own normalisation.
planPersonalProviderConfig is documented to never throw but was called
outside any try in the provisioner; a throwing exists bubbled straight
out. The public plan is now a wrapper that degrades any internal
throw to a conservative refusal, so provisioning and doctor are safe
by construction.
Family rules (templateId) resolved any model id without checking it.
The verdict now verifies the id against the builtin template's
builtinModelIds when the template list is available (injected document
or ZCODE_BUILTIN_PROVIDER_CONFIG_FILE / _BUNDLED_); without the list
the seed is refused conservatively and doctor says the verdict could
not be verified — a create-if-missing seed must not guess.
commit-msg and models test now have seeding tests: both commands are
spawned against a runtime stub that dies on contact, so they must
fail, but only after the seeding line ran — the assertions fail if
either call is removed. models test's fixture carries the v2 config
mirror because the command exits before its client without one.
Also: drop the unused cliUnreadable flag, stop shadowing node:path
with the local target path, label the existing-file verdict tests
precisely (missing model vs missing provider, exact detail asserted),
and give test-update-command's doctor fixture a models map with a
healthy exit-0 assertion so it covers the healthy path again.
|
agent-review (grok) at review我先读 PR 元数据、完整 diff 和仓库检查约定，再按文件核对正确性、安全性和测试。这是独立审阅，不改文件。接下来读 diff、提交说明和仓库检查命令。Memory flush started.
VERDICT: REQUEST_CHANGES |
Resolve the builtin template model list the way the runtime spawn sees it, reusing zagent's own resolvers instead of an env-only lookup: an injected document, then ZCODE_BUILTIN_PROVIDER_CONFIG_FILE, then the bundled copy beside the discovered runtime (builtinConfigPath — exactly what kernelEnv injects for our spawns), then the kernel's managed active cache (kernelActiveBuiltinPath under the data root). When none is readable the plan falls back to seeding exactly what the kernel migration would write and reports the model check as UNVERIFIED (resolves null plus a reason; doctor prints the caveat instead of failing) — a seed the kernel itself performs is never refused, so family rules no longer refuse on hosts without a resolvable list. Model id comparison now follows the kernel's own lookup semantics: the registry indexes models in plain Maps and the explicit-selection path reads them verbatim (getModel: this.#n.get(p)?.get(m), ~576830; models.find(g => g.modelId === t.modelId), ~579813) — exact and case-sensitive, which is why commit-msg canonicalises casing before sending. Measured live on 3.14.4: zai-api/glm-5.3 against the real upper-case template list answers model-not-found while zai-api/GLM-5.3 answers ok. Family verdicts compare exactly against the real shape (['GLM-5.3','GLM-5.3-Flash']) and custom selections against their own config's casing; a case mismatch is flagged, and family seeds still happen (their model list lives in the template, so a seed cannot brick a later selection). Also emit the family rule's access.type:'api-key' like every kernel xz serialization — without it the strict parser rejects the file and even the kernel's own -p fails (caught live: builtin:zai hosts seeded an unusable file). The family seed is now byte-identical to the kernel's own migration output. Tests: real upper-case builtin fixture, the unverified fallback, both resolver paths (runtime-anchored bundled copy, managed active cache), custom-casing semantics, and the family byte-parity oracle; the never-throw block no longer leaks its temp home and resets its threw flag between calls.
|
agent-review (grok) at review我先按这个 PR 的 head 做只读审查：读元数据、完整 diff、提交说明，并核对仓库自己的检查命令。审查范围是这个 PR 自身的三处改动。我先读 diff、提交说明和相关源码，再跑仓库自己的检查。Memory flush started.
VERDICT: REQUEST_CHANGES |
…eless access blocks
|
agent-review (grok) at review我先读 PR 元数据、完整 diff 和提交说明，再按文件核对正确性、安全性和测试。提交说明里没有 AI 署名。接下来按文件读 diff 和实现，并跑仓库自带的检查。Memory flush started. Findings
No AI-attribution trailers in VERDICT: REQUEST_CHANGES |
…access on every rule
|
agent-review (grok) at review我先读 PR 元数据、完整 diff 和仓库自检说明，再按文件核对这次提交。PR 元数据和仓库结构已经对齐。接下来核对提交说明、diff 和仓库自检命令。Memory flush started. Findings
已核对
VERDICT: REQUEST_CHANGES |
|
Independent review, owner-directed Sonnet high-quality lane (workflow wf_a064b94f-ab9). Two independent Sonnet reviewers looked at each PR through different lenses (correctness/test-oracle, and safety/scope/ops) at high effort. An adversarial Sonnet verifier then tried to refute every merged finding by execution and did a completeness pass. This is a same-family stand-in while grok is hanging and devin reports model-not-found. VERDICT: FIX-FIRST. Reviewed at pinned 7125946 and re-checked by the loop session at the current tip 08b350c. major (regression): The kernel schema allows absent or null access:
Session probe of Suggested fix:
Minor findings:
Pre-existing:
Session execution at 08b350c (env -i, temp HOME, ZCODE_RUNTIME=no-runtime): |
|
agent-review (grok) at review我先读 PR 元数据、完整 diff 和仓库自检说明，只做审阅，不改文件。Memory flush started. Findings
内核（本机 本 PR 在已有文件分支里对每条 rule 执行： if (!c.access || typeof c.access !== 'object' || typeof c.access.type !== 'string' || !c.access.type)
return malformed('the personal provider config is malformed (access.type)');实测（
Findings
No AI-attribution in VERDICT: REQUEST_CHANGES |
1 participant
Summary
Follow-up hardening of the personal provider seeding added in #7:
"zai "->zai) while the key gate read the untrimmed id, so an empty key could still be seeded and reported resolvable.templateIdnow resolve only when the selected model id is in the builtin template's model list; the list is read from the same file the kernel rebuilds its registry from (managed active cache first, then the preset/bundled copy) and ids compare exactly like the kernel. If the list is unavailable the seed still happens (family rules never brick a later selection) anddoctorreports the check as unverified.access.type(which the kernel's strict parser rejects) is repaired in place on the next-prun;doctorflags any rule whose access block is missing or untyped.commit-msgormodels testseed call is removed.pathshadowingnode:path, a mislabeled verdict test split into precise missing-model / missing-provider cases, and thetest-update-commanddoctor fixture now has a models map and asserts the healthy exit code.Verification
node packages/cli/test-personal-provider.mjs: 79 checks pass.make check: PASS.cli/config.json: the first-p --modelrun still succeeds, and an empty-key config still seeds nothing.