Quellcode durchsuchen

fix(web,llm): address review — document the card contract, pin the host diagnosis, gate the probe

Yichen Jiang vor 1 Monat
Ursprung
Commit
1019f149c4

+ 1 - 1
docs/cordis-catalog/services.md

@@ -941,7 +941,7 @@ stream(options: GenerateOptions): AsyncIterable<StreamChunk>
 
 Types: [AdapterRegistrationHandle](../core-data-structures/core.md) · [DirectoryRegistrationHandle](../core-data-structures/core.md) · [GenerateOptions](../core-data-structures/core.md) · [LlmAdapter](../core-data-structures/llm-streaming.md) · [LlmCallConfig](../core-data-structures/core.md) · [LlmConfigurableProvider](../core-data-structures/core.md) · [LlmDiscoveredModel](../core-data-structures/core.md) · [LlmModelDiscoveryRequest](../core-data-structures/core.md) · [LlmModelInfo](../core-data-structures/core.md) · [LlmProviderInfo](../core-data-structures/core.md) · [LlmResolvedModelInfo](../core-data-structures/core.md) · [PreparedLlmCall](../core-data-structures/llm-streaming.md) · [ResolvedRetryPolicy](../core-data-structures/llm-streaming.md) · [StreamChunk](../core-data-structures/llm-streaming.md)
 
-Source: [`packages/llm/llm/src/index.ts:287`](../../packages/llm/llm/src/index.ts)
+Source: [`packages/llm/llm/src/index.ts:292`](../../packages/llm/llm/src/index.ts)
 
 ## `ctx.permission` — `PermissionService`
 

+ 40 - 0
examples/headless-agent/tests/headless.snapshot.ts

@@ -31,6 +31,10 @@ const retryScenarioDir = join(snapshotsDir, 'provider-retry')
 const retryConfigPath = fileURLToPath(new URL('../retry.cordis.snapshot.yml', import.meta.url))
 const credentialsScenarioDir = join(snapshotsDir, 'missing-credential')
 const credentialsConfigPath = fileURLToPath(new URL('../credentials.cordis.snapshot.yml', import.meta.url))
+// Same keyless composition as the missing-credential scenario: the endpoint is
+// never dialed either way, because a supplied-but-unusable key fails credential
+// resolution exactly where an absent one does.
+const invalidCredentialScenarioDir = join(snapshotsDir, 'invalid-credential')
 const ralphScenarioDir = join(snapshotsDir, 'ralph-loop')
 const ralphConfigPath = fileURLToPath(new URL('../ralph.cordis.snapshot.yml', import.meta.url))
 const startupFailureConfigPath = fileURLToPath(new URL('./fixtures/startup-activation-error/cordis.yml', import.meta.url))
@@ -254,6 +258,42 @@ describe('headless stream-json snapshots', () => {
     expect(normalized).toContain('as a last resort')
   }, LOADER_SMOKE_TEST_TIMEOUT_MS)
 
