Skip to content

fix(server): allow more time for Pi model discovery - #15529

Open
MarcusNeufeldt wants to merge 1 commit into
pingdotgg:mainfrom
MarcusNeufeldt:fix/pi-discovery-timeout
Open

MarcusNeufeldt wants to merge 1 commit into
pingdotgg:mainfrom
MarcusNeufeldt:fix/pi-discovery-timeout

Conversation

@MarcusNeufeldt

Copy link
Copy Markdown

Problem

Pi's model picker can fall back to only the default and manually configured models when an extension-heavy Pi installation takes too long to start. Expected behavior is to discover the user's configured models, including extension-registered providers.

This occurred on Windows with T3 nightly 0.0.46-nightly.20261004.2644 and Pi 0.99.1. The provider cache showed the discovery-timeout fallback. Direct RPC probes returned 543 models and took 12.2 to 13.9 seconds to start, leaving little headroom under the 15-second deadline. This larger configuration may expose a startup limit that a minimal setup does not. Model count alone has not been established as the cause.

Change

Increase the Pi RPC discovery deadline from 15 to 30 seconds. Successful discovery still returns immediately, with no fixed delay. Keep extensions enabled so their registered models remain available.

The production change is one constant. Tests cover successful discovery after a 20-second startup and fallback for a stalled startup. The mock executable name avoids resolving an installed Windows pi.cmd during these tests.

Scope and approval

Submitted under the small, focused bug-fix exception. This restores existing model discovery for a supported Pi setup without introducing a feature, setting, dependency, or alternate discovery path. The deadline remains bounded. No prior maintainer approval is claimed.

Verification

  • Changing only the deadline in the installed nightly, then restarting, restored the model picker, confirmed by the user. Pi configuration and extensions were unchanged.
  • The 20-second startup regression fails with the original deadline and passes with this change. It verifies that extensions are not disabled and their models appear.
  • pnpm exec vp test run apps/server/src/provider/Layers/PiProvider.test.ts: all four tests pass, including stalled-startup fallback and existing compatibility checks.
  • Targeted vp lint, vp fmt --check, and git diff --check: pass.

Full typecheck, the full suite, and other operating systems were not checked. Direct-probe timings are not exact measurements of the failed T3 startup.

Model: GPT-6.1 Sol. Harness: Pi.

@github-actions github-actions Bot added size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Oct 4, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 4, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This focused fix increases the default Pi RPC discovery deadline from 15 to 30 seconds, changing startup behavior for existing users while delaying fallback on stalled discovery. Because it changes a product default, human review is required under the applicable policy.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 4, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

🧰 Additional context used
📚 Code guidelines (1)
docs/internals/effect-services.md — auto-discovered

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: pingdotgg/t3code/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 24491980-cb3f-46f0-a3d9-0948e0eee01c
📥 Commits

Reviewing files that changed from the base of the PR and between 0fe4fa4 and 0c276cf.

📒 Files selected for processing (2)
  • apps/server/src/provider/Layers/PiProvider.test.ts
  • apps/server/src/provider/Layers/PiProvider.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

Pi RPC discovery now waits up to 30 seconds before applying the existing timeout handling. Tests cover delayed startup that completes within the timeout and startup that exceeds it.

Changes

Pi RPC discovery

Layer / File(s) Summary
Discovery timeout and delayed-startup tests
apps/server/src/provider/Layers/PiProvider.ts, apps/server/src/provider/Layers/PiProvider.test.ts
The discovery timeout increases from 15 to 30 seconds. Tests check that a 20-second startup returns an authenticated snapshot with the extension model, while a 60-second startup returns a ready snapshot with unknown authentication and only the default model.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge

Merge Risk: ⚪ Minimal · up to 0c276

Pi model discovery has more time to include extension models while retaining the existing timeout fallback. No actionable merge-blocking risk is established.

Security Architecture Review

Security architecture risk: 🔵 Low · up to 0c276

The startup wait remains bounded, and existing failure handling and cleanup are preserved. Each attempt may now run twice as long; the limits on simultaneous attempts across separate configurations have not been fully established.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The demonstrated exposure change is a longer execution window for each existing configured discovery process and its extensions. Separate provider instances have separate scheduling bounds. Maximum aggregate concurrency and an untrusted caller's ability to create those instances remain unverified.

Trust Boundaries and Controls

  • observed — Discovery continues to use registry-decoded configuration and driver-bound runtime services. It launches an ephemeral no-session RPC process without a T3 MCP session. Configured extensions remain enabled; the deadline change adds no new tool authority or launch path.

Resilience and Maintainability Implications

  • observed — The existing RPC connection owns its child process and registers scoped cleanup. Replies are correlated by request ID, failed transports release pending requests, and interruption removes pending entries. Cleanup includes process-tree termination, preserving containment when the discovery deadline expires.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: increasing the time allowed for Pi model discovery.
Description check ✅ Passed The description covers the problem, change, scope and approval rationale, and focused verification. It also states which checks were not run and preserves relevant limitations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 4, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 6, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 7, 2026
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 8, 2026
pingdotgg#15529's new test parses and writes JSON with JSON.parse/stringify inside
Effect.gen, which the effect language service rejects (preferSchemaOverJson),
so the server typecheck failed on fork-stack. `ours` does not have that code to
change, so build-stack.sh now commits fixups/<pr>.patch once every PR is
merged; applying it earlier changes pingdotgg#14941's conflict with pingdotgg#15079 and its
recorded resolution no longer matches. The patch uses the Schema helpers pingdotgg#15079
adds to the same file.
aliceisjustplaying added a commit to aliceisjustplaying/t3code that referenced this pull request Oct 8, 2026

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS 0-9 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant