From b5c3cc897c931ef99856c04502acdb15879ff6ae Mon Sep 17 00:00:00 2001 From: Yichen Jiang Date: Wed, 26 Aug 2026 16:37:49 +0800 Subject: [PATCH] fix(llm-pi-ai): store the JSON image of a grant payload pi-ai credentials carry optional members as explicit undefined (a github.com Copilot grant holds enterpriseUrl: undefined), and the store bridge committed the object verbatim, so the credential store's strict validator refused the write and sign-in failed after the provider had already authorized it. toRecord now drops explicitly-undefined members and renders undefined array entries null, exactly as JSON.stringify would; everything else passes through untouched so genuinely unstorable values still fail loud at the store. --- ...6-pi-ai-grant-payload-json-image.i18n.yaml | 6 +++ ...26-08-26-pi-ai-grant-payload-json-image.md | 29 +++++++++++++ ...08-26-pi-ai-grant-payload-json-image.zh.md | 29 +++++++++++++ packages/llm/llm-pi-ai/src/auth.ts | 26 +++++++++++- packages/llm/llm-pi-ai/tests/auth.spec.ts | 41 +++++++++++++++++++ 5 files changed, 130 insertions(+), 1 deletion(-) create mode 100644 .agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.i18n.yaml create mode 100644 .agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.md create mode 100644 .agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.zh.md diff --git a/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.i18n.yaml b/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.i18n.yaml new file mode 100644 index 0000000000..de1f46c4a5 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.i18n.yaml @@ -0,0 +1,6 @@ +# Bilingual-pair consistency record (docs/i18n/README.md): the git blob hash of each +# side as of the last confirmed-consistent state. Both languages carry equal authority; +# after editing either side, bring the other along and re-record with: +# pnpm run verify-translation-pairing --write .agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.md +2026-08-26-pi-ai-grant-payload-json-image.md: 9eb5e5ffd9ccbc18d409bd50e05061006bdca094 +2026-08-26-pi-ai-grant-payload-json-image.zh.md: 8329b4acf062a073dfe78ec10fe03852e7a64fd9 diff --git a/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.md b/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.md new file mode 100644 index 0000000000..9eb5e5ffd9 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.md @@ -0,0 +1,29 @@ +# Agent Note: pi-ai grant payloads store their JSON image + +Status: implemented + +English | [中文](2026-08-26-pi-ai-grant-payload-json-image.zh.md) + +## Problem + +A GitHub Copilot sign-in against github.com failed at its commit step: `credentials-local: record "llm-pi-ai/github-copilot" payload holds a value JSON cannot represent`. pi-ai's Copilot credential carries its optional members as explicit `undefined` (`enterpriseUrl: undefined` when no Enterprise domain was given — idiomatic JavaScript that `JSON.stringify` would simply drop), and `llm-pi-ai`'s store bridge committed the credential object verbatim as the grant payload. The credential store's validator rightly refuses `undefined` as unrepresentable, so every grant whose flow left an optional member unset failed to store, and the sign-in reported failure after the provider had already authorized it. + +## Decision + +`toRecord` in `packages/llm/llm-pi-ai/src/auth.ts` stores the JSON image of a grant credential: `jsonImage` drops explicitly-undefined members of plain objects and renders undefined array entries as `null`, exactly as `JSON.stringify` would. Everything else — non-finite numbers, foreign-prototype objects — passes through untouched, so a genuinely unstorable value still fails loud at the store's validator rather than being silently reshaped. Reading back is unchanged: an absent member and an explicitly-undefined one are indistinguishable to pi-ai's consumers, which access optional members by property read. + +## Testing + +`tests/auth.spec.ts` writes the Copilot-shaped grant (explicit `undefined` member, nested drop, array hole) through the real `LocalCredentialProvider` and asserts the stored payload is the JSON image; a second case proves the fail-loud path survives by committing a `Date`-valued member and asserting the store's refusal reaches the caller. + +## Alternatives considered + +**`JSON.parse(JSON.stringify(credential))`.** Rejected: it also renders `NaN`/`Infinity` as `null` and runs `toJSON` methods, silently reshaping exactly the values the strict validator exists to refuse loudly. + +**Relaxing the store validator to skip `undefined` members.** Rejected: the seam stores payloads it never reads or reshapes, and every producer relies on byte-faithful round-trips; normalization belongs to the producer that knows its library's idiom, not to the store every plugin shares. + +**Fixing pi-ai upstream to omit unset members.** Out of this repository's hands and version-fragile: any future flow reintroducing the idiom would break sign-in again. The bridge owning the translation makes the harness robust against the whole class. + +## Consequences + +Grants from every pi-ai flow store regardless of which optional members the flow left unset. The bridge now owns a one-way normalization: a payload read back lacks members that were explicitly `undefined` at write time, which is indistinguishable from their absence for property access, and remains the documented JSON semantics. diff --git a/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.zh.md b/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.zh.md new file mode 100644 index 0000000000..8329b4acf0 --- /dev/null +++ b/.agents/notes/implemented/bug-fix/2026-08-26-pi-ai-grant-payload-json-image.zh.md @@ -0,0 +1,29 @@ +# Agent Note: pi-ai grant payload 落盘其 JSON 像 + +Status: implemented + +[English](2026-08-26-pi-ai-grant-payload-json-image.md) | 中文 + +## Problem + +一次面向 github.com 的 GitHub Copilot 登录在提交环节失败：`credentials-local: record "llm-pi-ai/github-copilot" payload holds a value JSON cannot represent`。pi-ai 的 Copilot 凭据以显式 `undefined` 携带可选成员（未填 Enterprise 域名时为 `enterpriseUrl: undefined`——这是 `JSON.stringify` 会直接丢弃的 JavaScript 惯用写法），而 `llm-pi-ai` 的存储桥接把凭据对象原样作为 grant payload 提交。凭据存储的校验器正当地拒绝 `undefined` 为不可表示，于是所有流程留有未填可选成员的 grant 都无法落盘，提供方已经完成授权之后登录却报失败。 + +## Decision + +`packages/llm/llm-pi-ai/src/auth.ts` 的 `toRecord` 改为落盘 grant 凭据的 JSON 像：`jsonImage` 丢弃普通对象里显式为 undefined 的成员，把数组中的 undefined 条目渲染为 `null`，与 `JSON.stringify` 完全一致。其余一切——非有限数、异种原型对象——原样透传，因此真正不可存储的值仍会在存储校验器处大声失败，而不是被静默改写。读回不变：成员缺失与显式 undefined 对以属性读取访问可选成员的 pi-ai 消费方不可区分。 + +## Testing + +`tests/auth.spec.ts` 经真实 `LocalCredentialProvider` 写入 Copilot 形状的 grant（显式 `undefined` 成员、嵌套丢弃、数组空洞），断言落盘 payload 为 JSON 像；第二个用例提交带 `Date` 成员的 grant 并断言存储的拒绝到达调用方，证明 fail-loud 路径仍在。 + +## Alternatives considered + +**`JSON.parse(JSON.stringify(credential))`。**否决：它还会把 `NaN`/`Infinity` 渲染为 `null` 并执行 `toJSON` 方法，把严格校验器本要大声拒绝的值静默改写掉。 + +**放宽存储校验器、跳过 undefined 成员。**否决：seam 存储的 payload 它从不读取或改写，所有生产方都依赖逐字节往返；归一化属于了解自家库惯用法的生产方，而不是所有插件共享的存储。 + +**在 pi-ai 上游修掉未填成员。**不在本仓库掌控内且随版本脆弱：将来任何流程重新引入该惯用法都会再次弄坏登录。由桥接层拥有这次翻译，让 harness 对整类问题免疫。 + +## Consequences + +所有 pi-ai 流程的 grant 无论留空哪些可选成员都能落盘。桥接层现在拥有一次单向归一化：写入时显式为 undefined 的成员在读回时缺失，这对属性访问不可区分，且正是文档化的 JSON 语义。 diff --git a/packages/llm/llm-pi-ai/src/auth.ts b/packages/llm/llm-pi-ai/src/auth.ts index f729a77f85..3d70de9815 100644 --- a/packages/llm/llm-pi-ai/src/auth.ts +++ b/packages/llm/llm-pi-ai/src/auth.ts @@ -37,6 +37,30 @@ export function recordKeyFor(providerId: string): CredentialKey { return credentialKey(RECORD_SCOPE, providerId) } +/** + * The JSON image of one grant payload: plain objects lose their + * explicitly-undefined members and array entries JSON cannot hold become + * null, exactly as `JSON.stringify` would render them. pi-ai credentials + * idiomatically carry optional members as explicit `undefined` (a github.com + * Copilot grant holds `enterpriseUrl: undefined`), which the credential + * store's strict validator refuses as unrepresentable. Everything else — + * non-finite numbers and foreign prototypes included — passes through + * untouched, so a genuinely unstorable value still fails loud at the store. + * @param value - the value to render. + * @returns the value's JSON image. + */ +function jsonImage(value: unknown): unknown { + if (Array.isArray(value)) return value.map(entry => entry === undefined ? null : jsonImage(entry)) + if (typeof value === 'object' && value !== null && Object.getPrototypeOf(value) === Object.prototype) { + const image: Record = {} + for (const [key, member] of Object.entries(value)) { + if (member !== undefined) image[key] = jsonImage(member) + } + return image + } + return value +} + /** * Translate a stored record into the credential pi-ai expects. * @@ -72,7 +96,7 @@ function toRecord(credential: Credential): CredentialRecord { ...credential.env === undefined ? {} : { env: { ...credential.env } }, } } - return { kind: 'grant', payload: credential } + return { kind: 'grant', payload: jsonImage(credential) } } /** diff --git a/packages/llm/llm-pi-ai/tests/auth.spec.ts b/packages/llm/llm-pi-ai/tests/auth.spec.ts index 27a496a2c3..def5b4229a 100644 --- a/packages/llm/llm-pi-ai/tests/auth.spec.ts +++ b/packages/llm/llm-pi-ai/tests/auth.spec.ts @@ -64,6 +64,47 @@ describe('pi-ai credential store over harness records', () => { await expect(ctx.credentials.readRecord(CODEX)).resolves.toEqual({ kind: 'grant', payload: granted }) }) + it('stores the JSON image of a grant, dropping explicitly-undefined members', async () => { + const ctx = await stored() + const store = credentialStoreFrom(ctx) + // The github.com Copilot shape: pi-ai sets optional members to explicit + // undefined, which the strict record validator refuses verbatim. + const granted = { + type: 'oauth' as const, + access: 'at', + refresh: 'rt', + expires: 42, + enterpriseUrl: undefined, + nested: { keep: 'x', drop: undefined }, + list: ['a', undefined, 'b'], + } + + await store.modify('github-copilot', () => Promise.resolve(granted)) + + await expect(ctx.credentials.readRecord(recordKeyFor('github-copilot'))).resolves.toEqual({ + kind: 'grant', + payload: { + type: 'oauth', + access: 'at', + refresh: 'rt', + expires: 42, + nested: { keep: 'x' }, + list: ['a', null, 'b'], + }, + }) + }) + + it('passes a genuinely unstorable grant value through to the store\'s loud refusal', async () => { + const ctx = await stored() + const store = credentialStoreFrom(ctx) + // A foreign-prototype member is not the undefined idiom: the image leaves + // it untouched and the record validator still refuses the write. + const granted = { type: 'oauth' as const, access: 'at', refresh: 'rt', expires: 42, issued: new Date(0) } + + await expect(store.modify('github-copilot', () => Promise.resolve(granted))) + .rejects.toThrow(/JSON cannot represent/) + }) + it('shows the mutation the current credential and leaves it alone when declined', async () => { const store = credentialStoreFrom(await stored()) await store.modify('openai-codex', () =>