+  it('logs actionable invalid-credential guidance through the one-shot app', async () => {
+    const streamExpected = join(invalidCredentialScenarioDir, 'stream-json.expected.jsonl')
+    let runCwd = ''
+    const result = await runLoaderSmoke({
+      label: 'invalid-credential headless stream-json snapshot',
+      tempDirPrefix: 'headless-snapshot-invalid-credential-',
+      binScript,
+      configPath: credentialsConfigPath,
+      binArgs: ['--config', credentialsConfigPath, '--output-format', 'stream-json', 'say pong'],
+      tsconfigPath,
+      env: {
+        // A key that exists but no HTTP header can carry — the paste this
+        // change exists for. Before it, `fetch` refused to build the header
+        // and the turn ended on a retried ByteString TypeError.
+        DEEPSEEK_API_KEY: 'sk-\u{1F600}pasted-from-a-chat-window',
+        DEEPSEEK_BASE_URL: '',
+        NODE_OPTIONS: [process.env.NODE_OPTIONS, '--disable-warning=ExperimentalWarning'].filter(Boolean).join(' '),
+      },
+      prepare: (cwd) => { runCwd = cwd },
+    })
+
+    expect(result.stderr).toBe('')
+    const normalized = normalizeHeadlessStream(result.stdout, runCwd)
+    if (refreshing) await writeFile(streamExpected, normalized)
+    expect(normalized).toBe(await readFile(streamExpected, 'utf8'))
+    // The durable failure names the reference to correct and the writer that
+    // usually owns it, and stays true in a composition that mounts no Models
+    // page at all.
+    expect(normalized).toContain('the API key resolved from DEEPSEEK_API_KEY contains characters')
+    expect(normalized).toContain('the web Models page writes it')
+    // Neither the key nor the transport-level symptom it used to produce may
+    // reach the user: the code point of one character is still the key.
+    expect(normalized).not.toContain('pasted-from-a-chat-window')
+    expect(normalized).not.toContain('ByteString')
+  }, LOADER_SMOKE_TEST_TIMEOUT_MS)
+
   it('logs the model default and a dynamic next-step reasoning effort', async () => {
     const result = await runLoaderSmoke({
       label: 'reasoning effort headless stream-json snapshot',

+ 12 - 0
examples/headless-agent/tests/snapshots/invalid-credential/stream-json.expected.jsonl

@@ -0,0 +1,12 @@
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"agent/inbox/spliced","seq":0,"time":0,"data":{"target":"next-turn","start":0,"inserted":[{"content":[{"type":"text","text":"say pong"}],"source":{"kind":"user"},"role":"user","id":"{{sessionId}}"}]}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"turn/start","seq":1,"time":0,"data":{"turn":1}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"agent/inbox/spliced","seq":2,"time":0,"data":{"target":"next-turn","start":0,"removedCount":1,"inserted":[]}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"step/start","seq":3,"time":0,"data":{"turn":1,"step":1}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"user/message","seq":4,"time":0,"data":{"content":[{"type":"text","text":"say pong"}],"source":{"kind":"user"},"role":"user","id":"{{sessionId}}"},"surfaceOp":"append"}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"session/title","seq":5,"time":0,"data":{"title":"say pong","messageSeqs":[4],"source":{"kind":"fallback"}}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"request/header","seq":6,"time":0,"data":{"header":{"config":{"provider":"deepseek-official","model":"deepseek-v4-flash","maxTokens":256000,"reasoningEffort":"high"},"adapterDefaults":{"reasoningEffort":true,"maxTokens":true},"system":"{{system}}","tools":"{{tools}}"},"reason":"initial"}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"request/context","seq":7,"time":0,"data":{"provider":"deepseek-official","model":"deepseek-v4-flash","contextWindow":1000000}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"assistant/chunk","seq":8,"time":0,"data":{"turn":1,"step":1,"chunk":{"type":"finish","reason":{"kind":"error","failure":{"message":"llm-deepseek: the API key resolved from DEEPSEEK_API_KEY contains characters no HTTP header can carry; set DEEPSEEK_API_KEY to the raw key alone (the web Models page writes it)","code":"INVALID_CREDENTIAL"}}}}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"step/end","seq":9,"time":0,"data":{"turn":1,"step":1}}}
+{"type":"session_event","sessionId":"{{sessionId}}","event":{"type":"turn/end","seq":10,"time":0,"data":{"turn":1,"reason":{"kind":"error","error":{"message":"llm-deepseek: the API key resolved from DEEPSEEK_API_KEY contains characters no HTTP header can carry; set DEEPSEEK_API_KEY to the raw key alone (the web Models page writes it)","code":"INVALID_CREDENTIAL"}}}}}
+{"type":"result","sessionId":"{{sessionId}}","output":""}

+ 2 - 2
packages/client/ui-models/README.i18n.yaml

@@ -2,5 +2,5 @@
 # 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 packages/client/ui-models/README.md
-README.md: b55914197e472edec8a8b6d4d3e02036d1697728
-README.zh.md: ca93c3d5a2a85fffb22707f8389f1e979468e2ec
+README.md: e3328bb5fd2cf812b05dc26bf534226818132631
+README.zh.md: 20e40cc571a9123b50dfb28565c5562937e03189

Datei-Diff unterdrückt, da er zu groß ist
+ 0 - 1
packages/client/ui-models/README.md


Datei-Diff unterdrückt, da er zu groß ist
+ 0 - 1
packages/client/ui-models/README.zh.md


+ 7 - 1
packages/client/ui-models/src/client/CustomProviderCard.tsx

@@ -215,7 +215,12 @@ export function CustomProviderCard(props: CustomProviderCardProps): ReactNode {
           disabled={disabled}
           onChange={(event) => { setKeyDraft(event.target.value) }}
         />
-        {keyFailure === undefined ? null : <p className={styles['error']}>{t(keyFailure)}</p>}
+        {/* A create card has no stored key to keep, so the blank case says
+            what a blank field means here instead: this route may authenticate
+            through the provider's own ambient discovery or OAuth. */}
+        {keyFailure === undefined
+          ? null
+          : <p className={styles['error']}>{t(keyFailure === 'keyBlank' ? 'keyBlankNew' : keyFailure)}</p>}
       </div>
       <ModelListEditor
         models={models}
@@ -226,6 +231,7 @@ export function CustomProviderCard(props: CustomProviderCardProps): ReactNode {
           api: protocol,
           ...keyValue.length === 0 ? {} : { apiKey: keyValue },
         }}
+        probeBlocked={keyFailure === 'keyBlank' ? 'keyBlankNew' : keyFailure}
         api={api}
         t={t}
         disabled={disabled}

+ 11 - 2
packages/client/ui-models/src/client/ModelListEditor.tsx

@@ -74,6 +74,13 @@ export interface ModelListEditorProps {
   onReset?: () => void
   /** Endpoint facts for the fetch action. */
   probe: ProbeTarget
+  /**
+   * Copy key naming why the fetch action is unavailable, or `undefined` when
+   * it is. The card owns this because the key it would send is judged there:
+   * asking with a key the form has already refused spends a round trip to be
+   * told what the field already says.
+   */
+  probeBlocked?: keyof typeof en | undefined
   /** Wire face the fetch action calls. */
   api: Pick<IApiClient, 'llm'>
   /** Section copy. */
@@ -314,8 +321,10 @@ export function ModelListEditor(props: ModelListEditorProps): ReactNode {
         <button
           type="button"
           className={styles['linkButton']}
-          disabled={disabled || busy || !askable}
-          title={askable ? undefined : t('fetchNeedsBaseUrl')}
+          disabled={disabled || busy || !askable || props.probeBlocked !== undefined}
+          title={props.probeBlocked !== undefined
+            ? t(props.probeBlocked)
+            : askable ? undefined : t('fetchNeedsBaseUrl')}
           onClick={() => { void fetchModels() }}
         >
           {busy ? t('fetching') : t('fetchModels')}

+ 1 - 1
packages/client/ui-models/src/client/ProviderEditor.tsx

@@ -371,7 +371,7 @@ export function ProviderEditor(props: ProviderEditorProps): ReactNode {
                   defaultMaxTokens={typeof defaultMaxTokens === 'number' ? defaultMaxTokens : undefined}
                 />
               )
-              : <ModelListEditor {...catalogProps} probe={probe} api={api} />}
+              : <ModelListEditor {...catalogProps} probe={probe} probeBlocked={keyFailure} api={api} />}
           </div>
         </details>
       </>

+ 2 - 0
packages/client/ui-models/src/client/locales.ts

@@ -47,6 +47,7 @@ export const en = {
   removeModel: 'Delete model',
   modelsEmpty: 'No models will be shown in the selector. Unlisted IDs can still be sent directly.',
   keyBlank: 'Enter the API key, or leave the field empty to keep the stored one.',
+  keyBlankNew: 'Enter the API key, or leave the field empty if this provider authenticates another way.',
   keyIllegalCharacters: 'This API key is not in a valid format. Please check it.',
   keyLooksWrapped: 'Paste only the key itself — not a NAME=value line, and without surrounding quotes.',
   modelIdRequired: 'Model ID is required.',
@@ -134,6 +135,7 @@ export const zh: typeof en = {
   removeModel: '删除模型',
   modelsEmpty: '模型选择器中将不显示任何模型;目录外 ID 仍可直接发送。',
   keyBlank: '请输入 API 密钥;留空则保持已存储的密钥。',
+  keyBlankNew: '请输入 API 密钥;若该提供方以其他方式鉴权,可以留空。',
   keyIllegalCharacters: '该 API 密钥格式错误,请检查。',
   keyLooksWrapped: '请只粘贴密钥本身——不要带 NAME=value 整行,也不要带引号。',
   modelIdRequired: '模型 ID 不能为空。',

+ 13 - 0
packages/client/ui-models/tests/provider-form.spec.tsx

@@ -954,6 +954,19 @@ describe('API key field', () => {
     expect((set.mock.calls[0]?.[0] as { value: string }).value).toBe('sk-abc')
   })
 
+  it('blocks the interrogation too, rather than spending a round trip on a refused key', async () => {
+    const { discover } = await mountSection()
+    openEditor('openai')
+
+    fireEvent.change(screen.getByLabelText(en.keyInput), { target: { value: 'sk-\u{1F600}' } })
+
+    // The host would refuse this before building the header anyway; asking is
+    // a round trip to be told what the field already says.
+    expect(buttonNamed(en.fetchModels).disabled).toBe(true)
+    expect(buttonNamed(en.fetchModels).title).toBe(en.keyIllegalCharacters)
+    expect(discover).not.toHaveBeenCalled()
+  })
+
   it('carries the trimmed key into an interrogation, not the padded draft', async () => {
     const { discover } = await mountSection()
     openEditor('openai')

+ 4 - 2
packages/llm/llm-pi-ai/tests/discovery.spec.ts

@@ -325,8 +325,10 @@ describe('probe key format', () => {
   })
 
   it('reports a blank probe key as a credential fault too', async () => {
-    // A cleared form field arrives as '', not an absent key; it must fail the
-    // same way a typed-in illegal key does, rather than probing unauthenticated.
+    // The Models page omits `apiKey` entirely for a cleared field rather than
+    // sending '', so this pins the contract for every other caller: a supplied
+    // key is judged, and only an absent one probes unauthenticated. '' means
+    // "I have a key" and is answered as the empty key it is.
     await expect(discoverModels({
       baseURL: 'https://acme.test',
       api: 'openai-completions',

+ 8 - 3
packages/llm/llm/src/index.ts

@@ -145,11 +145,16 @@ export class LlmError extends HarnessError {
 export function assertUsableApiKey(raw: string, pkg: string, ref: string): string {
   const checked = normalizeApiKey(raw)
   if (checked.ok) return checked.value
+  // The Models page is named as the writer it usually is, not as the only one:
+  // the same value can arrive from a hand-edited .env or a shell export in a
+  // composition that mounts no credentials seam at all, where directing the
+  // user to a page that deployment does not serve would be a dead end.
   throw new LlmError(
     checked.reason === 'empty'
-      ? `${pkg}: the API key stored as ${ref} is blank; re-enter it on the web Models page`
-      : `${pkg}: the API key stored as ${ref} contains characters no HTTP header can carry;`
-        + ' re-enter it on the web Models page, pasting the raw key only',
+      ? `${pkg}: the API key resolved from ${ref} is blank; set ${ref} to the raw key`
+        + ' (the web Models page writes it) or export it in the launching environment'
+      : `${pkg}: the API key resolved from ${ref} contains characters no HTTP header can carry;`
+        + ` set ${ref} to the raw key alone (the web Models page writes it)`,
     INVALID_CREDENTIAL_CODE,
   )
 }

+ 1 - 1
packages/llm/llm/tests/api-key.spec.ts

@@ -45,7 +45,7 @@ describe('assertUsableApiKey', () => {
 
   it('refuses a blank stored credential, naming the reference', () => {
     expect(() => assertUsableApiKey('   ', 'llm-deepseek', 'DEEPSEEK_API_KEY'))
-      .toThrow(/llm-deepseek: the API key stored as DEEPSEEK_API_KEY is blank/)
+      .toThrow(/llm-deepseek: the API key resolved from DEEPSEEK_API_KEY is blank/)
   })
 
   it('refuses an unusable stored credential with the invalid-credential code', () => {

Einige Dateien werden nicht angezeigt, da zu viele Dateien in diesem Diff geändert wurden.