Repository navigation
Conversation
…ints Follow-up to #563's `f306d4c2` (OutputsStack + CancelStack call `loadPreEnginePlugins`): the correctness fix was the right ship, but it shifted every `sc stack outputs` / `sc cancel` invocation from ~200ms to ~1.5-3s on a forge-infra-sized stack (6 declared plugins spawned + Schema RPC'd on every invocation). ## What - New `preEnginePluginBundle.LoadForBackend(ctx, specs)` — cache-aware variant. For each declared plugin: - Consult the per-version schema cache at `~/.sc/plugins/<repo>/<version>/.schema-cache.json` (already written by `pkg/stackread/stackread.go`'s `loadSchema` on every full provision/deploy). - If the cached schema advertises NEITHER `state_store_types` NOR `secrets_provider_types`, skip the plugin — its sidecars don't contribute to backend open, so `sc stack outputs` / `sc cancel` don't need a live client. - If the cached schema does advertise one, spawn the plugin so its sidecar has a live client (same shape as full `Load`). - Cache miss → fall back to `loadOne` (full spawn). Preserves correctness: a stack with no cached schema (fresh install, `path:` spec, corrupt cache) still gets every plugin loaded, same as today's broken-slow shape. Peer's reflection gate test in `preengine_entrypoints_test.go` (f306d4c) uses a `path:` spec, so it still exercises the cache-miss → full-spawn fallback — passes unchanged. - Added `schemaContributesToBackend(schema)` — reports whether a plugin's schema includes any type SC core's backend-open path consults (state_store_types + secrets_provider_types). - `provisioner.go`: new `loadPreEnginePluginsForBackend` wrapper mirrors `loadPreEnginePlugins`'s shape (secrets-provider resolve + bundle tracking) but routes through `LoadForBackend`. - `preview.go` + `cancel.go`: wire the two read-only / teardown entry points to `loadPreEnginePluginsForBackend`. Everything else (the six write-path entries — ProvisionStack, DeployStack, PreviewStack, PreviewChildStack, DestroyParentStack, DestroyChildStack) stays on full `Load` for correctness. ## Tests - `TestLoadForBackend_CacheSkipsPluginWithoutBackendTypes` — seeds the cache with a schema advertising only `resource_types`, verifies the bundle stays empty (no spawn, no sidecar registered). The plugin binary does not exist at the resolved path, so a spawn attempt would fail with the loud "plugin failed to exit gracefully" shape; the silent pass proves the cache-hit short-circuit fired. - `TestLoadForBackend_CacheMissFallsBackToFullSpawn` — `path:` spec (bypasses the cache entirely), binary does not exist, verifies the fallback hits `loadOne` and fails the spawn. Correctness guard: the fallback path is exercised, not stubbed. - `TestBackendEntryPointsLoadPreEnginePlugins` (peer's reflection gate on OutputsStack/CancelStack + the other six) still passes unchanged — the fallback invariant holds for the path: specs the test uses. ## Correctness A stale cache WRONGLY saying "no state-store types" would skip a plugin that does provide one, reproducing exactly the empty-URL bug `f306d4c2` fixed. The cache is version-keyed (`<repo>/<version>/.schema-cache.json`), so a plugin version bump invalidates it automatically — bounds the window to the plugin version's own lifetime. A plugin author who adds a state-store type mid-version without bumping SHOULD ship a new version; if they don't, `sc stack outputs` fails loudly with "unsupported state-store type %q" from login.go (same shape as today's full load fails on an unknown type — not silently). This bound is documented inline on the method. ## Follow-up (not in this PR) - The cache path handles `github-release` specs cleanly (version-keyed). `path:` specs always miss and take the full spawn fallback, which matches dev-mode semantics (`path:` is "I'll change the binary any time, don't cache my schema"). No change needed there. - A follow-up could warm the cache during `sc stack outputs` by writing the just-loaded schemas back to disk after a cache miss — currently only `pkg/stackread/stackread.go` writes the cache, so the first `outputs`-only session pays the full spawn for every invocation until a provision/deploy warms it. Defer; most operators run `outputs` after a deploy, so the cache is already warm. ## Roadmap - forge/docs/roadmap/items/sc-plugin-stack-outputs-latency-full-schema-load.md Signed-off-by: Ilya Sadykov <smecsia@gmail.com>
smecsia
requested review from
Cre-eD,
Laboratory and
universe-ops
as code owners
November 8, 2026 17:20
Semgrep Scan ResultsRepository:
Scanned at 2026-11-08 19:04 UTC |
Security Scan ResultsRepository:
Scanned at 2026-11-08 19:04 UTC |
universe-ops
approved these changes
Nov 8, 2026
baasforger
approved these changes
Nov 8, 2026
baasforger
enabled auto-merge (squash)
November 8, 2026 18:12
…container-com/api into feat/pre-engine-cache-aware
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.
Summary
Follow-up to #563's
f306d4c2. That commit madeOutputsStack+CancelStackload pre-engine plugins before touching the backend (empty-URL fix). This PR
trims the subset of plugins loaded on those two paths using the
already-maintained per-version schema cache — read-only entry points stop
paying the full ~1.5–3s latency tax on every invocation.
The six write-path entries (Provision/Deploy/Preview/PreviewChild/Destroy{Parent,Child})
stay on full
Load— correctness-critical.What
preEnginePluginBundle.LoadForBackend(ctx, specs): cache-awarevariant. For each declared plugin:
~/.sc/plugins/<repo>/<version>/.schema-cache.json(populated by
pkg/stackread/stackread.go'sloadSchemaon everyprovision/deploy).
state_store_typesNORsecrets_provider_types, skip the plugin — its sidecars don'tcontribute to backend open.
Load).loadOne. Preserves correctness.schemaContributesToBackend(schema): the gate function.provisioner.loadPreEnginePluginsForBackend: wrapper mirroringloadPreEnginePlugins's shape (secrets-provider resolve + bundletracking).
preview.go+cancel.go: wire the two entries to the new wrapper.Correctness invariant
Peer's reflection gate (
TestBackendEntryPointsLoadPreEnginePlugins,f306d4c2) uses apath:spec which bypasses the cache entirely → thefallback path is exercised. Still passes unchanged.
A stale cache WRONGLY saying "no state-store types" would skip a plugin
that does provide one, reproducing the empty-URL bug `f306d4c2` fixed. The
cache is version-keyed (
<repo>/<version>/.schema-cache.json), so a pluginversion bump invalidates it automatically — bounded window. A plugin author
adding a state-store type mid-version without bumping fails loudly at
`login.go` with "unsupported state-store type" (same shape as a full load
on an unknown type).
Tests
TestLoadForBackend_CacheSkipsPluginWithoutBackendTypes: seeds cachewith a resource-only schema, verifies the bundle stays empty (no spawn,
no sidecar registered). Binary doesn't exist at the resolved path, so
a spawn would fail loudly — silent pass proves the short-circuit fired.
TestLoadForBackend_CacheMissFallsBackToFullSpawn:path:spec bypassescache, verifies fallback hits
loadOneand fails the spawn. Correctnessguard: fallback path is exercised, not stubbed.
TestBackendEntryPointsLoadPreEnginePlugins(peer's 8-entry reflectiongate) still passes unchanged.
go test ./pkg/clouds/pulumi/ ./pkg/plugin/... -count=1green locally (40s / 4.3s / 0.07s).
Follow-up (not in this PR)
outputs-only session (no provision/deploy in the same session) stillpays the full spawn once per plugin version — the cache gets warmed by the
full
Loadpaths. Most operators runoutputsafter a deploy, so the cacheis already warm. A "warm during outputs" follow-up could write the schemas
back on cache miss; defer until measured as a real pain point.
Roadmap
Test plan
go build ./...go test ./pkg/clouds/pulumi/ ./pkg/plugin/... -count=1— all green locallygofumpt+gci(this repo's 6-section config) appliedsc stack outputs -s infralatency against forge-infra;expected target sub-500ms (the item's falsifiable acceptance)