feat(cli): make optional-plugin loading intent-driven, fail-fast on declared-but-missing (#1597)#3228
Merged
Merged
Conversation
…1597) `os serve` auto-loaded the optional AI service plugins by speculatively importing the package and swallowing the error, conflating two things: "is the package installed?" (presence) and "does this app want AI?" (intent). Nothing ever failed loud — an app that genuinely REQUIRES AI but ships without the package booted "successfully" in a broken state, and the single catch couldn't tell "intentionally absent" from "plugin crashed at startup" (the #1595 false-alarm class). Drive enablement from declared INTENT instead, resolved deterministically at startup into three states (tier gating stays an orthogonal deny): required (requires: ['ai'|'ai-studio']) load; missing/broken => fail-fast (throw, exit 1) auto (package declared in app pkg) best-effort load off (neither, or tier denies) skip, with NO speculative import - AIService + AIStudio guards resolve via `Serve.resolveOptionalPluginLoad` (pure, unit-tested). Studio gains a `requires: ['ai-studio']` required path (opens the `ai` tier, implies the base service). - The `requires: [...]` capability resolver now fails fast for capabilities the app EXPLICITLY declared; platform-injected defaults (ALWAYS_ON, mcp, ...) stay best-effort. Declared caps are snapshotted before auto-injection. - Missing-vs-crashed detection consolidated into one `Serve.isModuleNotFoundError` (checks err.code first — the #1595 fix), replacing the duplicated string-matches. - apps/cloud constraint preserved: no AI Studio declared => skipped, clean boot (cloud#107); an undeclared Studio is no longer speculatively imported. Spec: document the intent / fail-fast contract on `requires`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TjHfkKmEvgk8v7N8nTe5sH
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 2 package(s): 108 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
…#1597) `os start` "What it boots" listed `requires: [...]` auto-registration but predated the intent-driven fail-fast behavior. Clarify that a declared service capability whose provider package isn't installed aborts boot (fail-fast), and call out that `auth`/`ui` are the tier-gated exceptions with their own opt-in rules. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TjHfkKmEvgk8v7N8nTe5sH
os-zhuang
marked this pull request as ready for review
July 18, 2026 15:58
os-zhuang
pushed a commit
that referenced
this pull request
Jul 18, 2026
…he MCP endpoint on `os dev` boot (#3167) #3167 PR-B — the direction-ratified, non-overlapping half. The HTTP identity-admission e2e proof landed separately (#3228: showcase-based, HIGH-RISK); this rebases onto it and keeps only what that PR did not cover. Decision 2 (off-switch semantics): the MCP HTTP surface and the long-lived stdio transport shared one env var — OS_MCP_SERVER_ENABLED=true turned HTTP on AND silently auto-started the UNSCOPED stdio bridge (raw services, no per-request principal). Split them: - types: new resolveMcpStdioAutoStart(); stdio is now OS_MCP_STDIO_ENABLED (default off). OS_MCP_SERVER_ENABLED governs only HTTP. Legacy OS_MCP_SERVER_ENABLED=true still starts stdio for one release, flagged deprecated (=false only ever gated HTTP, unchanged). - mcp: plugin.start() gates stdio on the new switch + warns once on the legacy alias. The bridgeResources(...) line (mcp-stdio-authority probe key) is untouched; the matrix row's env-var references are synced. Decision 3 (dev affordance): `os dev` prints the MCP endpoint, skill URL, and a ready-to-paste `claude mcp add` command on boot, gated on isMcpServerEnabled() (opted-out deployments advertise nothing). Decision 4 (exposure policy): documented the current default (non-system objects + ai.exposed actions over the ~10-tool spine) as the committed v1 policy in connect-mcp.mdx, with the metadata-authorable direction noted. Also: env-var docs updated for the split (+ deprecation callout); the blank scaffold README documents the serve-side MCP surface; unit tests pin the new switch semantics (canonical / legacy-deprecated / footgun-closed). Changeset covers types+mcp+cli (minor) and create-objectstack (patch). Refs #3167, #3228, ADR-0096, ADR-0097. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0115eg8dAaCfWaDYYAm3ma36
os-zhuang
added a commit
that referenced
this pull request
Jul 18, 2026
…t UX + exposure-policy docs (#3217) Decouple the MCP stdio auto-start switch (OS_MCP_STDIO_ENABLED) from the default-on HTTP surface (OS_MCP_SERVER_ENABLED), closing the footgun where =true silently attached an unscoped stdio bridge; print the MCP endpoint + connect command on `os dev` boot; document the v1 exposure policy and the env-var split. The HTTP identity-admission e2e proof landed separately via #3228; this is the de-duplicated remainder of #3167 PR-B. Refs #3167, #3228, ADR-0096, ADR-0097.
This was referenced Jul 19, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1597. Follow-up to #1595 (which fixed the
Cannot find packagefalse alarm) and the deeper design problem it exposed.Problem
os serveauto-loaded the optional AI service plugins (@objectstack/service-ai,@objectstack/service-ai-studio) by speculatively importing the package and swallowing the error, conflating two distinct questions:Consequences: installing a dependency silently auto-enabled a runtime feature; nothing ever failed loud (an app that genuinely requires AI but ships without the package booted "successfully" in a broken state); and the single catch couldn't distinguish "intentionally absent" from "plugin crashed at startup" — the string-matching that caused #1595.
Design — intent-driven, three states
Enablement is now driven by declared intent, resolved deterministically at startup. Tier gating stays an orthogonal deny.
requires: ['ai' | 'ai-studio'])package.json)Key changes
Serve.resolveOptionalPluginLoad({ tierAllowed, required, declared })— a pure, unit-tested resolver returning'required' | 'auto' | 'off'. Both the AIService and AIStudio guards map over it.requireswhose provider package is missing (or whose plugin throws while starting) now aborts boot with a clear message (the outer boot catch prints it and exits 1) instead ofconsole.warn-ing and booting degraded. Declared caps are snapshotted before the platform auto-injects its convenience defaults (ALWAYS_ON,mcp,pinyin-search,auth→email,queue/job), which stay best-effort.requires: ['ai-studio']required path (maps to theaitier; implies the base service). An undeclared Studio is no longer speculatively imported at all.Serve.isModuleNotFoundError(checkserr.codefirst — the perf(build): OS_SKIP_DTS gating + fix optional AI plugin "Cannot find package" skip #1595 fix), replacing every duplicated string-match inserve.ts(AI ×2, capability resolver, auth, datasource ×2).requiresfield (stack.zod.ts).Constraint preserved —
apps/cloudboots clean (cloud#107)apps/cloudships no AI Studio and must boot clean. It declares neither the Studio package norrequires: ['ai-studio'], so Studio resolves tooff→ skipped, no import, no error. This is strictly better than before: previously an app declaring only the base service still speculatively imported Studio and swallowed the failure.Why this is CI-safe
@objectstack/clidepends on every built-in capability provider package (service-automation,plugin-approvals,service-job,service-messaging,service-package,trigger-*, …), so bareimport(spec.pkg)always resolves wherever the CLI runs. The example apps' declared capabilities all resolve today, so fail-fast never wrongly fires.service-ai/service-ai-studioare external/private — exactly where declared-but-missing should fail fast.Verification
tsc -p packages/cli/tsconfig.build.json --noEmit→ exit 0packages/cli/test/serve-optional-plugin-intent.test.ts(intent resolver +isModuleNotFoundErrorperf(build): OS_SKIP_DTS gating + fix optional AI plugin "Cannot find package" skip #1595 regression guard) → 9 passingserve-defaults,serve-host-config,serve-log-level,serve-automation-summary,adr-0048-app-split→ all passingpackages/specstack.test.ts→ 89 passing; full spec build succeedseslinton changed files → clean🤖 Generated with Claude Code
Generated by Claude Code