Skip to content

fix: prevent daemon fsync crash and Pi catalog failures - #454

Merged
lucas77778 merged 2 commits into
masterfrom
lucas77778/code-578
Aug 17, 2026
Merged

fix: prevent daemon fsync crash and Pi catalog failures#454
lucas77778 merged 2 commits into
masterfrom
lucas77778/code-578

Conversation

@lucas77778

@lucas77778 lucas77778 commented Aug 17, 2026

Copy link
Copy Markdown
Member

Summary

  • fsync daemon config writes through the temporary file's writable descriptor, preventing the Windows credential-migration crash loop
  • resolve bare Pi model ids from the resumed provider, known provider, or a unique model/endpoint match
  • pause Pi catalog requests until its managed runtime is available, avoiding startup module-not-found noise

Verification

  • pnpm check:ci
  • pnpm test (2956 passed, 1 skipped)
  • regression coverage for opencode-go/deepseek-v4-flash

Closes #437
Closes #438

Copilot AI lite review requested due to automatic review settings August 17, 2026 09:32
@linear-code

linear-code Bot commented Aug 17, 2026

Copy link
Copy Markdown

CODE-578

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lucas77778
lucas77778 requested a review from PeronGH August 17, 2026 09:35

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ Both headline fixes are correct and independently verified. Two minor suggestions inline plus one doc-drift item below; nothing here blocks merge.

Reviewed changes

Three independent fixes — a Windows daemon crash, pi bare-model provider resolution, and an eager pi catalog request — each with new tests, no wire/schema/version changes.

  • Daemon config write stops fsyncing a read-only handle. writeConfigFields now opens the temp file once with openSync(path, 'wx', 0o600) and does write → chmodfsyncclose on that single write descriptor; the old code re-opened with 'r', which fails EPERM on Windows because FlushFileBuffers requires write access.

  • fsyncPath narrowed to the directory fsync. The helper survives but is now only used for the post-rename parent-directory sync, still skipped on Windows.

  • pi gains a fourth provider-precedence tier. When a bare model id has no provider from fallbackProvider or cred.knownProvider, createConfiguredRegistry scans modelRegistry.getAll() for a model whose id matches and whose baseUrl equals cred.baseUrl, accepting it only if the match is unique.

  • readAgentCredential hoisted above model parsing. Behavior-neutral — the function is pure over config fields.

  • pi catalog request is gated on runtime availability. use-agent-catalogs.ts passes null params (tayori's pause idiom) unless useAgentRuntimes() reports pi.status === 'available', ending the eager agent.catalog request that logged Failed to load agent catalog when pi isn't installed.

ℓ️ Verification notes

I checked the two claims that mocked tests can't establish, since both fixes depend on real SDK and real data behavior:

  • ModelRegistry.getAll() does exist and Model.baseUrl is required in the installed @earendil-works/pi-ai typings, so the new registry scan is real rather than mock-only fiction. (The test's local interface Model declares baseUrl?: string optional — a small fidelity gap, noted as a nit below.)
  • pi's built-in MODELS table (35 providers, 1057 models) contains opencode-go/deepseek-v4-flash at exactly https://opencode.ai/zen/go/v1, and packages/foundation/providers/src/catalog.ts has no opencode/zen service — so the account in the linked issue genuinely has no knownProvider, and the new endpoint tier is precisely the tier that resolves it. Separately, zero (modelId, baseUrl) pairs in that table map to more than one provider, so the uniqueness guard isn't a practical blocker today.

ℹ️ AGENTS.md provider-precedence documentation is now stale

packages/host/agent-adapter/AGENTS.md documents the provider-routed credential precedence as:

Precedence: model-ref → for pi, the resumed session's own last-routed provider (lastPiModelChange, direct evidence) → config.knownProvider → for pi, its first available provider

This PR adds a fourth tier (built-in endpoint match) and changes the documented outcome for a bare model id from "throws" to "resolved when provenance is unambiguous". That file isn't in the diff so I can't anchor a line comment, but it's the routing doc AGENTS.md points agents at before touching this code — worth updating in the same PR while the reasoning is fresh.

ℹ️ Nitpicks

  • The getAll() endpoint scan is built unconditionally, then discarded when fallbackProvider ?? cred.knownProvider already resolves. Cheap at 1057 models and reads fine as-is, but the loop could sit behind the !provider check if you'd rather not walk the table on the common path.

  • pi-model.test.ts's local interface Model declares baseUrl?: string, whereas the real SDK type requires it. Making it required keeps the fixture honest and would catch a future fixture that forgets the field.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using Claude Opus𝕏

Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts
Comment thread packages/host/agent-adapter/src/native/pi/adapter.ts

@PeronGH PeronGH left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the three production changes. The fsync-through-the-writable-handle fix is correct: the old fsyncPath reopened the temp file read-only, so the sync targeted a different descriptor than the one that wrote the bytes. Write/chmod/fsync/close ordering and the post-rename directory fsync are preserved. Pi bare-model resolution correctly refuses ambiguous endpoint matches, and gating the Pi catalog on runtime availability leaves the other four agents untouched. CI green.

@lucas77778
lucas77778 merged commit 964f04c into master Aug 17, 2026
12 checks passed
@lucas77778
lucas77778 deleted the lucas77778/code-578 branch August 17, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants