Updates from Upstream - #3

PR Review

The fork's scheduled-prompts feature coexists with upstream's experimental scheduler without any table, migration, route, event, flag, or tool-name collisions, and the new migrations apply cleanly on top of the fork's tables. The deprecated Schedule* client aliases are wired correctly. Three issues found: one UI data bug, one destructive side effect in the auth-session fix, and a broken macOS build script.

Findings (highest priority first)

1. [Warning] packages/app/src/home/schedules/dialog.tsx:286 — the schedule dialog submits the agent's display name as the agent ID, and the server stores it without validating.

   • Why it matters: a scheduled task created from the UI with an agent whose configured name differs from its ID fails every run with "Agent not found". The name is stored as the agent reference, and session startup rejects it when the task fires.
   • Where:
     • packages/app/src/home/schedules/dialog.tsx:286 — the picker items use agent.name as their value.
     • packages/app/src/home/schedules/dialog.tsx:218 — submit casts form.agentId to Agent.ID and sends it.
     • packages/core/src/scheduled-task.ts:495 and :540 — create and update store agent_id with no validation, unlike the tool path in packages/core/src/tool/plugin/schedule.ts, which checks the ID against the agent list.
     • packages/core/src/session/context.ts:144 — AgentNotFoundError raised when the stored reference does not resolve.
   • Fix: make the picker send agent.id (SegmentedControlItem value={agent.id}), and reject unknown agent IDs in ScheduleTask.create/update with InvalidSpecError so the HTTP path matches the tool path.

2. [Warning] packages/core/src/database/auth-session.ts:55 — a server started with credentials that differ from the shared database's stored verifier deletes every remembered login.

   • Why it matters: any second server process on the shared data directory (a stdio/ACP server, a second opencode serve) logs out all browsers with 30-day remembered sessions and flips the stored verifier to its own password. The fork's own comments describe auxiliary processes sharing the data directory as a supported setup.
   • Where:
     • packages/core/src/database/auth-session.ts:55 — the configure loop treats a verifier mismatch as "this identity owns the row", deletes the auth_session table, and overwrites auth_config.
     • packages/server/src/routes.ts:219 — runs configure at startup for every server built from the standard routes.
     • packages/cli/src/server-process.ts — a server without an explicit password gets a random one, so a shared-DB auxiliary process is guaranteed to mismatch.
   • Fix: do not reconcile a row written by a different identity. When an existing verifier does not match this process's credentials, log a warning and leave both auth_config and auth_session untouched (first writer wins per database), or store the identity in auth_config and only rewrite rows you wrote.

3. [Warning] packages/desktop/build-mac.sh — the macOS build script calls package scripts that do not exist.

   • Why it matters: the script aborts at the "Prebuilding" step, so it cannot produce a macOS build as written.
   • Where:
     • packages/desktop/build-mac.sh — runs bun run prebuild and bun run package:mac.
     • packages/desktop/package.json — the scripts block has no prebuild or package:mac entries.
   • Fix: mirror build-windows.bat: run bun run build, then bunx electron-builder --mac --publish never --config electron-builder.config.ts (or add the missing scripts to package.json).

Following up on the review above: finding #1 is a false positive. The schedule dialog was already correct.

The picker reads from normalizeAgentList (packages/app/src/runtime/server/global-sync/utils.ts), which stores the agent's id in the name field (name: agent.id). The app's Agent type (packages/app/src/runtime/server/types.ts) has no id field — name is the id. So SegmentedControlItem value={agent.name} at dialog.tsx:286 already sends the agent's id, and the server stores a valid agent reference. No change is needed there; the "name vs id" framing in the review misread the normalized representation.

The other two findings are fixed in this branch:

• fix(core): wait for plugin activation between steps #2 (ephemeral credential wipes shared sessions) — fixed in 019360cc07. An auxiliary stdio/ACP server mints its own random password and shares the data directory (and database) with the canonical server. On startup, AuthSession.configure treated the verifier mismatch as a credential change: it deleted the whole auth_session table, wiping the canonical server's remembered logins, and adopted a random verifier. A new ephemeral flag (set in the CLI when the password was minted rather than taken from env/config) is threaded down to configure, which now skips the reconcile for such a process. The canonical server's behavior, including invalidation on a real credential change, is unchanged.
• Updates from Upstream #3 (stale build-mac.sh) — fixed in 0aa53c4a94. It called bun run prebuild and bun run package:mac, neither of which exists in packages/desktop/package.json. It now runs bun run build then bunx electron-builder --mac --publish never --config electron-builder.config.ts, mirroring build-windows.bat.

Ran an independent review pass (neutral, no prior involvement in this work) over the fork's changes versus upstream: git diff 17b5826a5e...wellington-merge, 329 files (301 in packages/, 24 docs, plus root files).

The one warning-level finding is a false positive, and it's the same one as before: the agent picker "sends the name instead of the id" claim. normalizeAgentList stores the agent's id in the name field (packages/app/src/runtime/server/global-sync/utils.ts:21), and the app's Agent type has no id field, so value={agent.name} at packages/app/src/home/schedules/dialog.tsx:286 is already the id. The proposed value={agent.id} does not typecheck. See the earlier note: #3 (comment)

The remaining four are nits.

1. Schedules ship without experimental or a flag. Repo convention is that new endpoints and new user-visible behavior start experimental and get a feature flag. The fork adds stable /api/schedule endpoints with v2.schedule.* identifiers (packages/protocol/src/groups/scheduled-task.ts:25) and a schedule_create tool, and none of that reads a flag. The existing schedules flag (packages/schema/src/config/flag.ts:16) is V1-only. Likely intentional for a private fork, so I'm calling it out as a decision to make explicit rather than a bug.

2. session column default disagrees with the service default. The column default is "new" (packages/core/src/schedule/sql.ts:58), the service default is "reuse" (packages/core/src/scheduled-task.ts:480). Inserts always supply the value, so the column default is dead code and the two disagree. Aligning the column to "reuse" would keep the schema honest for future direct inserts.

3. The 50-task cap check is not atomic. ScheduleTask.create does a COUNT(*) check (packages/core/src/scheduled-task.ts:482) followed by a separate INSERT (:486). Two concurrent creates near the cap can both pass the check and both insert. Unlikely in practice, but wrapping the count and insert in one transaction would close it.

4. Transitional re-export. packages/core/src/schedule.ts:835 re-exports ScheduleTask from ./scheduled-task.js, giving it a second public identity. It is commented as transitional for fork callers. Once those callers import the module directly, the re-export can go.

Verified clean in the same pass: core schedule tests (74), auth session tests (6), and typecheck for core and app. The concern that the authorization middleware would redirect API 401s to the sign-in page does not hold: browser HTML pages go through the router middleware, which issues the full 302, and the HttpApi layer only sees API paths, where it returns a JSON 401.