fix: 回应独立 CR 的 P2/P3 意见(0×P1)
- P2-1 onOpen 收到真正落地的 tab:url 新建场景从 landed 态按 id 反查
(补丁后对象),回调不再拿到未打 url 的 tab
- P2-2 插件设置行显式取值源:FeatureSettingsRows 新增 valueSource prop,
插件行读自己的 pluginSettings blob——插件键与宿主 pref 同名时不再
静默读宿主值(原投影合并有读宿主写插件的读写不对称);prop 命名避开
Object.prototype.valueOf 冲突
- P2-3 consumer 类型检查接入工程:package.json 新增 check:consumer-types
脚本 + CI 在 build 后执行;脚本补 lib/types 存在性前置检查(防假绿)
- P3-1 url seed 只在新建时应用:dedupe 聚焦的 tab 不再被覆写 path
- P3-2 closeTab/activateTab 未知 id 严格 no-op:store.reduce 对同引用
结果短路(不 notify、不重写 localStorage),根因修复 + 回归测试
- P3-3 closeTab/activateTab 增加可选 scope(cwd 随回调传递),Sidebar
双树激活/关闭路径传入 { sessionId, cwd }
- P3-4 定向打开不再自动展开不可见 session 的面板
- P3-5 Sidebar 终端 pty 释放查找覆盖双树(底部面板终端不再漏掉 HTTP
兜底)
- P3-6/P3-7 与 Nits:AGENTS.md/指南文档化(dedupeKey 纯函数、meta 用
null 清除、回调 scope 的 cwd 语义);consumer fixture 补常量导出覆盖
测试 +6(url 落地保真 / url 不覆写聚焦 / 未知 id 无 notify / 定向打开
不展开 / scope 随回调 / valueSource 键隔离),464 全绿;typecheck、
build、consumer 校验全部通过。
This commit is contained in:
@@ -108,11 +108,6 @@ function viewerOrder(a: FileViewerDescriptor, b: FileViewerDescriptor): number {
|
||||
return (b.priority ?? 0) - (a.priority ?? 0)
|
||||
}
|
||||
|
||||
/** Read one boolean pref by declarative key (missing = false). */
|
||||
function prefBool(prefs: SidebarPrefs, key: string): boolean {
|
||||
return (prefs as unknown as Record<string, boolean>)[key] === true
|
||||
}
|
||||
|
||||
/** Whether a feature declares any secondary settings (gear button shows). */
|
||||
function hasSettings(feature: TabDescriptor | FileViewerDescriptor): boolean {
|
||||
const settings = feature.settings
|
||||
@@ -210,8 +205,15 @@ export function FeatureSettingsRows(props: {
|
||||
* display (clamped for numbers, the current pref when the input is
|
||||
* invalid). Optional: rows with no handler keep their draft. */
|
||||
onCommit?: (toggle: SidebarSettingToggle, raw: string) => string
|
||||
/** Explicit value source (v0.12.0+): when given, rows read their values
|
||||
* from it instead of the `prefs` face — plugin-owned rows read their
|
||||
* own blob, so a plugin key can never collide with (or silently read)
|
||||
* a host pref of the same name. (Named `valueSource`, not `valueOf`:
|
||||
* the latter collides with the inherited Object.prototype.valueOf.) */
|
||||
valueSource?: (key: string) => unknown
|
||||
}) {
|
||||
const { toggles, prefs, onToggle, onCommit } = props
|
||||
const { toggles, prefs, onToggle, onCommit, valueSource } = props
|
||||
const read = valueSource ?? ((key: string): unknown => (prefs as unknown as Record<string, unknown>)[key])
|
||||
return (
|
||||
<div className={css.popupRows}>
|
||||
{toggles.map(toggle => {
|
||||
@@ -225,13 +227,13 @@ export function FeatureSettingsRows(props: {
|
||||
</span>
|
||||
<Switch
|
||||
label={title}
|
||||
checked={prefBool(prefs, toggle.key)}
|
||||
checked={read(toggle.key) === true}
|
||||
onChange={(next) => { onToggle(toggle, next) }}
|
||||
/>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
const value = String((prefs as unknown as Record<string, unknown>)[toggle.key] ?? '')
|
||||
const value = String(read(toggle.key) ?? '')
|
||||
// Keyed by the committed value: a failed commit reverts prefs, the
|
||||
// key changes, and the row remounts with the stored value (typing
|
||||
// never changes the key, so mid-edit drafts survive re-renders).
|
||||
@@ -337,9 +339,11 @@ export function SettingsBody(props: {
|
||||
const toggles = feature.settings?.toggles ?? []
|
||||
const pluginToggles = feature.settings?.pluginToggles ?? []
|
||||
if (toggles.length === 0 && pluginToggles.length === 0) return null
|
||||
// Plugin rows read/write their values through a projected prefs face so
|
||||
// the shared row renderer works unchanged (its rows read prefs[key]).
|
||||
const effectivePrefs = { ...prefs, ...(prefs.pluginSettings[feature.id] ?? {}) } as SidebarPrefs
|
||||
// Plugin rows read their values from the descriptor's OWN blob through
|
||||
// an explicit value source — no projection onto the prefs face, so a
|
||||
// plugin key can never collide with (or silently read) a host pref of
|
||||
// the same name.
|
||||
const pluginBlob = prefs.pluginSettings[feature.id] ?? {}
|
||||
return (
|
||||
<div className={css.popupRows}>
|
||||
{toggles.length > 0 && (
|
||||
@@ -353,9 +357,10 @@ export function SettingsBody(props: {
|
||||
{pluginToggles.length > 0 && (
|
||||
<FeatureSettingsRows
|
||||
toggles={pluginToggles}
|
||||
prefs={effectivePrefs}
|
||||
prefs={prefs}
|
||||
onToggle={onPluginToggle}
|
||||
onCommit={onPluginCommit}
|
||||
valueSource={(key) => pluginBlob[key]}
|
||||
/>
|
||||
)}
|
||||
</div>
|
||||
|
||||
+12
-5
@@ -553,11 +553,17 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) {
|
||||
// TerminalView on unmount), and the agent-pty.close HTTP route is the
|
||||
// fallback when the WS is down.
|
||||
const current = store.getSnapshot().state
|
||||
const leaf = current === undefined ? undefined : leafWithTab(current.splits, tabId)
|
||||
// Terminal tabs may live in EITHER tree (the bottom panel hosts them
|
||||
// too) — the pty-release lookup covers both, or the HTTP fallback is
|
||||
// skipped for a bottom-panel terminal whose WS frame never arrived.
|
||||
const leaf = current === undefined
|
||||
? undefined
|
||||
: leafWithTab(current.splits, tabId) ?? leafWithTab(current.bottomSplits, tabId)
|
||||
const tab = leaf?.tabs.find(candidate => candidate.id === tabId)
|
||||
// Route through the service: the tab-bar close is the canonical close
|
||||
// path (finds the pane itself, fires descriptor.onClose).
|
||||
ctx.betterSidebar?.closeTab(tabId)
|
||||
// path (finds the pane itself, fires descriptor.onClose); the session
|
||||
// scope (with its cwd) rides to the callback.
|
||||
ctx.betterSidebar?.closeTab(tabId, sessionId === undefined ? undefined : { sessionId, cwd })
|
||||
if (tab?.type === 'terminal') {
|
||||
if (isAgentTabId(tabId)) {
|
||||
const uuid = agentUuidOf(tabId)
|
||||
@@ -569,8 +575,9 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) {
|
||||
},
|
||||
activateTab: (paneId, tabId) => {
|
||||
// Route through the service: same reducer (finds the pane in EITHER
|
||||
// tree, sets the active pane) and fires descriptor.onActivate.
|
||||
ctx.betterSidebar?.activateTab(tabId)
|
||||
// tree, sets the active pane) and fires descriptor.onActivate; the
|
||||
// session scope (with its cwd) rides to the callback.
|
||||
ctx.betterSidebar?.activateTab(tabId, sessionId === undefined ? undefined : { sessionId, cwd })
|
||||
},
|
||||
focusPane: (paneId) => { store.reduce(s => ({ ...s, activePane: paneId })) },
|
||||
moveTabToEdge: (payload: TabDragPayload, toPane: string, zone: DropZone) => {
|
||||
|
||||
+55
-32
@@ -321,8 +321,13 @@ export interface BetterSidebarService {
|
||||
* refuse `openTab` (only the settings disable switch does).
|
||||
*/
|
||||
openTab(seed: OpenTabSeed, scope?: SessionScope): void
|
||||
/** Close a tab by id. */
|
||||
closeTab(tabId: string): void
|
||||
/**
|
||||
* Close a tab by id (fires descriptor.onClose). An unknown tab id is a
|
||||
* strict no-op (no state churn, no callbacks). `scope` (v0.12.0+) rides
|
||||
* to the callback (its optional cwd included); absent, the callback gets
|
||||
* `{ sessionId }` of the active session.
|
||||
*/
|
||||
closeTab(tabId: string, scope?: SessionScope): void
|
||||
/** Subscribe to registry changes (register/dispose). */
|
||||
subscribe(listener: () => void): () => void
|
||||
/** The plugin version this service instance was built from ('0.12.0'). */
|
||||
@@ -344,8 +349,12 @@ export interface BetterSidebarService {
|
||||
subscribeState(listener: () => void): () => void
|
||||
/** Update an open tab's display fields (title / path / meta); a missing tab id is a no-op. */
|
||||
updateTab(tabId: string, patch: { title?: string; path?: string; meta?: unknown }): void
|
||||
/** Activate an open tab (the tab-bar activation path; fires descriptor.onActivate). */
|
||||
activateTab(tabId: string): void
|
||||
/**
|
||||
* Activate an open tab (the tab-bar activation path; fires
|
||||
* descriptor.onActivate). An unknown tab id is a strict no-op. `scope`
|
||||
* (v0.12.0+) rides to the callback like `closeTab`'s.
|
||||
*/
|
||||
activateTab(tabId: string, scope?: SessionScope): void
|
||||
/** Open a file in the sidebar editor of `scope`'s session (title defaults to the file name). */
|
||||
openFile(scope: SessionScope, path: string, title?: string): void
|
||||
}
|
||||
@@ -503,6 +512,11 @@ export function createBetterSidebarService(store: SidebarStore): BetterSidebarSe
|
||||
const targetSessionId = scope?.sessionId ?? store.getSnapshot().sessionId
|
||||
if (targetSessionId === undefined) return
|
||||
const callbackScope: SessionScope = scope ?? { sessionId: targetSessionId }
|
||||
// Whether this open targets a session that is NOT the one on screen: a
|
||||
// targeted open must not auto-expand panels the user cannot see (the
|
||||
// expansion is about landing "in sight" for the CURRENT viewer).
|
||||
const activeSessionId = store.getSnapshot().sessionId
|
||||
const targetsInactiveSession = scope !== undefined && scope.sessionId !== activeSessionId
|
||||
// Lifecycle capture: `created` when the open minted a NEW tab (a
|
||||
// dedupe/id-safety-net focus is an ACTIVATION, not an open).
|
||||
let created: SidebarTab | undefined
|
||||
@@ -530,20 +544,7 @@ export function createBetterSidebarService(store: SidebarStore): BetterSidebarSe
|
||||
}
|
||||
next = applyDedupe(state, tab, descriptor)
|
||||
}
|
||||
// A URL seed pre-fills the tab's path (the browser tab navigates to it
|
||||
// on mount). An explicit seed.title also wins over a createTab-minted
|
||||
// default title (e.g. the sidebar-browser's hostname title).
|
||||
let landed: SidebarState
|
||||
if (seed.url !== undefined) {
|
||||
landed = patchTab(next, tab.id, {
|
||||
path: seed.url,
|
||||
...(seed.title !== undefined ? { title: seed.title } : {}),
|
||||
})
|
||||
} else {
|
||||
landed = next
|
||||
}
|
||||
// Lifecycle capture (before the auto-expand block, which early-returns).
|
||||
// Classify the landing against the INPUT state: a FOCUS fires
|
||||
// Classify the landing against the INPUT state FIRST: a FOCUS fires
|
||||
// onActivate with the tab that is active NOW; a real creation fires
|
||||
// onOpen with the minted tab. Both the dedupeKey match AND the id
|
||||
// match count as a focus — a descriptor deduping by key (e.g. editor
|
||||
@@ -556,8 +557,24 @@ export function createBetterSidebarService(store: SidebarStore): BetterSidebarSe
|
||||
const existedByKey = key !== undefined
|
||||
&& inputTabs.some(candidate => candidate.type === tab.type && dedupeKey!(candidate) === key)
|
||||
const existedById = tabOpenIn(state, tab.id)
|
||||
if (!existedByKey && !existedById) {
|
||||
created = tab
|
||||
const isCreation = !existedByKey && !existedById
|
||||
// A URL seed pre-fills a NEWLY CREATED tab's path (the browser tab
|
||||
// navigates to it on mount); a FOCUS must never have its path
|
||||
// overwritten. An explicit seed.title still wins over a createTab-
|
||||
// minted default title (e.g. the sidebar-browser's hostname title).
|
||||
let landed: SidebarState = next
|
||||
if (seed.url !== undefined && isCreation) {
|
||||
landed = patchTab(next, tab.id, {
|
||||
path: seed.url,
|
||||
...(seed.title !== undefined ? { title: seed.title } : {}),
|
||||
})
|
||||
}
|
||||
// Lifecycle capture (before the auto-expand block, which early-returns).
|
||||
if (isCreation) {
|
||||
// Resolve the ACTUAL landed tab — the url patch mints a new object,
|
||||
// so the callback must see the tab that was really inserted.
|
||||
const landedTabs = allLeaves(landed.splits).concat(allLeaves(landed.bottomSplits)).flatMap(leaf => leaf.tabs)
|
||||
created = landedTabs.find(candidate => candidate.id === tab.id) ?? tab
|
||||
} else {
|
||||
// A focus happened: resolve the tab that is actually active now and
|
||||
// report THAT to onActivate (never the caller's un-inserted seed).
|
||||
@@ -576,9 +593,11 @@ export function createBetterSidebarService(store: SidebarStore): BetterSidebarSe
|
||||
// agent-terminal auto-tabs) never expand (the panel behavior is their
|
||||
// caller's business). The check runs on the post-dedupe state, so a
|
||||
// content open that merely FOCUSES an existing tab expands the panel
|
||||
// too — the open must never land out of sight.
|
||||
// too — the open must never land out of sight. Opens targeted at an
|
||||
// INACTIVE session never expand (nothing is in sight for the user).
|
||||
if (
|
||||
typeof window !== 'undefined'
|
||||
!targetsInactiveSession
|
||||
&& typeof window !== 'undefined'
|
||||
&& (seed.path !== undefined || seed.url !== undefined)
|
||||
) {
|
||||
if (isNarrowWidth(window.innerWidth)) {
|
||||
@@ -597,8 +616,7 @@ export function createBetterSidebarService(store: SidebarStore): BetterSidebarSe
|
||||
// A scope targeting ANOTHER session lands the open there without
|
||||
// switching the UI; a scope naming the active session (or no scope)
|
||||
// takes the regular reduce path so the UI notifies and re-renders.
|
||||
const activeSessionId = store.getSnapshot().sessionId
|
||||
if (scope !== undefined && scope.sessionId !== activeSessionId) {
|
||||
if (targetsInactiveSession) {
|
||||
store.reduceFor(scope.sessionId, reducer)
|
||||
} else {
|
||||
store.reduce(reducer)
|
||||
@@ -607,20 +625,23 @@ export function createBetterSidebarService(store: SidebarStore): BetterSidebarSe
|
||||
else if (activated !== undefined) safeCall(() => descriptor.onActivate?.(activated!, callbackScope))
|
||||
}
|
||||
|
||||
const closeTab = (tabId: string): void => {
|
||||
const closeTab = (tabId: string, scope?: SessionScope): void => {
|
||||
let closed: SidebarTab | undefined
|
||||
store.reduce((state) => {
|
||||
// Unknown tab ids are a strict no-op: no state churn, no notify, no
|
||||
// pointless localStorage rewrite (mirrors updateTab's short-circuit).
|
||||
if (!tabOpenIn(state, tabId)) return state
|
||||
const paneId = findPaneIdOf(state, tabId)
|
||||
if (paneId === '') return state
|
||||
const leaf = leafWithTab(state[treeOf(state, paneId)], tabId)
|
||||
closed = leaf?.tabs.find(tab => tab.id === tabId)
|
||||
return closeTabReducer(state, paneId, tabId)
|
||||
})
|
||||
if (closed !== undefined) {
|
||||
const sessionId = store.getSnapshot().sessionId
|
||||
const sessionId = scope?.sessionId ?? store.getSnapshot().sessionId
|
||||
if (sessionId !== undefined) {
|
||||
const descriptor = tabs.get(closed.type)
|
||||
safeCall(() => descriptor?.onClose?.(closed!, { sessionId }))
|
||||
// An explicit scope (with its optional cwd) rides to the callback.
|
||||
safeCall(() => descriptor?.onClose?.(closed!, scope ?? { sessionId }))
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -641,20 +662,22 @@ export function createBetterSidebarService(store: SidebarStore): BetterSidebarSe
|
||||
}
|
||||
|
||||
/** Activate an open tab (the tab-bar activation path; fires onActivate). */
|
||||
const activateTab = (tabId: string): void => {
|
||||
const activateTab = (tabId: string, scope?: SessionScope): void => {
|
||||
let activated: SidebarTab | undefined
|
||||
store.reduce((state) => {
|
||||
// Unknown tab ids are a strict no-op (no state churn / notify).
|
||||
if (!tabOpenIn(state, tabId)) return state
|
||||
const paneId = findPaneIdOf(state, tabId)
|
||||
if (paneId === '') return state
|
||||
const leaf = leafWithTab(state[treeOf(state, paneId)], tabId)
|
||||
activated = leaf?.tabs.find(tab => tab.id === tabId)
|
||||
return activateTabReducer(state, paneId, tabId)
|
||||
})
|
||||
if (activated !== undefined) {
|
||||
const sessionId = store.getSnapshot().sessionId
|
||||
const sessionId = scope?.sessionId ?? store.getSnapshot().sessionId
|
||||
if (sessionId !== undefined) {
|
||||
const descriptor = tabs.get(activated.type)
|
||||
safeCall(() => descriptor?.onActivate?.(activated!, { sessionId }))
|
||||
// An explicit scope (with its optional cwd) rides to the callback.
|
||||
safeCall(() => descriptor?.onActivate?.(activated!, scope ?? { sessionId }))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+9
-1
@@ -1025,6 +1025,11 @@ export class SidebarStore {
|
||||
const state = this.snapshot.state
|
||||
if (sessionId === undefined || state === undefined) return
|
||||
const next = reducer(state)
|
||||
// A reducer returning the SAME reference means "no change": skip the
|
||||
// persist + notify entirely — strict no-op paths (unknown tab ids,
|
||||
// patchTab on a missing tab) must not churn the state or rewrite
|
||||
// localStorage.
|
||||
if (next === state) return
|
||||
this.bySession.set(sessionId, next)
|
||||
this.snapshot = { sessionId, state: next, prefs: this.prefs }
|
||||
this.schedulePersist(sessionId, next)
|
||||
@@ -1056,8 +1061,11 @@ export class SidebarStore {
|
||||
nextIdCounter = maxCounterId(state)
|
||||
}
|
||||
const next = reducer(state)
|
||||
this.bySession.set(sessionId, next)
|
||||
// Same-reference result = no change: keep the counter restore (it may
|
||||
// have been seeded down) but skip the write.
|
||||
nextIdCounter = Math.max(nextIdCounter, counterBefore)
|
||||
if (next === state) return
|
||||
this.bySession.set(sessionId, next)
|
||||
this.schedulePersist(sessionId, next)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user