Skip to content

fix(app): block new sessions when no models are available - #399

Merged
oxwen11 merged 1 commit into
mainfrom
pie/e06640a5
Oct 6, 2026
Merged

oxwen11 merged 1 commit into
mainfrom
pie/e06640a5

Conversation

@oxwen11

@oxwen11 oxwen11 commented Sep 23, 2026

Copy link
Copy Markdown
Owner

What changed

  • Gate the new-session composer until model discovery confirms at least one available model.
  • Show an actionable empty state when no model provider is configured, plus Retry; keep discovery errors retryable and prevent submits while loading.
  • Update the draft verification recipe for the new no-model path.

Verification

  • Empty isolated Pi agent directory: /draft displays the gate, Retry keeps the gate, and no session file is created (0 files).
  • Configured model regression: app browser product flows 14/14 passed; full browser suite 83/83 passed.
  • Node tests after required CLI build: 1354 passed, 1 skipped; app typecheck, oxlint, oxfmt and React Doctor changed/design passed.

Scope

This prevents starting from /draft. Existing-session prompts, direct RPC callers, and scheduled runs are unchanged.

UI evidence

Before/after screenshots and recordings captured locally; attaching below.

@oxwen11

oxwen11 commented Sep 23, 2026

Copy link
Copy Markdown
Owner Author

UI 验证证据:

  • Before:无模型凭据时旧版仍显示新会话输入框;发送后出现 No API key found。
  • After:新版显示配置提示与 Retry;重试后仍无法提交,且没有新 session 文件。

下方附上前后截图及两段操作录像。

Before: composer with no models

Before: model request fails

After: model configuration gate

After: still gated after Retry

recording-001.webm
recording-002.webm

@pkg-pr-new

pkg-pr-new Bot commented Sep 23, 2026 •

Copy link
Copy Markdown
npx https://pkg.pr.new/oxwen11/pie/@getpie/cli@399

commit: 6cd6272

@github-actions

github-actions Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

React Doctor found no new issues. 🎉

Reviewed by React Doctor for commit 6cd6272.

@oxwen11

oxwen11 commented Sep 23, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head ad1a59e3abae36c4e5529e656861c1a00d7038bd — required CI green (Check, react-doctor, both Publish @getpie/cli preview), MERGEABLE.

out of .agents/rules/auto-merge-pr.md allowed groups; hits exclusions:

  • feature / new functional product behavior: draft route now blocks the composer until agent.listModels succeeds with ≥1 model; empty catalog shows a new No model provider connected empty state + Retry; pending/error paths add Loader / new error UI. That changes whether a session can start, not presentation.
  • new components + control flow: DraftModelsError, DraftNoModels, and early-return conditions in DraftPage (apps/app/src/routes/draft.tsx).
  • new user-facing copy and handlers (Retry /login guidance).
  • docs update in .agents/skills/verify-pie/features/draft-new-chat.md documents the new gate / verify path — not a docs-only PR.

not lint-format, not presentation-only UI. fail closed — no merge.

@oxwen11

oxwen11 commented Sep 28, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head aa14c3a7d6f3fe17c82d7a55e287364c6c4a533f — required CI green (Check, react-doctor, both Publish @getpie/cli preview), MERGEABLE (BEHIND). Prior no was on ad1a59e3abae36c4e5529e656861c1a00d7038bd; re-reviewed this updated head.

out of .agents/rules/auto-merge-pr.md allowed groups; hits exclusions:

  • feature / new functional product behavior: draft route now blocks the composer until agent.listModels succeeds with ≥1 model; empty catalog shows a new No model provider connected empty state + Retry; pending/error paths add Loader / new error UI. That changes whether a session can start, not presentation.
  • new components + control flow: DraftModelsError, DraftNoModels, and early-return conditions in DraftPage (apps/app/src/routes/draft.tsx).
  • new user-facing copy and handlers (Retry /login guidance).
  • docs update in .agents/skills/verify-pie/features/draft-new-chat.md documents the new gate / verify path — not a docs-only PR.

not lint-format, not presentation-only UI. fail closed — no merge.

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

