Browse Source

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.
Yichen Jiang 1 month ago
parent
commit
b5c3cc897c

+ 6 - 0
.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

+ 29 - 0
.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.

+ 29 - 0
.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 语义。

+ 25 - 1
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<string, unknown> = {}
+    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) }
 }
 
 /**

+ 41 - 0
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', () =>