Sync/upstream 20260827 215608

[theoneand33]

What Changed

Synced changes from upstream

Checklist

• This PR is small and focused
• I explained what changed and why
• I included before/after screenshots for any UI changes
• I included a video for animation/interaction changes

---

Note

^{Cursor Bugbot is generating a summary for commit c69d3b8. Configure here.}

Note

Add Pi provider driver and overhaul CI/release workflows to GitHub-hosted runners

• Introduces a full Pi provider driver: PiDriver, PiAdapter (RPC subprocess session management), PiProvider (snapshot/probe), PiSkills (skill discovery), PiTextGeneration, and PiSettings schema in contracts. Registered in BUILT_IN_DRIVERS and disabled by default.
• Overhauls CI and release workflows: replaces all blacksmith runners with ubuntu-24.04/macos-15/windows-2025; adds opencode-review and sync-upstream workflows; removes relay public config propagation, publish_cli, publish_aur, deploy_web, and announce_discord jobs from release.yml; finalize now depends on [preflight, quality, build].
• Extracts shared helpers makeWithInstanceIdentity and makeStandardTextGeneration across provider drivers and text-generation factories; converts label and check-status arrays to Set lookups.
• Fixes SettingsWatcherLive to acquire the subscription synchronously before forking, preventing dropped settings-change events.
• Fixes ghosttyCellText to use Array.from instead of new Array(count), avoiding sparse-hole iteration.
• Risk: release.yml no longer publishes CLI, AUR, web app, or Discord announcements — verify these are intentionally removed or handled elsewhere. cli-external-packages.test.ts now requires dependencies/optionalDependencies/peerDependencies to be defined on manifests.

📊 Macroscope summarized c69d3b8. 71 files reviewed, 30 issues evaluated, 13 issues filtered, 15 comments posted

🗂️ Filtered Issues

.github/workflows/release.yml — 4 comments posted, 5 evaluated, 1 filtered

• line 733: announce_discord was removed with no replacement notification step. Successful stable releases and nightlies now terminate after finalize without posting the previously configured release webhook message, so subscribers are not informed that a release is available. [ Out of scope (post-validation triage) ]

apps/server/src/provider/Drivers/PiDriver.ts — 1 comment posted, 3 evaluated, 2 filtered

• line 98: PiDriver declares ProviderEventLoggers as a required environment service but never retrieves or passes its native logger when constructing makePiAdapter. Unlike the other built-in drivers, Pi protocol events therefore cannot be written to the shared native event log, leaving Pi sessions absent from the provider diagnostic logs. [ Out of scope (post-validation triage) ]
• line 107: The per-instance environment is passed into checkPiProviderStatus, which invokes discoverPiSkills, but that function ignores its environment argument and calls NodeOS.homedir() against the server's own process.env. Consequently, a Pi instance configured with a different HOME launches Pi with that home but publishes skills from the server process's home, so its $ picker omits the instance's user skills (or exposes the wrong account's skills). [ Exceeded comment limit ]

apps/server/src/provider/Layers/GrokAdapter.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 746: interruptedTurnIds grows permanently for watchdog-cancelled turns: settleStalledTurn adds each timed-out turnId, but the stalled-settlement path never deletes it (unlike the normal completion path). A long-lived session that encounters many stalled or manually cancelled turns therefore retains an unbounded set of unique turn IDs, causing a per-session memory leak. [ Out of scope (post-validation triage) ]

apps/server/src/provider/Layers/PiAdapter.ts — 0 comments posted, 8 evaluated, 8 filtered

• line 134: getThreadSemaphore permanently inserts a semaphore for every thread ID, but neither stopSessionInternal nor stopAll removes the entry. Since conversations create unbounded new thread IDs, threadLocksRef grows for the lifetime of the provider instance even after all sessions have ended. [ Out of scope (post-validation triage) ]
• line 481: The new Pi session context is created without applying or recording input.modelSelection; the spawned pi --mode rpc command also receives no --model option. Consequently, choosing a custom Pi model when starting a thread always runs Pi's configured default model while the application persists the requested model selection, so chat output and usage are attributed to the wrong model. [ Cross-file consolidated ]
• line 540: The old child's exit handler unconditionally calls sessions.delete(ctx.threadId). Starting a replacement session for the same thread kills the old child and immediately installs the new context, but the old process's asynchronous exit can then delete that new context. Subsequent operations report the active replacement session as missing, and the emitted exit event is also attributed to its thread. [ Exceeded comment limit ]
• line 610: sendTurn sets ctx.activeTurnId and changes the session to running before validating that the turn has content or before reading every attachment. An empty turn, invalid attachment path, or attachment read failure returns an error on lines 624–655 without calling markSessionReady, leaving listSessions/subsequent state observers with a phantom running turn although no prompt was submitted. [ Exceeded comment limit ]
• line 709: The background completion poller stops after 600 successful streaming checks without emitting turn.completed or resetting the session. A Pi turn that remains streaming for more than ten minutes leaves ctx.activeTurnId and ctx.session.status permanently running, so clients never receive completion and later state remains stale. [ Exceeded comment limit ]
• line 755: interruptTurn clears ctx.activeTurnId and sends Pi's unscoped abort even when the supplied turnId is not the currently active turn. A delayed cancel request for an earlier turn therefore aborts a newer turn, while emitting a cancellation event for the stale ID. The adapter must ignore a provided ID that differs from the active turn. [ Exceeded comment limit ]
• line 786: respondToRequest ignores the returned RPC result's success flag and always emits request.resolved; respondToUserInput has the same behavior. If Pi rejects an extension_ui_response (for example, for an expired request), the server reports it as resolved even though the provider did not receive the decision, leaving the interactive operation blocked or in a different state. [ Exceeded comment limit ]
• line 850: readThread never checks the RPC response's success flag. When Pi replies to get_messages with { success: false, error: ... }, this code treats missing data.messages as [] and returns a seemingly valid empty thread snapshot, silently hiding the provider failure from the caller. [ Exceeded comment limit ]

apps/server/src/textGeneration/makeStandardTextGeneration.ts — 0 comments posted, 1 evaluated, 1 filtered

• line 28: RunJson accepts and forwards modelSelection, but the Pi implementation discards it (modelSelection: _modelSelection) and never adds its model to the CLI arguments. Thus selecting any configured custom Pi model for commit/PR/title generation still runs Pi's default model, even though Pi supports --model; the generated result is silently produced by a different model than the one selected. [ Cross-file consolidated ]

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 15ac0840-d4f2-4f5b-a7d3-c3c56d8bc2f5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

---

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

[macroscopeapp]

Reviewed the new/changed Effect service code in this sync (Pi driver/adapter/provider/text generation, Grok skills, shared providerIdentity, makeStandardTextGeneration).

Most of the new code follows the conventions (subpath namespace imports, Effect.fn constructors, environment-acquired FileSystem/Path/Crypto/ChildProcessSpawner, structured errors mirroring the existing provider patterns). Two dependency-acquisition / runtime-boundary violations in apps/server/src/provider/Layers/PiAdapter.ts need attention:

• the Pi process is spawned via a dynamic node:child_process import instead of the ChildProcessSpawner service, hiding an imperative runtime dependency from the service environment;
• node stdio/exit callbacks execute Effects with bare Effect.runSync/Effect.runPromise inside the service constructor, detaching them from the adapter runtime (contrast ClaudeAdapter, which bridges its SDK callbacks with Effect.runPromiseWith(runtimeContext)).

No other findings; error shapes such as detail: cause.message / stderr-derived detail match established patterns in CodexTextGeneration/ClaudeTextGeneration and are out of scope for this check.

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

makePiAdapter spawns Pi through a dynamic node:child_process import rather than acquiring the spawner from the Effect environment, so this imperative dependency never appears in the service's requirements and cannot be substituted by a test layer. Consider const spawner = yield* ChildProcessSpawner.ChildProcessSpawner plus ChildProcess.make(...) — the same acquisition CodexAdapter, GrokAdapter, GrokSkills, and PiProvider already use — and drive stdin/stdout through the returned handle's Sink/Stream.

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

These node callbacks run Effects with bare Effect.runSync/Effect.runPromise inside the service constructor (also lines 522-527, 542, and 554), which starts a fresh runtime per event and drops the adapter's context, logging/tracing configuration, interruption, and error propagation into the session scope. Consider offering raw stdio lines and the exit result into a Queue from the callback and consuming them in a fiber forked into scope; if a callback bridge is truly required, capture the ambient context once and use Effect.runPromiseWith(context) as ClaudeAdapter does.

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

🟡 Medium Drivers/PiSkills.ts:81

discoverPiSkills omits skills configured through Pi's settings.skills, package skills/ directories, and pi.skills package entries, so those skills are loaded by Pi but never appear in T3 Code's $ picker. The roots list only contains the two default global roots and ancestor-based project roots; include the configured and package-provided skill roots when building it.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/provider/Drivers/PiSkills.ts around line 81:

`discoverPiSkills` omits skills configured through Pi's `settings.skills`, package `skills/` directories, and `pi.skills` package entries, so those skills are loaded by Pi but never appear in T3 Code's `$` picker. The `roots` list only contains the two default global roots and ancestor-based project roots; include the configured and package-provided skill roots when building it.

[macroscopeapp]

🟡 Medium Drivers/PiSkills.ts:95

The $ picker exposes skills from <cwd>/.claude/skills as enabled even though the spawned Pi process does not load them unless they are explicitly configured in Pi's settings.skills, so selecting one exposes a skill that Pi cannot use. Remove this unconditional cross-harness root (or gate it on the corresponding Pi setting).

-  // Also include cwd's .claude/skills for shared skill reuse? Pi docs say
-  // users can add ~/.claude/skills via settings.skills, but discovery for
-  // `$` picker should surface project-local .claude/skills when present,
-  // mirroring Claude's behavior for cross-harness reuse.
-  const crossHarnessProjectRoots: ReadonlyArray<{ directory: string; scope: PiSkillScope }> = cwd
-    ? [{ directory: path.join(cwd, ".claude", "skills"), scope: "project" as const }]
-    : [];
-
   // Order: global first, then project ancestors (cwd first, then parents outward).
   // Later entries win on name collision, so project overrides user.
-  const roots = [...globalRoots, ...projectRoots, ...crossHarnessProjectRoots];
+  const roots = [...globalRoots, ...projectRoots];

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/provider/Drivers/PiSkills.ts around lines 95-105:

The `$` picker exposes skills from `<cwd>/.claude/skills` as enabled even though the spawned Pi process does not load them unless they are explicitly configured in Pi's `settings.skills`, so selecting one exposes a skill that Pi cannot use. Remove this unconditional cross-harness root (or gate it on the corresponding Pi setting).

[macroscopeapp]

🟡 Medium Drivers/PiSkills.ts:122

The picker omits valid Markdown skills that are not named SKILL.md, including root .md files under ~/.pi/agent/skills and .pi/skills and nested .md files under .agents/skills and ~/.agents/skills. The traversal only probes <entry>/SKILL.md and treats every entry as a directory, so these files are never added to skillPaths; inspect .md entries as skill files and recurse only into directories.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/provider/Drivers/PiSkills.ts around line 122:

The picker omits valid Markdown skills that are not named `SKILL.md`, including root `.md` files under `~/.pi/agent/skills` and `.pi/skills` and nested `.md` files under `.agents/skills` and `~/.agents/skills`. The traversal only probes `<entry>/SKILL.md` and treats every entry as a directory, so these files are never added to `skillPaths`; inspect `.md` entries as skill files and recurse only into directories.

[macroscopeapp]

🟠 High workflows/opencode-review.yml:18

Both third-party actions execute from mutable references (actions/checkout@v6 and anomalyco/opencode/github@latest), so a retagged or compromised release runs with OPENROUTER_API_KEY and write-capable repository permissions and can exfiltrate the key or modify the repository. Pin both actions to reviewed immutable commit SHAs.

🤖 Copy this AI Prompt to have your agent fix this:

In file @.github/workflows/opencode-review.yml around line 18:

Both third-party actions execute from mutable references (`actions/checkout@v6` and `anomalyco/opencode/github@latest`), so a retagged or compromised release runs with `OPENROUTER_API_KEY` and write-capable repository permissions and can exfiltrate the key or modify the repository. Pin both actions to reviewed immutable commit SHAs.

[macroscopeapp]

🟡 Medium Drivers/PiSkills.ts:135

A directory containing SKILL.md is still recursed into, so nested SKILL.md files under that skill's assets or references are advertised as separate invocable skills. Add a guard so directories already identified as skill roots are not pushed onto stack.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/provider/Drivers/PiSkills.ts around line 135:

A directory containing `SKILL.md` is still recursed into, so nested `SKILL.md` files under that skill's assets or references are advertised as separate invocable skills. Add a guard so directories already identified as skill roots are not pushed onto `stack`.

[macroscopeapp]

🟡 Medium Drivers/PiSkills.ts:156

A same-named skill in a parent directory overwrites the nearer skill from cwd, so /repo/.pi/skills/foo is replaced by /.pi/skills/foo and the nearest-project precedence is reversed. Because projectRoots lists ancestors from cwd outward but this loop always calls skillsByName.set, retain an existing project-scoped skill instead of overwriting it.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/provider/Drivers/PiSkills.ts around line 156:

A same-named skill in a parent directory overwrites the nearer skill from `cwd`, so `/repo/.pi/skills/foo` is replaced by `/.pi/skills/foo` and the nearest-project precedence is reversed. Because `projectRoots` lists ancestors from `cwd` outward but this loop always calls `skillsByName.set`, retain an existing project-scoped skill instead of overwriting it.

[macroscopeapp]

🟠 High Drivers/PiDriver.ts:98

With a missing or non-executable binaryPath, startSession reports a ready session and leaves hasSession true even though no child process is running; subsequent turns then fail. The adapter's error listener only resumes its fork and never marks or removes the session. Handle the spawn error by failing startup and cleaning up the session context.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/provider/Drivers/PiDriver.ts around line 98:

With a missing or non-executable `binaryPath`, `startSession` reports a ready session and leaves `hasSession` true even though no child process is running; subsequent turns then fail. The adapter's `error` listener only resumes its fork and never marks or removes the session. Handle the spawn error by failing startup and cleaning up the session context.

[macroscopeapp]

🟡 Medium Drivers/PiSkills.ts:148

discoverPiSkills advertises skills that the Pi CLI does not load when SKILL.md has no non-empty description or has no frontmatter, so those entries appear selectable but are unavailable. Require parsed frontmatter with a description before registering the skill.

🤖 Copy this AI Prompt to have your agent fix this:

In file @apps/server/src/provider/Drivers/PiSkills.ts around line 148:

`discoverPiSkills` advertises skills that the Pi CLI does not load when `SKILL.md` has no non-empty `description` or has no frontmatter, so those entries appear selectable but are unavailable. Require parsed frontmatter with a description before registering the skill.

[macroscopeapp]

🟡 Medium workflows/sync-upstream.yml:22

This workflow never synchronizes a distinct upstream: UPSTREAM_REPOSITORY points to the same repository as origin, so upstream/main is the checked-out main history and git merge-base --is-ancestor upstream/main HEAD causes every run to set needed=false. Set UPSTREAM_REPOSITORY to the intended upstream repository.

🤖 Copy this AI Prompt to have your agent fix this:

In file @.github/workflows/sync-upstream.yml around line 22:

This workflow never synchronizes a distinct upstream: `UPSTREAM_REPOSITORY` points to the same repository as `origin`, so `upstream/main` is the checked-out `main` history and `git merge-base --is-ancestor upstream/main HEAD` causes every run to set `needed=false`. Set `UPSTREAM_REPOSITORY` to the intended upstream repository.

[macroscopeapp]

🟡 Medium workflows/release.yml:618

After a GitHub release, no job invokes .github/workflows/publish-aur.yml, so new release tags never update the AUR package and Linux users installing through AUR receive an indefinitely stale version. Restore the publish_aur reusable-workflow job or otherwise trigger that workflow after release succeeds.

🤖 Copy this AI Prompt to have your agent fix this:

In file @.github/workflows/release.yml around line 618:

After a GitHub release, no job invokes `.github/workflows/publish-aur.yml`, so new release tags never update the AUR package and Linux users installing through AUR receive an indefinitely stale version. Restore the `publish_aur` reusable-workflow job or otherwise trigger that workflow after `release` succeeds.

[macroscopeapp]

Reviewed the new Pi service modules (PiDriver.ts, PiAdapter.ts, PiProvider.ts, PiSkills.ts, PiTextGeneration.ts, providerIdentity.ts, makeStandardTextGeneration.ts) plus the touched registry/hydration code.

Service definition, layering, and namespace-import conventions look correct, and the makeWithInstanceIdentity / makeStandardTextGeneration extractions are clean. Two convention issues in PiAdapter.ts / PiTextGeneration.ts:

• Nested Effect.runSync / Effect.runPromise inside the makePiAdapter service constructor for stdout/stderr/exit handling, instead of ChildProcessSpawner + Stream (as CodexSessionRuntime/opencodeRuntime do) or a context-aware runner captured at the callback boundary (as ClaudeAdapter does).
• Error detail fields populated from cause.message / String(cause), so the structural attribute and the resulting message just restate the underlying failure. Sibling providers keep detail a stable description and preserve the original error in cause.

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