Review-and-merge gate record — not merged

  • Head: e0eb997cb1a3184ff19730949e2ebaa94e319324.
  • Current target main and trusted rules commit: d7f4b7a95588e5de2aeefbe790e65111e89195f6.
  • Policy: trusted PR workflow, with review, security and acceptance rules from that base (not PR-proposed or retired rules).
  • Context: fix(app): block new sessions when no models are available. Same-repository, author oxwen11, open, non-draft.

Required CI observed for this head

Blocking gate and conclusion

  • Required CI has not all passed at this head (see failed or queued/in-progress results above).

Stopped before code review and independent verification; not merged. These are gate observations, not an implementation-quality verdict. No branch update, conflict resolution, CI rerun, protection bypass or fix-and-merge was performed.

Revisit only after a relevant change resolves the recorded blocker: an author-provided head containing current main with no conflict where necessary, and every required check present, executed and successful for the candidate version. The next eligible version starts again at CI; unchanged blockers need no duplicate review/comment.

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

CI 失败定位与修复方向

  • Head:e0eb997cb1a3184ff19730949e2ebaa94e319324
  • 当前 main / 可信规则 commit:8f7546548ca95b7167c2261ebe87f4425806cf65
  • 失败检查日志

e2e/product-flows.e2e.test.tsx 12 项失败。日志中的实际页面为 No model provider connected,后续 textbox/首条消息定位失败。需先核对 E2E 模型发现链路:正常聊天 fixture 必须确实返回可用模型;无模型场景则单独断言门禁和不创建 Session。若有模型仍被误判为空,修产品模型加载逻辑。当前日志尚不能确定是 fixture 还是实现错误,不能只延长超时或删断言。

这是对既有阻塞新增的日志定位,不是代码审查通过。未本地重现或验证修复方案,未修改分支、重跑 CI 或合并。修复/环境恢复并得到满足当前 base 的全绿候选后,重新从 CI → 审查 → 独立验收开始。

@oxwen11

oxwen11 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

Queue update after verified #423 merge — waiting at CI

Head 33c07232921fc342981774c7880e2ed69e238ba1 now contains main/trusted-rules 0920277492d381b108b520b341c449bcc3e3dbd8 (behind_by=0 verified). GitHub performed a conflict-free merge update guarded by the previous exact head; no conflict resolution or source repair.

Required CI for this new head is queued/in progress at this observation, so no old-head review or acceptance is carried forward. New-head full review and affected independent runtime verification remain outstanding.

