fix: trigger auto-update check on headless serve startup

[ralphstodomingo]

PINEAPPLE

Summary

Triggers a best-effort auto-update check when headless altimate serve starts, so extension-launched servers follow the same upgrade path as TUI.

Updated implementation details to match the final code:

• Schedules a one-time startup check (~1s after server listen) via a dedicated helper.
• Runs the check in Instance.provide({ directory: process.cwd(), init: InstanceBootstrap, fn: ... }) (no bootstrap() wrapper), avoiding unintended Instance.dispose() teardown during in-process server runtime.
• Keeps all update policy inside upgrade() (autoupdate gating, install method handling, downgrade guard, telemetry).
• Uses explicit error handling/logging for both upgrade execution failures and outer orchestration failures.
• Extracts logic into packages/opencode/src/cli/cmd/serve-upgrade-check.ts.
• Adds dependency injection for collaborators in the helper to avoid process-global module mocking in tests.

Test Plan

• Ran targeted tests for startup upgrade check behavior:
  • delayed scheduling
  • process.cwd() instance usage
  • non-throwing behavior when upgrade/provide fail
• Verified combined CLI test coverage after mock isolation fix:
  • packages/opencode/test/cli suite passing (512 pass, 0 fail).
• Verified CI TypeScript failure caused by leaked mock.module state is resolved by DI-based test rewrite.

Checklist

• Tests added/updated
• Documentation updated (if needed)
• CHANGELOG updated (if user-facing)

[coderabbitai]

📝 Walkthrough

Walkthrough

A new serve-upgrade-check module encapsulates startup upgrade logic with a configurable delay and non-blocking timer. The module exports a delay constant, an async startup check function that seeds an instance and runs upgrade with error logging, and a public scheduler that dispatches the check asynchronously. A comprehensive test suite validates the module's contract. The serve.ts command handler imports and calls the scheduler after the server starts, reusing the existing bootstrap and upgrade logic to bring extension users in line with TUI auto-update behavior.

Changes

Serve Startup Auto-Update

Layer / File(s)	Summary
Startup upgrade check module packages/opencode/src/cli/cmd/serve-upgrade-check.ts	Introduces STARTUP_UPGRADE_DELAY_MS, StartupUpgradeDeps interface for dependency injection, runStartupUpgradeCheck() that seeds an instance to the current directory and invokes upgrade with internal error logging, and scheduleStartupUpgradeCheck() that dispatches the check asynchronously with an unref'd timer to avoid blocking process exit.
Startup upgrade check tests packages/opencode/test/cli/serve-upgrade-check.test.ts	Injects mock dependencies via makeDeps() to track call counts and verify that runStartupUpgradeCheck calls upgrade exactly once with process.cwd(), resolves without throwing on upgrade errors or instance setup failures, and STARTUP_UPGRADE_DELAY_MS remains a positive delay within 5000ms.
Serve integration packages/opencode/src/cli/cmd/serve.ts	Imports scheduleStartupUpgradeCheck and calls it after the server starts listening, enabling non-blocking auto-update checks on headless serve startup for extension users.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

needs-review:blocked

Poem

🐇 A timer hops, the serve wakes,
Checking versions, what it takes,
Extensions now stay fresh and bright—
Auto-updates through the night! ✨
No fleet left frozen, old, or worn,
All users greet the updated dawn.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	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 'fix: trigger auto-update check on headless serve startup' directly and concisely describes the main change: adding auto-update triggering to the headless serve command.
Linked Issues check	✅ Passed	The PR fully addresses issue #939 by implementing an auto-update check on serve startup with dependency injection, testable functions, and proper error handling that respects existing upgrade policy.
Out of Scope Changes check	✅ Passed	All changes are directly related to issue #939: modifications to serve.ts to schedule the upgrade check, new serve-upgrade-check.ts module with testable functions, and comprehensive tests—no out-of-scope additions detected.
Description check	✅ Passed	The PR description includes the required PINEAPPLE marker, has all three template sections (Summary, Test Plan, Checklist), and comprehensively documents the changes, implementation rationale, and testing approach.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/serve-autoupdate-trigger

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[anandgupta42]

Code Review — /code-review (Claude)

Verdict: Request changes — one real concurrency/lifecycle risk to resolve first. The change is otherwise clean, well-commented, and correctly delegates all update policy to upgrade().

MAJOR — the transient Instance disposes state the live server shares (Bug / Logic Error) — serve.ts:59-61

This runs in-process with the HTTP server, unlike the TUI path it mirrors (worker.checkUpgrade runs in a separate worker thread, so its Instance.dispose() is isolated). Here it is not:

• The server resolves each request's directory as c.req.query("directory") || header || process.cwd() and wraps the handler in Instance.provide({ directory, … }) (server/server.ts:196,210). Requests that don't pass a directory default to process.cwd().
• Instance/State cache per-directory state in a process-global map keyed by the directory string (project/state.ts:10 recordsByKey).
• bootstrap(process.cwd(), …) runs upgrade() and then, in its finally, calls Instance.dispose() → State.dispose(process.cwd()) + disposeInstance(process.cwd()) (project/instance.ts:141-144), which disposes the entire process.cwd() bucket — including state created by server requests that defaulted to that directory.

Net effect: ~1s after the server starts listening, the upgrade check tears down the default-directory instance state. Best case it's lazily recreated on the next request (needless churn of config/storage/connections); worst case it races an in-flight request on process.cwd() → use-after-dispose.

Suggested fix: upgrade() only needs global config + Installation (both global, not directory-scoped), so give the check its own isolated instance key that the server never uses, instead of process.cwd():

import { tmpdir } from "os"
import { join } from "path"
// a sentinel dir the request path will never resolve to
const UPGRADE_INSTANCE_DIR = join(tmpdir(), "altimate-code-upgrade-check")
setTimeout(() => {
  bootstrap(UPGRADE_INSTANCE_DIR, () =>
    upgrade().catch((err) => log.error("startup upgrade check failed", { error: String(err) })),
  ).catch((err) => log.error("startup upgrade bootstrap failed", { error: String(err) }))
}, 1000).unref?.()

Please confirm upgrade() has no directory-scoped dependency (it reads Config.global() and Installation.*, which are global) — if so, the sentinel-dir fix fully removes the collision.

MINOR — magic 1000 delay — serve.ts:62

The 1s delay is an unexplained constant. A named const (const STARTUP_UPGRADE_DELAY_MS = 1000) with a one-line "let the listener settle before the best-effort check" makes intent explicit.

NIT — double-logging path

upgrade() is already .catch-wrapped, so the inner catch handles upgrade failures and the outer .catch only ever fires for bootstrap/dispose errors. That's fine and intentional, but worth a comment so a future reader doesn't think one is dead code.

Positives

• Correct marker hygiene (altimate_change start/end) on every added block.
• Faithfully mirrors the existing TUI trigger and keeps all policy inside upgrade() (autoupdate gate, downgrade guard, method detection, notify) — no policy duplication.
• Fire-and-forget with .unref?.() and logged-not-thrown errors: a flaky network/registry can't take the server down. Good instinct.

Missing tests

Hard to unit-test a detached setTimeout, but consider extracting the trigger body into a small named function so a test can assert it calls upgrade() and never rejects. At minimum, a manual note in the PR on how you verified the extension fleet now updates (since that's the whole motivation) would help.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

packages/opencode/src/cli/cmd/serve.ts (1)

58-63: ⚡ Quick win

Extract the delayed upgrade trigger and name the delay constant.

The inline setTimeout closure plus raw 1000 makes this path harder to test and reason about. A small extraction (e.g., triggerServeStartupUpgradeCheck) and SERVE_STARTUP_UPGRADE_DELAY_MS constant would make intent explicit and allow focused unit coverage for “calls upgrade” + “never rejects”.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/opencode/src/cli/cmd/serve.ts` around lines 58 - 63, Extract the
delayed upgrade check logic from the inline setTimeout closure into a separate
function named triggerServeStartupUpgradeCheck and create a constant
SERVE_STARTUP_UPGRADE_DELAY_MS set to 1000. Replace the current setTimeout block
with a call to setTimeout that uses the constant and function name, making the
intent clearer and improving testability of the upgrade check behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/opencode/src/cli/cmd/serve.ts`:
- Around line 59-61: The bootstrap() call on line 59 uses process.cwd() as the
instance key, which is the same bucket used by live serve requests. Since
bootstrap() calls Instance.dispose() in its finally block, this disposes the
active serve instance, causing lifecycle races. Fix this by passing an
isolated/unique instance key to the bootstrap() function instead of
process.cwd(), so the bootstrap upgrade check runs with its own separate
instance without interfering with the running server's instance lifecycle.

---

Nitpick comments:
In `@packages/opencode/src/cli/cmd/serve.ts`:
- Around line 58-63: Extract the delayed upgrade check logic from the inline
setTimeout closure into a separate function named
triggerServeStartupUpgradeCheck and create a constant
SERVE_STARTUP_UPGRADE_DELAY_MS set to 1000. Replace the current setTimeout block
with a call to setTimeout that uses the constant and function name, making the
intent clearer and improving testability of the upgrade check behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 620c7131-9228-4028-a46a-53c2a26e858e

📥 Commits

Reviewing files that changed from the base of the PR and between 146acea and b301805.

📒 Files selected for processing (1)

• packages/opencode/src/cli/cmd/serve.ts

[cubic-dev-ai]

1 issue found across 1 file

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[ralphstodomingo]

Thanks — the MAJOR is a real bug, confirmed all three claims against the code (bootstrap() finally → Instance.dispose() at cli/bootstrap.ts:12; server default dir = process.cwd() at server/server.ts:196; per-directory global cache in project/instance.ts). Fixed in 49f64ff.

On the fix — I went a slightly different route than the sentinel dir, and I think it's strictly better; flagging so you can sanity-check:

The root issue isn't which directory we dispose — it's that we dispose at all while in-process with the server. The TUI path I was mirroring (tui/worker.ts checkUpgrade) uses raw Instance.provide and never disposes; my mistake was reusing bootstrap(), which adds the finally dispose. So the faithful mirror is just Instance.provide({ directory: process.cwd(), init: InstanceBootstrap, fn: upgrade }) with no dispose.

Why this over the sentinel tmpdir() key:

• No teardown at all — addresses the root concern more directly than relocating the dispose to a throwaway bucket.
• No throwaway instance on a non-existent dir — InstanceBootstrap spins up LSP / FileWatcher / Vcs / Snapshot against Instance.directory; pointing that at a bogus tmpdir path is wasteful and a bit risky. Reusing the process.cwd() instance the server already seeds avoids that.
• Bus notifications still land — Bus.publish fans out to the instance PubSub and GlobalBus tagged with the directory; keeping process.cwd() means UpdateAvailable/Updated still reach default-directory SSE subscribers, which a sentinel key would orphan.

Confirmed upgrade() has no directory-scoped dependency: it reads Config.global() (process-global lazy()) + Installation.* (global); the instance exists only so Bus.publish has an ambient context.

MINOR / NIT / testability — all addressed: extracted the trigger into serve-upgrade-check.ts with named STARTUP_UPGRADE_DELAY_MS (+ the "let the listener settle" rationale), a comment explaining the inner-vs-outer catch split, and unit tests asserting it runs upgrade() exactly once in the process.cwd() instance and never rejects when upgrade() throws. 114 existing upgrade/installation tests still green.

Verification note (re: "how did you confirm the fleet updates"): the serve-side trigger can't unstick the existing 0.7.3 fleet on its own — they won't have this code until they upgrade once (catch-22). That half is handled from the extension in AltimateAI/vscode-altimate-mcp-server#369, which version-checks and re-installs on activation. End-to-end, a stuck machine gets pulled forward by #369; #940 keeps every future serve self-updating.

[Copilot]

Pull request overview

Adds a missing auto-update trigger to the headless altimate serve startup path so extension-launched servers perform the same best-effort upgrade check as the TUI, without blocking request handling.

Changes:

• Schedule a one-shot startup upgrade check shortly after Server.listen() in serve.
• Introduce serve-upgrade-check.ts to run upgrade() inside a process.cwd() Instance context without disposing the shared instance bucket.
• Add a Bun test validating orchestration and non-throwing behavior when upgrade() fails.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
packages/opencode/src/cli/cmd/serve.ts	Triggers the startup auto-update check from the headless server entrypoint.
packages/opencode/src/cli/cmd/serve-upgrade-check.ts	Implements the delayed, best-effort upgrade check within a shared Instance context.
packages/opencode/test/cli/serve-upgrade-check.test.ts	Adds unit coverage for the new startup upgrade check helper.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ralphstodomingo]

Pushed fixes for the CI failure and the remaining review comments.

CI red (TypeScript job) — fixed (d0b7b6e). The failure was SyntaxError: Export named 'isValidVersion' not found in module cli/upgrade.ts, caused by my test: bun's mock.module is process-global, so mocking ../upgrade clobbered compareVersions/isValidVersion for upgrade-decision.test.ts in the combined run (passed locally only because I ran the file in isolation). Exactly the leak @copilot flagged on the test. Rewrote the test to use dependency injection — runStartupUpgradeCheck takes an optional StartupUpgradeDeps ({ provide, run }) defaulting to the real collaborators; the test injects fakes, no global module state touched. Verified in the combined suite + full test/cli: 512 pass, 0 fail.

Copilot nits — fixed (e6c26a1):

• Unused Installation import — removed. Also removed Workspace and Project in the same file (same pre-existing dead-import issue, just not flagged).
• String(err) loses stack/cause — correct; Log.build special-cases Error via formatError (walks .cause). Switched both log.error calls to pass the raw error: err.
• Inaccurate "swallows its own errors" comment — reworded to attribute the never-rejects guarantee to the explicit inner .catch wrapper + outer try/catch, not to upgrade().

Dispose bug (MAJOR / coderabbit / cubic) — already resolved in 49f64ff: no longer wraps in bootstrap() (whose finally disposes), now mirrors the TUI worker with raw Instance.provide on process.cwd() and no dispose. Rationale for choosing that over the sentinel-dir suggestion is in my earlier reply.