detail here just copies the underlying error's message, so the wrapper adds no stable structural context (and ProviderAdapterRequestError.message is derived from it). A fixed description plus cause preserves both. Same at line 637.

Suggested change

	detail: cause.message,
	detail: "Failed to read Pi thread messages.",

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

detail is populated from cause.message/String(cause), so the TextGenerationError message merely restates the underlying failure and the structured field carries no stable operation context. GrokTextGeneration/ClaudeTextGeneration keep detail a fixed description and leave the underlying error in cause. Same pattern at lines 85, 106, and 137.

Suggested change

	detail: (cause as { message?: string })?.message ?? String(cause),
	detail: "Failed to resolve the Pi binary for text generation.",

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

makePiAdapter runs nested runtimes inside the service: Effect.runSync for the stderr log (494) and await Effect.runPromise(...) per parsed RPC line (522, 527). Bare run* discards the surrounding context (logger/tracer annotations, FileSystem, ServerConfig, Crypto) and detaches interruption, and run* should stay at application boundaries rather than inside a service constructor.

Consider spawning through ChildProcessSpawner.ChildProcessSpawner (already acquired by PiProvider/PiTextGeneration) and consuming child.stdout/child.stderr as Streams — Stream.decodeText() → Stream.runForEach(...) with a Ref holding the partial line — each forked with Effect.forkIn(scope), matching CodexSessionRuntime and opencodeRuntime. If raw Node stdio really is required here, capture the context once (const runPromise = Effect.runPromiseWith(yield* Effect.context<never>())) like ClaudeAdapter and use that only at the callback boundary.

^{Posted via Macroscope — Effect Service Conventions}

[macroscopeapp]

Same runtime-boundary issue in the exit handler: Effect.runSync (542) and Effect.runPromise (554) run new runtimes from inside the adapter. Suggest failing the pending deferreds and publishing session.exited via a runner derived from Effect.context<never>() (Effect.runForkWith/Effect.runPromiseWith), or modelling exit as child.exitCode on an Effect child process forked into scope, so annotations and interruption survive.

^{Posted via Macroscope — Effect Service Conventions}

[cursor]

Cursor Bugbot has reviewed your changes using high effort and found 4 potential issues.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.

^{Reviewed by Cursor Bugbot for commit 8fc1c65. Configure here.}

[cursor]

Pi turns can hang indefinitely

High Severity

The turn-completion poller is started with Effect.forkScoped and then ignored, instead of Effect.forkIn(ctx.scope) like the stdout and exit fibers. A missing Scope or a short-lived caller scope drops the poller with no turn.completed. Even when it runs, the 600-attempt loop can exit without emitting turn.completed, so the thread stays running.

Additional Locations (1)

• apps/server/src/provider/Layers/PiAdapter.ts#L532-L533

 

^{Reviewed by Cursor Bugbot for commit 8fc1c65. Configure here.}

[cursor]

Failed sendTurn leaves session running

High Severity

sendTurn sets ctx.session to running and assigns activeTurnId before attachment and empty-input checks. Those failures return without markSessionReady or a turn.completed event, so the session stays running and later turns or settle logic cannot proceed.

 

^{Reviewed by Cursor Bugbot for commit 8fc1c65. Configure here.}

[cursor]

Pi spawn failure leaves zombie session

High Severity

Node spawn reports a missing or failed binary on the error event, but that handler only resumes the watcher fiber. The session is still published as ready. Process exit and stopSessionInternal also omit turn.completed for an in-flight turn, and both can emit session.exited, so threads can stay running or tear down twice.

Additional Locations (1)

• apps/server/src/provider/Layers/PiAdapter.ts#L166-L197

 

^{Reviewed by Cursor Bugbot for commit 8fc1c65. Configure here.}

[cursor]

Pi approval options all accept

Medium Severity

Every string in an extension_ui_request options list is mapped to decision: "accept". Choosing Deny or any non-allow label still resolves as accept, so respondToRequest sends Allow to Pi.

 

^{Reviewed by Cursor Bugbot for commit 8fc1c65. Configure here.}

[macroscopeapp]

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — This change combines a substantial new Pi provider runtime with privileged AI workflows and major release-pipeline changes, including removal of CLI, hosted web, AUR, and production configuration paths. The scope and production/security blast radius are too broad for automatic approval.

Not approved because:

• 15 blocking correctness issues found at or above your repo's Minimum Blocking Severity

^{Adjust the Minimum Blocking Severity for this repo — including turning it Off — in Settings. You can add or adjust custom eligibility rules. Learn more.}