No merge or deferred auto-merge. Continue only after the applicable blocker is resolved and the new version passes required CI → review → independent acceptance.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Merged origin/main into pie/e06640a5 with merge commits (no rebase). Head is 4f36fcbd, which contains db2acfae (#447) and 257f0da0 (#402).

The only content conflict was .agents/skills/verify-pie/features/draft-new-chat.md. apps/app/src/routes/draft.tsx auto-merged: the no-models send block stays, and agent.listModels is the selected Environment's catalog (?environmentId=, otherwise local) via EnvironmentOrpcProvider. An empty catalog on that Environment replaces the composer; another Environment's models do not unblock send. The grouped project picker, environmentId search, and linked-host send disable from #402 are unchanged. The later #447 shell title-row merge was clean.

Checks: app browser tests 8/8 (-draft-loading, project-select, rail/shell/page-sidebar), including a case where the local Environment has a model and the selected remote catalog is empty. pnpm check passed after a workspace build. PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Blocker (CI gate) for head 4f36fcbd: required Check fails in app-e2e — e2e/product-flows.e2e.test.tsx has 12 failing flows where the draft composer textbox never renders (job). main db2acfae passes these flows, so this is attributable to the PR. Review/verification will restart on the next head once Check is green.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

The app-e2e failures were the no-models gate, not a missing textbox. agent.listModels used the daemon catalog, which does not run extensions, so the seeded e2e provider came back as { models: [] } and the draft showed No model provider connected. The session child already listed e2e/fake.

ad1cd7e5 keeps that daemon list, and when it is empty asks pie-pi-process get_available_models before the gate blocks. An actually empty child catalog still blocks.

Checks: list-available-models 3/3, including the extension-provider fallback. e2e/product-flows.e2e.test.tsx 14/14. PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review — #399 head ad1cd7e5 (base db2acfae): changes required

Required CI is green on this head. Review is blocked on one finding, so independent UI verification has not started.

Blocking: the model-list fallback has no timeout and cannot be interrupted

PiAgentService.listModels now starts a pie-pi-process (listSessionPiModels) whenever the daemon catalog is empty. That is exactly the unconfigured machine this PR targets. Nothing bounds that call.

How I reproduced it, against this head:

  • Setup: a temporary PI_CODING_AGENT_DIR with a single extension whose session_start handler awaits.
  • Result: listModels was still pending after 45 s, and the child pie-pi-process --mode rpc --approve outlived the caller until I stopped it.
  • The same setup without the extension resolves in about 0.4 s.

What a user would see: the draft page shows <Loader /> indefinitely, with no Retry button, and each draft load or refetch can leave another child running.

On top of that, Effect.tryPromise(() => Effect.runPromise(...NodeServices.layer)) starts a second runtime. Interrupting the RPC (client disconnects, query cancelled) therefore never reaches the inner scope, and the child is not cleaned up.

Needed:

  • Run the fallback inside the caller's Effect, without nesting runPromise, so interruption closes the scope and kills the child.
  • Bound it with a timeout that maps to AgentOperationError. t3code uses 15 s for the equivalent probe. The draft then reaches the existing retryable error state.
  • Add a test with a hanging session_start extension that asserts a timely failure and that the child has exited.

Needs a Developer decision (not decided here)

The fallback runs with the selected project's cwd and --approve. Global and project-local extension code now executes when the draft page loads its model list for a selected project — before the user sends anything. Previously this happened only once a session started (#443). This overlaps with the direction under discussion in #427 (transient-process model listing), so I am confirming the intended trust behavior with the Developer.

Re-review on the next head after CI passes.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Addressed the blocking review on ad1cd7e5.

listSessionPiModels is now an Effect on the caller fiber. It no longer nests Effect.runPromise or a NodeServices runtime, so interruption closes the scope and kills the child. A 15s timeout maps to AgentOperationError, which the draft already shows as the retryable model-check error.

A session_start extension that never returns fails in about 15s and the child is gone afterward. cwd and --approve are unchanged.

Pushed as a normal commit. PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Verification record — #399 head 89f9b526 (base db2acfae)

Conclusion: verified technically. Not merging yet — waiting on a Developer decision about extension trust (details below).

  • Required CI on this head: all pass (Check, react-doctor, and both Publish @getpie/cli preview jobs).
  • The blocking finding from my previous review (fix(app): block new sessions when no models are available #399 (comment)) is fixed:
    • listSessionPiModels now runs on the caller's fiber.
    • It has a 15 s timeout that maps to AgentOperationError.
    • When the call is interrupted or times out, the child process is killed.

Checks I ran (clean worktree at this head)

  • list-available-models.test.ts: 4/4 pass. This includes the new hang test, which fails in about 17 s (15 s timeout) and confirms the child process has exited.

  • -draft-loading.test.tsx: 3/3 pass. This includes "blocks send from the selected Environment's model list".

  • Runtime with a configured model (operator's Pi config, isolated PIE_HOME). On every surface below, the draft composer rendered with the model picker, and a send created a session that answered OK-399:

    Surface Code tested
    Web, before main db2acfae
    Web, after this PR's head
    Desktop, after this PR's head

    This confirms the gate does not regress the configured path.

  • The empty-catalog gate itself is covered by the browser test above, as the recipe asks. pie-verify refuses an empty PI_CODING_AGENT_DIR by design.

Open decision before merge

  • When the daemon catalog is empty, the fallback starts pie-pi-process --approve with the selected project's cwd. That runs global and project extensions while the draft is still only listing models, before anything is sent.
  • The existing test listAvailablePiModels › does not execute Project extensions while reading the model list encodes the opposite intent for the daemon path.
  • This overlaps the model-listing direction in fix(server): restore user Pi extension and plugin loading #427, so the intended trust behavior is with the Developer.
  • Merge waits for that decision. A change to the fallback's cwd or flags warrants re-review.

399-web-before-01-draft-composer

399-web-after-01-draft-composer

399-web-after-02-after-send

399-desktop-after-02-after-send

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Recordings for the verification record above (#399 (comment)): draft composer → send → reply on head 89f9b526, Web and Desktop.

399-web-after-send.webm
399-desktop-after-send.webm

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Developer decision (2026-10-06): the model list comes from a short-lived Pi process, as in t3code and paseo.

  • No --approve: project-local extensions follow Pi's project-trust setting, so they do not run on listing unless the project is trusted.
  • Keep the 15 s timeout and child cleanup.
  • Replaces the in-daemon listing and the empty-catalog fallback in this PR.
    Re-review and re-verification will run on the resulting head.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Developer decision applied: PiAgentService.listModels now always uses a short-lived pie-pi-process (get_available_models and get_state, then exit). The in-daemon SDK listing and the empty-catalog fallback are gone. No server-side cache.

The listing child does not pass --approve. Project extensions run only when Pi already trusts that directory; global extensions still load. cwd is the selected project path, or the agent dir when there is none. The 15s timeout still kills the child on timeout or interrupt. The default model comes from get_state, with resolveDefaultPiModel only if that model is not in the list.

Draft, the session model picker, and the schedule form still call agent.listModels. Tests cover an untrusted project extension (does not run), a trusted project extension provider, a global extension provider, and the hang/timeout kill.

PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Single-flight and a 60s server cache are now in front of the model-list child, keyed by the resolved cwd. Concurrent calls share one child. A success is reused until the TTL; a timeout or failure is not stored. Draft Retry calls listModels with refresh: true, which skips a completed entry.

Draft, the session model picker, and the schedule form share one React Query key per project (staleTime 60s, no refetch on focus), so switching a project away and back does not issue another request while that entry is fresh.

Commands stay a separate spawn. agent.commands still passes --approve, and folding that into the no-approve model probe would run untrusted Project extensions. get_state for the default model is already on the model probe. The session picker's live getModelState is a different process and was left alone.

Measured on this machine, empty agent dir: cold listModels 466ms, cached call 1ms.

PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Developer decision update (supersedes the cache request above): no server-side cache or single-flight for listModels. The b130226c server cache and the refresh input should be removed. Instead, rely on React Query's per-key dedupe and staleTime: Infinity, turn off window-focus refetch for the probe-backed keys, and invalidate only the affected project's key. Broader latency work is tracked in #450.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Developer decision (2026-10-06, supersedes the earlier no---approve note): adding a Project to Pie means approving its project-local files. The model probe therefore uses PI_PROJECT_PROCESS_ARGS (--approve), consistent with sessions and the command probe, with cwd = the imported Project (or the agent dir when none is selected). The other parts of the earlier update are unchanged: no server-side cache, focus refetch disabled for probe-backed keys, project-scoped invalidation.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Two Check failures on 67887217:

  • serve-prompt.test.ts timed out because the shipped CLI stand-in (PIE_E2E_PI_EXECUTABLE) only speaks --list-models, not get_available_models. A direct executable now uses that flag, so the child comes up and returns. The real pie-pi-process path is unchanged.
  • bundled-node.test.ts failed because moving the catalog out of the daemon tree-shook Pi's extension loader out of server.js, so PI_BUNDLED_NODE no longer folded there. The loader import is kept in the daemon bundle.

agent.commands now shares that same no---approve probe and the 60s single-flight cache, instead of spawning its own child. A commands call after listModels for the same cwd spawns nothing.

Cold listModels 466ms, cached call 1ms. PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Dropped the server catalog cache and single-flight. React Query already dedupes the same key, and the app staleTime is Infinity.

agent.listModels no longer refetches on window focus, so focus does not spawn another Pi process. Draft Retry still calls refetch(). After session.create and setModel, invalidation targets only that project's listModels key.

The two Check failures stay fixed: a direct PIE_E2E_PI_EXECUTABLE answers --list-models instead of hanging on RPC, and the daemon bundle still folds Pi's extension loader.

PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Imported Project = approved. The transient model probe now passes PI_PROJECT_PROCESS_ARGS (--approve), same as session and command children. cwd is still the selected Project path, or the agent dir when none is selected.

The untrusted-extension expectation is gone. Listing an imported Project includes that Project's extension provider models. The hang/timeout test stays.

No server cache, single-flight, or refresh input. listModels and commands do not refetch on window focus. session.create and setModel invalidate only that project's listModels key. Draft Retry still calls refetch(). The shipped CLI stand-in still uses --list-models, and the daemon bundle still folds Pi's extension loader.

PR not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review — #399 head e23c2182: changes required

Required CI passed on e23c2182, which was before the update-branch merge of main. Review is blocked on the two CI-repair approaches below. They make the tests pass by changing product code, not by fixing the root cause.

What is now correct:

  • The model list comes only from a transient child with --approve.
  • The default model comes from get_state.
  • The server cache and the refresh input are gone.
  • listModels and commands no longer refetch on window focus.
  • Invalidation is scoped to the project.
  • CONTEXT and README match the import-equals-approve decision.

Blocking 1 — a test seam now drives the product path (list-available-models.ts)

  • The problem: listSessionPiModels switches to --list-models stdout parsing whenever resolvePiExecutable() returns empty prefixArgs. Empty prefixArgs also happens in production when the pie-pi-process entry cannot be resolved: the command falls back to bun with no script (resolve-executable.ts).
  • Failure in that case:
    • The daemon runs bun --list-models and parses that output as a model table.
    • It treats the first row as the default model.
    • The draft then shows No model provider connected, or a garbage list, instead of a retryable error.
  • Needed:
    • Keep one RPC path in product code.
    • Make the shipped-CLI test's Pi stand-in answer get_available_models and get_state over RPC. tools/test/fake-pi.js already exists for RPC executable-replacement tests.
    • An unresolvable entry must surface as an error.

Blocking 2 — dead code plus an env flag to keep a bundler fold (keep-bundled-loader.ts, http/main.ts)

  • The problem:
    • The daemon no longer loads Pi extensions, so the bundled-node assertion on server.js is obsolete.
    • Instead of updating that test, the PR imports createAgentSessionServices into the server entry only so the fold survives.
    • It also adds PIE_RETAIN_PI_LOADER=1, which calls createAgentSessionServices({ cwd: process.cwd() }) at daemon startup.
  • Needed:
    • Remove both.
    • Change bundled-node.test.ts to assert the pie-pi-process bundle only, which is where extensions now load.
    • Update the tsdown.config.ts comment to match.

Should fix — stored-trust branch nobody uses (rpc/main.ts)

  • The problem:
    • storedProjectTrust reads Pi's ProjectTrustStore and the default trust when --approve is absent.
    • Every Pie child now passes --approve, so nothing uses this branch.
    • It also changes how a child without the override behaves.
  • Needed: remove it unless a caller needs it.

Re-review and Web + Desktop re-verification will run on the next head after CI passes.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Addressed the review on e23c2182 (now 7693d8cc, with origin/main at 98063ef1 merged first).

  • One catalog path: get_available_models + get_state. The --list-models branch is gone. The shipped-CLI stand-in is tools/test/fake-pi.js, which answers those RPC commands. serve-prompt.test.ts passed against dist/cli.js.
  • A missing pie-pi-process entry throws instead of spawning bun with no script.
  • Removed keep-bundled-loader.ts and the PIE_RETAIN_PI_LOADER startup call. bundled-node.test.ts asserts the pie-pi-process bundle only.
  • Removed the unused storedProjectTrust branch. Children still pass --approve.

Not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Review — #399 head 7693d8cc: one blocker remains

Resolved since e23c2182

  • --list-models branch: removed. The shipped-CLI test now uses tools/test/fake-pi.js over RPC.
  • keep-bundled-loader.ts and PIE_RETAIN_PI_LOADER: removed. bundled-node.test.ts now asserts the pie-pi-process bundle.
  • Stored-trust branch: removed.

Blocking: resolvePiExecutable() now throws, and the daemon calls it at module load

  • rpc/runtime.ts:38 evaluates const piExecutable = resolvePiExecutable() when the module loads.
  • agent.ts then passes that executable to checkPiAvailability. When the pie-pi-process entry is missing, that check reports pie-pi-process entry was not found.
  • With the new throw, the same condition crashes daemon startup instead. Pi becomes unavailable to every caller, and the server does not start.
  • In listSessionPiModels, the throw happens inside Effect.gen as a defect. mapError cannot map a defect, so the draft would see an internal error instead of an AgentOperationError.
  • Needed:
    • Leave resolvePiExecutable non-throwing.
    • In the probe, fail with AgentOperationError when the script is missing. Reuse checkPiAvailability, or check prefixArgs.
    • Add a test for the missing-entry case.

Note: the required Check failure is not caused by this PR

The only failing test is app-e2e session archive › archives the open session from the row menu (right-click a row → Archive menuitem times out). Main has the same failure on its own push run after #449 (98063ef1, run 37434700908). That file passes locally at both db2acfae and 64709013, so I am tracking it on main separately. A green Check on this PR still depends on that being resolved.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Correction to #399 (comment): main's next push run (bd1af63a, rn-pie-02) passed, so the archive-row e2e failure is a flake (recorded on #444), not a main regression. The blocker in that review is unchanged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Addressed the blocker on 7693d8cc (now this head).

resolvePiExecutable() no longer throws, so daemon startup still reports pie-pi-process entry was not found. through checkPiAvailability. The catalog probe fails immediately with AgentOperationError when bun has no script, instead of a defect or a 15s hang. Covered by list-available-models.test.ts.

Did not touch the app-e2e archive failure. Not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Typecheck fix on the missing-entry test, and merged origin/main (bd1af63a).

listSessionPiModels still requires ChildProcessSpawner on the early-fail path, so the test now provides the platform layer. pnpm exec turbo run typecheck --filter=@getpie/server passed.

Not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto current main (7ee17fe3, #459) as 898fbf9b.

The Check failure was not a product regression. After the titlebar started showing the session title, getByText("e2e archive me").first() right-clicked the titlebar, so the Archive menuitem never opened. #459 already targets the sidebar row button. Main's Check on 7ee17fe3 is green (run 37479570876). This branch was one commit behind that fix.

Replaying the old PR commits conflicted on the first commit, so the net change is one commit on top of main. The archive assertion is unchanged from #459.

Local evidence:

  • pnpm check passed (lint, format, typecheck).
  • vitest run --config vitest.browser.config.mts apps/app/e2e/product-flows.e2e.test.tsx -t "imports a project from the empty draft|archives the open session from the row menu" — 2 passed. The archive test needs the import test first; the sample project is not pre-seeded.

Not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head 898fbf9b85494d0b — all checks green (Check, react-doctor, Publish @getpie/cli preview ×2, Continuous Releases, React Doctor), but mergeable=CONFLICTING / mergeStateStatus=DIRTY against main 18fea6051977 (#461), so the gate fails. Rules: trusted copy of .agents/rules/auto-merge-pr.md + auto-merge/exclusions.md from 451a89407c6b.

Exclusion hits even once conflicts are resolved:

  • new functional product behavior: apps/app/src/routes/draft.tsx hides the composer while model discovery is pending/failed and adds a no-models gate with Retry that blocks sending from /draft.
  • non-presentational server change: packages/server/src/harness/pi/list-available-models.ts (+69/−30), agent-service.ts, protocol.ts, rpc/main.ts, project-resource-policy.ts change how the model list is produced; per the updated CONTEXT.md/README.md, the model-list child now runs with --approve and loads the imported Project's extensions, which is a trust-boundary change (topics/security.md territory).
  • apps/app/src/lib/orpc.ts changes query defaults (refetchOnWindowFocus: false for agent.listModels/agent.commands) and use-session-models.ts re-keys the query per projectId — client data-flow changes, not presentation.
  • tests are extensive but accompany product control-flow changes.

Also note main just landed #461 (list models through a short-lived pie-pi-process), which likely overlaps list-available-models.ts; rebase before manual review. Needs Din's manual review/merge. Re-review on a new head.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Rebased onto main (18fea605, #461) as 581c83ef.

#461 already lists models through a short-lived pie-pi-process (listAvailablePiModels, get_available_models, settings default). The no-model gate now sits on that path. Dropped the duplicate probe, get_state default, and the extra trust override so the gate does not fight the merged listing.

Local evidence:

  • pnpm check passed.
  • orpc.test.ts: 2 passed.
  • -draft-loading.test.tsx: 3 passed.
  • app-e2e import, send-draft, and archive row-menu: 3 passed.

Not merged.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head 581c83efd58d — all checks green and MERGEABLE (BEHIND main daec7bbaa077); prior no was on 898fbf9b8549 (conflicting), so re-reviewed the rebased head. Rules: trusted .agents/rules/auto-merge-pr.md + auto-merge/exclusions.md from 451a89407c6b.

Fails closed on exclusions (no allowed group covers these hunks):

  • apps/app/src/routes/draft.tsx (+52/-2): new functional product behavior — a no-models gate that replaces the composer with No model provider connected guidance and a Retry button, plus loader/error states; new user-facing copy, conditions, and handlers.
  • apps/app/src/lib/orpc.ts: new query defaults (refetchOnWindowFocus: false for agent.listModels and agent.commands) — runtime/perf behavior change, not presentation.
  • use-session-models.ts: narrows listModels invalidation to the session's projectId — non-presentational fix.
  • -draft-loading.test.tsx / orpc.test.ts pin that new control flow; verify-pie doc updated to match.

Looks like a reasonable fix, but it needs a human merge.

@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Independent review + verification: PR #399

  • Head reviewed and verified: a993aa429d94d420afbb45ab794ede76fd77936e
  • Base: daec7bbaa077936598907c5c51848725ca166b13 (main)
  • Rules followed: .agents/rules/workflows/pull-requests.md, review.md and acceptance.md at daec7bba
  • Conclusion: Ready, with no blocking findings. This is a comment and not an Approve, because the reviewer uses the author's GitHub identity. It is not merged.

CI

All required checks passed on this head: Check, react-doctor, and both Publish @getpie/cli preview checks. Mergeable state: clean.

Review

The diff matches the description. /draft now shows a loader while agent.listModels is pending. If the call fails, it shows an error with a Retry button. If it succeeds with zero models, it shows the No model provider connected gate with Retry. The composer appears only when the selected Environment has at least one model.

The other changes are scoped as described:

  • listModels invalidation now targets the specific project.
  • refetchOnWindowFocus: false is set for listModels and commands, so returning to the window no longer spawns a Pi process.
  • The recipe text is updated.

The new browser test (the remote Environment has an empty catalog, the local one has models) fails against base draft.tsx and passes on this head, so it does assert the changed behavior.

Independent verification

Clean worktree pinned to the head:

  • pnpm install --frozen-lockfile and turbo run build passed.
  • pnpm check passed (lint, format, typecheck).
  • orpc.test.ts passed 2/2 and -draft-loading.test.tsx passed 3/3.

Runtime used pie-verify web with its own root and ports, an isolated PIE_HOME and an isolated browser. PI_CODING_AGENT_DIR pointed to a temp dir. No real credentials or models were used. Results:

Scenario Observed
No models: agent dir holds only settings.json {} /draft shows the gate. Retry keeps it. The sidebar's per-project New chat is gated too. 0 session files written. (rec 001)
Discovery fails: a Project extension makes the Pi child exit Loader first, then "Couldn't check available models" with Retry after the query retries. Composer stays hidden. (rec 003)
Models available: stub registerProvider in the agent dir, fake baseUrl Gate shown before. After adding the provider, Retry shows the composer with Review Stub Model. Send created session review 399 ping (review-stub/stub-model) and opened it. The turn then failed with the expected unreachable-provider retry. (rec 005)
Provider only in a Project extension The project draft shows the composer with the stub model. A plain /draft (no project) is gated. (rec 002)

Non-blocking notes

  1. The gate hides the project picker. If models exist only through a Project's .pi/extensions, a plain New chat says "No model provider connected" and gives no way to pick that Project from the draft. The sidebar's "New chat in <project>" still works. A follow-up could keep the picker visible above the gate, or reword the copy.
  2. A hung discovery child means an endless loader. I tested an extension that keeps Pi alive but never finishes loading. After Retry, the draft showed only a loader for 150s+ with no error and no Retry (rec 004). Before this PR the composer would still render in that state. Main has no listModels timeout yet; fix(server): bound Pi discovery and stop children on interrupt #463 adds one, after which this becomes the error/Retry state above. Suggest landing fix(server): bound Pi discovery and stop children on interrupt #463 alongside or before this PR.

Gaps

  • Desktop (Electron) was not run. The draft route is shared SPA, so this is the outstanding Desktop gap.
  • No real-model run. All checks used stub providers.

Re-review is needed if the head changes, or if main's listModels contract changes (e.g. #463).

No models: gate on /draft

No models: project draft also gated

Discovery failure: retryable error

Hung discovery: loader after 150s

Models available: before Retry

Models available: after Retry, composer with stub model

Models available: send created session

Project-only provider: plain draft gated

recording-001.webm
recording-002.webm
recording-003.webm
recording-004.webm
recording-005.webm

Rebased onto main. The archive row-menu e2e now right-clicks the sidebar title, not the titlebar text that stole the first exact match.
@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Supplementary verification: PR #399 on Desktop, and combined with #463

  • Head verified: 6cd627215ae90103771e4eb99b6dcbb76dcb55d9. This is the rebase of a993aa42, and git range-diff shows the patch is identical.
  • Base: c7d9205aa8b4c807fd5320717a89f8c284d1f389 (main, which includes fix(server): bound Pi discovery and stop children on interrupt #463's 30s discovery timeout)
  • Rules: .agents/rules/workflows/pull-requests.md and acceptance.md at c7d9205a
  • Conclusion: Ready. Both gaps from the earlier record are now closed, with no blocking findings. Not merged, and the branch was not updated.

CI

All required checks pass on 6cd62721: Check, react-doctor, and both Publish @getpie/cli preview checks. Mergeable.

Setup

  • Clean worktree pinned to the head.
  • pnpm install --frozen-lockfile, then turbo run build for @getpie/server (forced, no cache) and @getpie/desktop.
  • pie-verify desktop ran with its own root and CDP port, plus an isolated PIE_HOME and Electron profile. PI_CODING_AGENT_DIR pointed to a temp dir, and the daemon process was confirmed to inherit it.
  • No real models or credentials were used.

Observed (Desktop / Electron)

Scenario Result
No models: the agent dir holds only settings.json {} /draft shows No model provider connected with Retry. Retry keeps the gate. 0 session files were written.
Models available: a stub pi.registerProvider (fake baseUrl 127.0.0.1:9) was added to the same agent dir Retry immediately replaces the gate with the composer, showing Review Stub Model.
With #463: a Project .pi/extensions file that stalls forever while keeping Pi alive. Opened via sidebar New chat in A loader first, then "Couldn't check available models. Retry before starting a session." with Retry at t+127s. The loader does not run forever. Daemon log: 4 get_available_models-timeout failures about 31–34s apart. No pie-pi-process children were left behind. After the stall file was removed, Retry showed the project composer within about 1s.

Note on the elapsed time (non-blocking): the error appears after about 127s, not about 30s. Each attempt is bounded at 30s by #463, but the draft's listModels query uses React Query's default retry: 3, so the app makes 4 attempts plus backoff. If about 30s is the intended user-facing bound, a follow-up could set retry: false (or 1) on listModels, with refetchOnWindowFocus: false already set. An error toast with the timeout message also appears alongside the inline error.

Gaps

  • No real-model run: all checks used stub providers.
  • The no-project draft with a stalled agent-dir extension was not run. Only the Project-extension stall path was exercised.

Re-review is needed if the head changes, or if main's listModels or query-retry behaviour changes.

Videos: desktop-no-models-then-models.webm covers the gate, then Retry, then the composer. desktop-stalled-discovery-timeout.webm covers the loader, the error at 127s, and recovery.

Desktop no models: gate (before)

Desktop no models: gate kept after Retry

Desktop models available: composer after Retry (after)

Stalled project extension: before opening project draft

Stalled project extension: loader at ~20s

Stalled project extension: retryable error at t+127s (after)

Stall removed: Retry shows project composer

desktop-no-models-then-models.webm
desktop-stalled-discovery-timeout.webm

@oxwen11
oxwen11 merged commit 047a49c into main Oct 6, 2026
6 checks passed
@oxwen11
oxwen11 deleted the pie/e06640a5 branch October 6, 2026 16:29
@oxwen11

oxwen11 commented Oct 6, 2026

Copy link
Copy Markdown
Owner Author

Merged head 6cd627215ae90103771e4eb99b6dcbb76dcb55d9 (base c7d9205a, patch identical to reviewed a993aa42 per range-diff) after rechecking SHAs, required CI and mergeability. Records: Web #399 (comment), Desktop + #463 combination #399 (comment). Confirmed merge commit: 047a49cf68beb9764c0ed6374525a0c743b9227c.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants