Stop overlays from rendering fixture data on missing dependencies - #366
Merged
Conversation
Settings, plugins, resume, and mentions all read-site-defaulted to hardcoded rows from residuals.ts whenever a caller omitted items, indistinguishable from real content. Settings now surfaces an honest message when its dependency is missing; help always renders the shell's real keybinding catalog; the overlay openers require items from their caller instead of silently substituting a fixture. The in-shell /plugins and /settings palette entries duplicated slash commands already wired to real data and are dropped. /resume had no backing session source in-session (resume only exists as a pre-launch picker) and is dropped rather than left pointing at fake sessions. /mentions now drives the real @-mention path instead of opening a fixture list. The observe fixture fallback is removed; a missing host handler now leaves observe closed with an honest status line, matching the existing empty-state message. Fixture data used only by the interactive demo now lives in demo.ts, outside the production module graph.
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
residuals.tsheld demo/fixture rows (settings, help, plugins, resume, mentions, a fake child-session transcript) that were exported fromindex.tsand compiled into the production bundle.shell.ts's overlay openers used them as read-site defaults (items: opts?.items ?? makeSettingsItems()), so a caller that omitteditemsrendered fabricated content indistinguishably from real data. Two call sites did exactly that in production:openSettingsSurfacewhen its settings dependency was missing, andopenHelpOverlay(shell)on every call.Fix
OpenResidualListOpts.itemsis now required — the overlay openers (openSettingsOverlay,openMentionsOverlay) no longer have a fixture to fall back to; TypeScript enforces that every caller supplies real data.openHelpOverlayalways renders the shell's real keybinding catalog (SHELL_SHORTCUTS) — it never took a host dependency, so there was nothing to be missing; the read-site-default shape it copied from the others is gone.openSettingsSurfacenow callsdeps.notify(...)when its settings dependency is missing, matching the pattern already used byopenPermissionsSurface/openPluginsSurface/openHooksSurface/openMcpSurface— an honest message instead of a painted overlay.onObserveRequest ? onObserveRequest() : makeObserveFixture()) is gone; a missing host handler now leaves observe closed with the existing "no subagent session to observe" status line.residuals.tsis reduced to genuinely production logic:ResidualCatalogEntry,ResidualListPayload,ObserveSession,residualListFromCatalog,residualIdFromSelection. All fixture generators moved intodemo.ts, which is not exported fromindex.tsand never ships./resume and /plugins
/plugins: real plugin administration already exists —openPluginsSurfaceincommand-surfaces.tsis wired todeps.pluginsand is discoverable via the registry-backed/pluginsslash command (which already shadows the palette's residualpluginsentry viapreferRegistry). The residual palette entry was pure duplication of a path that only ever rendered the fixture, so I removed it (DEFAULT_PALETTE_COMMANDS,PaletteActionId, and therunPaletteActioncase)./settings: same story — the registry-backed/settingscommand already routes throughopenSettingsSurfacewith real data, and the residual palette entry always shadowed itself in production. Removed for the same reason./resume: I searched for a real in-session resume data source and found none.pick-session.ts/list-modal.tsimplement a real resume flow, but only as a pre-launch picker before the shell starts — there is no wiring to list or resume sessions from inside a running shell (ShellOverlayHooks.onResumewas never set by any production caller). This matches the operator report cited in the issue ("not sure what this is"). Since there's nothing real to wire it to, I removed it from the palette rather than leave an overlay that can only ever show fake sessions./mentions: this one does have a real backing implementation (openAtMentionSuggestions, which reads the live@-token under the cursor via the injectableMentionSuggestionSource). The palette's residual "mentions" action was bypassing it and opening the fixture list directly; it now delegates to the real function, so the palette action either shows the same suggestions as typing@or honestly does nothing when there's no@token to complete.Validation
bun run typecheck,bun run build,bun run testall pass (3985 tests, 0 fail).overlay-fixture-fallback.test.tsas a red/green test: it opens settings with no settings dependency and asserts none of the old fixture strings appear — committed failing first, confirmed red, then made to pass by the fix.wave7.test.tsandoverlay-paint.test.tsto pass real items into the now-required-items openers instead of relying on the fixture fallback; updatedobserve-live.test.ts's "falls back to the fixture" test to assert the honest empty state instead.bun src/tui-opentui/demo.ts) in a real TTY via tmux and captured frames:h(help) renders the actualSHELL_SHORTCUTScatalog (verified againstSHELL_SHORTCUTS.map(...)in the frame), and the demo's own settings overlay (now sourced from a localDEMO_SETTINGS_ITEMSindemo.ts, not the production module) still renders correctly.Judgment calls / other notes
ShellOverlayHooks(onSettings,onPlugins,onResume,onMentions) and the"resume"/"plugins"PrimaryOverlayKindvalues alone — they're generic accept-dispatch plumbing and the"resume"kind is still legitimately used bylist-modal.ts's pre-launch picker. Out of scope for this fix.shell.ts(palette/overlay-geometry and model-surface branches); rebased ontoorigin/mainbefore opening this PR and resolved one small conflict inpalette.tswhere upstream had already dropped thecategoryfield fromPaletteCommand.