From 90e79b04a4b6f0fdf850ae6b2d7b8006faadc9d9 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 04:26:39 +0000 Subject: [PATCH 01/17] fix(sessions): fall back when lastDirectory is a deleted worktree New chats inherited a persisted lastDirectory even after that worktree was removed, so the first message saved but the prompt never started. Validate the implicit draft directory, fall back to the active project only when OpenCode confirms the path is missing, and leave explicit worktree targets and unknown probes unchanged. Co-authored-by: serkraser --- CHANGELOG.md | 1 + packages/ui/src/lib/opencode/client.test.ts | 21 +++ packages/ui/src/lib/opencode/client.ts | 40 +++++- packages/ui/src/sync/DOCUMENTATION.md | 3 +- packages/ui/src/sync/session-ui-store.test.js | 124 ++++++++++++++++++ packages/ui/src/sync/session-ui-store.ts | 63 ++++++++- packages/vscode/CHANGELOG.md | 4 + 7 files changed, 246 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a980e698..c047c348 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ All notable changes to this project will be documented in this file. ## [Unreleased] +- Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. - **Usage/Claude:** Claude plan limits now work when you are signed in through Claude Code, without also signing into Anthropic in OpenCode; the account is read from Claude Code's own login on macOS, Linux, and WSL. The page shows your session and weekly limits again, adds per-model weekly limits and extra usage spending, and names your plan. Limits are kept on screen instead of disappearing when Anthropic temporarily blocks refreshes. - **Settings/Integrations:** a new Integrations settings page lists Claude Code, Command Code, and Cursor plugins with install, update, setup, and remove actions, plus Discord and Telegram Coming soon placeholders. - Chat: opening a busy subagent in the context panel now shows its history instead of only the working-status line (thanks to @makeittech). diff --git a/packages/ui/src/lib/opencode/client.test.ts b/packages/ui/src/lib/opencode/client.test.ts index a691a79d..824b5827 100644 --- a/packages/ui/src/lib/opencode/client.test.ts +++ b/packages/ui/src/lib/opencode/client.test.ts @@ -9,6 +9,7 @@ let configCalls = 0; let runtimeKey = 'test-runtime'; const promptAsyncCalls: unknown[][] = []; const promptAsyncResults: Array = []; +const pathGetResults: Array = []; const promptAsyncMock = mock(async (...args: unknown[]) => { promptAsyncCalls.push(args); @@ -17,6 +18,12 @@ const promptAsyncMock = mock(async (...args: unknown[]) => { return next ?? { response: new Response(null, { status: 200 }) }; }); +const pathGetMock = mock(async () => { + const next = pathGetResults.shift(); + if (next instanceof Error) throw next; + return next ?? { data: { directory: '/workspace/project' } }; +}); + mock.module('@opencode-ai/sdk/v2', () => ({ createOpencodeClient: mock(() => ({ config: { @@ -30,6 +37,9 @@ mock.module('@opencode-ai/sdk/v2', () => ({ session: { promptAsync: promptAsyncMock, }, + path: { + get: pathGetMock, + }, })), })); @@ -64,6 +74,17 @@ beforeEach(() => { runtimeKey = 'test-runtime'; promptAsyncCalls.length = 0; promptAsyncResults.length = 0; + pathGetResults.length = 0; +}); + +describe('opencodeClient directory availability', () => { + test('distinguishes a missing directory from an unavailable path probe', async () => { + pathGetResults.push({ error: { code: 'ENOENT', message: 'no such file or directory' } }); + expect(await opencodeClient.getDirectoryAvailability('/private/deleted-worktree')).toBe('missing'); + + pathGetResults.push(new Error('offline')); + expect(await opencodeClient.getDirectoryAvailability('/private/deleted-worktree')).toBe('unknown'); + }); }); describe('opencodeClient getConfig cache', () => { diff --git a/packages/ui/src/lib/opencode/client.ts b/packages/ui/src/lib/opencode/client.ts index c817acb3..1af33e97 100644 --- a/packages/ui/src/lib/opencode/client.ts +++ b/packages/ui/src/lib/opencode/client.ts @@ -68,6 +68,21 @@ type SdkResult = { response?: { status?: number }; }; +export type DirectoryAvailability = "available" | "missing" | "unknown"; + +const isMissingDirectoryError = (error: unknown): boolean => { + if (error instanceof FilesystemError) { + return error.reason === "not-found" || error.reason === "not-directory"; + } + if (error && typeof error === "object") { + const code = (error as { code?: unknown }).code; + if (code === "ENOENT" || code === "ENOTDIR") { + return true; + } + } + return /\bENOENT\b|\bENOTDIR\b|no such file or directory/i.test(formatSdkError(error)); +}; + function unwrapSdkData(result: SdkResult, operation: string): T { if (result.error) { const status = result.response?.status; @@ -506,17 +521,28 @@ class OpencodeService { * This is intentionally NOT the same as local filesystem access in the UI runtime. */ async probeDirectory(directory: string): Promise { + return (await this.getDirectoryAvailability(directory)) === "available"; + } + + /** + * Distinguishes a confirmed-missing directory from an unavailable probe. + * Offline, permission, and other transport failures stay `unknown` so callers + * do not treat a temporary outage as proof the path was deleted. + */ + async getDirectoryAvailability(directory: string): Promise { const normalized = this.normalizeCandidatePath(directory); if (!normalized) { - return false; + return "unknown"; } try { - const response = await this.client.path.get({ directory: normalized }); - const info = response.data as { directory?: unknown } | undefined; - const returned = typeof info?.directory === 'string' ? info.directory : null; - return Boolean(returned && returned.trim().length > 0); - } catch { - return false; + const response = await this.client.path.get({ directory: normalized }) as SdkResult<{ directory?: unknown }>; + if (response.error) { + return isMissingDirectoryError(response.error) ? "missing" : "unknown"; + } + const returned = typeof response.data?.directory === "string" ? response.data.directory.trim() : ""; + return returned ? "available" : "unknown"; + } catch (error) { + return isMissingDirectoryError(error) ? "missing" : "unknown"; } } diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index 7dde6772..f094e0e4 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -251,7 +251,8 @@ Rules: 4. Sending after a revert commits the new branch optimistically: remove the reverted tail and marker before inserting the new message, and restore both if the send is rejected. 5. Composer and queued sends carry their captured runtime, directory, and session through asynchronous preparation. A runtime change cancels the send instead of re-resolving it against the new runtime. 6. After session creation, the directory returned by the server is authoritative over the requested draft directory. The server may canonicalize a worktree path, and the first prompt must use the same directory identity as the created session. -7. A prompt send that fails **after** the request left the client is ambiguous, never a definite failure: the server may already be answering it. Transports tag those errors (`markAmbiguousTransportFailure` in `@/lib/relay/transport-error`; the relay tunnel tags every stream that dies with a request in flight), and `isAmbiguousSendFailure` reads the tag before falling back to status/text heuristics. An ambiguous failure waits for the connection to return, refetches recent messages, and confirms the optimistic message in place instead of rolling it back — rolling it back lets the message queue re-send a prompt the engine is already running, producing two independent AI responses for one user message. +7. Regular new-chat drafts that inherit the persisted current/last directory must not create a session against a confirmed-missing path. Fall back to the active project only when OpenCode reports the directory missing; keep explicit worktree targets, in-flight worktree creation, and unknown/offline probes unchanged, and do not persist the fallback until session creation succeeds. +8. A prompt send that fails **after** the request left the client is ambiguous, never a definite failure: the server may already be answering it. Transports tag those errors (`markAmbiguousTransportFailure` in `@/lib/relay/transport-error`; the relay tunnel tags every stream that dies with a request in flight), and `isAmbiguousSendFailure` reads the tag before falling back to status/text heuristics. An ambiguous failure waits for the connection to return, refetches recent messages, and confirms the optimistic message in place instead of rolling it back — rolling it back lets the message queue re-send a prompt the engine is already running, producing two independent AI responses for one user message. Examples of global-store updates performed in `session-actions.ts`: diff --git a/packages/ui/src/sync/session-ui-store.test.js b/packages/ui/src/sync/session-ui-store.test.js index 23ace4aa..565d5294 100644 --- a/packages/ui/src/sync/session-ui-store.test.js +++ b/packages/ui/src/sync/session-ui-store.test.js @@ -9,6 +9,7 @@ import { useSkillsStore } from '@/stores/useSkillsStore'; import { useCommandsStore } from '@/stores/useCommandsStore'; import { useConfigStore } from '@/stores/useConfigStore'; import { getRuntimeKey } from '@/lib/runtime-switch'; +import { getDeferredSafeStorage } from '@/stores/utils/safeStorage'; /** * Unit tests for session worktree routing through the authoritative store. @@ -408,9 +409,21 @@ describe('openNewSessionDraft project binding', () => { describe('createSession draft lifecycle', () => { let originalCreateSession; + let originalGetDirectoryAvailability; + let originalProjects; + let originalActiveProjectId; + let originalDirectoryState; + let originalClientDirectory; + let originalLastDirectory; beforeEach(() => { originalCreateSession = opencodeClient.createSession; + originalGetDirectoryAvailability = opencodeClient.getDirectoryAvailability; + originalProjects = useProjectsStore.getState().projects; + originalActiveProjectId = useProjectsStore.getState().activeProjectId; + originalDirectoryState = useDirectoryStore.getState(); + originalClientDirectory = opencodeClient.getDirectory(); + originalLastDirectory = getDeferredSafeStorage().getItem('lastDirectory'); useSessionUIStore.setState({ currentSessionId: null, currentSessionDirectory: null, @@ -420,6 +433,15 @@ describe('createSession draft lifecycle', () => { afterEach(() => { opencodeClient.createSession = originalCreateSession; + opencodeClient.getDirectoryAvailability = originalGetDirectoryAvailability; + useProjectsStore.setState({ projects: originalProjects, activeProjectId: originalActiveProjectId }); + useDirectoryStore.setState(originalDirectoryState, true); + opencodeClient.setDirectory(originalClientDirectory ?? undefined); + if (originalLastDirectory === null) { + getDeferredSafeStorage().removeItem('lastDirectory'); + } else { + getDeferredSafeStorage().setItem('lastDirectory', originalLastDirectory); + } }); test('keeps the draft open when session creation fails', async () => { @@ -433,6 +455,108 @@ describe('createSession draft lifecycle', () => { expect(useSessionUIStore.getState().newSessionDraft.open).toBe(true); expect(useSessionUIStore.getState().newSessionDraft.title).toBe('Draft title'); }); + + test('falls back to the current active project when a regular new-chat directory is missing', async () => { + const createSessionCalls = []; + useProjectsStore.setState({ + projects: [ + { id: 'project-draft', path: '/projects/draft', label: 'Draft' }, + { id: 'project-active', path: '/projects/active', label: 'Active' }, + ], + activeProjectId: 'project-active', + }); + useDirectoryStore.getState().setDirectory('/private/deleted-worktree', { showOverlay: false }); + useSessionUIStore.getState().openNewSessionDraft(); + opencodeClient.getDirectoryAvailability = async () => 'missing'; + opencodeClient.createSession = async (_params, directory) => { + createSessionCalls.push(directory); + return { id: 'session-fallback', directory }; + }; + + await useSessionUIStore.getState().createSession('Draft title', '/private/deleted-worktree'); + + expect(createSessionCalls).toEqual(['/projects/active']); + expect(useDirectoryStore.getState().currentDirectory).toBe('/projects/active'); + expect(getDeferredSafeStorage().getItem('lastDirectory')).toBe('/projects/active'); + }); + + test('keeps an explicitly pinned worktree directory unchanged', async () => { + const createSessionCalls = []; + useProjectsStore.setState({ + projects: [{ id: 'project-main', path: '/projects/main', label: 'Main' }], + activeProjectId: 'project-main', + }); + useSessionUIStore.getState().openNewSessionDraft({ directoryOverride: '/private/deleted-worktree', preserveDirectoryOverride: true }); + expect(useSessionUIStore.getState().newSessionDraft.preserveDirectoryOverride).toBe(true); + opencodeClient.getDirectoryAvailability = async () => 'missing'; + opencodeClient.createSession = async (_params, directory) => { + createSessionCalls.push(directory); + return { id: 'session-pinned', directory }; + }; + + await useSessionUIStore.getState().createSession('Draft title', '/private/deleted-worktree'); + + expect(createSessionCalls).toEqual(['/private/deleted-worktree']); + }); + + test('keeps a ChatInput-style current-directory draft recoverable when that path is missing', async () => { + const createSessionCalls = []; + useProjectsStore.setState({ + projects: [{ id: 'project-main', path: '/projects/main', label: 'Main' }], + activeProjectId: 'project-main', + }); + useDirectoryStore.getState().setDirectory('/private/deleted-worktree', { showOverlay: false }); + useSessionUIStore.getState().openNewSessionDraft({ directoryOverride: '/private/deleted-worktree' }); + expect(useSessionUIStore.getState().newSessionDraft.preserveDirectoryOverride).not.toBe(true); + opencodeClient.getDirectoryAvailability = async () => 'missing'; + opencodeClient.createSession = async (_params, directory) => { + createSessionCalls.push(directory); + return { id: 'session-chat-input', directory }; + }; + + await useSessionUIStore.getState().createSession('Draft title', '/private/deleted-worktree'); + + expect(createSessionCalls).toEqual(['/projects/main']); + }); + + test('keeps the stale directory when its availability cannot be confirmed', async () => { + const createSessionCalls = []; + useProjectsStore.setState({ + projects: [{ id: 'project-main', path: '/projects/main', label: 'Main' }], + activeProjectId: 'project-main', + }); + useDirectoryStore.getState().setDirectory('/private/unavailable-worktree', { showOverlay: false }); + useSessionUIStore.getState().openNewSessionDraft(); + opencodeClient.getDirectoryAvailability = async () => 'unknown'; + opencodeClient.createSession = async (_params, directory) => { + createSessionCalls.push(directory); + return { id: 'session-unavailable', directory }; + }; + + await useSessionUIStore.getState().createSession('Draft title', '/private/unavailable-worktree'); + + expect(createSessionCalls).toEqual(['/private/unavailable-worktree']); + expect(useDirectoryStore.getState().currentDirectory).toBe('/private/unavailable-worktree'); + }); + + test('does not persist a fallback when session creation fails', async () => { + useProjectsStore.setState({ + projects: [{ id: 'project-main', path: '/projects/main', label: 'Main' }], + activeProjectId: 'project-main', + }); + useDirectoryStore.getState().setDirectory('/private/deleted-worktree', { showOverlay: false }); + useSessionUIStore.getState().openNewSessionDraft(); + opencodeClient.getDirectoryAvailability = async () => 'missing'; + opencodeClient.createSession = async () => { + throw new Error('offline'); + }; + + const session = await useSessionUIStore.getState().createSession('Draft title', '/private/deleted-worktree'); + + expect(session).toBeNull(); + expect(useDirectoryStore.getState().currentDirectory).toBe('/private/deleted-worktree'); + expect(getDeferredSafeStorage().getItem('lastDirectory')).toBe('/private/deleted-worktree'); + }); }); // --------------------------------------------------------------------------- diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index 1db06f4c..ac4fa453 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -603,6 +603,63 @@ const waitForWorktreeBootstrapIfConfigured = async (directory: string | null, pr } } +const resolveActiveProjectDirectory = (draft: NewSessionDraftState): string | null => { + const projectsState = useProjectsStore.getState() + return normalizePath( + projectsState.getActiveProject()?.path + ?? (draft.selectedProjectId + ? projectsState.projects.find((project) => project.id === draft.selectedProjectId)?.path + : null) + ?? null, + ) +} + +/** + * Regular new-chat drafts inherit the persisted current/last directory. If that + * path is confirmed missing (deleted worktree), fall back to the active project. + * Explicit worktree targets, in-flight worktree creation, and unknown/offline + * probes stay unchanged so a temporary outage cannot rewrite the destination. + */ +const resolveCreatableDraftDirectory = async ( + draft: NewSessionDraftState, + requestedDirectory: string | null | undefined, +): Promise<{ status: "ok"; directory: string | null | undefined } | { status: "aborted" }> => { + const directory = requestedDirectory ?? opencodeClient.getDirectory() ?? null + const isRecoverableDraftDirectory = + draft.open + && draft.preserveDirectoryOverride !== true + && !draft.pendingWorktreeRequestId + && !draft.bootstrapPendingDirectory + && normalizePath(draft.directoryOverride) === normalizePath(directory) + + if (!isRecoverableDraftDirectory || !directory) { + return { status: "ok", directory } + } + + const activeProjectDirectory = resolveActiveProjectDirectory(draft) + if (!activeProjectDirectory || normalizePath(directory) === activeProjectDirectory) { + return { status: "ok", directory } + } + + const runtimeKey = getRuntimeKey() + const draftDirectory = draft.directoryOverride + const availability = await opencodeClient.getDirectoryAvailability(directory) + const currentDraft = useSessionUIStore.getState().newSessionDraft + const draftChanged = !currentDraft.open + || currentDraft.preserveDirectoryOverride !== draft.preserveDirectoryOverride + || currentDraft.pendingWorktreeRequestId !== draft.pendingWorktreeRequestId + || normalizePath(currentDraft.directoryOverride) !== normalizePath(draftDirectory) + + if (getRuntimeKey() !== runtimeKey || draftChanged) { + return { status: "aborted" } + } + + return { + status: "ok", + directory: availability === "missing" ? activeProjectDirectory : directory, + } +} + export async function materializeOpenDraftSession(selection: { providerID: string modelID: string @@ -1416,14 +1473,16 @@ export const useSessionUIStore = create()((set, get) => ({ const targetFolderId = draft.targetFolderId try { - const dir = directoryOverride ?? opencodeClient.getDirectory() + const resolved = await resolveCreatableDraftDirectory(draft, directoryOverride) + if (resolved.status === "aborted") return null + const dir = resolved.directory const session = await createSessionAction(title, dir, parentID ?? null, metadata) if (!session) return null get().closeNewSessionDraft() if (targetFolderId) { - const scopeKey = directoryOverride || get().lastLoadedDirectory || session.directory + const scopeKey = dir || get().lastLoadedDirectory || session.directory if (scopeKey) { useSessionFoldersStore.getState().addSessionToFolder(scopeKey, targetFolderId, session.id) } diff --git a/packages/vscode/CHANGELOG.md b/packages/vscode/CHANGELOG.md index 8132a578..86d45514 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -1,3 +1,7 @@ +## [Unreleased] + +- Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. + ## [1.18.4] - 2026-08-14 - **Chat:** new messages now remain at the end of the conversation instead of jumping before older messages after the message ID sequence rolls over; history loading, revert, and redo follow the same chronological order. From 909073390860e47663c9f65f29cec12894913e14 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 04:28:07 +0000 Subject: [PATCH 02/17] fix(sessions): rewrite stale new-chat drafts before send When a regular new-chat draft inherits a deleted lastDirectory, update the visible draft target to the active project immediately. lastDirectory still stays unchanged until session creation succeeds. Co-authored-by: serkraser --- packages/ui/src/sync/session-ui-store.test.js | 16 ++++++++++ packages/ui/src/sync/session-ui-store.ts | 29 +++++++++++++++++++ 2 files changed, 45 insertions(+) diff --git a/packages/ui/src/sync/session-ui-store.test.js b/packages/ui/src/sync/session-ui-store.test.js index 565d5294..7d016857 100644 --- a/packages/ui/src/sync/session-ui-store.test.js +++ b/packages/ui/src/sync/session-ui-store.test.js @@ -456,6 +456,22 @@ describe('createSession draft lifecycle', () => { expect(useSessionUIStore.getState().newSessionDraft.title).toBe('Draft title'); }); + test('rewrites an implicit new-chat draft to the active project before the session is created', async () => { + useProjectsStore.setState({ + projects: [{ id: 'project-main', path: '/projects/main', label: 'Main' }], + activeProjectId: 'project-main', + }); + useDirectoryStore.getState().setDirectory('/private/deleted-worktree', { showOverlay: false }); + opencodeClient.getDirectoryAvailability = async () => 'missing'; + + useSessionUIStore.getState().openNewSessionDraft(); + await Bun.sleep(0); + + expect(useSessionUIStore.getState().newSessionDraft.directoryOverride).toBe('/projects/main'); + expect(useSessionUIStore.getState().newSessionDraft.selectedProjectId).toBe('project-main'); + expect(getDeferredSafeStorage().getItem('lastDirectory')).toBe('/private/deleted-worktree'); + }); + test('falls back to the current active project when a regular new-chat directory is missing', async () => { const createSessionCalls = []; useProjectsStore.setState({ diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index ac4fa453..7ca29a6a 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -660,6 +660,33 @@ const resolveCreatableDraftDirectory = async ( } } +const recoverStaleDraftDirectory = async (openedDraft: NewSessionDraftState): Promise => { + const resolved = await resolveCreatableDraftDirectory(openedDraft, openedDraft.directoryOverride) + if (resolved.status !== "ok") return + const recovered = normalizePath(resolved.directory ?? null) + const original = normalizePath(openedDraft.directoryOverride) + if (!recovered || recovered === original) return + + const currentDraft = useSessionUIStore.getState().newSessionDraft + if (!currentDraft.open) return + if (currentDraft.preserveDirectoryOverride === true) return + if (currentDraft.pendingWorktreeRequestId) return + if (normalizePath(currentDraft.directoryOverride) !== original) return + + const recoveredProject = useProjectsStore.getState().projects.find((project) => ( + normalizePath(project.path) === recovered + )) + const nextDraft: NewSessionDraftState = { + ...currentDraft, + selectedProjectId: recoveredProject?.id ?? currentDraft.selectedProjectId, + directoryOverride: recovered, + } + useSessionUIStore.setState({ newSessionDraft: nextDraft }) + writeRuntimeSessionMemory(runtimeMemoryKey(), { draft: nextDraft }) + persistDraftTarget({ projectId: nextDraft.selectedProjectId ?? null, directory: recovered }) + void activateConfigForDirectory(recovered) +} + export async function materializeOpenDraftSession(selection: { providerID: string modelID: string @@ -1021,6 +1048,8 @@ export const useSessionUIStore = create()((set, get) => ({ if (directory && directory !== useDirectoryStore.getState().currentDirectory) { useDirectoryStore.getState().setDirectory(directory) } + + void recoverStaleDraftDirectory(nextDraft) }, // --------------------------------------------------------------------------- From 7db8b2c463c195c6ddf53db7e14a1afe7c4ac251 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 04:47:15 +0000 Subject: [PATCH 03/17] chore(ui): keep directory availability type module-private The availability union is only used by the OpenCode client probe. Co-authored-by: serkraser --- packages/ui/src/lib/opencode/client.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/ui/src/lib/opencode/client.ts b/packages/ui/src/lib/opencode/client.ts index 1af33e97..37e12620 100644 --- a/packages/ui/src/lib/opencode/client.ts +++ b/packages/ui/src/lib/opencode/client.ts @@ -68,7 +68,7 @@ type SdkResult = { response?: { status?: number }; }; -export type DirectoryAvailability = "available" | "missing" | "unknown"; +type DirectoryAvailability = "available" | "missing" | "unknown"; const isMissingDirectoryError = (error: unknown): boolean => { if (error instanceof FilesystemError) { From d817c44c46b96b7cdb3f0d7425fca38f94bfa374 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Sat, 15 Aug 2026 06:33:28 +0000 Subject: [PATCH 04/17] fix(sessions): accept in-flight draft rewrite to the fallback directory Create no longer aborts when recoverStaleDraftDirectory rewrites the implicit new-chat draft to the active project during the create probe. Also rank the Unreleased Chat bullet below the changelog highlights. Co-authored-by: serkraser --- CHANGELOG.md | 2 +- packages/ui/src/sync/DOCUMENTATION.md | 2 +- packages/ui/src/sync/session-ui-store.test.js | 31 +++++++++++++++++++ packages/ui/src/sync/session-ui-store.ts | 15 ++++++++- 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9813325e..e3073032 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,10 +4,10 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -- Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. - Git: the pull request panel now follows the current open PR for the branch instead of keeping a merged or closed one after reload or a later open PR (thanks to @makeittech). - **Usage/Claude:** Claude plan limits now work when you are signed in through Claude Code, without also signing into Anthropic in OpenCode; the account is read from Claude Code's own login on macOS, Linux, and WSL. The page shows your session and weekly limits again, adds per-model weekly limits and extra usage spending, and names your plan. Limits are kept on screen instead of disappearing when Anthropic temporarily blocks refreshes. - **Settings/Integrations:** a new Integrations settings page lists Claude Code, Command Code, and Cursor plugins with install, update, setup, and remove actions, plus Discord and Telegram Coming soon placeholders. +- Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. - Chat: opening a busy subagent in the context panel now shows its history instead of only the working-status line (thanks to @makeittech). ## [1.18.4] - 2026-08-14 diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index f094e0e4..7c615190 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -251,7 +251,7 @@ Rules: 4. Sending after a revert commits the new branch optimistically: remove the reverted tail and marker before inserting the new message, and restore both if the send is rejected. 5. Composer and queued sends carry their captured runtime, directory, and session through asynchronous preparation. A runtime change cancels the send instead of re-resolving it against the new runtime. 6. After session creation, the directory returned by the server is authoritative over the requested draft directory. The server may canonicalize a worktree path, and the first prompt must use the same directory identity as the created session. -7. Regular new-chat drafts that inherit the persisted current/last directory must not create a session against a confirmed-missing path. Fall back to the active project only when OpenCode reports the directory missing; keep explicit worktree targets, in-flight worktree creation, and unknown/offline probes unchanged, and do not persist the fallback until session creation succeeds. +7. Regular new-chat drafts that inherit the persisted current/last directory must not create a session against a confirmed-missing path. Fall back to the active project only when OpenCode reports the directory missing; keep explicit worktree targets, in-flight worktree creation, and unknown/offline probes unchanged, and do not persist the fallback until session creation succeeds. A concurrent draft rewrite to that same active-project fallback must not abort session creation. 8. A prompt send that fails **after** the request left the client is ambiguous, never a definite failure: the server may already be answering it. Transports tag those errors (`markAmbiguousTransportFailure` in `@/lib/relay/transport-error`; the relay tunnel tags every stream that dies with a request in flight), and `isAmbiguousSendFailure` reads the tag before falling back to status/text heuristics. An ambiguous failure waits for the connection to return, refetches recent messages, and confirms the optimistic message in place instead of rolling it back — rolling it back lets the message queue re-send a prompt the engine is already running, producing two independent AI responses for one user message. Examples of global-store updates performed in `session-actions.ts`: diff --git a/packages/ui/src/sync/session-ui-store.test.js b/packages/ui/src/sync/session-ui-store.test.js index 7d016857..07a8a19f 100644 --- a/packages/ui/src/sync/session-ui-store.test.js +++ b/packages/ui/src/sync/session-ui-store.test.js @@ -555,6 +555,37 @@ describe('createSession draft lifecycle', () => { expect(useDirectoryStore.getState().currentDirectory).toBe('/private/unavailable-worktree'); }); + test('still creates against the active project when the draft is rewritten during the create probe', async () => { + const createSessionCalls = []; + const availabilityResolvers = []; + useProjectsStore.setState({ + projects: [{ id: 'project-main', path: '/projects/main', label: 'Main' }], + activeProjectId: 'project-main', + }); + useDirectoryStore.getState().setDirectory('/private/deleted-worktree', { showOverlay: false }); + opencodeClient.getDirectoryAvailability = () => new Promise((resolve) => { + availabilityResolvers.push(resolve); + }); + opencodeClient.createSession = async (_params, directory) => { + createSessionCalls.push(directory); + return { id: 'session-race', directory }; + }; + + useSessionUIStore.getState().openNewSessionDraft(); + const createPromise = useSessionUIStore.getState().createSession('Draft title', '/private/deleted-worktree'); + expect(availabilityResolvers.length).toBe(2); + + availabilityResolvers[0]('missing'); + await Bun.sleep(0); + expect(useSessionUIStore.getState().newSessionDraft.directoryOverride).toBe('/projects/main'); + + availabilityResolvers[1]('missing'); + const session = await createPromise; + + expect(session).not.toBeNull(); + expect(createSessionCalls).toEqual(['/projects/main']); + }); + test('does not persist a fallback when session creation fails', async () => { useProjectsStore.setState({ projects: [{ id: 'project-main', path: '/projects/main', label: 'Main' }], diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index 7ca29a6a..0fc4ebb1 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -619,6 +619,8 @@ const resolveActiveProjectDirectory = (draft: NewSessionDraftState): string | nu * path is confirmed missing (deleted worktree), fall back to the active project. * Explicit worktree targets, in-flight worktree creation, and unknown/offline * probes stay unchanged so a temporary outage cannot rewrite the destination. + * A concurrent rewrite of the same implicit draft to that fallback is accepted + * instead of aborting create. */ const resolveCreatableDraftDirectory = async ( draft: NewSessionDraftState, @@ -645,15 +647,26 @@ const resolveCreatableDraftDirectory = async ( const draftDirectory = draft.directoryOverride const availability = await opencodeClient.getDirectoryAvailability(directory) const currentDraft = useSessionUIStore.getState().newSessionDraft + const currentDirectory = normalizePath(currentDraft.directoryOverride) + const capturedDirectory = normalizePath(draftDirectory) + // openNewSessionDraft may rewrite the same implicit draft to this fallback + // while createSession's probe is still in flight. That is the intended + // destination, not a user change, so do not abort the create. + const recoveredToActiveProject = currentDirectory === activeProjectDirectory + && capturedDirectory !== activeProjectDirectory const draftChanged = !currentDraft.open || currentDraft.preserveDirectoryOverride !== draft.preserveDirectoryOverride || currentDraft.pendingWorktreeRequestId !== draft.pendingWorktreeRequestId - || normalizePath(currentDraft.directoryOverride) !== normalizePath(draftDirectory) + || (currentDirectory !== capturedDirectory && !recoveredToActiveProject) if (getRuntimeKey() !== runtimeKey || draftChanged) { return { status: "aborted" } } + if (recoveredToActiveProject) { + return { status: "ok", directory: activeProjectDirectory } + } + return { status: "ok", directory: availability === "missing" ? activeProjectDirectory : directory, From 9032dfa5c015fd5b629adce7f6494a1a7171dfd3 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 15 Aug 2026 10:16:24 +0300 Subject: [PATCH 05/17] fix(ui): show project names exactly as the folder is named Auto-derived project labels were title-cased, turning .ssh into .Ssh and opencode-claude into Opencode Claude. Show the folder name verbatim in the sidebar, window title, settings selector and notification templates, and migrate persisted legacy labels back to the folder name (manual renames are preserved). --- .../sections/shared/SettingsProjectSelector.tsx | 4 +--- .../ui/src/components/session/sidebar/utils.tsx | 7 ++----- packages/ui/src/hooks/useWindowTitle.ts | 4 +--- packages/ui/src/stores/useProjectsStore.ts | 17 ++++++++++++++--- .../lib/notifications/template-runtime.js | 5 ++--- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/packages/ui/src/components/sections/shared/SettingsProjectSelector.tsx b/packages/ui/src/components/sections/shared/SettingsProjectSelector.tsx index 709d5c50..9acbd7b0 100644 --- a/packages/ui/src/components/sections/shared/SettingsProjectSelector.tsx +++ b/packages/ui/src/components/sections/shared/SettingsProjectSelector.tsx @@ -12,9 +12,7 @@ import { isVSCodeRuntime } from '@/lib/desktop'; import { cn } from '@/lib/utils'; import { useI18n } from '@/lib/i18n'; -const formatProjectLabel = (label: string): string => { - return label.replace(/[-_]/g, ' ').replace(/\b\w/g, (char) => char.toUpperCase()); -}; +const formatProjectLabel = (label: string): string => label.trim(); export const SettingsProjectSelector: React.FC<{ className?: string }> = ({ className }) => { const { t } = useI18n(); diff --git a/packages/ui/src/components/session/sidebar/utils.tsx b/packages/ui/src/components/session/sidebar/utils.tsx index 05e57157..3d10eb4c 100644 --- a/packages/ui/src/components/session/sidebar/utils.tsx +++ b/packages/ui/src/components/session/sidebar/utils.tsx @@ -156,11 +156,8 @@ export const resolveArchivedFolderName = (session: Session, projectRoot: string return segments[segments.length - 1] ?? 'unassigned'; }; -export const formatProjectLabel = (label: string): string => { - return label - .replace(/[-_]/g, ' ') - .replace(/\b\w/g, (char) => char.toUpperCase()); -}; +// Folder names are shown exactly as they are on disk — no title-casing. +export const formatProjectLabel = (label: string): string => label.trim(); export const renderHighlightedText = (text: string, query: string): React.ReactNode => { if (!query) { diff --git a/packages/ui/src/hooks/useWindowTitle.ts b/packages/ui/src/hooks/useWindowTitle.ts index 87e64019..7fb31e4c 100644 --- a/packages/ui/src/hooks/useWindowTitle.ts +++ b/packages/ui/src/hooks/useWindowTitle.ts @@ -7,9 +7,7 @@ import { getRuntimeApiBaseUrl } from '@/lib/runtime-switch'; const APP_TITLE = 'OpenChamber'; -const formatProjectLabel = (label: string): string => { - return label.replace(/[-_]/g, ' ').replace(/\b\w/g, (char) => char.toUpperCase()); -}; +const formatProjectLabel = (label: string): string => label.trim(); const getProjectNameFromPath = (path: string): string => { const normalized = path.replace(/\\/g, '/').replace(/\/+$/, ''); diff --git a/packages/ui/src/stores/useProjectsStore.ts b/packages/ui/src/stores/useProjectsStore.ts index fbf279b3..0f37f137 100644 --- a/packages/ui/src/stores/useProjectsStore.ts +++ b/packages/ui/src/stores/useProjectsStore.ts @@ -166,14 +166,22 @@ const normalizeProjectPath = (value: string): string => { return normalized.length > 1 ? normalized.replace(/\/+$/, '') : normalized; }; +// Folder names are shown verbatim: title-casing them turned `.ssh` into `.Ssh` +// and made every project look like a name the user never chose. const deriveProjectLabel = (path: string): string => { const normalized = normalizeProjectPath(path); if (!normalized || normalized === '/') { return 'Root'; } const segments = normalized.split('/').filter(Boolean); - const raw = segments[segments.length - 1] || normalized; - return raw.replace(/[-_]/g, ' ').replace(/\b\w/g, (c) => c.toUpperCase()); + return segments[segments.length - 1] || normalized; +}; + +// Labels auto-derived by older versions were title-cased and persisted. Drop +// them back to the folder name; labels the user typed themselves are kept. +const legacyAutoProjectLabel = (path: string): string => { + const derived = deriveProjectLabel(path); + return derived.replace(/[-_]/g, ' ').replace(/\b\w/g, (c) => c.toUpperCase()); }; const sanitizeProjectIconImage = (value: unknown): ProjectEntry['iconImage'] | undefined => { @@ -261,7 +269,10 @@ const sanitizeProjects = (value: unknown): ProjectEntry[] => { }; if (typeof candidate.label === 'string' && candidate.label.trim().length > 0) { - project.label = candidate.label.trim(); + const storedLabel = candidate.label.trim(); + project.label = storedLabel === legacyAutoProjectLabel(normalizedPath) + ? deriveProjectLabel(normalizedPath) + : storedLabel; } if (typeof candidate.icon === 'string' && candidate.icon.trim().length > 0) { project.icon = candidate.icon.trim(); diff --git a/packages/web/server/lib/notifications/template-runtime.js b/packages/web/server/lib/notifications/template-runtime.js index b7233c3e..7400e24b 100644 --- a/packages/web/server/lib/notifications/template-runtime.js +++ b/packages/web/server/lib/notifications/template-runtime.js @@ -27,9 +27,8 @@ export const createNotificationTemplateRuntime = (deps) => { const formatProjectLabel = (label) => { if (!label || typeof label !== 'string') return ''; - return label - .replace(/[-_]/g, ' ') - .replace(/\b\w/g, (char) => char.toUpperCase()); + // Folder names are shown exactly as they are on disk — no title-casing. + return label.trim(); }; const resolveNotificationTemplate = (template, variables) => { From 52ac367b1e9fbba1e0daf884f2b66bb9ef0511aa Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 15 Aug 2026 16:32:57 +0300 Subject: [PATCH 06/17] feat: update annotate toolbar icon to markup --- packages/ui/src/components/browser/BrowserToolbar.tsx | 2 +- packages/ui/src/components/icon/sprite.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/ui/src/components/browser/BrowserToolbar.tsx b/packages/ui/src/components/browser/BrowserToolbar.tsx index c56aca09..f2eaf046 100644 --- a/packages/ui/src/components/browser/BrowserToolbar.tsx +++ b/packages/ui/src/components/browser/BrowserToolbar.tsx @@ -226,7 +226,7 @@ export const BrowserToolbar: React.FC = ({ ) : null} {onAnnotate ? ( `, "loop-right-ai": ``, "macbook": ``, + "markup": ``, "menu-2": ``, "menu-fold-2": ``, "menu-search": ``, From 268f9ea9f283cf9455b21a8fa76009c91da63e7e Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 15 Aug 2026 17:56:55 +0300 Subject: [PATCH 07/17] fix(github): keep merged PRs as branch history instead of hiding them Branch status resolves an open PR across the whole fork network first, so a merged fork PR can never hide an open upstream PR for the same head. Only when no target has an open PR does the branch's newest closed/merged PR come back, as history. The panel shows that history as a compact note and offers creating the next PR below it, instead of either sticking on a terminal PR or going blank after a merge. Terminal associations stay persisted for reload continuity but are never treated as authority: they revalidate on the discovery cadence and on focus. History is looked up only for the branch's own remote and name, and remembered per repo+branch, so the extra lookup cannot exhaust the route's resolve budget. The checks summary and merge-permission lookup are skipped for a closed or merged PR, where neither is actionable. --- CHANGELOG.md | 2 +- .../views/git/PullRequestSection.tsx | 61 +++++-- packages/ui/src/lib/i18n/messages/de.ts | 2 + packages/ui/src/lib/i18n/messages/en.ts | 2 + packages/ui/src/lib/i18n/messages/es.ts | 2 + packages/ui/src/lib/i18n/messages/fr.ts | 2 + packages/ui/src/lib/i18n/messages/ja.ts | 2 + packages/ui/src/lib/i18n/messages/ko.ts | 2 + packages/ui/src/lib/i18n/messages/pl.ts | 2 + packages/ui/src/lib/i18n/messages/pt-BR.ts | 2 + packages/ui/src/lib/i18n/messages/uk.ts | 2 + packages/ui/src/lib/i18n/messages/zh-CN.ts | 2 + packages/ui/src/lib/i18n/messages/zh-TW.ts | 2 + packages/ui/src/stores/DOCUMENTATION.md | 8 +- .../src/stores/useGitHubPrStatusStore.test.ts | 30 +-- .../ui/src/stores/useGitHubPrStatusStore.ts | 54 ++---- packages/vscode/CHANGELOG.md | 1 - .../web/server/lib/github/DOCUMENTATION.md | 11 +- packages/web/server/lib/github/pr-status.js | 169 +++++++++++++---- .../web/server/lib/github/pr-status.test.js | 171 +++++++++++++----- packages/web/server/lib/github/routes.js | 66 ++++--- 21 files changed, 405 insertions(+), 190 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e3073032..2313f45a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,7 +4,7 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -- Git: the pull request panel now follows the current open PR for the branch instead of keeping a merged or closed one after reload or a later open PR (thanks to @makeittech). +- Git: the pull request panel now follows the branch's current open PR, and an open PR always wins over an older merged or closed one. After a PR is merged or closed the panel keeps showing it as the branch's last PR and offers creating the next one right below it (thanks to @makeittech). - **Usage/Claude:** Claude plan limits now work when you are signed in through Claude Code, without also signing into Anthropic in OpenCode; the account is read from Claude Code's own login on macOS, Linux, and WSL. The page shows your session and weekly limits again, adds per-model weekly limits and extra usage spending, and names your plan. Limits are kept on screen instead of disappearing when Anthropic temporarily blocks refreshes. - **Settings/Integrations:** a new Integrations settings page lists Claude Code, Command Code, and Cursor plugins with install, update, setup, and remove actions, plus Discord and Telegram Coming soon placeholders. - Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. diff --git a/packages/ui/src/components/views/git/PullRequestSection.tsx b/packages/ui/src/components/views/git/PullRequestSection.tsx index f85727ad..3a258526 100644 --- a/packages/ui/src/components/views/git/PullRequestSection.tsx +++ b/packages/ui/src/components/views/git/PullRequestSection.tsx @@ -508,8 +508,13 @@ export const PullRequestSection: React.FC<{ }, [useDetectedUpstream, detectedUpstream?.defaultBranch]); const pr = status?.pr ?? null; + // A closed/merged PR is the branch's history, not its live status: it still + // deserves to be shown (you just merged it), but the branch is free again, so + // the panel offers creating the next PR instead of a read-only detail view. + const isHistoricalPr = pr?.state === 'merged' || pr?.state === 'closed'; + const livePr = isHistoricalPr ? null : pr; - const prContextKey = pr ? getPrContextKey(directory, pr.number) : null; + const prContextKey = livePr ? getPrContextKey(directory, livePr.number) : null; const prContextEntry = usePrContextStore((state) => (prContextKey ? state.entries[prContextKey] : undefined)); const ensurePrContext = usePrContextStore((state) => state.ensure); const prContext = prContextEntry?.result ?? null; @@ -525,14 +530,14 @@ export const PullRequestSection: React.FC<{ // Load the context the active segment needs; checks include details. React.useEffect(() => { - if (!pr || !github?.prContext || activeSegment === 'overview') { + if (!livePr || !github?.prContext || activeSegment === 'overview') { return; } - void ensurePrContext(github, directory, pr.number, { + void ensurePrContext(github, directory, livePr.number, { includeCheckDetails: activeSegment === 'checks', sourceRepo: status?.repo ?? null, }); - }, [activeSegment, directory, ensurePrContext, github, pr, status?.repo]); + }, [activeSegment, directory, ensurePrContext, github, livePr, status?.repo]); const checks = status?.checks ?? null; const checksArePending = (checks?.pending ?? 0) > 0; @@ -1167,12 +1172,11 @@ export const PullRequestSection: React.FC<{ }, [remotes, status?.resolvedRemoteName]); React.useEffect(() => { - // Terminal (closed/merged) status must still revalidate on focus/visibility: - // the branch may now have a newer open PR, or the association may clear. - // Recompute staleness inside the handlers — a captured boolean freezes after - // the first fresh refresh until lastRefreshAt changes again. - const onFocus = () => { - const lastRefreshAt = statusEntry?.lastRefreshAt ?? 0; + // Coming back to the app is the moment a PR is most likely to have changed + // elsewhere — including a merged one being replaced by a newer open PR — so + // staleness is read from the store when the event fires, not captured here. + const refreshWhenStale = () => { + const lastRefreshAt = useGitHubPrStatusStore.getState().entries[prStatusKey]?.lastRefreshAt ?? 0; if (Date.now() - lastRefreshAt > 60_000) { void refresh({ force: true, silent: true }); } @@ -1181,19 +1185,16 @@ export const PullRequestSection: React.FC<{ if (document.visibilityState !== 'visible') { return; } - const lastRefreshAt = statusEntry?.lastRefreshAt ?? 0; - if (Date.now() - lastRefreshAt > 60_000) { - void refresh({ force: true, silent: true }); - } + refreshWhenStale(); }; - window.addEventListener('focus', onFocus); + window.addEventListener('focus', refreshWhenStale); document.addEventListener('visibilitychange', onVisibility); return () => { - window.removeEventListener('focus', onFocus); + window.removeEventListener('focus', refreshWhenStale); document.removeEventListener('visibilitychange', onVisibility); }; - }, [refresh, statusEntry?.lastRefreshAt]); + }, [prStatusKey, refresh]); React.useEffect(() => { if (githubAuthChecked && githubAuthStatus?.connected === false) { @@ -1614,7 +1615,7 @@ export const PullRequestSection: React.FC<{ {t('gitView.pr.checkingStatus')} - ) : pr ? ( + ) : pr && !isHistoricalPr ? (
) : (
+ {pr && isHistoricalPr ? ( +
+ +
+ {pr.state === 'merged' + ? t('gitView.pr.history.merged', { number: pr.number, base: pr.base || targetBaseBranch }) + : t('gitView.pr.history.closed', { number: pr.number })} +
+ +
+ ) : null}
{t('gitView.pr.createTitle')}
diff --git a/packages/ui/src/lib/i18n/messages/de.ts b/packages/ui/src/lib/i18n/messages/de.ts index 8c9f215d..fbb8a6cb 100644 --- a/packages/ui/src/lib/i18n/messages/de.ts +++ b/packages/ui/src/lib/i18n/messages/de.ts @@ -933,6 +933,8 @@ export const dict = { 'gitView.pr.field.draft': 'Entwurf', 'gitView.pr.field.title': 'Titel', 'gitView.pr.githubNotConnected': 'GitHub ist nicht verbunden', + 'gitView.pr.history.merged': 'PR #{number} wurde in {base} gemergt.', + 'gitView.pr.history.closed': 'PR #{number} wurde geschlossen.', 'gitView.pr.loadingDescription': 'Beschreibung wird geladen...', 'gitView.pr.mergeMethod.merge': 'Einen Merge-Commit erstellen', 'gitView.pr.mergeMethod.rebase': 'Rebase und Merge', diff --git a/packages/ui/src/lib/i18n/messages/en.ts b/packages/ui/src/lib/i18n/messages/en.ts index 9d60f0d7..52e6c94d 100644 --- a/packages/ui/src/lib/i18n/messages/en.ts +++ b/packages/ui/src/lib/i18n/messages/en.ts @@ -997,6 +997,8 @@ export const dict = { 'gitView.pr.field.draft': 'Draft', 'gitView.pr.field.title': 'Title', 'gitView.pr.githubNotConnected': 'GitHub is not connected', + 'gitView.pr.history.merged': 'PR #{number} was merged into {base}.', + 'gitView.pr.history.closed': 'PR #{number} was closed.', 'gitView.pr.loadingDescription': 'Loading description...', 'gitView.pr.mergeMethod.merge': 'Create a merge commit', 'gitView.pr.mergeMethod.rebase': 'Rebase and merge', diff --git a/packages/ui/src/lib/i18n/messages/es.ts b/packages/ui/src/lib/i18n/messages/es.ts index e30ea4ad..52e3b35f 100644 --- a/packages/ui/src/lib/i18n/messages/es.ts +++ b/packages/ui/src/lib/i18n/messages/es.ts @@ -998,6 +998,8 @@ export const dict: Record = { "gitView.pr.field.draft": "Borrador", "gitView.pr.field.title": "Título", "gitView.pr.githubNotConnected": "GitHub no está conectado", + "gitView.pr.history.merged": "La PR #{number} se fusionó en {base}.", + "gitView.pr.history.closed": "La PR #{number} se cerró.", "gitView.pr.loadingDescription": "Cargando descripción...", "gitView.pr.mergeMethod.merge": "Crear un merge commit", "gitView.pr.mergeMethod.rebase": "Rebase y merge", diff --git a/packages/ui/src/lib/i18n/messages/fr.ts b/packages/ui/src/lib/i18n/messages/fr.ts index f9c74dcf..9aed071b 100644 --- a/packages/ui/src/lib/i18n/messages/fr.ts +++ b/packages/ui/src/lib/i18n/messages/fr.ts @@ -817,6 +817,8 @@ export const dict = { 'gitView.pr.field.draft': 'Brouillon', 'gitView.pr.field.title': 'Titre', 'gitView.pr.githubNotConnected': 'GitHub n\'est pas connecté', + 'gitView.pr.history.merged': 'La PR #{number} a été fusionnée dans {base}.', + 'gitView.pr.history.closed': 'La PR #{number} a été fermée.', 'gitView.pr.loadingDescription': 'Chargement de la description de la PR...', 'gitView.pr.mergeMethod.merge': 'Créer un commit de fusion', 'gitView.pr.mergeMethod.rebase': 'Rebase et fusionner', diff --git a/packages/ui/src/lib/i18n/messages/ja.ts b/packages/ui/src/lib/i18n/messages/ja.ts index 441d7fd0..1d4959f7 100644 --- a/packages/ui/src/lib/i18n/messages/ja.ts +++ b/packages/ui/src/lib/i18n/messages/ja.ts @@ -994,6 +994,8 @@ export const dict: Record = { 'gitView.pr.field.draft': '下書き', 'gitView.pr.field.title': 'タイトル', 'gitView.pr.githubNotConnected': 'GitHubが接続されていません', + 'gitView.pr.history.merged': 'PR #{number} は {base} にマージされました。', + 'gitView.pr.history.closed': 'PR #{number} はクローズされました。', 'gitView.pr.loadingDescription': '説明を読み込み中...', 'gitView.pr.mergeMethod.merge': 'マージコミットを作成', 'gitView.pr.mergeMethod.rebase': 'リベースしてマージ', diff --git a/packages/ui/src/lib/i18n/messages/ko.ts b/packages/ui/src/lib/i18n/messages/ko.ts index 5c9499dd..52449bc8 100644 --- a/packages/ui/src/lib/i18n/messages/ko.ts +++ b/packages/ui/src/lib/i18n/messages/ko.ts @@ -998,6 +998,8 @@ export const dict: Record = { 'gitView.pr.field.draft': '드래프트', 'gitView.pr.field.title': '제목', 'gitView.pr.githubNotConnected': 'GitHub에 연결되지 않음', + 'gitView.pr.history.merged': 'PR #{number}이(가) {base}에 병합되었습니다.', + 'gitView.pr.history.closed': 'PR #{number}이(가) 닫혔습니다.', 'gitView.pr.loadingDescription': '설명 로드 중…', 'gitView.pr.mergeMethod.merge': '병합 커밋 생성', 'gitView.pr.mergeMethod.rebase': '리베이스 후 병합', diff --git a/packages/ui/src/lib/i18n/messages/pl.ts b/packages/ui/src/lib/i18n/messages/pl.ts index 54b53ee9..b903014d 100644 --- a/packages/ui/src/lib/i18n/messages/pl.ts +++ b/packages/ui/src/lib/i18n/messages/pl.ts @@ -2187,6 +2187,8 @@ export const dict: Record = { 'gitView.pr.field.draft': 'Szkic', 'gitView.pr.field.title': 'Tytuł', 'gitView.pr.githubNotConnected': 'GitHub nie jest połączony', + 'gitView.pr.history.merged': 'PR #{number} został scalony do {base}.', + 'gitView.pr.history.closed': 'PR #{number} został zamknięty.', 'gitView.pr.loadingDescription': 'Loading description...', 'gitView.pr.mergeMethod.merge': 'Create a merge commit', 'gitView.pr.mergeMethod.rebase': 'Rebase and merge', diff --git a/packages/ui/src/lib/i18n/messages/pt-BR.ts b/packages/ui/src/lib/i18n/messages/pt-BR.ts index 8f377453..500cc98b 100644 --- a/packages/ui/src/lib/i18n/messages/pt-BR.ts +++ b/packages/ui/src/lib/i18n/messages/pt-BR.ts @@ -998,6 +998,8 @@ export const dict: Record = { "gitView.pr.field.draft": "Borrador", "gitView.pr.field.title": "Título", "gitView.pr.githubNotConnected": "GitHub não está conectado", + "gitView.pr.history.merged": "A PR #{number} foi mesclada em {base}.", + "gitView.pr.history.closed": "A PR #{number} foi fechada.", "gitView.pr.loadingDescription": "Carregando descrição...", "gitView.pr.mergeMethod.merge": "Criar um merge commit", "gitView.pr.mergeMethod.rebase": "Rebase e merge", diff --git a/packages/ui/src/lib/i18n/messages/uk.ts b/packages/ui/src/lib/i18n/messages/uk.ts index d9f07823..fa1496f5 100644 --- a/packages/ui/src/lib/i18n/messages/uk.ts +++ b/packages/ui/src/lib/i18n/messages/uk.ts @@ -998,6 +998,8 @@ export const dict: Record = { "gitView.pr.field.draft": "Чернетка", "gitView.pr.field.title": "Назва", "gitView.pr.githubNotConnected": "GitHub не підключено", + "gitView.pr.history.merged": "PR #{number} злито в {base}.", + "gitView.pr.history.closed": "PR #{number} закрито.", "gitView.pr.loadingDescription": "Завантаження опису...", "gitView.pr.mergeMethod.merge": "Створити коміт злиття", "gitView.pr.mergeMethod.rebase": "Перебазувати та злити", diff --git a/packages/ui/src/lib/i18n/messages/zh-CN.ts b/packages/ui/src/lib/i18n/messages/zh-CN.ts index 7edfc6b3..2b670e97 100644 --- a/packages/ui/src/lib/i18n/messages/zh-CN.ts +++ b/packages/ui/src/lib/i18n/messages/zh-CN.ts @@ -998,6 +998,8 @@ export const dict: Record = { 'gitView.pr.field.draft': '草稿', 'gitView.pr.field.title': '标题', 'gitView.pr.githubNotConnected': 'GitHub 未连接', + 'gitView.pr.history.merged': 'PR #{number} 已合并到 {base}。', + 'gitView.pr.history.closed': 'PR #{number} 已关闭。', 'gitView.pr.loadingDescription': '正在加载描述...', 'gitView.pr.mergeMethod.merge': '创建合并提交', 'gitView.pr.mergeMethod.rebase': '变基并合并', diff --git a/packages/ui/src/lib/i18n/messages/zh-TW.ts b/packages/ui/src/lib/i18n/messages/zh-TW.ts index def005d2..313d59ed 100644 --- a/packages/ui/src/lib/i18n/messages/zh-TW.ts +++ b/packages/ui/src/lib/i18n/messages/zh-TW.ts @@ -1010,6 +1010,8 @@ export const dict: Record = { 'gitView.pr.field.draft': '草稿', 'gitView.pr.field.title': '標題', 'gitView.pr.githubNotConnected': 'GitHub 未連線', + 'gitView.pr.history.merged': 'PR #{number} 已合併到 {base}。', + 'gitView.pr.history.closed': 'PR #{number} 已關閉。', 'gitView.pr.loadingDescription': '正在載入描述...', 'gitView.pr.mergeMethod.merge': '建立合併提交', 'gitView.pr.mergeMethod.rebase': 'Rebase 並合併', diff --git a/packages/ui/src/stores/DOCUMENTATION.md b/packages/ui/src/stores/DOCUMENTATION.md index ee2295e0..7bd82bd1 100644 --- a/packages/ui/src/stores/DOCUMENTATION.md +++ b/packages/ui/src/stores/DOCUMENTATION.md @@ -173,10 +173,10 @@ Important properties: - `refreshTargets()` supports one-shot multi-target bootstrap without turning on live watching - runtime reset disposes timers, watchers, API references, and request ownership while inert namespaced snapshots remain isolated - persisted cache is versioned, TTL-filtered, and bounded for page refresh continuity, not broad background syncing -- closed/merged associations use the same `5m` discovery cadence as missing PRs so a newer open PR (or authoritative `pr: null`) can replace them without a manual refresh -- closed/merged branch associations are not persisted; legacy hydrated terminal PRs are stripped to `pr: null` and marked unresolved until refresh -- sibling remote-key seeding never copies a closed/merged association -- a successful refresh that returns `pr: null` replaces any previously cached PR authoritatively +- a closed/merged PR is the branch's history, not live status: it is displayed and persisted, but never treated as authority +- closed/merged associations use the same `5m` discovery cadence as missing PRs so a newer open PR (or authoritative `pr: null`) replaces them without a manual refresh +- hydrate restores a persisted closed/merged PR but resets its `lastDiscoveryPollAt`, so revalidation runs on the first watcher tick after a reload +- a successful refresh that returns `pr: null` replaces any previously cached PR authoritatively; a failed refresh keeps the previous one ## Ownership Rules diff --git a/packages/ui/src/stores/useGitHubPrStatusStore.test.ts b/packages/ui/src/stores/useGitHubPrStatusStore.test.ts index ff02ff90..fb376750 100644 --- a/packages/ui/src/stores/useGitHubPrStatusStore.test.ts +++ b/packages/ui/src/stores/useGitHubPrStatusStore.test.ts @@ -371,7 +371,7 @@ describe("GitHub PR status stale terminal associations", () => { expect(useGitHubPrStatusStore.getState().entries[key]?.status?.pr).toBeNull() }) - test("does not seed sibling entries from a closed PR", () => { + test("seeds sibling entries from a closed PR without freezing discovery", () => { const closed: GitHubPullRequestStatus = { connected: true, fetchedAt: 1_000, @@ -405,8 +405,12 @@ describe("GitHub PR status stale terminal associations", () => { }) useGitHubPrStatusStore.getState().ensureEntry(originKey) - expect(useGitHubPrStatusStore.getState().entries[originKey]?.status).toBeNull() - expect(useGitHubPrStatusStore.getState().entries[originKey]?.isInitialStatusResolved).toBe(false) + const seeded = useGitHubPrStatusStore.getState().entries[originKey] + expect(seeded?.status?.pr?.number).toBe(9) + // Seeding is display continuity only: the seeded entry has never refreshed + // or polled, so its own discovery still runs immediately. + expect(seeded?.lastRefreshAt).toBe(0) + expect(seeded?.lastDiscoveryPollAt).toBe(0) }) test("keeps a cached PR when a forced refresh fails", async () => { @@ -441,7 +445,7 @@ describe("GitHub PR status stale terminal associations", () => { expect(useGitHubPrStatusStore.getState().entries[key]?.error).toBe("GitHub unavailable") }) - test("does not persist a merged branch association", () => { + test("persists a merged branch association as history", () => { const merged: GitHubPullRequestStatus = { connected: true, fetchedAt: 1_000, @@ -464,8 +468,8 @@ describe("GitHub PR status stale terminal associations", () => { const persisted = useGitHubPrStatusStore.persist.getOptions().partialize?.( useGitHubPrStatusStore.getState(), - ) as { entries?: Record } | undefined - expect(persisted?.entries?.[key]).toBe(undefined) + ) as { entries?: Record } | undefined + expect(persisted?.entries?.[key]?.status?.pr?.number).toBe(12) }) test("still persists an open branch association", () => { @@ -495,7 +499,7 @@ describe("GitHub PR status stale terminal associations", () => { expect(persisted?.entries?.[key]?.status?.pr?.number).toBe(15) }) - test("hydrate strips a legacy persisted merged PR and marks it unresolved", () => { + test("hydrate keeps a persisted merged PR but forces the next discovery poll", () => { const key = getGitHubPrStatusKey("/repo", "feature", "origin") const hydrated = useGitHubPrStatusStore.persist.getOptions().merge?.( { @@ -511,7 +515,7 @@ describe("GitHub PR status stale terminal associations", () => { }, isInitialStatusResolved: true, lastRefreshAt: Date.now(), - lastDiscoveryPollAt: 0, + lastDiscoveryPollAt: Date.now(), identity: { runtimeKey: "runtime-a", directory: "/repo", @@ -527,17 +531,19 @@ describe("GitHub PR status stale terminal associations", () => { entries: Record } - expect(hydrated.entries[key]?.status?.pr).toBeNull() + expect(hydrated.entries[key]?.status?.pr?.number).toBe(12) expect(hydrated.entries[key]?.status?.repo).toEqual({ owner: "acme", repo: "app", url: "https://github.com/acme/app", }) - expect(hydrated.entries[key]?.status?.checks).toBe(undefined) - expect(hydrated.entries[key]?.status?.canMerge).toBe(undefined) - expect(hydrated.entries[key]?.isInitialStatusResolved).toBe(false) + expect(hydrated.entries[key]?.isInitialStatusResolved).toBe(true) + // Restored history must not inherit a fresh discovery timestamp, otherwise + // a newer open PR would wait a full discovery interval after every reload. + expect(hydrated.entries[key]?.lastDiscoveryPollAt).toBe(0) }) }) diff --git a/packages/ui/src/stores/useGitHubPrStatusStore.ts b/packages/ui/src/stores/useGitHubPrStatusStore.ts index e8583a44..a2e82053 100644 --- a/packages/ui/src/stores/useGitHubPrStatusStore.ts +++ b/packages/ui/src/stores/useGitHubPrStatusStore.ts @@ -212,11 +212,6 @@ const findResolvedSiblingEntry = ( if (entryKey === key || !entry.isInitialStatusResolved || !entry.status) { continue; } - // Never seed a fresh key from a closed/merged association — that is what - // made stale terminal PRs reappear after remote-key switches. - if (isTerminalPrState(entry.status.pr?.state)) { - continue; - } const parsed = parseStatusKey(entryKey); if (!parsed || parsed.runtimeKey !== target.runtimeKey @@ -364,35 +359,18 @@ const toPersistedEntry = (entry: PrStatusEntry): PersistedPrStatusEntry => ({ resolvedRemoteName: entry.resolvedRemoteName ?? entry.status?.resolvedRemoteName ?? null, }); -const stripTerminalPersistedStatus = ( - status: GitHubPullRequestStatus | null | undefined, -): GitHubPullRequestStatus | null => { - if (!status) { - return null; - } - if (!isTerminalPrState(status.pr?.state)) { - return status; - } - // Persisted closed/merged branch associations are not live authority. Keep - // repo/remote continuity so refresh can resume without briefly showing the - // stale terminal PR. - return { - ...status, - pr: null, - checks: undefined, - canMerge: undefined, - }; -}; - const hydrateEntry = (entry: PersistedPrStatusEntry | undefined): PrStatusEntry => { - const status = stripTerminalPersistedStatus(entry?.status); - const hadTerminalPr = Boolean(entry?.status?.pr) && !status?.pr; + // A persisted closed/merged PR is restored so the panel keeps showing the + // branch's PR history across a reload. It is never treated as live authority: + // `lastDiscoveryPollAt` is reset so the watcher revalidates it immediately and + // an open PR (or an authoritative empty result) replaces it. + const hasTerminalPr = isTerminalPrState(entry?.status?.pr?.state); return { ...createEntry(), - status, - isInitialStatusResolved: hadTerminalPr ? false : (entry?.isInitialStatusResolved ?? false), + status: entry?.status ?? null, + isInitialStatusResolved: entry?.isInitialStatusResolved ?? false, lastRefreshAt: entry?.lastRefreshAt ?? 0, - lastDiscoveryPollAt: entry?.lastDiscoveryPollAt ?? 0, + lastDiscoveryPollAt: hasTerminalPr ? 0 : (entry?.lastDiscoveryPollAt ?? 0), identity: entry?.identity ?? null, resolvedRemoteName: entry?.resolvedRemoteName ?? entry?.status?.resolvedRemoteName ?? null, }; @@ -501,8 +479,9 @@ export const useGitHubPrStatusStore = create()( if (!entry || entry.watchers <= 0) { return; } - // Bootstrap retries only help discovery before any PR is known. Once a - // terminal PR is cached, the discovery interval owns revalidation. + // Bootstrap retries only help discovery before any PR is known. + // Once a PR is cached — open or historical — the discovery interval + // owns revalidation. if (entry.status?.pr) { return; } @@ -527,10 +506,10 @@ export const useGitHubPrStatusStore = create()( } const hasPr = Boolean(entry.status?.pr); + // A closed/merged PR is history, not live status. It stays on the + // discovery cadence like a branch with no PR at all, so a newer open + // PR — or an authoritative empty result — replaces it on its own. const isTerminal = isTerminalPrState(entry.status?.pr?.state); - // Missing PR and terminal (closed/merged) PRs both need discovery: - // a new open PR may exist for the same head, or the association may - // need to clear to an authoritative empty result. if (!hasPr || isTerminal) { const now = Date.now(); if (now - entry.lastDiscoveryPollAt < PR_DISCOVERY_INTERVAL_MS) { @@ -881,11 +860,6 @@ export const useGitHubPrStatusStore = create()( if (!identity?.directory || !identity.branch) { return false; } - // Do not persist closed/merged branch associations — they become - // permanently sticky without a discovery refresh after reload. - if (isTerminalPrState(entry.status?.pr?.state)) { - return false; - } const freshness = Math.max(entry.lastRefreshAt, entry.lastDiscoveryPollAt); return freshness > 0 && Date.now() - freshness < PR_PERSIST_TTL_MS; }) diff --git a/packages/vscode/CHANGELOG.md b/packages/vscode/CHANGELOG.md index e97fd5f8..86d45514 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -1,7 +1,6 @@ ## [Unreleased] - Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. -- Git: the pull request panel now follows the current open PR for the branch instead of keeping a merged or closed one after reload or a later open PR (thanks to @makeittech). ## [1.18.4] - 2026-08-14 diff --git a/packages/web/server/lib/github/DOCUMENTATION.md b/packages/web/server/lib/github/DOCUMENTATION.md index c1ed499a..41d978e9 100644 --- a/packages/web/server/lib/github/DOCUMENTATION.md +++ b/packages/web/server/lib/github/DOCUMENTATION.md @@ -77,7 +77,12 @@ - It skips PR lookup when the current branch matches that repo's default branch. - It first searches for **open** PRs by likely source owner plus exact head branch. - If that fails, it falls back to broader GitHub search for open PRs on the branch name. -- Closed/merged PRs are intentionally not associated with branch status; historical PR browsing stays on explicit list/detail endpoints. +- An **open PR from any candidate repo always wins** over a closed/merged one, so a merged fork PR can never hide an open upstream PR for the same head. +- Only when no target has an open PR does it return the branch's newest closed/merged PR, as history. +- History is looked up **only for the ranked-first remote and the branch's own name** — the repo it actually pushes to. Live status is worth searching the whole fork network for; history is not, and asking every target for it multiplies serial GitHub calls until the route hits its `12s` resolve timeout and returns no status at all. +- The history answer is remembered per repo+branch so discovery polls do not re-query it: a found closed/merged record for `6h`, and "no history yet" for `10m`. A found record only changes if a second PR appears on the same head, and while that one is open the open-PR path wins without ever reading this cache. +- Creating, merging, or closing a PR invalidates both the shared repo pull list and that remembered history. +- The route skips the checks summary and the merge-permission lookup for a closed/merged PR: neither is actionable, and both cost extra GitHub calls. - `403` and `404` during repo lookups are treated as expected gaps, not hard errors. ## Shared client state model @@ -116,8 +121,8 @@ ## Persistence notes for terminal PRs -- Closed/merged branch-status entries are not written to local storage. -- Legacy persisted terminal entries are stripped on hydrate (`pr: null`) and marked unresolved until the next refresh. +- Closed/merged branch associations are persisted like open ones, so a reload still shows that the branch's PR was merged. +- Hydrate resets `lastDiscoveryPollAt` for them, so restored history revalidates on the first watcher tick instead of waiting out a discovery interval. ## Background tracking rules diff --git a/packages/web/server/lib/github/pr-status.js b/packages/web/server/lib/github/pr-status.js index f31cb30c..8e88122c 100644 --- a/packages/web/server/lib/github/pr-status.js +++ b/packages/web/server/lib/github/pr-status.js @@ -332,6 +332,38 @@ const safeListPulls = async (octokit, options) => { const REPO_PULLS_CACHE_TTL_MS = 45_000; const repoPullsCache = new Map(); +// Remembered answer to "what is the newest closed/merged PR for this head?", +// so discovery polls do not re-ask GitHub every few minutes. +// +// A found record barely ever changes: it would take a second PR on the same +// head, and while that one is open the open-PR path wins and never reads this +// cache at all. "No history yet" is the volatile answer, since closing or +// merging a PR elsewhere flips it, so it expires far sooner. Either way, doing +// it from OpenChamber invalidates the entry immediately. +const HISTORICAL_PR_FOUND_TTL_MS = 6 * 60 * 60 * 1000; +const HISTORICAL_PR_ABSENT_TTL_MS = 10 * 60 * 1000; +const HISTORICAL_PR_CACHE_MAX_ENTRIES = 500; +const _historicalPrCache = new Map(); + +const isHistoricalPrCacheFresh = (entry) => { + if (!entry) { + return false; + } + const ttl = entry.pr ? HISTORICAL_PR_FOUND_TTL_MS : HISTORICAL_PR_ABSENT_TTL_MS; + return Date.now() - entry.fetchedAt < ttl; +}; + +const rememberHistoricalPr = (key, pr) => { + _historicalPrCache.delete(key); + _historicalPrCache.set(key, { pr, fetchedAt: Date.now() }); + if (_historicalPrCache.size > HISTORICAL_PR_CACHE_MAX_ENTRIES) { + const oldest = _historicalPrCache.keys().next().value; + if (oldest !== undefined) { + _historicalPrCache.delete(oldest); + } + } +}; + export const invalidateRepoPullsCache = (owner, repo) => { const prefix = `${normalizeText(owner)}/${normalizeText(repo)}::`; for (const key of repoPullsCache.keys()) { @@ -347,6 +379,13 @@ export const invalidateRepoPullsCache = (owner, repo) => { _searchMissCache.delete(key); } } + // A merge or close changes the branch's PR history, so drop it too. + const historicalPrefix = `${normalizeRepoKey(owner, repo)}::`; + for (const key of _historicalPrCache.keys()) { + if (key.startsWith(historicalPrefix)) { + _historicalPrCache.delete(key); + } + } }; const getRepoPulls = (octokit, repo, state, { force = false } = {}) => { @@ -440,8 +479,8 @@ const searchFallbackPr = async ({ octokit, branch, repoNames }) => { const normalizedRepoNames = new Set(repoNames.map((name) => normalizeLower(name)).filter(Boolean)); - // Branch status only discovers open PRs. Closed/merged history belongs to - // explicit PR list/detail workflows, not automatic branch association. + // The Search API has a tiny quota, so it is only spent on live branch status. + // Closed/merged history is resolved by the cheaper per-head repo queries. let response; try { response = await octokit.rest.search.issuesAndPullRequests({ @@ -502,7 +541,22 @@ const searchFallbackPr = async ({ octokit, branch, repoNames }) => { return null; }; -const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates, force = false, coverage = null }) => { +const isTerminalPr = (pr) => Boolean(pr) && (pr.state === 'closed' || Boolean(pr.merged_at)); + +/** + * Resolve the PRs a branch is associated with in one repo target. + * + * Returns both candidates because they answer different questions: + * `open` is live branch status, `historical` is the last closed/merged PR for + * the same head. The caller must prefer an open PR from ANY target over a + * historical one — otherwise a merged fork PR hides an open upstream PR. + * + * `includeHistory` is off by default and must stay that way for secondary + * targets. Live status is worth searching the whole fork network for; history + * is not, and doing it per target multiplied the serial GitHub calls until the + * route hit its resolve timeout and reported no status at all. + */ +const findBranchPrCandidates = async ({ octokit, target, branch, sourceCandidates, force = false, coverage = null, includeHistory = false }) => { const matcher = buildSourceMatcher(sourceCandidates); const sourceOwners = []; sourceCandidates.forEach((candidate) => pushUnique(sourceOwners, candidate.repo?.owner)); @@ -512,46 +566,71 @@ const findFirstMatchingPr = async ({ octokit, target, branch, sourceCandidates, .filter((pr) => matcher.matches(pr, target.repo.repo)) .sort((left, right) => matcher.compare(left, right, target.repo.repo))[0] ?? null; - // Branch status associates the current head with an open PR only. Returning a - // closed/merged PR here made the client cache a terminal status that could not - // self-heal until a manual forced refresh. A miss also lets the next repo - // target run, so an open upstream PR wins over a merged fork PR. - let listWasComplete = false; + // The shared repo-level open list answers every branch of the repo within the + // TTL. A miss in a complete list is authoritative: no open PR exists here. + let openListWasComplete = false; try { const listEntry = await getRepoPulls(octokit, target.repo, 'open', { force }); const fromList = pickPreferred(listEntry.prs); if (fromList) { - return fromList; + return { open: fromList, historical: null }; } - listWasComplete = listEntry.complete; + openListWasComplete = listEntry.complete; } catch { - // fall through to the precise per-branch queries + // fall through to the precise per-head queries } - if (!listWasComplete) { - if (coverage) { - coverage.authoritative = false; - } - for (const owner of sourceOwners) { - const directCandidates = await safeListPulls(octokit, { - owner: target.repo.owner, - repo: target.repo.repo, - state: 'open', - head: `${owner}:${branch}`, - per_page: 100, - }); - const direct = pickPreferred(directCandidates); - if (direct) { - return direct; - } + if (!openListWasComplete && coverage) { + coverage.authoritative = false; + } + + // A complete open list already proved there is no open PR in this repo. With + // no history to look up there is nothing left to ask GitHub. + if (openListWasComplete && !includeHistory) { + return { open: null, historical: null }; + } + + const historicalKey = `${normalizeRepoKey(target.repo?.owner, target.repo?.repo)}::${branch}`; + if (includeHistory && !force && openListWasComplete) { + const cached = _historicalPrCache.get(historicalKey); + if (isHistoricalPrCacheFresh(cached)) { + return { open: null, historical: cached.pr }; } } - return null; + // One query per source owner. With history enabled `state: 'all'` answers + // both questions at once, so asking for history never costs an extra call. + let historical = null; + for (const owner of sourceOwners) { + const directCandidates = await safeListPulls(octokit, { + owner: target.repo.owner, + repo: target.repo.repo, + state: includeHistory ? 'all' : 'open', + head: `${owner}:${branch}`, + per_page: 100, + }); + const openMatch = pickPreferred(directCandidates.filter((pr) => !isTerminalPr(pr))); + if (openMatch) { + return { open: openMatch, historical: null }; + } + if (includeHistory && !historical) { + // Among past PRs for the same head the newest one is the relevant record. + historical = directCandidates + .filter((pr) => normalizeText(pr?.head?.ref) === branch) + .filter((pr) => matcher.matches(pr, target.repo.repo)) + .filter(isTerminalPr) + .sort((left, right) => (right?.number ?? 0) - (left?.number ?? 0))[0] ?? null; + } + } + + if (includeHistory) { + rememberHistoricalPr(historicalKey, historical); + } + return { open: null, historical }; }; -// Exported for focused unit tests of open-only branch matching. -export { findFirstMatchingPr }; +// Exported for focused unit tests of open-versus-historical branch matching. +export { findBranchPrCandidates }; export async function resolveGitHubPrStatus({ octokit, directory, branch, remoteName, force = false }) { // A deleted worktree can still have a session in the sidebar that keeps @@ -604,6 +683,11 @@ export async function resolveGitHubPrStatus({ octokit, directory, branch, remote let fallbackRemoteName = resolvedTargets[0].remoteName; let fallbackDefaultBranch = await getRepoDefaultBranch(octokit, fallbackRepo); + // The first closed/merged PR found, in target priority order. It is only + // returned once every target has been checked for an open PR, so an open + // upstream PR always wins over a merged fork PR for the same head. + let historicalMatch = null; + for (const target of resolvedTargets) { const defaultBranch = await getRepoDefaultBranch(octokit, target.repo); if (!fallbackRepo) { @@ -618,18 +702,33 @@ export async function resolveGitHubPrStatus({ octokit, directory, branch, remote continue; } - const pr = await findFirstMatchingPr({ + // History is only asked of the branch's own repo and its own name: the + // ranked-first target is the remote this branch actually pushes to. + // Searching the rest of the fork network for history would multiply + // serial GitHub calls for no additional user-visible information. + const isPrimaryAssociation = target === resolvedTargets[0] && candidateBranch === branchCandidates[0]; + + const { open, historical } = await findBranchPrCandidates({ octokit, target, branch: candidateBranch, sourceCandidates, force, coverage, + includeHistory: isPrimaryAssociation, }); - if (pr) { + if (open) { return { repo: target.repo, - pr, + pr: open, + defaultBranch, + resolvedRemoteName: target.remoteName, + }; + } + if (historical && !historicalMatch) { + historicalMatch = { + repo: target.repo, + pr: historical, defaultBranch, resolvedRemoteName: target.remoteName, }; @@ -656,6 +755,10 @@ export async function resolveGitHubPrStatus({ octokit, directory, branch, remote } } + if (historicalMatch) { + return historicalMatch; + } + return { repo: fallbackRepo, pr: null, diff --git a/packages/web/server/lib/github/pr-status.test.js b/packages/web/server/lib/github/pr-status.test.js index a481deca..bbde6c78 100644 --- a/packages/web/server/lib/github/pr-status.test.js +++ b/packages/web/server/lib/github/pr-status.test.js @@ -1,4 +1,4 @@ -import { beforeEach, describe, expect, mock, test } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, mock, setSystemTime, test } from 'bun:test'; const listMock = mock(async () => ({ data: [] })); @@ -15,7 +15,7 @@ mock.module('./rate-limit.js', () => ({ noteIfGitHubRateLimit: () => {}, })); -const { findFirstMatchingPr, invalidateRepoPullsCache } = await import('./pr-status.js'); +const { findBranchPrCandidates, invalidateRepoPullsCache } = await import('./pr-status.js'); const openPr = { number: 15, @@ -28,7 +28,7 @@ const openPr = { }, }; -const closedPr = { +const mergedPr = { number: 12, state: 'closed', merged_at: '2026-01-01T00:00:00Z', @@ -40,66 +40,141 @@ const closedPr = { }, }; -describe('findFirstMatchingPr open-only branch status', () => { +const olderMergedPr = { + ...mergedPr, + number: 7, + merged_at: '2025-11-01T00:00:00Z', +}; + +const call = (overrides = {}) => findBranchPrCandidates({ + octokit: { rest: { pulls: { list: listMock } } }, + target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' }, + branch: 'feature', + sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }], + force: true, + includeHistory: true, + ...overrides, +}); + +describe('findBranchPrCandidates', () => { beforeEach(() => { listMock.mockReset(); invalidateRepoPullsCache('acme', 'app'); }); - test('returns a matching open PR', async () => { - listMock.mockImplementation(async ({ state }) => { - if (state === 'open') { - return { data: [openPr] }; - } - return { data: [closedPr] }; - }); - - const pr = await findFirstMatchingPr({ - octokit: { rest: { pulls: { list: listMock } } }, - target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' }, - branch: 'feature', - sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }], - force: true, - }); - - expect(pr?.number).toBe(15); - expect(listMock.mock.calls.every((call) => call[0]?.state === 'open')).toBe(true); + afterEach(() => { + setSystemTime(); }); - test('returns null when only a closed/merged PR exists for the head branch', async () => { - listMock.mockImplementation(async ({ state }) => { - if (state === 'open') { - return { data: [] }; - } - return { data: [closedPr] }; - }); + test('an open PR wins and no history lookup is spent', async () => { + listMock.mockImplementation(async ({ state }) => ( + state === 'open' ? { data: [openPr] } : { data: [mergedPr] } + )); - const pr = await findFirstMatchingPr({ - octokit: { rest: { pulls: { list: listMock } } }, - target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' }, - branch: 'feature', - sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }], - force: true, - }); + const { open, historical } = await call(); - expect(pr).toBeNull(); - expect(listMock.mock.calls.every((call) => call[0]?.state === 'open')).toBe(true); - expect(listMock.mock.calls.some((call) => call[0]?.state === 'closed')).toBe(false); + expect(open?.number).toBe(15); + expect(historical).toBeNull(); + expect(listMock.mock.calls.every((entry) => entry[0]?.state === 'open')).toBe(true); }); - test('does not query closed PRs when the open list is complete and empty', async () => { + test('an open PR still wins when the shared open list missed it', async () => { + // A repo with more than one page of open PRs: the shared list is incomplete, + // so the per-head query is the one that must find the open PR. + listMock.mockImplementation(async ({ head }) => ( + head ? { data: [mergedPr, openPr] } : { data: new Array(100).fill(null).map((_, index) => ({ number: index, state: 'open', head: { ref: 'other' } })) } + )); + + const { open, historical } = await call(); + + expect(open?.number).toBe(15); + expect(historical).toBeNull(); + }); + + test('returns the branch history when no open PR exists', async () => { + listMock.mockImplementation(async ({ head }) => ( + head ? { data: [olderMergedPr, mergedPr] } : { data: [] } + )); + + const { open, historical } = await call(); + + expect(open).toBeNull(); + // The newest past PR for the head is the relevant record. + expect(historical?.number).toBe(12); + }); + + test('returns no history for a branch that never had a PR', async () => { listMock.mockImplementation(async () => ({ data: [] })); - const pr = await findFirstMatchingPr({ - octokit: { rest: { pulls: { list: listMock } } }, - target: { repo: { owner: 'acme', repo: 'app' }, remoteName: 'origin' }, - branch: 'feature', - sourceCandidates: [{ repo: { owner: 'acme', repo: 'app' } }], - force: true, - }); + const { open, historical } = await call(); - expect(pr).toBeNull(); + expect(open).toBeNull(); + expect(historical).toBeNull(); + expect(listMock.mock.calls.some((entry) => entry[0]?.state === 'all')).toBe(true); + }); + + test('spends no call on history for a secondary target', async () => { + listMock.mockImplementation(async ({ head }) => ( + head ? { data: [mergedPr] } : { data: [] } + )); + + const { open, historical } = await call({ includeHistory: false }); + + expect(open).toBeNull(); + expect(historical).toBeNull(); + // The complete open list already answered the only question that matters + // for a secondary repo in the fork network. expect(listMock.mock.calls).toHaveLength(1); expect(listMock.mock.calls[0]?.[0]?.state).toBe('open'); }); + + test('reuses the cached history instead of re-querying every poll', async () => { + listMock.mockImplementation(async ({ head }) => ( + head ? { data: [mergedPr] } : { data: [] } + )); + + await call(); + const callsAfterFirst = listMock.mock.calls.length; + + // A non-forced poll is answered entirely from the shared open list cache + // plus the remembered history — no extra GitHub call. + const { open, historical } = await call({ force: false }); + + expect(open).toBeNull(); + expect(historical?.number).toBe(12); + expect(listMock.mock.calls.length).toBe(callsAfterFirst); + }); + + test('a found record outlives the shorter "no history" window', async () => { + const startedAt = Date.now(); + listMock.mockImplementation(async ({ head }) => ( + head ? { data: [mergedPr] } : { data: [] } + )); + + await call(); + const callsAfterFirst = listMock.mock.calls.length; + + // Past the "no history" expiry, but far short of the found-record one. The + // shared open list is re-fetched; the history answer is not re-queried. + setSystemTime(new Date(startedAt + 30 * 60 * 1000)); + const { historical } = await call({ force: false }); + + expect(historical?.number).toBe(12); + expect(listMock.mock.calls.length).toBe(callsAfterFirst + 1); + expect(listMock.mock.calls.at(-1)?.[0]?.state).toBe('open'); + }); + + test('re-queries a branch with no history once its shorter window passes', async () => { + const startedAt = Date.now(); + listMock.mockImplementation(async () => ({ data: [] })); + + await call(); + const callsAfterFirst = listMock.mock.calls.length; + + setSystemTime(new Date(startedAt + 30 * 60 * 1000)); + await call({ force: false }); + + expect(listMock.mock.calls.some((entry) => entry[0]?.state === 'all')).toBe(true); + expect(listMock.mock.calls.length).toBeGreaterThan(callsAfterFirst + 1); + }); }); diff --git a/packages/web/server/lib/github/routes.js b/packages/web/server/lib/github/routes.js index 427348aa..c083c236 100644 --- a/packages/web/server/lib/github/routes.js +++ b/packages/web/server/lib/github/routes.js @@ -574,10 +574,17 @@ export function registerGitHubRoutes(app) { return res.json({ connected: true, repo: searchRepo, branch, pr: null, checks: null, canMerge: false }); } + const isMerged = Boolean(prData.merged || prData.merged_at); + const prState = isMerged ? 'merged' : (prData.state === 'closed' ? 'closed' : 'open'); + // A closed/merged PR is a historical record for this branch: its checks + // are no longer actionable and it can never be merged from here, so skip + // the extra GitHub calls those two fields would cost. + const isHistorical = prState !== 'open'; + // Checks summary: prefer check-runs (Actions), fallback to classic statuses. let checks = null; const sha = prData.head?.sha; - if (sha) { + if (sha && !isHistorical) { try { const runs = await octokit.rest.checks.listForRef({ owner: searchRepo.owner, @@ -610,38 +617,37 @@ export function registerGitHubRoutes(app) { // Permission check (best-effort) let canMerge = false; - try { - const auth = getGitHubAuth(); - // gh-CLI tokens have no persisted user record; resolve the login from - // the API once (memoized) so permissions still resolve for them. - let username = auth?.user?.login; - if (!username) { - if (!resolvedAuthLoginPromise) { - resolvedAuthLoginPromise = octokit.rest.users.getAuthenticated() - .then((resp) => resp?.data?.login || null) - .catch(() => { - resolvedAuthLoginPromise = null; - return null; - }); + if (!isHistorical) { + try { + const auth = getGitHubAuth(); + // gh-CLI tokens have no persisted user record; resolve the login from + // the API once (memoized) so permissions still resolve for them. + let username = auth?.user?.login; + if (!username) { + if (!resolvedAuthLoginPromise) { + resolvedAuthLoginPromise = octokit.rest.users.getAuthenticated() + .then((resp) => resp?.data?.login || null) + .catch(() => { + resolvedAuthLoginPromise = null; + return null; + }); + } + username = await resolvedAuthLoginPromise; } - username = await resolvedAuthLoginPromise; + if (username) { + const perm = await octokit.rest.repos.getCollaboratorPermissionLevel({ + owner: searchRepo.owner, + repo: searchRepo.repo, + username, + }); + const level = perm?.data?.permission; + canMerge = level === 'admin' || level === 'maintain' || level === 'write'; + } + } catch { + canMerge = false; } - if (username) { - const perm = await octokit.rest.repos.getCollaboratorPermissionLevel({ - owner: searchRepo.owner, - repo: searchRepo.repo, - username, - }); - const level = perm?.data?.permission; - canMerge = level === 'admin' || level === 'maintain' || level === 'write'; - } - } catch { - canMerge = false; } - const isMerged = Boolean(prData.merged || prData.merged_at); - const mergedState = isMerged ? 'merged' : (prData.state === 'closed' ? 'closed' : 'open'); - return res.json({ connected: true, repo: searchRepo, @@ -651,7 +657,7 @@ export function registerGitHubRoutes(app) { title: prData.title, body: prData.body || '', url: prData.html_url, - state: mergedState, + state: prState, draft: Boolean(prData.draft), base: prData.base?.ref, head: prData.head?.ref, From e3094ee676d651e501d3beaf021c262487146ff9 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sat, 15 Aug 2026 17:58:31 +0300 Subject: [PATCH 08/17] docs(changelog): add pending unreleased entries and reorder by impact --- CHANGELOG.md | 9 +++++++-- packages/vscode/CHANGELOG.md | 3 +++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2313f45a..46731e19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,11 +4,16 @@ All notable changes to this project will be documented in this file. ## [Unreleased] -- Git: the pull request panel now follows the branch's current open PR, and an open PR always wins over an older merged or closed one. After a PR is merged or closed the panel keeps showing it as the branch's last PR and offers creating the next one right below it (thanks to @makeittech). -- **Usage/Claude:** Claude plan limits now work when you are signed in through Claude Code, without also signing into Anthropic in OpenCode; the account is read from Claude Code's own login on macOS, Linux, and WSL. The page shows your session and weekly limits again, adds per-model weekly limits and extra usage spending, and names your plan. Limits are kept on screen instead of disappearing when Anthropic temporarily blocks refreshes. - **Settings/Integrations:** a new Integrations settings page lists Claude Code, Command Code, and Cursor plugins with install, update, setup, and remove actions, plus Discord and Telegram Coming soon placeholders. +- Usage/Claude: Claude plan limits now work when you are signed in through Claude Code, without also signing into Anthropic in OpenCode; the account is read from Claude Code's own login on macOS, Linux, and WSL. The page shows your session and weekly limits again, adds per-model weekly limits and extra usage spending, and names your plan. Limits are kept on screen instead of disappearing when Anthropic temporarily blocks refreshes. +- Git: the pull request panel now follows the branch's current open PR, and an open PR always wins over an older merged or closed one. After a PR is merged or closed the panel keeps showing it as the branch's last PR and offers creating the next one right below it (thanks to @makeittech). - Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. - Chat: opening a busy subagent in the context panel now shows its history instead of only the working-status line (thanks to @makeittech). +- Chat: saved chats in the context panel open again instead of staying blank. +- Projects: project names now match the folder name exactly, so `.ssh` and `opencode-claude` are no longer shown as `.Ssh` and `Opencode Claude` in the sidebar, window title, settings and notifications; names you renamed yourself are kept. +- Settings: the session retention action you pick is now saved instead of being dropped (thanks to @Gautam0507). +- Browser: typing a comment on a page no longer triggers app shortcuts. +- Skills Catalog: the source is now named ClawHub instead of "ClawdHub" (thanks to @makeittech). ## [1.18.4] - 2026-08-14 diff --git a/packages/vscode/CHANGELOG.md b/packages/vscode/CHANGELOG.md index 86d45514..44f43241 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -1,6 +1,9 @@ ## [Unreleased] - Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. +- Chat: opening a busy subagent in the context panel now shows its history instead of only the working-status line (thanks to @makeittech). +- Projects: project names now match the folder name exactly, so `.ssh` and `opencode-claude` are no longer shown as `.Ssh` and `Opencode Claude`; names you renamed yourself are kept. +- Skills Catalog: the source is now named ClawHub instead of "ClawdHub" (thanks to @makeittech). ## [1.18.4] - 2026-08-14 From 51aef5e3165de9b2d96bbfa74ac52177bf066c9c Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 15:55:08 +0300 Subject: [PATCH 09/17] chore(lint): vendor anti-slop oxlint plugin and add batched cleanup pipeline Vendor the anti-slop Oxlint plugin at tools/oxlint/anti-slop and register it in oxlint.config.ts, with Oxlint's own rule categories disabled so ESLint stays the general-purpose linter. Add scripts/anti-slop.mjs (bun run deslop) mirroring the React Doctor batch interface: next-batch, check-batch, active, release, top, file. Batch handoff directories now double as file claims shared across clones via ~/.openchamber/maintenance-claims, so concurrent maintenance batches from either pipeline never select the same file. Harden both scheduled maintenance flows: stop on a dirty worktree, stop on NO BATCH AVAILABLE, validate per package instead of workspace-wide, and pin react-doctor to 0.9.12. The anti-slop task command documents concrete good and bad fixes and forbids laundering types to satisfy a rule. --- .opencode/commands/as-fixes.md | 344 +++++++++++ .opencode/commands/as-follow-up.md | 69 +++ .opencode/commands/rd-fixes.md | 28 +- .opencode/commands/rd-follow-up.md | 40 +- AGENTS.md | 1 + bun.lock | 52 +- oxlint.config.ts | 47 ++ package.json | 6 +- scripts/anti-slop.mjs | 540 ++++++++++++++++++ scripts/lib/batch-claims.mjs | 107 ++++ scripts/react-doctor.mjs | 90 ++- tools/oxlint/anti-slop/index.ts | 41 ++ .../rules/no-chained-type-assertions.ts | 77 +++ .../no-conditional-empty-object-spread.ts | 49 ++ .../rules/no-known-value-widening.ts | 247 ++++++++ .../anti-slop/rules/no-module-mocking.ts | 91 +++ .../anti-slop/rules/no-object-parameters.ts | 126 ++++ .../anti-slop/rules/no-reflect-apply.ts | 28 + .../oxlint/anti-slop/rules/no-reflect-get.ts | 28 + .../anti-slop/rules/no-runtime-typeof.ts | 67 +++ .../rules/no-shape-in-symbol-names.ts | 39 ++ .../anti-slop/rules/no-unknown-parameters.ts | 83 +++ .../anti-slop/rules/no-unknown-returns.ts | 115 ++++ .../rules/no-unknown-type-aliases.ts | 70 +++ .../rules/no-unsafe-dictionary-type.ts | 134 +++++ .../anti-slop/rules/no-widen-then-assert.ts | 366 ++++++++++++ ...quire-safety-comment-for-type-assertion.ts | 62 ++ .../anti-slop/shared/dictionary-types.ts | 502 ++++++++++++++++ .../shared/lexical-type-parameters.ts | 61 ++ .../oxlint/anti-slop/shared/reflect-method.ts | 35 ++ 30 files changed, 3512 insertions(+), 33 deletions(-) create mode 100644 .opencode/commands/as-fixes.md create mode 100644 .opencode/commands/as-follow-up.md create mode 100644 oxlint.config.ts create mode 100644 scripts/anti-slop.mjs create mode 100644 scripts/lib/batch-claims.mjs create mode 100644 tools/oxlint/anti-slop/index.ts create mode 100644 tools/oxlint/anti-slop/rules/no-chained-type-assertions.ts create mode 100644 tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts create mode 100644 tools/oxlint/anti-slop/rules/no-known-value-widening.ts create mode 100644 tools/oxlint/anti-slop/rules/no-module-mocking.ts create mode 100644 tools/oxlint/anti-slop/rules/no-object-parameters.ts create mode 100644 tools/oxlint/anti-slop/rules/no-reflect-apply.ts create mode 100644 tools/oxlint/anti-slop/rules/no-reflect-get.ts create mode 100644 tools/oxlint/anti-slop/rules/no-runtime-typeof.ts create mode 100644 tools/oxlint/anti-slop/rules/no-shape-in-symbol-names.ts create mode 100644 tools/oxlint/anti-slop/rules/no-unknown-parameters.ts create mode 100644 tools/oxlint/anti-slop/rules/no-unknown-returns.ts create mode 100644 tools/oxlint/anti-slop/rules/no-unknown-type-aliases.ts create mode 100644 tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts create mode 100644 tools/oxlint/anti-slop/rules/no-widen-then-assert.ts create mode 100644 tools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.ts create mode 100644 tools/oxlint/anti-slop/shared/dictionary-types.ts create mode 100644 tools/oxlint/anti-slop/shared/lexical-type-parameters.ts create mode 100644 tools/oxlint/anti-slop/shared/reflect-method.ts diff --git a/.opencode/commands/as-fixes.md b/.opencode/commands/as-fixes.md new file mode 100644 index 00000000..cdf6f494 --- /dev/null +++ b/.opencode/commands/as-fixes.md @@ -0,0 +1,344 @@ +--- +description: Create an anti-slop lint cleanup PR from the next generated batch +agent: build +--- + +You are working in the OpenChamber repository. + +Goal: reduce anti-slop Oxlint findings in a small, reviewable maintenance PR. + +This task can run unattended on a schedule, so it must be safe to start at any moment and must stop cleanly when there is nothing to do. + +First, verify the worktree is safe to use: + +`git status --porcelain` + +If the output is not empty, stop immediately and report that the worktree has uncommitted changes. Do not stash, reset, discard, commit, or switch branches. Local work in progress must never end up in a maintenance PR. + +Then run: + +`bun run deslop -- next-batch --min-issues 25 --max-issues 60` + +Use the command output as the source of truth for this task scope. + +If the output contains `NO BATCH AVAILABLE`, stop immediately and report the printed reason. Do not create a branch, do not create a pull request, and do not look for other work. Concurrency is already handled: the command excludes files claimed by other active batches and refuses to exceed the active-batch limit. + +Background: anti-slop is a vendored Oxlint plugin at `tools/oxlint/anti-slop/`, configured in `oxlint.config.ts`. It rejects low-evidence typing: unjustified type assertions, `unknown`/`object`/`Record` contracts, ad hoc `typeof` narrowing, conditional `{}` spreads, and module mocking. Fixing a finding means giving the code real type evidence, never hiding the symptom. + +Workflow: +- Before generating the batch, switch to `main` and pull the latest remote changes. +- Read the `next-batch` output carefully. +- Use the exact `Run ID`, `Batch name`, `Branch name`, and `PR title` printed by the command. +- Create the branch using the printed `Branch name`. +- Work only on the selected files listed in the batch output. +- Treat the selected files as complete-file scope. Do not cherry-pick only the first N findings. +- Read each selected file fully before editing it. These findings sit on type contracts, so a local edit can change behavior at a distant call site. +- Fix as many findings as practical in the selected files. Your default should be to fix selected findings, not to skip them. + +## What a good fix looks like + +Every finding is the same underlying complaint: the code claims less about a value than it actually knows. A good fix restores the missing knowledge. A bad fix hides the complaint while the knowledge stays missing. The rule cannot tell the difference, so you must. + +Before editing, answer one question for the value in question: where does it actually come from? There are only three answers, and each has one correct fix. + +1. It comes from code in this repository. The real type already exists somewhere upstream. Find it and use it. No parsing, no assertion. +2. It crosses an I/O boundary: HTTP response, `postMessage`, file contents, `localStorage`, a child process, the OpenCode SDK edge. Parse it once at that boundary, then let the parsed type flow onward untouched. + +On parsing style, follow local precedent and do not introduce a new one. `zod` is declared as a dependency but is not currently used in the source, so a maintenance PR is the wrong place to start spreading it. Unless the file or package you are editing already parses with a schema library, write a small local parse function that takes the raw input, returns the domain type or `undefined`, and lives next to the boundary it guards. If you believe a schema library is genuinely warranted, skip the finding and say so in the PR body instead of introducing the pattern yourself. +3. It is genuinely dynamic, such as a plugin registry keyed by arbitrary strings. Then keep the open key but make the value type precise, and say so in the contract's name. + +### `no-unsafe-dictionary-type` + +Bad, and the most common lazy fix. The shape is known; the annotation throws it away. + +```ts +type QuotaSnapshot = Record; + +function readLimit(snapshot: QuotaSnapshot) { + return snapshot.limit; +} +``` + +Good. Name the contract and state the fields the code actually reads. + +```ts +type QuotaSnapshot = { + limit: number; + used: number; + resetsAt: string; +}; + +function readLimit(snapshot: QuotaSnapshot) { + return snapshot.limit; +} +``` + +Also good, when keys really are open but values are not. + +```ts +type ProviderQuotas = Record; +``` + +Still bad, and does not count as a fix: + +```ts +type QuotaSnapshot = Record; +type QuotaSnapshot = { [key: string]: object }; +type QuotaSnapshot = Record; +``` + +The third one is the sneaky one. Widening to a union of primitives satisfies the rule without describing anything. If you cannot name the fields, that is a signal the value is unparsed I/O; go to the boundary and parse it. + +### `no-unknown-parameters`, `no-unknown-returns`, `no-unknown-type-aliases` + +Bad. The function accepts anything and immediately guesses. + +```ts +function applyThemeMessage(message: unknown) { + const theme = message as { themeId: string }; + setTheme(theme.themeId); +} +``` + +Good. Parse at the boundary; the domain function receives a real type. + +```ts +type ThemeMessage = { themeId: string }; + +function parseThemeMessage(data: MessageEvent["data"]): ThemeMessage | undefined { + if (data === null || typeof data !== "object") return undefined; + const themeId = Reflect.get(data, "themeId"); + return typeof themeId === "string" ? { themeId } : undefined; +} + +function applyThemeMessage(message: ThemeMessage) { + setTheme(message.themeId); +} + +window.addEventListener("message", (event) => { + const message = parseThemeMessage(event.data); + if (message === undefined) return; + applyThemeMessage(message); +}); +``` + +The parse function itself will still report `no-runtime-typeof` and `no-reflect-get`, because it is doing exactly what those rules describe. That is expected and acceptable: the checks are now concentrated in one named boundary function instead of scattered through domain logic, and the domain function above is genuinely typed. Report these remaining findings in the PR body rather than hiding them. Do not silence them with inline suppressions. + +Note what changed at runtime: a malformed message is now ignored instead of silently producing `undefined` deeper in the call stack. That is a deliberate behavior decision and it belongs in the PR body. Never introduce a throw on a path that previously degraded quietly. + +The `cause` convention is the single allowed exception: `unknown` is correct for an error cause. + +### `no-known-value-widening` + +Bad. The annotation erases the known keys, so callers lose autocomplete and typo safety. + +```ts +const settingsBySlug: Record = { + appearance: appearanceSection, + keybindings: keybindingsSection, +}; +``` + +Good. Keep inference and validate the shape. + +```ts +const settingsBySlug = { + appearance: appearanceSection, + keybindings: keybindingsSection, +} satisfies Record; +``` + +`satisfies` checks every value against the contract while preserving the literal keys. Reach for it before anything else here. + +### `no-chained-type-assertions` and `no-widen-then-assert` + +Bad. The precise type existed and was thrown away, then guessed back. + +```ts +const raw = loadSession() as unknown as SessionSnapshot; +``` + +Good. Fix the upstream contract so the round trip is unnecessary. + +```ts +const snapshot = loadSession(); +``` + +If `loadSession` genuinely returns something imprecise, that function is the real defect. Fix it there when it is inside the batch scope; if it is outside, make the minimal supporting change and say so in the PR body. + +### `require-safety-comment-for-type-assertion` + +The first move is always to delete the assertion, not to document it. Only a small minority of these findings deserve a comment. + +Bad, and an automatic rejection at review: + +```ts +// SAFETY: this is safe. +const session = value as Session; + +// SAFETY: value is a Session. +const session = value as Session; + +// SAFETY: required by TypeScript. +const session = value as Session; +``` + +These say nothing. A valid comment names the check that already ran and the line or function that ran it, so a reviewer can verify the claim without trusting you. + +Good: + +```ts +const parsed = sessionSchema.safeParse(payload); +if (!parsed.success) return undefined; +// SAFETY: sessionSchema.safeParse above confirmed every field of Session. +const session = parsed.data as Session; +``` + +If you cannot write such a sentence truthfully, you do not have an assertion problem, you have a missing check. Add the check. + +### `no-conditional-empty-object-spread` + +This one changes behavior more often than it looks, so read the consumer before editing. + +Bad: + +```ts +const body = { + sessionId, + ...(title !== undefined ? { title } : {}), +}; +``` + +Good, when the consumer distinguishes a missing key from an explicit `undefined`, which is true for anything serialized to JSON or merged over defaults: + +```ts +const body: CreateSessionBody = { sessionId }; +if (title !== undefined) body.title = title; +``` + +Good, when the consumer treats both the same: + +```ts +const body = { sessionId, title }; +``` + +Choosing wrongly here sends `"title": null` or drops a field on a real API call. If you cannot determine which behavior the consumer needs by reading it, skip the finding and say why. + +### `no-runtime-typeof` + +Bad. An ad hoc check in the middle of domain logic. + +```ts +function resolveHost(stored: unknown) { + if (typeof stored === "string") return stored; + return DEFAULT_HOST; +} +``` + +Good. Read and validate where the value enters the program, then branch on real domain values. + +```ts +function readStoredHost(): string { + const stored = localStorage.getItem(STORED_HOST_KEY); + return stored !== null && stored.length > 0 ? stored : DEFAULT_HOST; +} +``` + +Here the fix removed the check entirely, because `localStorage.getItem` already has a precise contract: `string | null`. The original `unknown` was self-inflicted. Look for this case first; it is more common than it seems. + +When a real check is unavoidable, keep it inside one named boundary function as shown above, and accept that the boundary function keeps its finding. What is not acceptable is spreading the same check across domain code, or renaming it into a type predicate so it reads as intentional while nothing was actually established. + +### `no-module-mocking` + +Bad. The test mocks a module and therefore tests the mock. + +```ts +mock.module("../lib/runtimeFetch", () => ({ runtimeFetch: async () => ({ ok: true }) })); +``` + +Good. Pass the dependency in, and let the test supply a real function. + +```ts +async function loadStatus(fetchStatus: () => Promise) { + return fetchStatus(); +} + +test("returns the fetched status", async () => { + const status = await loadStatus(async () => ({ ok: true })); + expect(status.ok).toBe(true); +}); +``` + +If introducing the seam would restructure production code well beyond the batch, skip the finding and say so. Do not fake a seam you do not believe in. + +## How to know your fix is real + +Before moving to the next finding, check all four: + +- The code now knows something it did not know before. If you only rearranged syntax, it is not a fix. +- No new `any`, no new assertion, no new broad union invented to satisfy the checker. +- If you added parsing, you decided explicitly what happens on invalid input, and that decision is written in the PR body. +- If you changed a type used elsewhere, you searched for its call sites and updated them, rather than casting at the call site. + +Handle findings deliberately instead of skipping them: for parsing work, add the smallest schema that covers the fields actually used; for contract changes, follow call sites with search and update them; for tests, prefer real seams over widened fixtures. + +Skip a finding only when the fix would require broad architectural changes, unclear behavior changes, or changes outside the selected batch scope. If skipped, mention it in the PR body. + +Hard prohibitions. Each of these makes the lint output greener while making the code worse, and each is grounds for rejecting the whole PR: +- Do not disable, downgrade, or ignore anti-slop rules, in configuration or with inline comments. +- Do not add `any`, widen a type, or add an assertion in order to satisfy a rule. +- Do not write a generic or placeholder `// SAFETY:` comment. A comment that does not name a real, already-performed check is worse than the original finding. +- Do not invent a union of primitives to escape a dictionary rule. +- Do not move a rejected `typeof` check into a hand-written type predicate to get it out of the linter's way. +- Do not delete code, tests, or fields to make a finding disappear. +- Do not rename a symbol solely to dodge `no-shape-in-symbol-names`; rename it to what it actually is. +- Do not introduce a throw where the previous code degraded quietly. A parse failure on a path that used to fall back must keep falling back. +- Do not introduce a schema library, a new utility module, or a new architectural pattern as part of a lint cleanup. +- Do not edit `oxlint.config.ts` or `tools/oxlint/anti-slop/`. +- Do not edit `CHANGELOG.md`, package versions, or release metadata. This is internal maintenance with no user-facing change. +- Do not fix findings outside the selected files. + +After edits, run: + +`bun run deslop -- check-batch --run ` + +Then validate the packages you actually touched, not the whole workspace. For each affected package run its own checks, for example: + +`bun run --cwd packages/ui type-check` + +`bun run --cwd packages/ui lint` + +`bun run --cwd packages/ui test` + +Workspace-wide `bun run type-check` and `bun run lint` are CI's job. Run them locally only when a change crosses package boundaries or touches shared contracts. + +For files that TypeScript does not cover, such as server or CLI JavaScript, run the focused tests for that surface instead, for example `bun run --cwd packages/web test`. + +Validation and delivery: +- Confirm selected files have fewer findings than before. +- Confirm `Findings outside selected files delta` is not positive. If it is, you introduced new findings elsewhere; fix them before continuing. +- If validation fails, fix failures only if the fixes stay within the task scope. Otherwise stop and report the blocker. +- Commit the changes with a concise message. +- Push the branch. +- Create exactly one PR with `gh pr create` using the exact printed `PR title`. +- After the PR is created, switch back to `main` and pull the latest remote changes again. + +PR requirements: +- Use the exact printed `PR title`. +- Include the `Run ID`, `Batch name`, and `Branch name`. +- Include selected files. +- Include findings fixed according to `check-batch`. +- Include remaining findings in selected files. +- Include validation results for `check-batch` and every package-scoped type-check, lint, and test command you ran, naming the packages. +- Include a `Manual testing recommendations` section with focused checks for the changed behavior, based on the selected files and actual edits. Type-contract changes can alter runtime behavior at call sites, so name the affected surfaces concretely. +- Include any skipped findings and why. +- Include any `// SAFETY:` comment you added, with the invariant it documents. +- Include every parsing decision you introduced: what schema was added, and what now happens when input fails to parse. Reviewers must be able to see where behavior changed without reading the whole diff. + +Constraints: +- Keep the PR small and reviewable. +- Do not auto-merge. +- Do not modify unrelated files except minimal supporting changes required by selected-file fixes. +- Do not run broad formatting. +- Leave the batch's run directory intact after creating the PR. `next-batch` prints its location. That directory is both the handoff for the review follow-up task and the claim that stops another batch, including the React Doctor pipeline, from touching the same files. Deleting it early lets a parallel batch collide with this PR. Never delete it by hand; use `bun run deslop -- release --run `. +- If you stop before creating a PR for any reason, release the claim with `bun run deslop -- release --run ` so the files return to the pool. diff --git a/.opencode/commands/as-follow-up.md b/.opencode/commands/as-follow-up.md new file mode 100644 index 00000000..51b62b9f --- /dev/null +++ b/.opencode/commands/as-follow-up.md @@ -0,0 +1,69 @@ +--- +description: Follow up on an anti-slop PR by addressing review feedback +agent: build +--- + +You are working in the OpenChamber repository. + +Goal: follow up on an existing anti-slop maintenance PR, address Greptile/review bot feedback, and clean up the local batch handoff files when done. + +This task can run unattended on a schedule, so it must be safe to start at any moment and must stop cleanly when there is nothing to do. + +First, verify the worktree is safe to use: + +`git status --porcelain` + +If the output is not empty, stop immediately and report that the worktree has uncommitted changes. Do not stash, reset, discard, or switch branches. + +List the active batches: + +`bun run deslop -- active` + +The listing may include batches owned by the React Doctor pipeline; those are shown as `[pipeline rd]`. Never touch them. + +Workflow: +- If there are no active batches, stop and report that there is nothing to follow up. +- Each active batch corresponds to one open PR. Read its `batch.json` for `runId`, `branchName`, `batchName`, `prTitle`, and selected files. +- Use `gh` to find the open PR for each batch branch. +- Work on the oldest batch that has an open PR with unaddressed feedback. If several qualify, handle exactly one and leave the rest. +- If a batch's PR was already merged or closed, do not treat it as follow-up work. Release its claim with `bun run deslop -- release --run ` so its files return to the pool, then continue looking. +- If no batch has an open PR with actionable feedback, stop and report that. +- Switch to the batch branch using the exact `branchName`. +- Pull or update the branch from remote if needed. +- Use `gh` to inspect PR review comments, PR issue comments, review threads if available, and check run summaries if relevant. +- Focus specifically on Greptile/review bot feedback and actionable reviewer comments. +- Pay particular attention to comments questioning whether a type contract is now wrong, whether a `// SAFETY:` comment is accurate, or whether a call site was missed. These are the likely real defects in this kind of PR. +- Address actionable comments with minimal follow-up fixes. +- Keep changes within the original selected files whenever possible. +- If a review comment requires changes outside the selected files, make only the minimal required supporting change. +- Do not perform unrelated cleanup. +- Do not rewrite the original PR. +- Do not force-push. +- Do not disable, downgrade, or ignore anti-slop rules, and do not add `any`, widen a type, or add an assertion to satisfy a reviewer comment. +- Follow the same fix standards as the original batch task, described in `.opencode/commands/as-fixes.md` under "What a good fix looks like" and "Hard prohibitions". Read that section before editing. Review pressure is exactly when a laundered fix is most tempting. + +After fixes, run: + +`bun run deslop -- check-batch --run ` + +Then re-run the package-scoped checks for the packages you touched, for example `bun run --cwd packages/ui type-check`, `bun run --cwd packages/ui lint`, and `bun run --cwd packages/ui test`. Workspace-wide checks are CI's job. + +Delivery: +- Commit follow-up fixes with a concise message. +- Push the branch. +- Reply to addressed review comments using `gh`. +- For each specific review comment you addressed, reply with what was changed and the follow-up commit hash. +- If the feedback was a general PR comment, add one general PR comment summarizing what was addressed, commit hashes, and validation results. +- If a comment is intentionally not addressed, reply with a concise reason. +- Do not release the batch while its PR is still open and awaiting review. The claim is what keeps parallel batches off these files. +- Release the batch only once its PR has been merged or closed: `bun run deslop -- release --run `. +- After the follow-up is complete, switch back to `main` and pull the latest remote changes. + +Constraints: +- Work on exactly one anti-slop batch PR. +- Prefer the oldest batch with an open PR. +- Do not auto-merge. +- Do not close the PR. +- Do not edit `CHANGELOG.md`, package versions, or release metadata. +- Do not release or delete handoff directories for batches you did not handle. +- If validation fails and cannot be fixed safely within scope, leave the batch claimed and report the blocker. diff --git a/.opencode/commands/rd-fixes.md b/.opencode/commands/rd-fixes.md index 381711b7..bc026e08 100644 --- a/.opencode/commands/rd-fixes.md +++ b/.opencode/commands/rd-fixes.md @@ -7,12 +7,22 @@ You are working in the OpenChamber repository. Goal: reduce React Doctor diagnostics in a small, reviewable maintenance PR. -Start by running: +This task can run unattended on a schedule, so it must be safe to start at any moment and must stop cleanly when there is nothing to do. + +First, verify the worktree is safe to use: + +`git status --porcelain` + +If the output is not empty, stop immediately and report that the worktree has uncommitted changes. Do not stash, reset, discard, commit, or switch branches. Local work in progress must never end up in a maintenance PR. + +Then run: `bun run doctor -- next-batch --min-issues 75 --max-issues 120` Use the command output as the source of truth for this task scope. +If the output contains `NO BATCH AVAILABLE`, stop immediately and report the printed reason. Do not create a branch, do not create a pull request, and do not look for other work. Concurrency is already handled: the command excludes files claimed by other active batches and refuses to exceed the active-batch limit. + Workflow: - Before generating the batch, switch to `main` and pull the latest remote changes. - Read the `next-batch` output carefully. @@ -31,11 +41,15 @@ After edits, run: `bun run doctor -- check-batch --run ` -Then run: +Then validate the packages you actually touched, not the whole workspace. For each affected package run its own checks, for example: -`bun run type-check` +`bun run --cwd packages/ui type-check` -`bun run lint` +`bun run --cwd packages/ui lint` + +`bun run --cwd packages/ui test` + +Workspace-wide `bun run type-check` and `bun run lint` are CI's job. Run them locally only when a change crosses package boundaries or touches shared contracts. For files that TypeScript does not cover, such as server or CLI JavaScript, run the focused tests for that surface instead. Validation and delivery: - Confirm selected files have fewer diagnostics than before. @@ -51,7 +65,7 @@ PR requirements: - Include selected files. - Include diagnostics fixed according to `check-batch`. - Include remaining diagnostics in selected files. -- Include validation results for `bun run type-check` and `bun run lint`. +- Include validation results for every package-scoped type-check, lint, and test command you ran, naming the packages. - Include a `Manual testing recommendations` section with focused checks for the changed behavior. Base it on the selected files and actual edits, for example checking affected dropdowns, keyboard navigation, model/agent selection, settings controls, or mobile/desktop variants. - Include any skipped diagnostics and why. @@ -61,4 +75,6 @@ Constraints: - Do not modify unrelated files except minimal supporting changes required by selected-file fixes. - Do not run broad formatting. - Do not fix diagnostics outside the selected files. -- Leave `.tmp/react-doctor/runs//` intact after creating the PR. These files are the handoff for the review follow-up task. +- Do not edit `CHANGELOG.md`, package versions, or release metadata. This is internal maintenance with no user-facing change. +- Leave the batch's run directory intact after creating the PR. `next-batch` prints its location. That directory is both the handoff for the review follow-up task and the claim that stops another batch, including the anti-slop pipeline, from touching the same files. Deleting it early lets a parallel batch collide with this PR. Never delete it by hand; use `bun run doctor -- release --run `. +- If you stop before creating a PR for any reason, release the claim with `bun run doctor -- release --run ` so the files return to the pool. diff --git a/.opencode/commands/rd-follow-up.md b/.opencode/commands/rd-follow-up.md index b3ee888b..6bd75325 100644 --- a/.opencode/commands/rd-follow-up.md +++ b/.opencode/commands/rd-follow-up.md @@ -7,16 +7,27 @@ You are working in the OpenChamber repository. Goal: follow up on an existing React Doctor maintenance PR, address Greptile/review bot feedback, and clean up the local batch handoff files when done. -Inspect local React Doctor batch handoff files: +This task can run unattended on a schedule, so it must be safe to start at any moment and must stop cleanly when there is nothing to do. -`find .tmp/react-doctor/runs -maxdepth 2 -name batch.json -print 2>/dev/null || true` +First, verify the worktree is safe to use: + +`git status --porcelain` + +If the output is not empty, stop immediately and report that the worktree has uncommitted changes. Do not stash, reset, discard, or switch branches. + +List the active batches: + +`bun run doctor -- active` + +The listing may include batches owned by the anti-slop pipeline; those are shown as `[pipeline as]`. Never touch them. Workflow: -- Read the available `.tmp/react-doctor/runs/*/batch.json` files. -- Find the most recent batch that has `branchName`, `batchName`, and `prTitle`. -- Read its `Run ID`, `Batch name`, `Branch name`, `PR title`, and selected files. -- Use `gh` to find the open PR for that branch or title. -- If no open PR exists for the batch, stop and report that there is no PR to follow up. +- If there are no active batches, stop and report that there is nothing to follow up. +- Each active batch corresponds to one open PR. Read its `batch.json` for `runId`, `branchName`, `batchName`, `prTitle`, and selected files. +- Use `gh` to find the open PR for each batch branch. +- Work on the oldest batch that has an open PR with unaddressed feedback. If several qualify, handle exactly one and leave the rest. +- If a batch's PR was already merged or closed, do not treat it as follow-up work. Release its claim with `bun run doctor -- release --run ` so its files return to the pool, then continue looking. +- If no batch has an open PR with actionable feedback, stop and report that. - Switch to the batch branch using the exact `branchName`. - Pull or update the branch from remote if needed. - Use `gh` to inspect PR review comments, PR issue comments, review threads if available, and check run summaries if relevant. @@ -32,9 +43,7 @@ After fixes, run: `bun run doctor -- check-batch --run ` -`bun run type-check` - -`bun run lint` +Then re-run the package-scoped checks for the packages you touched, for example `bun run --cwd packages/ui type-check`, `bun run --cwd packages/ui lint`, and `bun run --cwd packages/ui test`. Workspace-wide checks are CI's job. Delivery: - Commit follow-up fixes with a concise message. @@ -43,14 +52,15 @@ Delivery: - For each specific review comment you addressed, reply with what was changed and the follow-up commit hash. - If the feedback was a general PR comment, add one general PR comment summarizing what was addressed, commit hashes, and validation results. - If a comment is intentionally not addressed, reply with a concise reason. -- After successful push and replies, delete only the completed batch handoff directory: `.tmp/react-doctor/runs//`. +- Do not release the batch while its PR is still open and awaiting review. The claim is what keeps parallel batches off these files. +- Release the batch only once its PR has been merged or closed: `bun run doctor -- release --run `. - After the follow-up is complete, switch back to `main` and pull the latest remote changes. Constraints: - Work on exactly one React Doctor batch PR. -- Prefer the most recent batch with an open PR. +- Prefer the oldest batch with an open PR. - Do not auto-merge. - Do not close the PR. -- Do not delete handoff files until comments are addressed, validation passes, and follow-up commits are pushed. -- Do not delete unrelated `.tmp/react-doctor/runs/*` directories. -- If validation fails and cannot be fixed safely within scope, do not delete the handoff directory. +- Do not edit `CHANGELOG.md`, package versions, or release metadata. +- Do not release or delete handoff directories for batches you did not handle. +- If validation fails and cannot be fixed safely within scope, leave the batch claimed and report the blocker. diff --git a/AGENTS.md b/AGENTS.md index f19ac241..0ea1ebb6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -123,6 +123,7 @@ Before adding guidance to a skill, identify its canonical owner. If another skil - Prefer focused tests and package-scoped type-check/lint for executable source changes. - Use workspace-wide checks for cross-workspace contracts, root tooling, dependencies, or shared generated assets. - Run `bun run dead-code` when source files are added/deleted/renamed or exports, types, entrypoints, or import shape change; inspect its report because it is non-blocking. +- Run `bunx oxlint ` on TypeScript/JavaScript files you created or substantially rewrote. This runs the vendored `anti-slop` plugin, which rejects low-evidence typing: unjustified type assertions, `unknown`/`object`/`Record` contracts, ad hoc `typeof` narrowing, and module mocking. Fix findings in code you authored. Pre-existing findings elsewhere are a known backlog: do not mass-fix them, and never silence a rule, weaken severity, or launder types to make the check pass. - Do not assume TypeScript/lint covers server JS, CLI JS, Electron helpers, or native behavior; run focused tests, syntax checks, builds, or runtime validation for the touched surface. - For docs-only or isolated config changes, run the narrowest relevant validation. - Report exactly what was and was not validated. Static checks alone do not prove runtime, relay, performance, or platform correctness. diff --git a/bun.lock b/bun.lock index 17212605..7dff8dfe 100644 --- a/bun.lock +++ b/bun.lock @@ -65,6 +65,7 @@ "devDependencies": { "@clack/prompts": "^1.1.0", "@eslint/js": "^9.33.0", + "@oxlint/plugins": "1.78.0", "@remixicon/react": "^4.7.0", "@tailwindcss/postcss": "^4.0.0", "@types/dom-speech-recognition": "^0.0.12", @@ -83,6 +84,7 @@ "globals": "^16.3.0", "node-addon-api": "7.1.1", "nodemon": "^3.1.7", + "oxlint": "1.78.0", "patch-package": "^8.0.0", "sharp": "^0.35.0", "tailwindcss": "^4.0.0", @@ -95,7 +97,7 @@ }, "packages/electron": { "name": "@openchamber/electron", - "version": "1.18.2", + "version": "1.18.4", "dependencies": { "@openchamber/web": "workspace:*", "electron-context-menu": "^4.1.2", @@ -132,7 +134,7 @@ }, "packages/ui": { "name": "@openchamber/ui", - "version": "1.18.2", + "version": "1.18.4", "dependencies": { "@aparajita/capacitor-secure-storage": "^8.0.0", "@base-ui/react": "^1.4.0", @@ -236,7 +238,7 @@ }, "packages/vscode": { "name": "openchamber", - "version": "1.18.2", + "version": "1.18.4", "dependencies": { "@openchamber/ui": "workspace:*", "@opencode-ai/sdk": "1.18.18", @@ -259,7 +261,7 @@ }, "packages/web": { "name": "@openchamber/web", - "version": "1.18.2", + "version": "1.18.4", "bin": { "openchamber": "./bin/cli.js", }, @@ -995,6 +997,46 @@ "@opencode-ai/sdk": ["@opencode-ai/sdk@1.18.18", "", { "dependencies": { "cross-spawn": "7.0.6" } }, "sha512-zJlwXskIR47V1dkPJqeKBgq7nejG1uU8lJaGIGqbX3MWRCT8vKn0fEotbxuPCKnTdmWsDyNGNg9q1qIliDSMDA=="], + "@oxlint/binding-android-arm-eabi": ["@oxlint/binding-android-arm-eabi@1.78.0", "", { "os": "android", "cpu": "arm" }, "sha512-Bu819lmAfZMUHErrpe0cEWj3iaefuUODHSU8+UbXy67V/r7/7f4K3FL0NmbD85E+wiFLDYuhP8Zlv0XnVeXshw=="], + + "@oxlint/binding-android-arm64": ["@oxlint/binding-android-arm64@1.78.0", "", { "os": "android", "cpu": "arm64" }, "sha512-CDfxZgB61B7buRdY2FJoAYYPPXCZ1EoC1LKscnC5dg3kjobdxiconvAvvN1BmHyW4PyFT3jRLDag/BY/roSNBQ=="], + + "@oxlint/binding-darwin-arm64": ["@oxlint/binding-darwin-arm64@1.78.0", "", { "os": "darwin", "cpu": "arm64" }, "sha512-2Y2U9Ahrz+OO0Ej88f9SJYq51/jUBp1Mc7iZu0ukrbeeZ3gpRGfzIFnoqfHDY96xr0GEfNrPUBFEy0nN5aD7HA=="], + + "@oxlint/binding-darwin-x64": ["@oxlint/binding-darwin-x64@1.78.0", "", { "os": "darwin", "cpu": "x64" }, "sha512-rpych6eJq6m9jDRypTEaPD1xysaEW5h9+xuxhGK/QhOg+/xaqPZrCrTNoIl/f3nEjuJeCEmstNDlrE9rJi/3/g=="], + + "@oxlint/binding-freebsd-x64": ["@oxlint/binding-freebsd-x64@1.78.0", "", { "os": "freebsd", "cpu": "x64" }, "sha512-IcMGrQT3QizkOESUJd5et+rOhVqSkNDfNik1cvrKDqIbzqx9KMtRswpFgkCuNTSwylCFLKhGUu8KmqY1ZnC0Dg=="], + + "@oxlint/binding-linux-arm-gnueabihf": ["@oxlint/binding-linux-arm-gnueabihf@1.78.0", "", { "os": "linux", "cpu": "arm" }, "sha512-/uLdoJ0IXE6vo/0f0LKjinQAp+re+VMaCWaNT8ENIv2EOCkSsc8SGaflXAuW0Jua2dq5+GLVWm1NQK7P3UFSNQ=="], + + "@oxlint/binding-linux-arm-musleabihf": ["@oxlint/binding-linux-arm-musleabihf@1.78.0", "", { "os": "linux", "cpu": "arm" }, "sha512-7xi4Wb/O8NRJhLoUXmDJMUVpNYvB5kefdhFU1Jb8rtae4QoXlTiLwI14X4YvAXVZLNZChP8m5qO9SQAlWQTbkQ=="], + + "@oxlint/binding-linux-arm64-gnu": ["@oxlint/binding-linux-arm64-gnu@1.78.0", "", { "os": "linux", "cpu": "arm64" }, "sha512-4hFW0+fVXa3OIh1Y4A5SPkmvI4wuuBSrCVKzOyE7PTjhc7yEqZ1pmvEEeS5Lj/MaqvegFxXyF33N+6jkehxdyg=="], + + "@oxlint/binding-linux-arm64-musl": ["@oxlint/binding-linux-arm64-musl@1.78.0", "", { "os": "linux", "cpu": "arm64" }, "sha512-oC0mvsgBJjlMijSDEhx9KuvR9zYeHXceA9MjbuXB1F8NSR78Yj2unOBrstEvTVaq+pko+kuue6DajC00eqvTdg=="], + + "@oxlint/binding-linux-ppc64-gnu": ["@oxlint/binding-linux-ppc64-gnu@1.78.0", "", { "os": "linux", "cpu": "ppc64" }, "sha512-XAllT5SUZS+ohjuZ3/5S0cwe0r7eboiuigeStCZ5DXRYx/2KVM2UvQXvAfyzXEimtQjAB7cDQ2YxDe2Zl2WNQQ=="], + + "@oxlint/binding-linux-riscv64-gnu": ["@oxlint/binding-linux-riscv64-gnu@1.78.0", "", { "os": "linux", "cpu": "none" }, "sha512-trucMER/0QtecoXvc1y/UVqE3kwJipDwrx4oHfj+nNm3dq2zjP44WT0CfHNDPM3G1DXIkx/gY6lAD21NSCZVhA=="], + + "@oxlint/binding-linux-riscv64-musl": ["@oxlint/binding-linux-riscv64-musl@1.78.0", "", { "os": "linux", "cpu": "none" }, "sha512-cm3O4F/HQbdzOUX5mKHqG5KDL6E5w0pnlZ+fbBy2rmLryPOowkuLagFHTopQsEIpjcaZoPOrL+BmmAytAG9HFg=="], + + "@oxlint/binding-linux-s390x-gnu": ["@oxlint/binding-linux-s390x-gnu@1.78.0", "", { "os": "linux", "cpu": "s390x" }, "sha512-33wRf6HqGNsybJ3qX4cGaQN2ODPxNmc1rMa0mrTmx3eFq1VzOnvQooi9bIGVYakW8a/wmqVx1mgsUm8R2xfTiw=="], + + "@oxlint/binding-linux-x64-gnu": ["@oxlint/binding-linux-x64-gnu@1.78.0", "", { "os": "linux", "cpu": "x64" }, "sha512-rRdISSYegj6VganMZ9tjRjijowfHJ09IZU01i0toBAqr6n5LEtwHq2IeS4FjW2RoskOHlb6efB26H5izYb3GEQ=="], + + "@oxlint/binding-linux-x64-musl": ["@oxlint/binding-linux-x64-musl@1.78.0", "", { "os": "linux", "cpu": "x64" }, "sha512-GmsP4rW0xTL6u5CVdcDsaN5Fbc7hBc382Wmar1kttbnwSEviM+rSINKOMQ+UQ6iH+AGwC+8gaAiwu134Tgh6Lg=="], + + "@oxlint/binding-openharmony-arm64": ["@oxlint/binding-openharmony-arm64@1.78.0", "", { "os": "none", "cpu": "arm64" }, "sha512-sy9yeYuADc8a+n4TLBayzMCZiHPW78DcIFVpOXTmdKHWQeM9xe5uzkqIIZmi326D5hY9XVwacipEB1p7tQjPAg=="], + + "@oxlint/binding-win32-arm64-msvc": ["@oxlint/binding-win32-arm64-msvc@1.78.0", "", { "os": "win32", "cpu": "arm64" }, "sha512-rjc2hF1KfMi8fZj1X/m3AmnHbdsF3rL0v6KQg0Uc880Yb2khjz+3U14sfdZ7jWTpRnN1m1NQa/TT7uU9lJWPrA=="], + + "@oxlint/binding-win32-ia32-msvc": ["@oxlint/binding-win32-ia32-msvc@1.78.0", "", { "os": "win32", "cpu": "ia32" }, "sha512-zcuXFVrEFHIafRfkCQT8w/Xe41o07ozl/vwHq7p94vB29xVzsB0sZGYORU1jhcYKv3Lr0J3HbJ2T4fHH5rWmvA=="], + + "@oxlint/binding-win32-x64-msvc": ["@oxlint/binding-win32-x64-msvc@1.78.0", "", { "os": "win32", "cpu": "x64" }, "sha512-Sb5ocmLSuYeOuXd+CFOToGKp/gjXUEWDnvIGwhnh8aq8wY4TMmEnKnvbogSW7RdMZv77JSARduS7/gv+khYEjA=="], + + "@oxlint/plugins": ["@oxlint/plugins@1.78.0", "", {}, "sha512-Ypt8KeRYw+4jUtlPirfcHWMrn5ms12VrrFPD+Mds477/7tJxG1Kcz2Yrg2nVcTQEUx/GdlhS+BUg1kmxNm04Ug=="], + "@paralleldrive/cuid2": ["@paralleldrive/cuid2@2.3.1", "", { "dependencies": { "@noble/hashes": "^1.1.5" } }, "sha512-XO7cAxhnTZl0Yggq6jOgjiOHhbgcO4NqFqwSmQpjK3b6TEE6Uj/jfSk6wzYyemh3+I0sHirKSetjQwn5cZktFw=="], "@peculiar/asn1-android": ["@peculiar/asn1-android@2.6.0", "", { "dependencies": { "@peculiar/asn1-schema": "^2.6.0", "asn1js": "^3.0.6", "tslib": "^2.8.1" } }, "sha512-cBRCKtYPF7vJGN76/yG8VbxRcHLPF3HnkoHhKOZeHpoVtbMYfY9ROKtH3DtYUY9m8uI1Mh47PRhHf2hSK3xcSQ=="], @@ -2663,6 +2705,8 @@ "own-keys": ["own-keys@1.0.1", "", { "dependencies": { "get-intrinsic": "^1.2.6", "object-keys": "^1.1.1", "safe-push-apply": "^1.0.0" } }, "sha512-qFOyK5PjiWZd+QQIh+1jhdb9LpxTF0qs7Pm8o5QHYZ0M3vKqSqzsZaEB6oWlxZ+q2sJBMI/Ktgd2N5ZwQoRHfg=="], + "oxlint": ["oxlint@1.78.0", "", { "optionalDependencies": { "@oxlint/binding-android-arm-eabi": "1.78.0", "@oxlint/binding-android-arm64": "1.78.0", "@oxlint/binding-darwin-arm64": "1.78.0", "@oxlint/binding-darwin-x64": "1.78.0", "@oxlint/binding-freebsd-x64": "1.78.0", "@oxlint/binding-linux-arm-gnueabihf": "1.78.0", "@oxlint/binding-linux-arm-musleabihf": "1.78.0", "@oxlint/binding-linux-arm64-gnu": "1.78.0", "@oxlint/binding-linux-arm64-musl": "1.78.0", "@oxlint/binding-linux-ppc64-gnu": "1.78.0", "@oxlint/binding-linux-riscv64-gnu": "1.78.0", "@oxlint/binding-linux-riscv64-musl": "1.78.0", "@oxlint/binding-linux-s390x-gnu": "1.78.0", "@oxlint/binding-linux-x64-gnu": "1.78.0", "@oxlint/binding-linux-x64-musl": "1.78.0", "@oxlint/binding-openharmony-arm64": "1.78.0", "@oxlint/binding-win32-arm64-msvc": "1.78.0", "@oxlint/binding-win32-ia32-msvc": "1.78.0", "@oxlint/binding-win32-x64-msvc": "1.78.0" }, "peerDependencies": { "oxlint-tsgolint": ">=7.0.2001", "vite-plus": "*" }, "optionalPeers": ["oxlint-tsgolint", "vite-plus"], "bin": { "oxlint": "bin/oxlint" } }, "sha512-QgQePuxIqKOzo1KSjG2EnITEeWvWnKAm77eq8nrMtf6AGoA+zyGc4PFYtDNJSD25g/ibOwfQ851hZ4/SPkMVoA=="], + "p-cancelable": ["p-cancelable@2.1.1", "", {}, "sha512-BZOr3nRQHOntUjTrH8+Lh54smKHoHyur8We1V8DSMVrl5A2malOOwuJRnKRDjSnkoeBh4at6BwEnb5I7Jl31wg=="], "p-limit": ["p-limit@3.1.0", "", { "dependencies": { "yocto-queue": "^0.1.0" } }, "sha512-TYOanM3wGwNGsZN2cVTYPArw454xnXj5qmWF1bEoAc4+cU/ol7GVh7odevjp1FNHduHc3KZMcFduxU5Xc6uJRQ=="], diff --git a/oxlint.config.ts b/oxlint.config.ts new file mode 100644 index 00000000..00409e7d --- /dev/null +++ b/oxlint.config.ts @@ -0,0 +1,47 @@ +import { defineConfig } from "oxlint"; + +// Oxlint here runs only the vendored anti-slop plugin; ESLint remains the +// general-purpose linter for this repository. +export default defineConfig({ + categories: { + correctness: "off", + }, + ignorePatterns: [ + "**/node_modules/**", + "**/dist/**", + "**/build/**", + "**/out/**", + "**/.next/**", + "**/ios/**", + "**/android/**", + ".agents/**", + ".claude/**", + ".conductor/**", + ".opencode/**", + ".openchamber/**", + ".tmp/**", + "patches/**", + "bun-patches/**", + "tools/oxlint/anti-slop/**", + ], + jsPlugins: [ + { name: "anti-slop", specifier: "./tools/oxlint/anti-slop/index.ts" }, + ], + rules: { + "anti-slop/no-chained-type-assertions": "error", + "anti-slop/no-conditional-empty-object-spread": "error", + "anti-slop/no-known-value-widening": "error", + "anti-slop/no-module-mocking": "error", + "anti-slop/no-object-parameters": "error", + "anti-slop/no-reflect-apply": "error", + "anti-slop/no-reflect-get": "error", + "anti-slop/no-runtime-typeof": "error", + "anti-slop/no-shape-in-symbol-names": "error", + "anti-slop/no-unknown-parameters": "error", + "anti-slop/no-unknown-returns": "error", + "anti-slop/no-unknown-type-aliases": "error", + "anti-slop/no-unsafe-dictionary-type": "error", + "anti-slop/no-widen-then-assert": "error", + "anti-slop/require-safety-comment-for-type-assertion": "error", + }, +}); diff --git a/package.json b/package.json index 0e100b17..f9c145f5 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "lint:ui": "bun run --cwd packages/ui lint", "lint:electron": "bun run --cwd packages/electron lint", "lint:mobile": "bun run --cwd packages/mobile lint", + "lint:anti-slop": "oxlint", "test": "node scripts/run-isolated-tests.mjs scripts && bun run --cwd packages/ui test && bun run --cwd packages/vscode test && bun run --cwd packages/electron test && bun run --cwd packages/web test", "clean": "bun run --filter '*' clean", "changelog-card": "node scripts/changelog-card/generate.mjs", @@ -74,6 +75,7 @@ "docs:validate": "node scripts/docs/validate-docs.mjs", "dead-code": "bunx knip@5.80.0 --no-exit-code --include files,exports,nsExports,types,nsTypes,enumMembers,duplicates", "doctor": "node scripts/react-doctor.mjs", + "deslop": "node scripts/anti-slop.mjs", "profile:browser": "node scripts/profile-browser.mjs", "icons:sprite": "node scripts/generate-file-type-sprite.mjs", "icons:generate": "bun run scripts/generate-icon-sprite.mjs", @@ -152,6 +154,8 @@ "devDependencies": { "@clack/prompts": "^1.1.0", "@eslint/js": "^9.33.0", + "@oxlint/plugins": "1.78.0", + "@remixicon/react": "^4.7.0", "@tailwindcss/postcss": "^4.0.0", "@types/dom-speech-recognition": "^0.0.12", "@types/node": "^24.3.1", @@ -169,8 +173,8 @@ "globals": "^16.3.0", "node-addon-api": "7.1.1", "nodemon": "^3.1.7", + "oxlint": "1.78.0", "patch-package": "^8.0.0", - "@remixicon/react": "^4.7.0", "sharp": "^0.35.0", "tailwindcss": "^4.0.0", "tsx": "^4.20.6", diff --git a/scripts/anti-slop.mjs b/scripts/anti-slop.mjs new file mode 100644 index 00000000..ca91c8e1 --- /dev/null +++ b/scripts/anti-slop.mjs @@ -0,0 +1,540 @@ +#!/usr/bin/env node + +import { execFileSync } from "node:child_process"; +import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { join } from "node:path"; + +import { + claimedFilePaths, + printClaims, + readActiveClaims, + releaseRun, + resolveRunsDir, + runDirPath, +} from "./lib/batch-claims.mjs"; + +const PIPELINE = "as"; +// Resolved before command dispatch so every command shares one claims location. +const { runsDir: RUNS_DIR, shared: SHARED_CLAIMS } = resolveRunsDir(parseArgs(process.argv.slice(2))["claims-dir"]); +const DEFAULT_MAX_ACTIVE = 3; +const DEFAULT_CLAIM_TTL_DAYS = 3; + +// Rules ordered by how mechanical and behavior-safe their fixes are. Higher +// scores are preferred when selecting the next batch. +const PRIORITY_RULES = new Map([ + ["no-object-parameters", 100], + ["no-shape-in-symbol-names", 95], + ["no-unknown-type-aliases", 90], + ["no-unknown-returns", 85], + ["no-unknown-parameters", 80], + ["no-unsafe-dictionary-type", 75], + ["no-conditional-empty-object-spread", 70], + ["no-known-value-widening", 65], + ["no-chained-type-assertions", 60], + ["no-widen-then-assert", 55], + ["no-reflect-get", 50], + ["no-reflect-apply", 50], + ["no-module-mocking", 30], + ["require-safety-comment-for-type-assertion", 20], + ["no-runtime-typeof", 10], +]); + +// Excluded by default because they account for most of the existing backlog and +// their fixes are the least mechanical. Opt in with --include-noisy. +const NOISY_RULES = new Set(["no-runtime-typeof", "require-safety-comment-for-type-assertion"]); + +function usage(exitCode = 0) { + const out = exitCode === 0 ? console.log : console.error; + out(`Usage: + bun run deslop -- next-batch [--min-issues 25] [--max-issues 60] [--max-files 4] + [--max-active ${DEFAULT_MAX_ACTIVE}] [--claim-ttl ${DEFAULT_CLAIM_TTL_DAYS}] [--include-noisy] + bun run deslop -- check-batch --run + bun run deslop -- active [--claim-ttl ${DEFAULT_CLAIM_TTL_DAYS}] + +Every command accepts --claims-dir to isolate a working copy. + bun run deslop -- release --run + bun run deslop -- file [--include-noisy] + bun run deslop -- top [--limit 10] [--include-noisy] + +Files selected by an active batch are excluded from later batches, so concurrent +batches never touch the same file, including batches created by the React Doctor +pipeline. Claims are shared across clones by default. A batch stays active until +it is released. + +Examples: + bun run deslop -- next-batch --min-issues 25 --max-issues 60 + bun run deslop -- file packages/ui/src/lib/settings/metadata.ts + bun run deslop -- check-batch --run 2026-08-16T10-12-44Z + bun run deslop -- release --run 2026-08-16T10-12-44Z`); + process.exit(exitCode); +} + +function parseArgs(argv) { + const args = { _: [] }; + for (let i = 0; i < argv.length; i += 1) { + const arg = argv[i]; + if (!arg.startsWith("--")) { + args._.push(arg); + continue; + } + const key = arg.slice(2); + const next = argv[i + 1]; + if (!next || next.startsWith("--")) { + args[key] = true; + continue; + } + args[key] = next; + i += 1; + } + return args; +} + +function asPositiveInt(value, fallback, name) { + if (value === undefined) return fallback; + const parsed = Number(value); + if (!Number.isInteger(parsed) || parsed <= 0) { + throw new Error(`Invalid --${name}: expected a positive integer.`); + } + return parsed; +} + +function runOxlint() { + // Oxlint exits non-zero whenever it reports findings, so the report has to be + // read from stdout of the failed invocation rather than treated as an error. + let output; + try { + output = execFileSync("bunx", ["oxlint", "--format", "json"], { + cwd: process.cwd(), + encoding: "utf8", + maxBuffer: 256 * 1024 * 1024, + stdio: ["ignore", "pipe", "pipe"], + }); + } catch (error) { + // The utf8 encoding above makes stdout a string whenever the run produced + // a report; an empty stdout means the run itself failed. + if (!error.stdout) throw error; + output = error.stdout; + } + const report = JSON.parse(output); + return { diagnostics: normalizeDiagnostics(report.diagnostics ?? []) }; +} + +function ruleOf(code) { + const match = /^anti-slop\((.+)\)$/.exec(code ?? ""); + return match ? match[1] : (code ?? "unknown"); +} + +function normalizeDiagnostics(rawDiagnostics) { + return rawDiagnostics.map((diagnostic) => { + const span = diagnostic.labels?.[0]?.span; + return { + filePath: diagnostic.filename, + rule: ruleOf(diagnostic.code), + severity: diagnostic.severity ?? "error", + message: diagnostic.message, + line: span?.line, + column: span?.column, + }; + }); +} + +function selectableDiagnostics(report, includeNoisy) { + if (includeNoisy) return report.diagnostics; + return report.diagnostics.filter((diagnostic) => !NOISY_RULES.has(diagnostic.rule)); +} + +function groupByFile(diagnostics) { + const byFile = new Map(); + for (const diagnostic of diagnostics) { + const list = byFile.get(diagnostic.filePath) ?? []; + list.push(diagnostic); + byFile.set(diagnostic.filePath, list); + } + return byFile; +} + +function rulePriority(rule) { + return PRIORITY_RULES.get(rule) ?? 50; +} + +function filePriority(diagnostics) { + const score = diagnostics.reduce((sum, diagnostic) => sum + rulePriority(diagnostic.rule), 0); + const mechanicalCount = diagnostics.filter((diagnostic) => rulePriority(diagnostic.rule) >= 75).length; + return score + mechanicalCount * 20; +} + +function sortedFileEntries(diagnostics) { + return [...groupByFile(diagnostics).entries()].sort((a, b) => { + const scoreDiff = filePriority(b[1]) - filePriority(a[1]); + if (scoreDiff !== 0) return scoreDiff; + const countDiff = b[1].length - a[1].length; + if (countDiff !== 0) return countDiff; + return a[0].localeCompare(b[0]); + }); +} + +function summarizeRules(diagnostics) { + const counts = new Map(); + for (const diagnostic of diagnostics) { + counts.set(diagnostic.rule, (counts.get(diagnostic.rule) ?? 0) + 1); + } + return [...counts.entries()].sort((a, b) => b[1] - a[1] || a[0].localeCompare(b[0])); +} + +function createRunId() { + return new Date().toISOString().replace(/:/g, "-").replace(/\.\d{3}Z$/, "Z"); +} + +function titleCase(value) { + return value + .replace(/[-_]+/g, " ") + .replace(/\b\w/g, (char) => char.toUpperCase()) + .trim(); +} + +function fileNameWithoutExtension(filePath) { + const fileName = filePath.split("/").at(-1) ?? filePath; + return fileName.replace(/\.[^.]+$/, ""); +} + +function slugify(value) { + return value + .replace(/([a-z0-9])([A-Z])/g, "$1-$2") + .toLowerCase() + .replace(/[^a-z0-9]+/g, "-") + .replace(/^-+|-+$/g, "") + .slice(0, 80); +} + +function createBatchMetadata(runId, selectedFiles) { + const [datePart, timePart = ""] = runId.replace(/Z$/, "").split("T"); + const timestamp = `${datePart.replace(/-/g, "")}-${timePart.replace(/-/g, "")}`; + const stems = selectedFiles.map((file) => fileNameWithoutExtension(file.filePath)); + const readableArea = stems.length === 1 + ? stems[0] + : `${stems.slice(0, 2).join(" and ")}${stems.length > 2 ? ` plus ${stems.length - 2}` : ""}`; + const areaSlug = slugify(stems.slice(0, 3).join("-")) || "batch"; + const batchName = `as-${timestamp}-${areaSlug}`; + + return { + batchName, + branchName: `anti-slop/${batchName}`, + prTitle: `Reduce anti-slop findings in ${titleCase(readableArea)}`, + }; +} + +function selectBatch(entries, minIssues, maxIssues, maxFiles) { + if (entries.length === 0) { + return { selected: [], oversized: false, belowTarget: false, reason: "No findings available for selection." }; + } + + const firstFitting = entries.find(([, diagnostics]) => diagnostics.length >= minIssues && diagnostics.length <= maxIssues); + if (firstFitting) { + return { + selected: [firstFitting], + oversized: false, + belowTarget: false, + reason: "A prioritized file already fits the target window.", + }; + } + + const oversized = entries.find(([, diagnostics]) => diagnostics.length > maxIssues); + if (oversized) { + return { + selected: [oversized], + oversized: true, + belowTarget: false, + reason: "A prioritized file exceeds the target window and was selected as a single complete-file batch.", + }; + } + + const selected = []; + let total = 0; + for (const entry of entries) { + if (selected.length >= maxFiles) break; + const count = entry[1].length; + if (total + count > maxIssues) { + if (total >= minIssues) break; + continue; + } + selected.push(entry); + total += count; + if (total >= minIssues) break; + } + + if (selected.length > 0) { + return { + selected, + oversized: false, + belowTarget: total < minIssues, + reason: total >= minIssues + ? "Added complete files until the batch reached the target window." + : "No combination reached the minimum without exceeding the maximum; selected the best smaller complete-file batch.", + }; + } + + return { + selected: [entries[0]], + oversized: false, + belowTarget: entries[0][1].length < minIssues, + reason: "Selected the best available complete file below the target window.", + }; +} + +function writeRun(runId, payload) { + const dir = runDirPath(RUNS_DIR, PIPELINE, runId); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, "baseline.json"), `${JSON.stringify(payload.report, null, 2)}\n`); + writeFileSync(join(dir, "batch.json"), `${JSON.stringify(payload.batch, null, 2)}\n`); + return dir; +} + +function readRun(runId) { + const dir = runDirPath(RUNS_DIR, PIPELINE, runId); + const baselinePath = join(dir, "baseline.json"); + const batchPath = join(dir, "batch.json"); + if (!existsSync(baselinePath) || !existsSync(batchPath)) { + throw new Error(`Unknown run: ${runId}`); + } + return { + baseline: JSON.parse(readFileSync(baselinePath, "utf8")), + batch: JSON.parse(readFileSync(batchPath, "utf8")), + }; +} + +function printReportHeader(report) { + const total = report.diagnostics.length; + const affected = groupByFile(report.diagnostics).size; + const noisy = report.diagnostics.filter((diagnostic) => NOISY_RULES.has(diagnostic.rule)).length; + console.log(`Total findings: ${total} across ${affected} files`); + console.log(`Excluded-by-default findings: ${noisy} (${[...NOISY_RULES].join(", ")})`); +} + +function commandNextBatch(args) { + const minIssues = asPositiveInt(args["min-issues"], 25, "min-issues"); + const maxIssues = asPositiveInt(args["max-issues"], 60, "max-issues"); + const maxFiles = asPositiveInt(args["max-files"], 4, "max-files"); + if (minIssues > maxIssues) throw new Error("--min-issues cannot be greater than --max-issues."); + const maxActive = asPositiveInt(args["max-active"], DEFAULT_MAX_ACTIVE, "max-active"); + const claimTtlDays = asPositiveInt(args["claim-ttl"], DEFAULT_CLAIM_TTL_DAYS, "claim-ttl"); + const includeNoisy = args["include-noisy"] === true; + + const claims = readActiveClaims(RUNS_DIR, claimTtlDays); + if (claims.length >= maxActive) { + console.log("Anti-Slop Next Batch"); + console.log(""); + console.log("NO BATCH AVAILABLE"); + console.log(`Reason: ${claims.length} active batches already exist and the limit is ${maxActive}.`); + console.log("Stop here. Do not create a branch or a pull request."); + console.log(""); + printClaims(claims, PIPELINE); + return; + } + + const report = runOxlint(); + const claimedPaths = claimedFilePaths(claims); + const candidates = selectableDiagnostics(report, includeNoisy) + .filter((diagnostic) => !claimedPaths.has(diagnostic.filePath)); + const entries = sortedFileEntries(candidates); + + if (entries.length === 0) { + console.log("Anti-Slop Next Batch"); + console.log(""); + console.log("NO BATCH AVAILABLE"); + console.log("Reason: no unclaimed findings remain for the selected rules."); + console.log("Stop here. Do not create a branch or a pull request."); + console.log(""); + printClaims(claims, PIPELINE); + return; + } + const selection = selectBatch(entries, minIssues, maxIssues, maxFiles); + const runId = createRunId(); + const selectedFiles = selection.selected.map(([filePath, fileDiagnostics]) => ({ + filePath, + diagnosticCount: fileDiagnostics.length, + rules: summarizeRules(fileDiagnostics), + })); + const metadata = createBatchMetadata(runId, selectedFiles); + const batch = { + runId, + ...metadata, + minIssues, + maxIssues, + maxFiles, + maxActive, + includeNoisy, + selectedFiles, + oversized: selection.oversized, + belowTarget: selection.belowTarget, + reason: selection.reason, + }; + const runDir = writeRun(runId, { report, batch }); + + console.log("Anti-Slop Next Batch"); + console.log(""); + console.log(`Run ID: ${runId}`); + console.log(`Batch name: ${batch.batchName}`); + console.log(`Branch name: ${batch.branchName}`); + console.log(`PR title: ${batch.prTitle}`); + console.log(`Baseline: ${join(runDir, "baseline.json")}`); + console.log(`Batch metadata: ${join(runDir, "batch.json")}`); + console.log(""); + printReportHeader(report); + console.log(""); + console.log(`Batch window: ${minIssues}-${maxIssues} findings`); + console.log(`Noisy rules included: ${includeNoisy ? "yes" : "no"}`); + console.log(`Active batches before this one: ${claims.length} of ${maxActive}`); + console.log(`Claims directory: ${RUNS_DIR} (${SHARED_CLAIMS ? "shared default" : "override"})`); + console.log(`Files excluded as claimed by active batches: ${claimedPaths.size}`); + console.log(`Selection mode: complete files only`); + console.log(`Batch total: ${selectedFiles.reduce((sum, file) => sum + file.diagnosticCount, 0)} findings`); + console.log(`Oversized: ${selection.oversized ? "yes" : "no"}`); + console.log(`Below target: ${selection.belowTarget ? "yes" : "no"}`); + console.log(`Selection reason: ${selection.reason}`); + console.log(""); + console.log("Selected files:"); + selection.selected.forEach(([filePath, fileDiagnostics], index) => { + console.log(`${index + 1}. ${filePath}`); + console.log(` Findings: ${fileDiagnostics.length}`); + console.log(" Rules:"); + for (const [rule, count] of summarizeRules(fileDiagnostics)) { + console.log(` ${String(count).padStart(3)} ${rule}`); + } + console.log(" Findings detail:"); + for (const diagnostic of fileDiagnostics) { + console.log(` line ${diagnostic.line ?? "?"}:${diagnostic.column ?? "?"} ${diagnostic.severity} ${diagnostic.rule}`); + console.log(` ${diagnostic.message}`); + } + console.log(""); + }); +} + +function commandActive(args) { + const claimTtlDays = asPositiveInt(args["claim-ttl"], DEFAULT_CLAIM_TTL_DAYS, "claim-ttl"); + console.log(`Claims directory: ${RUNS_DIR} (${SHARED_CLAIMS ? "shared default" : "override"})`); + printClaims(readActiveClaims(RUNS_DIR, claimTtlDays), PIPELINE); +} + +function commandRelease(args) { + const runId = args.run; + if (!runId || runId === true) throw new Error("Missing --run ."); + const dir = releaseRun(RUNS_DIR, PIPELINE, runId); + console.log(`Released batch ${runId}`); + console.log(`Removed ${dir}`); +} + +function commandTop(args) { + const limit = asPositiveInt(args.limit, 10, "limit"); + const includeNoisy = args["include-noisy"] === true; + const report = runOxlint(); + const entries = sortedFileEntries(selectableDiagnostics(report, includeNoisy)).slice(0, limit); + console.log(`Top ${limit} files by prioritized anti-slop findings`); + console.log(""); + for (const [filePath, diagnostics] of entries) { + console.log(`${String(diagnostics.length).padStart(4)} ${filePath}`); + console.log(` ${summarizeRules(diagnostics).map(([rule, count]) => `${rule} ${count}`).join(", ")}`); + } +} + +function commandFile(args) { + const filePath = args._[1]; + if (!filePath) throw new Error("Missing file path. Usage: bun run deslop -- file "); + const includeNoisy = args["include-noisy"] === true; + const report = runOxlint(); + const diagnostics = groupByFile(selectableDiagnostics(report, includeNoisy)).get(filePath) ?? []; + console.log(filePath); + console.log(`${diagnostics.length} findings`); + console.log(""); + if (diagnostics.length === 0) return; + console.log("Rules:"); + for (const [rule, count] of summarizeRules(diagnostics)) { + console.log(`${String(count).padStart(4)} ${rule}`); + } + console.log(""); + console.log("Findings:"); + for (const diagnostic of diagnostics) { + console.log(`line ${diagnostic.line ?? "?"}:${diagnostic.column ?? "?"} ${diagnostic.severity} ${diagnostic.rule}`); + console.log(` ${diagnostic.message}`); + } +} + +function commandCheckBatch(args) { + const runId = args.run; + if (!runId || runId === true) throw new Error("Missing --run ."); + const { baseline, batch } = readRun(runId); + const current = runOxlint(); + const includeNoisy = batch.includeNoisy === true; + const beforeDiagnostics = selectableDiagnostics(baseline, includeNoisy); + const afterDiagnostics = selectableDiagnostics(current, includeNoisy); + const beforeByFile = groupByFile(beforeDiagnostics); + const afterByFile = groupByFile(afterDiagnostics); + const selected = batch.selectedFiles ?? []; + let beforeTotal = 0; + let afterTotal = 0; + + console.log("Anti-Slop Batch Check"); + console.log(""); + console.log(`Run ID: ${runId}`); + if (batch.batchName) console.log(`Batch name: ${batch.batchName}`); + if (batch.branchName) console.log(`Branch name: ${batch.branchName}`); + if (batch.prTitle) console.log(`PR title: ${batch.prTitle}`); + console.log(""); + console.log("Selected files:"); + for (const file of selected) { + const before = beforeByFile.get(file.filePath)?.length ?? 0; + const after = afterByFile.get(file.filePath)?.length ?? 0; + beforeTotal += before; + afterTotal += after; + console.log(file.filePath); + console.log(` Before: ${before}`); + console.log(` After: ${after}`); + console.log(` Delta: ${after - before}`); + } + + const selectedPaths = new Set(selected.map((file) => file.filePath)); + const beforeOutside = beforeDiagnostics.filter((diagnostic) => !selectedPaths.has(diagnostic.filePath)).length; + const afterOutside = afterDiagnostics.filter((diagnostic) => !selectedPaths.has(diagnostic.filePath)).length; + + console.log(""); + console.log("Batch result:"); + console.log(`Fixed findings in selected files: ${Math.max(0, beforeTotal - afterTotal)}`); + console.log(`Remaining findings in selected files: ${afterTotal}`); + console.log(`Findings outside selected files delta: ${afterOutside - beforeOutside}`); + console.log(""); + console.log("Current repository summary:"); + printReportHeader(current); +} + +async function main() { + const args = parseArgs(process.argv.slice(2)); + const command = args._[0]; + if (!command || command === "help" || args.help) usage(0); + + switch (command) { + case "next-batch": + commandNextBatch(args); + break; + case "top": + commandTop(args); + break; + case "file": + commandFile(args); + break; + case "check-batch": + commandCheckBatch(args); + break; + case "active": + commandActive(args); + break; + case "release": + commandRelease(args); + break; + default: + throw new Error(`Unknown command: ${command}`); + } +} + +main().catch((error) => { + console.error(error.message); + process.exit(1); +}); diff --git a/scripts/lib/batch-claims.mjs b/scripts/lib/batch-claims.mjs new file mode 100644 index 00000000..fab1b8e5 --- /dev/null +++ b/scripts/lib/batch-claims.mjs @@ -0,0 +1,107 @@ +import { readdirSync, readFileSync, rmSync, existsSync } from "node:fs"; +import { homedir } from "node:os"; +import { join } from "node:path"; + +// Batch handoff directories double as file claims. A run directory exists from +// the moment its batch is generated until its follow-up task releases it, so +// concurrent maintenance batches can be kept file-disjoint. +// +// Maintenance pipelines are expected to run from dedicated clones of the same +// repository, so claims live outside the working copy by default. Every clone +// and every pipeline therefore sees the same claims without any per-scheduler +// configuration. Override with --claims-dir or OPENCHAMBER_BATCH_CLAIMS_DIR +// only when a working copy must be isolated, for example while experimenting. + +const DAY_MS = 24 * 60 * 60 * 1000; +const SHARED_CLAIMS_ENV = "OPENCHAMBER_BATCH_CLAIMS_DIR"; +const DEFAULT_CLAIMS_DIR = join(homedir(), ".openchamber", "maintenance-claims"); + +function expandHome(path) { + if (path === "~") return homedir(); + if (path.startsWith("~/")) return join(homedir(), path.slice(2)); + return path; +} + +export function resolveRunsDir(claimsDirArgument) { + const override = claimsDirArgument ?? process.env[SHARED_CLAIMS_ENV]; + if (override !== undefined && override !== true) { + return { runsDir: join(expandHome(override), "runs"), shared: false }; + } + return { runsDir: join(DEFAULT_CLAIMS_DIR, "runs"), shared: true }; +} + +export function runDirName(pipeline, runId) { + return `${pipeline}-${runId}`; +} + +export function runDirPath(runsDir, pipeline, runId) { + return join(runsDir, runDirName(pipeline, runId)); +} + +function parseDirName(dirName) { + const match = /^([a-z]+)-(\d{4}-\d{2}-\d{2}T\d{2}-\d{2}-\d{2}Z?)$/.exec(dirName); + if (!match) return undefined; + const [, pipeline, stamp] = match; + const [date, time] = stamp.replace(/Z$/, "").split("T"); + const createdAt = Date.parse(`${date}T${time.replace(/-/g, ":")}Z`); + return { pipeline, runId: stamp, createdAt: Number.isNaN(createdAt) ? undefined : createdAt }; +} + +export function readActiveClaims(runsDir, claimTtlDays) { + if (!existsSync(runsDir)) return []; + const now = Date.now(); + const claims = []; + + for (const entry of readdirSync(runsDir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const parsed = parseDirName(entry.name); + if (!parsed) continue; + + const batchPath = join(runsDir, entry.name, "batch.json"); + if (!existsSync(batchPath)) continue; + + let batch; + try { + batch = JSON.parse(readFileSync(batchPath, "utf8")); + } catch { + continue; + } + + const expired = parsed.createdAt !== undefined && now - parsed.createdAt > claimTtlDays * DAY_MS; + if (expired) continue; + + claims.push({ + pipeline: parsed.pipeline, + runId: batch.runId ?? parsed.runId, + branchName: batch.branchName, + createdAt: parsed.createdAt, + filePaths: (batch.selectedFiles ?? []).map((file) => file.filePath), + }); + } + + return claims.sort((a, b) => (a.createdAt ?? 0) - (b.createdAt ?? 0)); +} + +export function claimedFilePaths(claims) { + return new Set(claims.flatMap((claim) => claim.filePaths)); +} + +export function releaseRun(runsDir, pipeline, runId) { + const dir = runDirPath(runsDir, pipeline, runId); + if (!existsSync(dir)) throw new Error(`Unknown run: ${runId}`); + rmSync(dir, { recursive: true, force: true }); + return dir; +} + +export function printClaims(claims, ownPipeline) { + if (claims.length === 0) { + console.log("Active batches: none"); + return; + } + console.log(`Active batches: ${claims.length}`); + for (const claim of claims) { + const owner = claim.pipeline === ownPipeline ? "this pipeline" : `pipeline ${claim.pipeline}`; + console.log(` ${claim.runId} ${claim.branchName ?? "(no branch)"} [${owner}]`); + for (const filePath of claim.filePaths) console.log(` ${filePath}`); + } +} diff --git a/scripts/react-doctor.mjs b/scripts/react-doctor.mjs index f800ebb4..5ed108b5 100644 --- a/scripts/react-doctor.mjs +++ b/scripts/react-doctor.mjs @@ -4,8 +4,24 @@ import { execFileSync } from "node:child_process"; import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs"; import { join } from "node:path"; +import { + claimedFilePaths, + printClaims, + readActiveClaims, + releaseRun, + resolveRunsDir, + runDirPath, +} from "./lib/batch-claims.mjs"; + const PROJECT_NAME = "openchamber-monorepo"; -const RUNS_DIR = join(process.cwd(), ".tmp", "react-doctor", "runs"); +// Pinned so unattended batch runs cannot change diagnostics or output shape +// without an explicit update here. +const REACT_DOCTOR_VERSION = "0.9.12"; +const PIPELINE = "rd"; +// Resolved before command dispatch so every command shares one claims location. +const { runsDir: RUNS_DIR, shared: SHARED_CLAIMS } = resolveRunsDir(parseArgs(process.argv.slice(2))["claims-dir"]); +const DEFAULT_MAX_ACTIVE = 3; +const DEFAULT_CLAIM_TTL_DAYS = 3; const PRIORITY_RULES = new Map([ ["effect-needs-cleanup", 100], @@ -88,14 +104,25 @@ function usage(exitCode = 0) { const out = exitCode === 0 ? console.log : console.error; out(`Usage: bun run doctor -- next-batch [--min-issues 75] [--max-issues 120] [--max-files 4] + [--max-active ${DEFAULT_MAX_ACTIVE}] [--claim-ttl ${DEFAULT_CLAIM_TTL_DAYS}] bun run doctor -- check-batch --run + bun run doctor -- active [--claim-ttl ${DEFAULT_CLAIM_TTL_DAYS}] + +Every command accepts --claims-dir to isolate a working copy. + bun run doctor -- release --run bun run doctor -- file bun run doctor -- top [--limit 10] +Files selected by an active batch are excluded from later batches, so concurrent +batches never touch the same file, including batches created by the anti-slop +pipeline. Claims are shared across clones by default. A batch stays active until +it is released. + Examples: bun run doctor -- next-batch --min-issues 75 --max-issues 120 bun run doctor -- file packages/ui/src/components/chat/ChatInput.tsx - bun run doctor -- check-batch --run 2026-05-14T12-31-44`); + bun run doctor -- check-batch --run 2026-05-14T12-31-44Z + bun run doctor -- release --run 2026-05-14T12-31-44Z`); process.exit(exitCode); } @@ -132,7 +159,7 @@ function runReactDoctor() { const output = execFileSync( "npx", [ - "react-doctor@latest", + `react-doctor@${REACT_DOCTOR_VERSION}`, "--project", PROJECT_NAME, "--json", @@ -296,7 +323,7 @@ function selectBatch(entries, minIssues, maxIssues, maxFiles) { } function writeRun(runId, payload) { - const dir = join(RUNS_DIR, runId); + const dir = runDirPath(RUNS_DIR, PIPELINE, runId); mkdirSync(dir, { recursive: true }); writeFileSync(join(dir, "baseline.json"), `${JSON.stringify(payload.report, null, 2)}\n`); writeFileSync(join(dir, "batch.json"), `${JSON.stringify(payload.batch, null, 2)}\n`); @@ -304,7 +331,7 @@ function writeRun(runId, payload) { } function readRun(runId) { - const dir = join(RUNS_DIR, runId); + const dir = runDirPath(RUNS_DIR, PIPELINE, runId); const baselinePath = join(dir, "baseline.json"); const batchPath = join(dir, "batch.json"); if (!existsSync(baselinePath) || !existsSync(batchPath)) { @@ -336,10 +363,36 @@ function commandNextBatch(args) { const maxIssues = asPositiveInt(args["max-issues"], 120, "max-issues"); const maxFiles = asPositiveInt(args["max-files"], 4, "max-files"); if (minIssues > maxIssues) throw new Error("--min-issues cannot be greater than --max-issues."); + const maxActive = asPositiveInt(args["max-active"], DEFAULT_MAX_ACTIVE, "max-active"); + const claimTtlDays = asPositiveInt(args["claim-ttl"], DEFAULT_CLAIM_TTL_DAYS, "claim-ttl"); + + const claims = readActiveClaims(RUNS_DIR, claimTtlDays); + if (claims.length >= maxActive) { + console.log("React Doctor Next Batch"); + console.log(""); + console.log("NO BATCH AVAILABLE"); + console.log(`Reason: ${claims.length} active batches already exist and the limit is ${maxActive}.`); + console.log("Stop here. Do not create a branch or a pull request."); + console.log(""); + printClaims(claims, PIPELINE); + return; + } const report = runReactDoctor(); - const diagnostics = allDiagnostics(report); + const claimedPaths = claimedFilePaths(claims); + const diagnostics = allDiagnostics(report).filter((diagnostic) => !claimedPaths.has(diagnostic.filePath)); const entries = sortedFileEntries(diagnostics); + + if (entries.length === 0) { + console.log("React Doctor Next Batch"); + console.log(""); + console.log("NO BATCH AVAILABLE"); + console.log("Reason: no unclaimed diagnostics remain."); + console.log("Stop here. Do not create a branch or a pull request."); + console.log(""); + printClaims(claims, PIPELINE); + return; + } const selection = selectBatch(entries, minIssues, maxIssues, maxFiles); const runId = createRunId(); const selectedFiles = selection.selected.map(([filePath, fileDiagnostics]) => ({ @@ -348,7 +401,7 @@ function commandNextBatch(args) { rules: summarizeRules(fileDiagnostics), })); const metadata = createBatchMetadata(runId, selectedFiles); - const batch = { runId, ...metadata, minIssues, maxIssues, maxFiles, selectedFiles, oversized: selection.oversized, belowTarget: selection.belowTarget, reason: selection.reason }; + const batch = { runId, ...metadata, minIssues, maxIssues, maxFiles, maxActive, selectedFiles, oversized: selection.oversized, belowTarget: selection.belowTarget, reason: selection.reason }; const runDir = writeRun(runId, { report, batch }); console.log("React Doctor Next Batch"); @@ -363,6 +416,9 @@ function commandNextBatch(args) { printReportHeader(report); console.log(""); console.log(`Batch window: ${minIssues}-${maxIssues} diagnostics`); + console.log(`Active batches before this one: ${claims.length} of ${maxActive}`); + console.log(`Claims directory: ${RUNS_DIR} (${SHARED_CLAIMS ? "shared default" : "override"})`); + console.log(`Files excluded as claimed by active batches: ${claimedPaths.size}`); console.log(`Selection mode: complete files only`); console.log(`Batch total: ${selectedFiles.reduce((sum, file) => sum + file.diagnosticCount, 0)} diagnostics`); console.log(`Oversized: ${selection.oversized ? "yes" : "no"}`); @@ -387,6 +443,20 @@ function commandNextBatch(args) { }); } +function commandActive(args) { + const claimTtlDays = asPositiveInt(args["claim-ttl"], DEFAULT_CLAIM_TTL_DAYS, "claim-ttl"); + console.log(`Claims directory: ${RUNS_DIR} (${SHARED_CLAIMS ? "shared default" : "override"})`); + printClaims(readActiveClaims(RUNS_DIR, claimTtlDays), PIPELINE); +} + +function commandRelease(args) { + const runId = args.run; + if (!runId || runId === true) throw new Error("Missing --run ."); + const dir = releaseRun(RUNS_DIR, PIPELINE, runId); + console.log(`Released batch ${runId}`); + console.log(`Removed ${dir}`); +} + function commandTop(args) { const limit = asPositiveInt(args.limit, 10, "limit"); const report = runReactDoctor(); @@ -479,6 +549,12 @@ async function main() { case "check-batch": commandCheckBatch(args); break; + case "active": + commandActive(args); + break; + case "release": + commandRelease(args); + break; default: throw new Error(`Unknown command: ${command}`); } diff --git a/tools/oxlint/anti-slop/index.ts b/tools/oxlint/anti-slop/index.ts new file mode 100644 index 00000000..2b4ae222 --- /dev/null +++ b/tools/oxlint/anti-slop/index.ts @@ -0,0 +1,41 @@ +import { eslintCompatPlugin } from "@oxlint/plugins"; + +import { noChainedTypeAssertionsRule } from "./rules/no-chained-type-assertions.ts"; +import { noConditionalEmptyObjectSpreadRule } from "./rules/no-conditional-empty-object-spread.ts"; +import { noKnownValueWideningRule } from "./rules/no-known-value-widening.ts"; +import { noModuleMockingRule } from "./rules/no-module-mocking.ts"; +import { noObjectParametersRule } from "./rules/no-object-parameters.ts"; +import { noReflectApplyRule } from "./rules/no-reflect-apply.ts"; +import { noReflectGetRule } from "./rules/no-reflect-get.ts"; +import { noRuntimeTypeofRule } from "./rules/no-runtime-typeof.ts"; +import { noForbiddenTermInSymbolNamesRule } from "./rules/no-shape-in-symbol-names.ts"; +import { noUnknownParametersRule } from "./rules/no-unknown-parameters.ts"; +import { noUnknownReturnsRule } from "./rules/no-unknown-returns.ts"; +import { noUnknownTypeAliasesRule } from "./rules/no-unknown-type-aliases.ts"; +import { noUnsafeDictionaryTypeRule } from "./rules/no-unsafe-dictionary-type.ts"; +import { noWidenThenAssertRule } from "./rules/no-widen-then-assert.ts"; +import { requireSafetyCommentForTypeAssertionRule } from "./rules/require-safety-comment-for-type-assertion.ts"; + +/** Generic Oxlint rules that reject low-evidence and low-signal implementation patterns. */ +const antiSlopPlugin = eslintCompatPlugin({ + meta: { name: "anti-slop" }, + rules: { + "no-chained-type-assertions": noChainedTypeAssertionsRule, + "no-conditional-empty-object-spread": noConditionalEmptyObjectSpreadRule, + "no-known-value-widening": noKnownValueWideningRule, + "no-module-mocking": noModuleMockingRule, + "no-object-parameters": noObjectParametersRule, + "no-reflect-apply": noReflectApplyRule, + "no-reflect-get": noReflectGetRule, + "no-runtime-typeof": noRuntimeTypeofRule, + "no-unsafe-dictionary-type": noUnsafeDictionaryTypeRule, + "no-shape-in-symbol-names": noForbiddenTermInSymbolNamesRule, + "no-unknown-parameters": noUnknownParametersRule, + "no-unknown-returns": noUnknownReturnsRule, + "no-unknown-type-aliases": noUnknownTypeAliasesRule, + "no-widen-then-assert": noWidenThenAssertRule, + "require-safety-comment-for-type-assertion": requireSafetyCommentForTypeAssertionRule, + }, +}); + +export default antiSlopPlugin; diff --git a/tools/oxlint/anti-slop/rules/no-chained-type-assertions.ts b/tools/oxlint/anti-slop/rules/no-chained-type-assertions.ts new file mode 100644 index 00000000..0d118527 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-chained-type-assertions.ts @@ -0,0 +1,77 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +type TypeAssertionExpression = ESTree.TSAsExpression | ESTree.TSTypeAssertion; + +function isTypeAssertionExpression(node: ESTree.Node): node is TypeAssertionExpression { + return node.type === "TSAsExpression" || node.type === "TSTypeAssertion"; +} + +function unwrapParenthesizedExpression(expression: ESTree.Expression): ESTree.Expression { + let current = expression; + while (current.type === "ParenthesizedExpression") { + current = current.expression; + } + return current; +} + +function isConstAssertion(node: TypeAssertionExpression): boolean { + const { typeAnnotation } = node; + return ( + typeAnnotation.type === "TSTypeReference" && + typeAnnotation.typeName.type === "Identifier" && + typeAnnotation.typeName.name === "const" + ); +} + +function isOutermostAssertionInChain(node: TypeAssertionExpression): boolean { + let current: ESTree.Expression = node; + let parent = node.parent; + + while (parent.type === "ParenthesizedExpression" && parent.expression === current) { + current = parent; + parent = parent.parent; + } + + return !isTypeAssertionExpression(parent) || parent.expression !== current; +} + +function isForbiddenAssertionChain(node: TypeAssertionExpression): boolean { + let assertionCount = 0; + let hasNonConstAssertion = false; + let current: ESTree.Expression = node; + + while (isTypeAssertionExpression(current)) { + assertionCount += 1; + hasNonConstAssertion ||= !isConstAssertion(current); + current = unwrapParenthesizedExpression(current.expression); + } + + return assertionCount > 1 && hasNonConstAssertion; +} + +/** Disallow nested TypeScript type assertions, while permitting chains made only of const assertions. */ +export const noChainedTypeAssertionsRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow chained TypeScript as and angle-bracket assertions, including parenthesized chains.", + }, + messages: { + chained: + "This assertion chain discards type evidence. Keep the original precise type, or parse untrusted input at its boundary before narrowing it.", + }, + }, + createOnce(context) { + const checkTypeAssertion = (node: TypeAssertionExpression) => { + if (!isOutermostAssertionInChain(node) || !isForbiddenAssertionChain(node)) return; + context.report({ node, messageId: "chained" }); + }; + + return { + TSAsExpression: checkTypeAssertion, + TSTypeAssertion: checkTypeAssertion, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts b/tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts new file mode 100644 index 00000000..ae7248d3 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-conditional-empty-object-spread.ts @@ -0,0 +1,49 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +function unwrapParentheses(node: ESTree.Expression): ESTree.Expression { + let current = node; + while (current.type === "ParenthesizedExpression") { + current = current.expression; + } + return current; +} + +function isEmptyObjectExpression(node: ESTree.Expression): boolean { + return node.type === "ObjectExpression" && node.properties.length === 0; +} + +function isConditionalEmptyObjectSpread(node: ESTree.Expression): boolean { + const conditional = unwrapParentheses(node); + return ( + conditional.type === "ConditionalExpression" && + (isEmptyObjectExpression(conditional.consequent) || + isEmptyObjectExpression(conditional.alternate)) + ); +} + +/** Ban conditional empty-object spreads without changing their omission semantics. */ +export const noConditionalEmptyObjectSpreadRule = defineRule({ + meta: { + type: "suggestion", + docs: { + description: + "Disallow object spreads that conditionally spread an empty object to omit fields.", + }, + messages: { + avoid: + "This conditional spread hides property omission behind an empty object. Build the object in separate statements and add the property only when present.", + }, + }, + createOnce(context) { + return { + SpreadElement(node) { + if (node.parent.type !== "ObjectExpression") return; + + if (isConditionalEmptyObjectSpread(node.argument)) { + context.report({ node, messageId: "avoid" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-known-value-widening.ts b/tools/oxlint/anti-slop/rules/no-known-value-widening.ts new file mode 100644 index 00000000..2a6806c6 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-known-value-widening.ts @@ -0,0 +1,247 @@ +import { defineRule } from "@oxlint/plugins"; + +import { + classifyWideningTarget, + createTypeEnvironment, + isKnownEvidenceExpression, + type TypeEnvironment, + type WideningTarget, +} from "../shared/dictionary-types.ts"; + +import type { ESTree, Scope, SourceCode, Variable } from "@oxlint/plugins"; + +type FunctionExpression = ESTree.ArrowFunctionExpression | ESTree.Function; + +function unwrapExpression(expression: ESTree.Expression): ESTree.Expression { + let current = expression; + while ( + current.type === "ParenthesizedExpression" || + current.type === "TSAsExpression" || + current.type === "TSSatisfiesExpression" || + current.type === "TSTypeAssertion" || + current.type === "TSNonNullExpression" + ) { + current = current.expression; + } + return current; +} + +function resolveVariable( + sourceCode: SourceCode, + identifier: ESTree.IdentifierReference, +): Variable | null { + let scope: Scope | null = sourceCode.getScope(identifier); + while (scope !== null) { + const variable = scope.set.get(identifier.name); + if (variable !== undefined) return variable; + scope = scope.upper; + } + return null; +} + +function variableDeclarator(variable: Variable): ESTree.VariableDeclarator | null { + if (variable.defs.length !== 1) return null; + const [definition] = variable.defs; + return definition?.type === "Variable" && definition.node.type === "VariableDeclarator" + ? definition.node + : null; +} + +function isStableConstVariable(variable: Variable, declarator: ESTree.VariableDeclarator): boolean { + return ( + declarator.parent.type === "VariableDeclaration" && + declarator.parent.kind === "const" && + variable.references.every((reference) => reference.init || !reference.isWrite()) + ); +} + +function hasKnownEvidence( + sourceCode: SourceCode, + expression: ESTree.Expression, + visitedVariables = new Set(), +): boolean { + if (isKnownEvidenceExpression(expression)) return true; + const unwrapped = unwrapExpression(expression); + if (unwrapped.type !== "Identifier") return false; + const variable = resolveVariable(sourceCode, unwrapped); + if (variable === null || visitedVariables.has(variable)) return false; + const declarator = variableDeclarator(variable); + if ( + declarator === null || + declarator.init === null || + !isStableConstVariable(variable, declarator) + ) { + return false; + } + visitedVariables.add(variable); + return hasKnownEvidence(sourceCode, declarator.init, visitedVariables); +} + +function annotationTarget( + annotation: ESTree.TSTypeAnnotation | null | undefined, + environment: TypeEnvironment, +): WideningTarget | null { + return annotation === null || annotation === undefined + ? null + : classifyWideningTarget(annotation.typeAnnotation, environment); +} + +function enclosingFunction(node: ESTree.Node): FunctionExpression | null { + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if ( + current.type === "ArrowFunctionExpression" || + current.type === "FunctionDeclaration" || + current.type === "FunctionExpression" + ) { + return current; + } + current = current.parent; + } + return null; +} + +function sourceKeyName(sourceCode: SourceCode, key: ESTree.PropertyKey): string { + if (key.type === "Identifier" || key.type === "PrivateIdentifier") return key.name; + if (key.type === "Literal") return String(key.value); + return sourceCode.getText(key); +} + +function functionName(sourceCode: SourceCode, owner: FunctionExpression | null): string { + if (owner === null) return "anonymous function"; + if (owner.id !== null) return owner.id.name; + const parent = owner.parent; + if (parent.type === "VariableDeclarator" && parent.id.type === "Identifier") + return parent.id.name; + if (parent.type === "MethodDefinition") return sourceKeyName(sourceCode, parent.key); + return "anonymous function"; +} + +function isEmptyObjectExpression(expression: ESTree.Expression): boolean { + const unwrapped = unwrapExpression(expression); + return unwrapped.type === "ObjectExpression" && unwrapped.properties.length === 0; +} + +function isDictionaryAccumulatorTarget(destination: WideningTarget): boolean { + return destination.kind === "open dictionary" || destination.kind === "generic container"; +} + +function hasParentAssertion(node: ESTree.Node): boolean { + return node.parent?.type === "TSAsExpression" || node.parent?.type === "TSTypeAssertion"; +} + +/** Detect sound syntactic cases where a known value is explicitly widened and loses evidence. */ +export const noKnownValueWideningRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow syntactically established values from flowing into explicitly broad or anonymous target types that discard useful evidence.", + }, + messages: { + widening: + "The explicit {{target}} type on {{subject}} discards known type evidence. Keep inference, validate with `satisfies`, or use a named owner contract.", + }, + }, + createOnce(context) { + let environment: TypeEnvironment | null = null; + + const reportFlow = ( + expression: ESTree.Expression, + destination: WideningTarget | null, + subject: string, + ) => { + if (destination === null) return; + if ( + isDictionaryAccumulatorTarget(destination) && + isEmptyObjectExpression(expression) + ) { + return; + } + if (!hasKnownEvidence(context.sourceCode, expression)) return; + context.report({ + node: expression, + messageId: "widening", + data: { subject, target: destination.kind }, + }); + }; + + const targetFromAnnotation = (annotation: ESTree.TSTypeAnnotation | null | undefined) => + environment === null ? null : annotationTarget(annotation, environment); + + return { + Program(node) { + environment = createTypeEnvironment(node); + }, + VariableDeclarator(node) { + if (node.init === null || node.id.type !== "Identifier") return; + reportFlow( + node.init, + targetFromAnnotation(node.id.typeAnnotation), + `binding \`${node.id.name}\``, + ); + }, + PropertyDefinition(node) { + if (node.value === null) return; + reportFlow( + node.value, + targetFromAnnotation(node.typeAnnotation), + `property \`${sourceKeyName(context.sourceCode, node.key)}\``, + ); + }, + AccessorProperty(node) { + if (node.value === null) return; + reportFlow( + node.value, + targetFromAnnotation(node.typeAnnotation), + `property \`${sourceKeyName(context.sourceCode, node.key)}\``, + ); + }, + AssignmentExpression(node) { + if (node.operator !== "=" || node.left.type !== "Identifier") return; + const variable = resolveVariable(context.sourceCode, node.left); + if (variable === null) return; + const declarator = variableDeclarator(variable); + if (declarator === null || declarator.id.type !== "Identifier") return; + reportFlow( + node.right, + targetFromAnnotation(declarator.id.typeAnnotation), + `binding \`${declarator.id.name}\``, + ); + }, + ReturnStatement(node) { + if (node.argument === null) return; + const owner = enclosingFunction(node); + reportFlow( + node.argument, + targetFromAnnotation(owner?.returnType), + `return value of \`${functionName(context.sourceCode, owner)}\``, + ); + }, + ArrowFunctionExpression(node) { + if (node.body.type === "BlockStatement") return; + reportFlow( + node.body, + targetFromAnnotation(node.returnType), + `return value of \`${functionName(context.sourceCode, node)}\``, + ); + }, + TSAsExpression(node) { + if (environment === null || hasParentAssertion(node)) return; + reportFlow( + node.expression, + classifyWideningTarget(node.typeAnnotation, environment), + "assertion", + ); + }, + TSTypeAssertion(node) { + if (environment === null || hasParentAssertion(node)) return; + reportFlow( + node.expression, + classifyWideningTarget(node.typeAnnotation, environment), + "assertion", + ); + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-module-mocking.ts b/tools/oxlint/anti-slop/rules/no-module-mocking.ts new file mode 100644 index 00000000..d6fb5b45 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-module-mocking.ts @@ -0,0 +1,91 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree, Scope, SourceCode, Variable } from "@oxlint/plugins"; + +const moduleMockMethods = new Set(["doMock", "mock", "unstable_mockModule"]); + +function resolveVariable( + sourceCode: SourceCode, + identifier: ESTree.IdentifierReference, +): Variable | null { + let scope: Scope | null = sourceCode.getScope(identifier); + while (scope !== null) { + const variable = scope.set.get(identifier.name); + if (variable !== undefined) return variable; + scope = scope.upper; + } + return null; +} + +function importedName(node: ESTree.Node): string | null { + if (node.type !== "ImportSpecifier") return null; + return node.imported.type === "Identifier" ? node.imported.name : node.imported.value; +} + +function isTestFrameworkObject( + sourceCode: SourceCode, + expression: ESTree.Expression, +): expression is ESTree.IdentifierReference { + if (expression.type !== "Identifier") return false; + if ( + (expression.name === "vi" || expression.name === "jest") && + sourceCode.isGlobalReference(expression) + ) { + return true; + } + + const variable = resolveVariable(sourceCode, expression); + if (variable === null || variable.defs.length === 0) { + return expression.name === "vi" || expression.name === "jest"; + } + return variable.defs.some((definition) => { + if (definition.type !== "ImportBinding" || definition.parent?.type !== "ImportDeclaration") { + return false; + } + const source = definition.parent.source.value; + const name = importedName(definition.node); + return (source === "vitest" && name === "vi") || (source === "@jest/globals" && name === "jest"); + }); +} + +function moduleMockCall(sourceCode: SourceCode, callee: ESTree.Expression): boolean { + if (!("property" in callee) || !("object" in callee) || !("computed" in callee)) return false; + if (!isTestFrameworkObject(sourceCode, callee.object)) return false; + const property = callee.property; + const method = callee.computed + ? property.type === "Literal" && + (property.value === "doMock" || + property.value === "mock" || + property.value === "unstable_mockModule") + ? property.value + : null + : property.type === "Identifier" + ? property.name + : null; + return method !== null && moduleMockMethods.has(method); +} + +/** Ban test framework module mocking in favor of real dependency seams. */ +export const noModuleMockingRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow Vitest and Jest module mocking; tests must replace dependencies through real interfaces.", + }, + messages: { + moduleMock: + "Replace module mocking with dependency injection through a real interface, service layer, or faithful test implementation.", + }, + }, + createOnce(context) { + return { + CallExpression(node) { + if (node.callee.type === "Super" || node.callee.type === "V8IntrinsicExpression") return; + if (moduleMockCall(context.sourceCode, node.callee)) { + context.report({ node, messageId: "moduleMock" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-object-parameters.ts b/tools/oxlint/anti-slop/rules/no-object-parameters.ts new file mode 100644 index 00000000..29b990f3 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-object-parameters.ts @@ -0,0 +1,126 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree, SourceCode } from "@oxlint/plugins"; + +import { lexicalTypeParameterNames } from "../shared/lexical-type-parameters.ts"; + +type Parameter = ESTree.ParamPattern; +type ParameterOwner = + | ESTree.ArrowFunctionExpression + | ESTree.Function + | ESTree.TSCallSignatureDeclaration + | ESTree.TSConstructSignatureDeclaration + | ESTree.TSConstructorType + | ESTree.TSFunctionType + | ESTree.TSMethodSignature; + +function parameterAnnotation(parameter: Parameter): ESTree.TSTypeAnnotation | null | undefined { + if (parameter.type === "TSParameterProperty") { + return parameterAnnotation(parameter.parameter); + } + if (parameter.type === "RestElement") { + return parameter.typeAnnotation ?? parameterAnnotation(parameter.argument); + } + if (parameter.type === "AssignmentPattern") { + return parameter.typeAnnotation ?? parameter.left.typeAnnotation; + } + return parameter.typeAnnotation; +} + +function parameterName(parameter: Parameter, sourceCode: SourceCode): string { + return parameter.type === "Identifier" + ? parameter.name + : sourceCode.getText(parameter).replace(/\s*:\s*object\s*$/u, ""); +} + +/** Ban the broad object type on function inputs, including local aliases to object. */ +export const noObjectParametersRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow object function parameters; inputs must use an owner-provided type and be parsed at their boundary.", + }, + messages: { + objectParameter: + "Parameter `{{parameter}}` uses the broad `object` type. Accept a named owner type; parse external input at its boundary before calling this function.", + }, + }, + createOnce(context) { + const aliases = new Map(); + + const resolvesToObject = ( + type: ESTree.TSType, + shadowedAliases: ReadonlySet, + visited = new Set(), + ): boolean => { + if (type.type === "TSObjectKeyword") return true; + if (type.type === "TSParenthesizedType") + return resolvesToObject(type.typeAnnotation, shadowedAliases, visited); + if (type.type === "TSUnionType") { + return type.types.some((member) => + resolvesToObject(member, shadowedAliases, visited), + ); + } + if ( + type.type !== "TSTypeReference" || + type.typeName.type !== "Identifier" || + (type.typeArguments !== null && + type.typeArguments !== undefined && + type.typeArguments.params.length > 0) || + visited.has(type.typeName.name) || + shadowedAliases.has(type.typeName.name) + ) { + return false; + } + const alias = aliases.get(type.typeName.name); + if (alias === undefined) return false; + const nextVisited = new Set(visited); + nextVisited.add(type.typeName.name); + return resolvesToObject(alias, shadowedAliases, nextVisited); + }; + + const checkParameters = (node: ParameterOwner) => { + const shadowedAliases = lexicalTypeParameterNames( + node, + context.sourceCode.visitorKeys, + ); + for (const parameter of node.params) { + const annotation = parameterAnnotation(parameter); + if (annotation === null || annotation === undefined) continue; + if (!resolvesToObject(annotation.typeAnnotation, shadowedAliases)) continue; + context.report({ + node: annotation.typeAnnotation, + messageId: "objectParameter", + data: { parameter: parameterName(parameter, context.sourceCode) }, + }); + } + }; + + return { + Program(node) { + aliases.clear(); + for (const statement of node.body) { + const declaration = + statement.type === "ExportNamedDeclaration" ? statement.declaration : statement; + if ( + declaration?.type === "TSTypeAliasDeclaration" && + (declaration.typeParameters === null || declaration.typeParameters === undefined) + ) { + aliases.set(declaration.id.name, declaration.typeAnnotation); + } + } + }, + ArrowFunctionExpression: checkParameters, + FunctionDeclaration: checkParameters, + FunctionExpression: checkParameters, + TSCallSignatureDeclaration: checkParameters, + TSConstructSignatureDeclaration: checkParameters, + TSConstructorType: checkParameters, + TSDeclareFunction: checkParameters, + TSEmptyBodyFunctionExpression: checkParameters, + TSFunctionType: checkParameters, + TSMethodSignature: checkParameters, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-reflect-apply.ts b/tools/oxlint/anti-slop/rules/no-reflect-apply.ts new file mode 100644 index 00000000..2cc30451 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-reflect-apply.ts @@ -0,0 +1,28 @@ +import { defineRule } from "@oxlint/plugins"; + +import { isGlobalReflectMethodCall } from "../shared/reflect-method.ts"; + +/** Ban Reflect.apply, which bypasses ordinary typed function calls. */ +export const noReflectApplyRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow Reflect.apply; call typed functions directly or model dynamic dispatch behind an interface.", + }, + messages: { + reflectApply: + "Replace `Reflect.apply` with a typed function call. Model dynamic dispatch behind a named interface.", + }, + }, + createOnce(context) { + return { + CallExpression(node) { + if (node.callee.type === "Super" || node.callee.type === "V8IntrinsicExpression") return; + if (isGlobalReflectMethodCall(context.sourceCode, node.callee, "apply")) { + context.report({ node, messageId: "reflectApply" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-reflect-get.ts b/tools/oxlint/anti-slop/rules/no-reflect-get.ts new file mode 100644 index 00000000..cf630ecc --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-reflect-get.ts @@ -0,0 +1,28 @@ +import { defineRule } from "@oxlint/plugins"; + +import { isGlobalReflectMethodCall } from "../shared/reflect-method.ts"; + +/** Ban Reflect.get, which bypasses ordinary property access and useful type evidence. */ +export const noReflectGetRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow Reflect.get; use typed property access or parse dynamic input into a domain type.", + }, + messages: { + reflectGet: + "Replace `Reflect.get` with typed property access. Parse dynamic input into a named domain type before reading it.", + }, + }, + createOnce(context) { + return { + CallExpression(node) { + if (node.callee.type === "Super" || node.callee.type === "V8IntrinsicExpression") return; + if (isGlobalReflectMethodCall(context.sourceCode, node.callee, "get")) { + context.report({ node, messageId: "reflectGet" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-runtime-typeof.ts b/tools/oxlint/anti-slop/rules/no-runtime-typeof.ts new file mode 100644 index 00000000..6a25c247 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-runtime-typeof.ts @@ -0,0 +1,67 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree } from "@oxlint/plugins"; + +type RuntimeFunction = ESTree.ArrowFunctionExpression | ESTree.Function; + +function isRuntimeFunction(node: ESTree.Node): node is RuntimeFunction { + return ( + node.type === "ArrowFunctionExpression" || + node.type === "FunctionDeclaration" || + node.type === "FunctionExpression" + ); +} + +function isInsideTypeGuard(node: ESTree.Node): boolean { + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if (isRuntimeFunction(current)) { + return current.returnType?.typeAnnotation.type === "TSTypePredicate"; + } + current = current.parent; + } + return false; +} + +/** Disallow runtime typeof checks that narrow unparsed values instead of decoding them. */ +export const noRuntimeTypeofRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow runtime typeof checks; external values must be decoded into meaningful types at their I/O boundary.", + }, + messages: { + runtimeTypeof: + "A `typeof` check narrows a representation without establishing its contract. Parse input at its I/O boundary, then branch on the domain value.", + }, + schema: [ + { + type: "object", + properties: { + allowInTypeGuards: { type: "boolean" }, + }, + additionalProperties: false, + }, + ], + defaultOptions: [{ allowInTypeGuards: false }], + }, + createOnce(context) { + return { + UnaryExpression(node) { + const option = context.options?.[0]; + const allowInTypeGuards = + typeof option === "object" && + option !== null && + !Array.isArray(option) && + option.allowInTypeGuards === true; + if ( + node.operator === "typeof" && + (!allowInTypeGuards || !isInsideTypeGuard(node)) + ) { + context.report({ node, messageId: "runtimeTypeof" }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-shape-in-symbol-names.ts b/tools/oxlint/anti-slop/rules/no-shape-in-symbol-names.ts new file mode 100644 index 00000000..afc00dd4 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-shape-in-symbol-names.ts @@ -0,0 +1,39 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +const FORBIDDEN_SYMBOL_NAME = "shape"; + +function containsForbiddenSymbolName(name: string): boolean { + return name.toLowerCase().includes(FORBIDDEN_SYMBOL_NAME); +} + +/** Ban the case-insensitive substring "shape" in every JavaScript and TypeScript symbol name. */ +export const noForbiddenTermInSymbolNamesRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + 'Disallow the case-insensitive substring "shape" in JavaScript, TypeScript, private, and JSX symbol names.', + }, + messages: { + forbiddenSymbolName: + 'Rename symbol "{{name}}" for its domain role; "shape" describes structure rather than ownership.', + }, + }, + createOnce(context) { + const reportForbiddenSymbolName = (node: ESTree.Node & { name: string }) => { + if (!containsForbiddenSymbolName(node.name)) return; + context.report({ + node, + messageId: "forbiddenSymbolName", + data: { name: node.name }, + }); + }; + + return { + Identifier: reportForbiddenSymbolName, + PrivateIdentifier: reportForbiddenSymbolName, + JSXIdentifier: reportForbiddenSymbolName, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unknown-parameters.ts b/tools/oxlint/anti-slop/rules/no-unknown-parameters.ts new file mode 100644 index 00000000..cdc6c235 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unknown-parameters.ts @@ -0,0 +1,83 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree } from "@oxlint/plugins"; + +type Parameter = ESTree.ParamPattern; +type ParameterOwner = + | ESTree.ArrowFunctionExpression + | ESTree.Function + | ESTree.TSCallSignatureDeclaration + | ESTree.TSConstructSignatureDeclaration + | ESTree.TSConstructorType + | ESTree.TSFunctionType + | ESTree.TSMethodSignature; + +function parameterAnnotation(parameter: Parameter): ESTree.TSTypeAnnotation | null | undefined { + if (parameter.type === "TSParameterProperty") { + return parameterAnnotation(parameter.parameter); + } + if (parameter.type === "RestElement") { + return parameter.typeAnnotation ?? parameterAnnotation(parameter.argument); + } + if (parameter.type === "AssignmentPattern") { + return parameter.typeAnnotation ?? parameter.left.typeAnnotation; + } + return parameter.typeAnnotation; +} + +function parameterName(parameter: Parameter, sourceText: string): string { + if (parameter.type === "TSParameterProperty") { + return parameterName(parameter.parameter, sourceText); + } + if (parameter.type === "AssignmentPattern") { + return parameterName(parameter.left, sourceText); + } + if (parameter.type === "RestElement") { + return parameterName(parameter.argument, sourceText); + } + return parameter.type === "Identifier" + ? parameter.name + : sourceText.replace(/\s*:\s*unknown\s*$/u, ""); +} + +/** Disallow unknown inputs except explicitly named error-cause enrichment. */ +export const noUnknownParametersRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow explicitly unknown function parameters except `cause`; decode unknown input at its I/O boundary instead.", + }, + messages: { + unknownParameter: + "Parameter `{{parameter}}` leaves input unparsed. Accept a named domain type; run the expected schema or parser at the I/O boundary before calling this function.", + }, + }, + createOnce(context) { + const checkParameters = (node: ParameterOwner) => { + for (const parameter of node.params) { + const annotation = parameterAnnotation(parameter); + if (annotation?.typeAnnotation.type !== "TSUnknownKeyword") continue; + const name = parameterName(parameter, context.sourceCode.getText(parameter)); + if (name === "cause") continue; + context.report({ + node: annotation.typeAnnotation, + messageId: "unknownParameter", + data: { parameter: name }, + }); + } + }; + + return { + ArrowFunctionExpression: checkParameters, + FunctionDeclaration: checkParameters, + FunctionExpression: checkParameters, + TSCallSignatureDeclaration: checkParameters, + TSConstructSignatureDeclaration: checkParameters, + TSConstructorType: checkParameters, + TSDeclareFunction: checkParameters, + TSEmptyBodyFunctionExpression: checkParameters, + TSFunctionType: checkParameters, + TSMethodSignature: checkParameters, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unknown-returns.ts b/tools/oxlint/anti-slop/rules/no-unknown-returns.ts new file mode 100644 index 00000000..4b16d6ef --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unknown-returns.ts @@ -0,0 +1,115 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree } from "@oxlint/plugins"; + +import { lexicalTypeParameterNames } from "../shared/lexical-type-parameters.ts"; + +type FunctionWithReturnType = + | ESTree.ArrowFunctionExpression + | ESTree.Function + | ESTree.TSCallSignatureDeclaration + | ESTree.TSConstructSignatureDeclaration + | ESTree.TSConstructorType + | ESTree.TSFunctionType + | ESTree.TSMethodSignature; + +function referencedAliasName(type: ESTree.TSType): string | null { + if (type.type === "TSParenthesizedType") return referencedAliasName(type.typeAnnotation); + if (type.type !== "TSTypeReference" || type.typeName.type !== "Identifier") return null; + return type.typeArguments === null || + type.typeArguments === undefined || + type.typeArguments.params.length === 0 + ? type.typeName.name + : null; +} + +/** Ban function contracts that return unknown instead of a parsed domain type. */ +export const noUnknownReturnsRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow functions whose explicit return contract is unknown or Promise.", + }, + messages: { + unknownReturn: + "This function exposes `unknown` to its caller. Parse the value at its boundary and return a named domain type.", + }, + }, + createOnce(context) { + const aliases = new Map(); + + const resolvesToUnknown = ( + type: ESTree.TSType, + shadowedAliases: ReadonlySet, + visited = new Set(), + ): boolean => { + if (type.type === "TSUnknownKeyword") return true; + if (type.type === "TSParenthesizedType") { + return resolvesToUnknown(type.typeAnnotation, shadowedAliases, visited); + } + if (type.type === "TSUnionType") { + return type.types.some((member) => + resolvesToUnknown(member, shadowedAliases, visited), + ); + } + if ( + type.type === "TSTypeReference" && + type.typeName.type === "Identifier" && + (type.typeName.name === "Promise" || type.typeName.name === "PromiseLike") + ) { + const value = type.typeArguments?.params[0]; + return value !== undefined && resolvesToUnknown(value, shadowedAliases, visited); + } + const name = referencedAliasName(type); + if (name === null || visited.has(name) || shadowedAliases.has(name)) return false; + const alias = aliases.get(name); + if ( + alias === undefined || + (alias.typeParameters !== null && alias.typeParameters !== undefined) + ) { + return false; + } + const nextVisited = new Set(visited); + nextVisited.add(name); + return resolvesToUnknown(alias.typeAnnotation, shadowedAliases, nextVisited); + }; + + const checkReturnType = (node: FunctionWithReturnType) => { + const annotation = node.returnType; + if (annotation === null || annotation === undefined) return; + if ( + !resolvesToUnknown( + annotation.typeAnnotation, + lexicalTypeParameterNames(node, context.sourceCode.visitorKeys), + ) + ) { + return; + } + context.report({ node: annotation.typeAnnotation, messageId: "unknownReturn" }); + }; + + return { + Program(node) { + aliases.clear(); + for (const statement of node.body) { + const declaration = + statement.type === "ExportNamedDeclaration" ? statement.declaration : statement; + if (declaration?.type === "TSTypeAliasDeclaration") { + aliases.set(declaration.id.name, declaration); + } + } + }, + ArrowFunctionExpression: checkReturnType, + FunctionDeclaration: checkReturnType, + FunctionExpression: checkReturnType, + TSCallSignatureDeclaration: checkReturnType, + TSConstructSignatureDeclaration: checkReturnType, + TSConstructorType: checkReturnType, + TSDeclareFunction: checkReturnType, + TSEmptyBodyFunctionExpression: checkReturnType, + TSFunctionType: checkReturnType, + TSMethodSignature: checkReturnType, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unknown-type-aliases.ts b/tools/oxlint/anti-slop/rules/no-unknown-type-aliases.ts new file mode 100644 index 00000000..3e328fdf --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unknown-type-aliases.ts @@ -0,0 +1,70 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree } from "@oxlint/plugins"; + +function referencedAliasName(type: ESTree.TSType): string | null { + if (type.type === "TSParenthesizedType") return referencedAliasName(type.typeAnnotation); + if (type.type !== "TSTypeReference" || type.typeName.type !== "Identifier") return null; + return type.typeArguments === null || + type.typeArguments === undefined || + type.typeArguments.params.length === 0 + ? type.typeName.name + : null; +} + +/** Ban named aliases that merely conceal TypeScript's unknown top type. */ +export const noUnknownTypeAliasesRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow type aliases whose resolved type is unknown; unknown must remain visible at an allowed boundary.", + }, + messages: { + unknownAlias: + "Type alias `{{alias}}` hides `unknown`. Keep `unknown` explicit at the parsing boundary or on an allowed `cause` field; otherwise use the parsed owner type.", + }, + }, + createOnce(context) { + const aliases = new Map(); + + const resolvesToUnknown = (type: ESTree.TSType, visited = new Set()): boolean => { + if (type.type === "TSUnknownKeyword") return true; + if (type.type === "TSParenthesizedType") + return resolvesToUnknown(type.typeAnnotation, visited); + const name = referencedAliasName(type); + if (name === null || visited.has(name)) return false; + const alias = aliases.get(name); + if ( + alias === undefined || + (alias.typeParameters !== null && alias.typeParameters !== undefined) + ) { + return false; + } + const nextVisited = new Set(visited); + nextVisited.add(name); + return resolvesToUnknown(alias.typeAnnotation, nextVisited); + }; + + return { + Program(node) { + aliases.clear(); + for (const statement of node.body) { + const declaration = + statement.type === "ExportNamedDeclaration" ? statement.declaration : statement; + if (declaration?.type === "TSTypeAliasDeclaration") { + aliases.set(declaration.id.name, declaration); + } + } + for (const alias of aliases.values()) { + if (!resolvesToUnknown(alias.typeAnnotation, new Set([alias.id.name]))) continue; + context.report({ + node: alias.id, + messageId: "unknownAlias", + data: { alias: alias.id.name }, + }); + } + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts b/tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts new file mode 100644 index 00000000..8c45eed2 --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-unsafe-dictionary-type.ts @@ -0,0 +1,134 @@ +import { defineRule } from "@oxlint/plugins"; + +import { + classifyUnsafeDictionary, + classifyUnsafeDictionaryValue, + createTypeEnvironment, + type TypeEnvironment, +} from "../shared/dictionary-types.ts"; + +import type { ESTree } from "@oxlint/plugins"; + +const typeNodeKinds: ReadonlySet = new Set([ + "JSDocNonNullableType", + "JSDocNullableType", + "JSDocUnknownType", + "TSAnyKeyword", + "TSArrayType", + "TSBigIntKeyword", + "TSBooleanKeyword", + "TSConditionalType", + "TSConstructorType", + "TSFunctionType", + "TSImportType", + "TSIndexedAccessType", + "TSInferType", + "TSIntersectionType", + "TSIntrinsicKeyword", + "TSLiteralType", + "TSMappedType", + "TSNamedTupleMember", + "TSNeverKeyword", + "TSNullKeyword", + "TSNumberKeyword", + "TSObjectKeyword", + "TSParenthesizedType", + "TSStringKeyword", + "TSSymbolKeyword", + "TSTemplateLiteralType", + "TSThisType", + "TSTupleType", + "TSTypeLiteral", + "TSTypeOperator", + "TSTypePredicate", + "TSTypeQuery", + "TSTypeReference", + "TSUndefinedKeyword", + "TSUnionType", + "TSUnknownKeyword", + "TSVoidKeyword", +]); + +function isTypeNode(node: ESTree.Node): node is ESTree.TSType { + return typeNodeKinds.has(node.type); +} + +function typeReferenceName(type: ESTree.TSTypeReference): string | null { + return type.typeName.type === "Identifier" ? type.typeName.name : null; +} + +function isInsideTypeAliasDeclaration(node: ESTree.Node): boolean { + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if (current.type === "TSTypeAliasDeclaration") return true; + current = current.parent; + } + return false; +} + +function isPlainAliasConsumerUse(node: ESTree.TSType, environment: TypeEnvironment): boolean { + if (node.type !== "TSTypeReference" || node.typeArguments?.params.length) return false; + const name = typeReferenceName(node); + return name !== null && environment.aliases.has(name) && !isInsideTypeAliasDeclaration(node); +} + +function shouldReportType(node: ESTree.TSType, environment: TypeEnvironment): boolean { + if (isPlainAliasConsumerUse(node, environment)) return false; + if (classifyUnsafeDictionary(node, environment) === null) return false; + let current: ESTree.Node | null = node.parent; + while (current !== null && current.type !== "Program") { + if (isTypeNode(current) && classifyUnsafeDictionary(current, environment) !== null) + return false; + current = current.parent; + } + return true; +} + +/** Disallow object-dictionary contracts whose direct value type is an unsafe escape hatch. */ +export const noUnsafeDictionaryTypeRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow object-dictionary contracts whose direct value type is unknown, any, object, {}, or a union/alias containing one of those escape hatches.", + }, + messages: { + unsafeDictionary: + "This dictionary's {{value}} value type gives callers no concrete value contract. Use an owner/schema-derived value type; parse external payloads before insertion.", + }, + }, + createOnce(context) { + let environment: TypeEnvironment | null = null; + const report = (node: ESTree.Node, value: string) => { + context.report({ node, messageId: "unsafeDictionary", data: { value } }); + }; + const reportIfUnsafe = (node: ESTree.TSType) => { + if (environment === null || !shouldReportType(node, environment)) return; + const unsafe = classifyUnsafeDictionary(node, environment); + if (unsafe === null) return; + report(node, unsafe.unsafeValue); + }; + + return { + Program(node) { + environment = createTypeEnvironment(node); + }, + TSTypeReference: reportIfUnsafe, + TSTypeLiteral: reportIfUnsafe, + TSMappedType: reportIfUnsafe, + TSIndexSignature(node) { + if ( + environment === null || + node.typeAnnotation === null || + node.parent.type === "TSTypeLiteral" + ) + return; + const unsafe = classifyUnsafeDictionaryValue( + node.typeAnnotation.typeAnnotation, + environment, + ); + if (unsafe !== null) report(node, unsafe.unsafeValue); + }, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/no-widen-then-assert.ts b/tools/oxlint/anti-slop/rules/no-widen-then-assert.ts new file mode 100644 index 00000000..c5e07f7f --- /dev/null +++ b/tools/oxlint/anti-slop/rules/no-widen-then-assert.ts @@ -0,0 +1,366 @@ +import { defineRule } from "@oxlint/plugins"; +import type { ESTree, Variable } from "@oxlint/plugins"; + +type BroadTypeKind = "top" | "object" | "record"; + +type KnownValueEvidence = { + readonly type: ESTree.TSType | null; +}; + +const functionBoundaryTypes = new Set([ + "ArrowFunctionExpression", + "FunctionDeclaration", + "FunctionExpression", + "TSDeclareFunction", + "TSEmptyBodyFunctionExpression", +]); + +function unwrapExpressionParentheses(expression: ESTree.Expression): ESTree.Expression { + let current = expression; + while (current.type === "ParenthesizedExpression") current = current.expression; + return current; +} + +function unwrapTypeParentheses(type: ESTree.TSType): ESTree.TSType { + let current = type; + while (current.type === "TSParenthesizedType") current = current.typeAnnotation; + return current; +} + +function typeReferenceName(type: ESTree.TSTypeReference): string | null { + return type.typeName.type === "Identifier" ? type.typeName.name : null; +} + +function isUnknownOrAnyType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + return unwrapped.type === "TSUnknownKeyword" || unwrapped.type === "TSAnyKeyword"; +} + +function isBroadRecordKeyType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + if ( + unwrapped.type === "TSStringKeyword" || + unwrapped.type === "TSNumberKeyword" || + unwrapped.type === "TSSymbolKeyword" + ) { + return true; + } + if (unwrapped.type === "TSUnionType") return unwrapped.types.every(isBroadRecordKeyType); + return unwrapped.type === "TSTypeReference" && typeReferenceName(unwrapped) === "PropertyKey"; +} + +function isBroadRecordType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + + if (unwrapped.type === "TSTypeReference") { + if (typeReferenceName(unwrapped) === "Readonly") { + const [inner] = unwrapped.typeArguments?.params ?? []; + return inner !== undefined && isBroadRecordType(inner); + } + + if (typeReferenceName(unwrapped) !== "Record") return false; + const parameters = unwrapped.typeArguments?.params ?? []; + return ( + parameters.length === 2 && + parameters[0] !== undefined && + parameters[1] !== undefined && + isBroadRecordKeyType(parameters[0]) && + isUnknownOrAnyType(parameters[1]) + ); + } + + if (unwrapped.type !== "TSTypeLiteral" || unwrapped.members.length !== 1) return false; + const [member] = unwrapped.members; + const [parameter] = member?.type === "TSIndexSignature" ? member.parameters : []; + return ( + member?.type === "TSIndexSignature" && + member.parameters.length === 1 && + parameter !== undefined && + isBroadRecordKeyType(parameter.typeAnnotation.typeAnnotation) && + isUnknownOrAnyType(member.typeAnnotation.typeAnnotation) + ); +} + +function broadTypeKind(type: ESTree.TSType): BroadTypeKind | null { + const unwrapped = unwrapTypeParentheses(type); + if (unwrapped.type === "TSUnknownKeyword" || unwrapped.type === "TSAnyKeyword") return "top"; + if (unwrapped.type === "TSObjectKeyword") return "object"; + return isBroadRecordType(unwrapped) ? "record" : null; +} + +function assertedExpression( + node: ESTree.TSAsExpression | ESTree.TSTypeAssertion, +): ESTree.Expression { + return unwrapExpressionParentheses(node.expression); +} + +function assertionFromExpression( + expression: ESTree.Expression, +): ESTree.TSAsExpression | ESTree.TSTypeAssertion | null { + const unwrapped = unwrapExpressionParentheses(expression); + return unwrapped.type === "TSAsExpression" || unwrapped.type === "TSTypeAssertion" + ? unwrapped + : null; +} + +function normalizedTypeText(sourceText: string, type: ESTree.TSType): string { + return sourceText.slice(type.start, type.end).replaceAll(/\s+/gu, ""); +} + +function typesHaveSameSyntax( + sourceText: string, + left: ESTree.TSType | null, + right: ESTree.TSType, +): boolean { + return ( + left !== null && + normalizedTypeText(sourceText, unwrapTypeParentheses(left)) === + normalizedTypeText(sourceText, unwrapTypeParentheses(right)) + ); +} + +function isDefinitelyObjectType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + switch (unwrapped.type) { + case "TSArrayType": + case "TSConstructorType": + case "TSFunctionType": + case "TSMappedType": + case "TSObjectKeyword": + case "TSTupleType": + return true; + case "TSTypeLiteral": + return unwrapped.members.length > 0; + case "TSIntersectionType": + return unwrapped.types.every(isDefinitelyObjectType); + case "TSTypeOperator": + return unwrapped.operator === "readonly" && isDefinitelyObjectType(unwrapped.typeAnnotation); + default: + return false; + } +} + +function isDefinitelyNarrowerRecordType(type: ESTree.TSType): boolean { + const unwrapped = unwrapTypeParentheses(type); + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.some((member) => member.type !== "TSIndexSignature"); + } + + if (unwrapped.type !== "TSTypeReference") return false; + if (typeReferenceName(unwrapped) === "Readonly") { + const [inner] = unwrapped.typeArguments?.params ?? []; + return inner !== undefined && isDefinitelyNarrowerRecordType(inner); + } + if (typeReferenceName(unwrapped) !== "Record") return false; + + const parameters = unwrapped.typeArguments?.params ?? []; + return ( + parameters.length === 2 && parameters[1] !== undefined && !isUnknownOrAnyType(parameters[1]) + ); +} + +function functionBoundary(node: ESTree.Node): ESTree.Node | null { + let current = node.parent; + while (current !== null && current.type !== "Program") { + if (functionBoundaryTypes.has(current.type)) return current; + current = current.parent; + } + return null; +} + +function resolvedVariableForIdentifier( + scopes: readonly { + readonly references: readonly { + readonly identifier: ESTree.Node; + readonly resolved: Variable | null; + }[]; + }[], + identifier: ESTree.IdentifierReference, +): Variable | null { + for (const scope of scopes) { + const reference = scope.references.find( + (candidate) => + candidate.identifier.start === identifier.start && + candidate.identifier.end === identifier.end, + ); + if (reference !== undefined) return reference.resolved; + } + return null; +} + +function variableDeclarator(variable: Variable): ESTree.VariableDeclarator | null { + for (const definition of variable.defs) { + if (definition.type === "Variable" && definition.node.type === "VariableDeclarator") { + return definition.node; + } + } + return null; +} + +function knownValueEvidence( + expression: ESTree.Expression, + scopes: Parameters[0], + boundary: ESTree.Node | null, + visitedVariables: ReadonlySet, +): KnownValueEvidence | null { + const unwrapped = unwrapExpressionParentheses(expression); + + if (unwrapped.type === "TSAsExpression" || unwrapped.type === "TSTypeAssertion") { + if (broadTypeKind(unwrapped.typeAnnotation) !== null) return null; + return { type: unwrapped.typeAnnotation }; + } + + if (unwrapped.type === "Literal" || unwrapped.type === "TemplateLiteral") { + return { type: null }; + } + + if ( + unwrapped.type === "ArrayExpression" || + unwrapped.type === "ArrowFunctionExpression" || + unwrapped.type === "ClassExpression" || + unwrapped.type === "FunctionExpression" || + unwrapped.type === "NewExpression" || + unwrapped.type === "ObjectExpression" + ) { + return { type: null }; + } + + if (unwrapped.type !== "Identifier") return null; + const variable = resolvedVariableForIdentifier(scopes, unwrapped); + if (variable === null || visitedVariables.has(variable)) return null; + + const annotatedIdentifier = variable.identifiers.find( + (identifier) => identifier.typeAnnotation !== null && identifier.typeAnnotation !== undefined, + ); + const annotation = annotatedIdentifier?.typeAnnotation?.typeAnnotation; + if (annotation !== undefined && annotatedIdentifier !== undefined) { + if (functionBoundary(annotatedIdentifier) !== boundary || broadTypeKind(annotation) !== null) { + return null; + } + return { type: annotation }; + } + + const declarator = variableDeclarator(variable); + if ( + declarator === null || + declarator.parent.type !== "VariableDeclaration" || + declarator.parent.kind !== "const" || + declarator.init === null || + variable.references.some((reference) => reference.isWrite() && !reference.init) || + functionBoundary(declarator) !== boundary + ) { + return null; + } + + return knownValueEvidence( + declarator.init, + scopes, + boundary, + new Set([...visitedVariables, variable]), + ); +} + +function widenedBinding( + variable: Variable, + scopes: Parameters[0], +): { + readonly broadKind: BroadTypeKind; + readonly evidence: KnownValueEvidence; + readonly declaredAt: number; + readonly boundary: ESTree.Node | null; +} | null { + const declarator = variableDeclarator(variable); + if ( + declarator === null || + declarator.parent.type !== "VariableDeclaration" || + declarator.parent.kind !== "const" || + declarator.id.type !== "Identifier" || + declarator.init === null || + variable.references.some((reference) => reference.isWrite() && !reference.init) + ) { + return null; + } + + const boundary = functionBoundary(declarator); + const declaredType = declarator.id.typeAnnotation?.typeAnnotation; + const initializerAssertion = assertionFromExpression(declarator.init); + const initializerBroadKind = + initializerAssertion === null ? null : broadTypeKind(initializerAssertion.typeAnnotation); + const declaredBroadKind = declaredType === undefined ? null : broadTypeKind(declaredType); + const broadKind = declaredBroadKind ?? initializerBroadKind; + if (broadKind === null) return null; + + const originalExpression = + initializerAssertion !== null && initializerBroadKind !== null + ? assertedExpression(initializerAssertion) + : declarator.init; + const evidence = knownValueEvidence(originalExpression, scopes, boundary, new Set([variable])); + return evidence === null ? null : { broadKind, evidence, declaredAt: declarator.end, boundary }; +} + +function assertionIsNarrower( + sourceText: string, + broadKind: BroadTypeKind, + evidence: KnownValueEvidence, + assertedType: ESTree.TSType, +): boolean { + if (broadTypeKind(assertedType) !== null) return false; + if (broadKind === "top") return true; + if (typesHaveSameSyntax(sourceText, evidence.type, assertedType)) return true; + if (broadKind === "object") return isDefinitelyObjectType(assertedType); + return isDefinitelyNarrowerRecordType(assertedType); +} + +/** Detect immutable local bindings that erase a known type and are later asserted back to a narrower type. */ +export const noWidenThenAssertRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Disallow local const flows that explicitly widen a known value before asserting the widened binding to a narrower type.", + }, + messages: { + widenThenAssert: + 'Binding "{{name}}" discards type evidence and later recreates it with an assertion. Keep the precise type from initialization through use; parse boundary input once.', + }, + }, + createOnce(context) { + let scopes: Parameters[0] = []; + + const checkAssertion = (node: ESTree.TSAsExpression | ESTree.TSTypeAssertion) => { + const expression = assertedExpression(node); + if (expression.type !== "Identifier") return; + + const variable = resolvedVariableForIdentifier(scopes, expression); + if (variable === null) return; + const widened = widenedBinding(variable, scopes); + if ( + widened === null || + node.start <= widened.declaredAt || + functionBoundary(node) !== widened.boundary || + !assertionIsNarrower( + context.sourceCode.text, + widened.broadKind, + widened.evidence, + node.typeAnnotation, + ) + ) { + return; + } + + context.report({ + node, + messageId: "widenThenAssert", + data: { name: expression.name }, + }); + }; + + return { + Program() { + scopes = context.sourceCode.scopeManager.scopes; + }, + TSAsExpression: checkAssertion, + TSTypeAssertion: checkAssertion, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.ts b/tools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.ts new file mode 100644 index 00000000..f1a2ffcf --- /dev/null +++ b/tools/oxlint/anti-slop/rules/require-safety-comment-for-type-assertion.ts @@ -0,0 +1,62 @@ +import { defineRule } from "@oxlint/plugins"; + +import type { ESTree, SourceCode } from "@oxlint/plugins"; + +type TypeAssertion = ESTree.TSAsExpression | ESTree.TSTypeAssertion; + +const commentOwnerKinds = new Set([ + "ExpressionStatement", + "PropertyDefinition", + "ReturnStatement", + "ThrowStatement", + "VariableDeclaration", +]); + +function isConstAssertion(node: TypeAssertion): boolean { + return ( + node.typeAnnotation.type === "TSTypeReference" && + node.typeAnnotation.typeName.type === "Identifier" && + node.typeAnnotation.typeName.name === "const" + ); +} + +function hasSafetyComment(sourceCode: SourceCode, node: TypeAssertion): boolean { + let current: ESTree.Node = node; + while (true) { + if ( + sourceCode + .getCommentsBefore(current) + .some((comment) => comment.end <= node.start && /\bSAFETY\s*:/u.test(comment.value)) + ) { + return true; + } + if (commentOwnerKinds.has(current.type) || current.parent.type === "Program") return false; + current = current.parent; + } +} + +/** Require every non-const type assertion to state the invariant TypeScript cannot express. */ +export const requireSafetyCommentForTypeAssertionRule = defineRule({ + meta: { + type: "problem", + docs: { + description: + "Require a nearby SAFETY comment for every TypeScript type assertion except const assertions.", + }, + messages: { + missingSafetyComment: + "This type assertion has no `SAFETY:` justification. State the checked invariant immediately before the assertion or its containing statement.", + }, + }, + createOnce(context) { + const checkAssertion = (node: TypeAssertion) => { + if (isConstAssertion(node) || hasSafetyComment(context.sourceCode, node)) return; + context.report({ node, messageId: "missingSafetyComment" }); + }; + + return { + TSAsExpression: checkAssertion, + TSTypeAssertion: checkAssertion, + }; + }, +}); diff --git a/tools/oxlint/anti-slop/shared/dictionary-types.ts b/tools/oxlint/anti-slop/shared/dictionary-types.ts new file mode 100644 index 00000000..86517004 --- /dev/null +++ b/tools/oxlint/anti-slop/shared/dictionary-types.ts @@ -0,0 +1,502 @@ +import type { ESTree } from "@oxlint/plugins"; + +const BUILT_INS = new Set([ + "Record", + "Readonly", + "Partial", + "Required", + "Pick", + "Omit", + "PropertyKey", + "NonNullable", +]); +const TRANSPARENT_WRAPPERS = new Set(["Readonly", "Partial", "Required", "NonNullable"]); + +type TypeAliasEnvironment = ReadonlyMap; + +type ResolvedType = { + readonly type: ESTree.TSType; + readonly substitutions: TypeAliasEnvironment; +}; + +export type UnsafeDictionary = { + readonly kind: "unsafe-dictionary"; + readonly unsafeValue: "any" | "empty-object" | "object" | "union" | "unknown"; +}; + +export type WideningTargetKind = + | "anonymous object" + | "generic container" + | "object" + | "open dictionary" + | "unknown"; + +export type WideningTarget = { + readonly kind: WideningTargetKind; +}; + +export type TypeEnvironment = { + readonly aliases: ReadonlyMap; + readonly interfaces: ReadonlyMap; + readonly shadowedBuiltIns: ReadonlySet; +}; + +function declaredStatement(statement: ESTree.Statement): ESTree.Node | null { + return statement.type === "ExportNamedDeclaration" || + statement.type === "ExportDefaultDeclaration" + ? (statement.declaration ?? null) + : statement; +} + +export function createTypeEnvironment(program: ESTree.Program): TypeEnvironment { + const aliases = new Map(); + const interfaces = new Map(); + const shadowedBuiltIns = new Set(); + + for (const statement of program.body) { + const declaration = declaredStatement(statement); + if (declaration?.type === "ImportDeclaration") { + for (const specifier of declaration.specifiers) { + if (BUILT_INS.has(specifier.local.name)) shadowedBuiltIns.add(specifier.local.name); + } + continue; + } + + if (declaration?.type === "TSTypeAliasDeclaration") { + const existing = aliases.get(declaration.id.name); + if (existing === undefined) aliases.set(declaration.id.name, declaration); + else shadowedBuiltIns.add(declaration.id.name); + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + continue; + } + + if (declaration?.type === "TSInterfaceDeclaration") { + const declarations = interfaces.get(declaration.id.name) ?? []; + declarations.push(declaration); + interfaces.set(declaration.id.name, declarations); + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + continue; + } + + if (declaration?.type === "TSEnumDeclaration") { + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + continue; + } + + if ( + (declaration?.type === "ClassDeclaration" || + declaration?.type === "FunctionDeclaration") && + declaration.id !== null + ) { + if (BUILT_INS.has(declaration.id.name)) shadowedBuiltIns.add(declaration.id.name); + } + } + + return { aliases, interfaces, shadowedBuiltIns }; +} + +function typeReferenceName(type: ESTree.TSTypeReference): string | null { + return type.typeName.type === "Identifier" ? type.typeName.name : null; +} + +function isBuiltIn(name: string, environment: TypeEnvironment): boolean { + return BUILT_INS.has(name) && !environment.shadowedBuiltIns.has(name); +} + +function isUnappliedReferenceTo(type: ESTree.TSType, name: string): boolean { + const unwrapped = unwrapTransparentType(type); + return ( + unwrapped.type === "TSTypeReference" && + typeReferenceName(unwrapped) === name && + (unwrapped.typeArguments === null || + unwrapped.typeArguments === undefined || + unwrapped.typeArguments.params.length === 0) + ); +} + +function unwrapTransparentType(type: ESTree.TSType): ESTree.TSType { + let current = type; + while ( + current.type === "TSParenthesizedType" || + (current.type === "TSTypeOperator" && current.operator === "readonly") + ) { + current = current.typeAnnotation; + } + return current; +} + +function isNeverType(type: ESTree.TSType): boolean { + return unwrapTransparentType(type).type === "TSNeverKeyword"; +} + +function isEffectivelyEmptyMember(member: ESTree.TSSignature): boolean { + return ( + member.type === "TSPropertySignature" && + member.optional === true && + member.typeAnnotation !== null && + member.typeAnnotation !== undefined && + isNeverType(member.typeAnnotation.typeAnnotation) + ); +} + +function isEffectivelyEmptyTypeLiteral(type: ESTree.TSTypeLiteral): boolean { + return type.members.length === 0 || type.members.every(isEffectivelyEmptyMember); +} + +function isEffectivelyEmptyInterface( + declarations: readonly ESTree.TSInterfaceDeclaration[], +): boolean { + if (declarations.length !== 1) return false; + const [type] = declarations; + return ( + type !== undefined && + type.extends.length === 0 && + (type.body.body.length === 0 || type.body.body.every(isEffectivelyEmptyMember)) + ); +} + +function resolvedSubstitutionArgument( + type: ESTree.TSType, + base: TypeAliasEnvironment, + resolving: ReadonlySet = new Set(), +): ESTree.TSType { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type !== "TSTypeReference") return type; + const name = typeReferenceName(unwrapped); + if (name === null || resolving.has(name)) return type; + const substitution = base.get(name); + if (substitution === undefined) return type; + const nextResolving = new Set(resolving); + nextResolving.add(name); + return resolvedSubstitutionArgument(substitution, base, nextResolving); +} + +function aliasSubstitution( + alias: ESTree.TSTypeAliasDeclaration, + type: ESTree.TSTypeReference, + base: TypeAliasEnvironment, +): TypeAliasEnvironment | null { + const parameters = alias.typeParameters?.params ?? []; + const arguments_ = type.typeArguments?.params ?? []; + const next = new Map(base); + for (const [index, parameter] of parameters.entries()) { + const argument = arguments_[index] ?? parameter.default; + if (argument === null || argument === undefined) return null; + next.set(parameter.name.name, resolvedSubstitutionArgument(argument, next)); + } + return next; +} + +function unsafeDirectValue( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): UnsafeDictionary["unsafeValue"] | null { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type === "TSUnknownKeyword") return "unknown"; + if (unwrapped.type === "TSAnyKeyword") return "any"; + if (unwrapped.type === "TSObjectKeyword") return "object"; + if (unwrapped.type === "TSTypeLiteral" && isEffectivelyEmptyTypeLiteral(unwrapped)) + return "empty-object"; + if (unwrapped.type === "TSUnionType") { + return unwrapped.types.some( + (member) => unsafeDirectValue(member, environment, substitutions, resolvingAliases) !== null, + ) + ? "union" + : null; + } + if (unwrapped.type === "TSIntersectionType") { + const unsafeMembers = unwrapped.types.map((member) => + unsafeDirectValue(member, environment, substitutions, resolvingAliases), + ); + if (unsafeMembers.includes("any")) return "any"; + return unsafeMembers.length > 0 && unsafeMembers.every((member) => member !== null) + ? unsafeMembers[0] + : null; + } + if (unwrapped.type !== "TSTypeReference") return null; + const name = typeReferenceName(unwrapped); + if (name === null) return null; + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined + ? null + : unsafeDirectValue(wrapped, environment, substitutions, resolvingAliases); + } + const substitution = substitutions.get(name); + if (substitution !== undefined) { + return isUnappliedReferenceTo(substitution, name) + ? null + : unsafeDirectValue(substitution, environment, substitutions, resolvingAliases); + } + const interfaceDeclarations = environment.interfaces.get(name); + if (interfaceDeclarations !== undefined) { + return isEffectivelyEmptyInterface(interfaceDeclarations) ? "empty-object" : null; + } + const alias = environment.aliases.get(name); + if (alias === undefined || resolvingAliases.has(name)) return null; + const nextSubstitutions = aliasSubstitution(alias, unwrapped, substitutions); + if (nextSubstitutions === null) return null; + const nextResolving = new Set(resolvingAliases); + nextResolving.add(name); + return unsafeDirectValue(alias.typeAnnotation, environment, nextSubstitutions, nextResolving); +} + +function dictionaryValueTypes( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): readonly ResolvedType[] { + const unwrapped = unwrapTransparentType(type); + + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.flatMap((member): readonly ResolvedType[] => + member.type === "TSIndexSignature" && member.typeAnnotation !== null + ? [{ type: member.typeAnnotation.typeAnnotation, substitutions }] + : [], + ); + } + + if (unwrapped.type === "TSMappedType") { + return unwrapped.typeAnnotation === null + ? [] + : [{ type: unwrapped.typeAnnotation, substitutions }]; + } + + if (unwrapped.type !== "TSTypeReference") return []; + const name = typeReferenceName(unwrapped); + if (name === null) return []; + + const substitution = substitutions.get(name); + if (substitution !== undefined) { + return isUnappliedReferenceTo(substitution, name) + ? [] + : dictionaryValueTypes(substitution, environment, substitutions, resolvingAliases); + } + + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined + ? [] + : dictionaryValueTypes(wrapped, environment, substitutions, resolvingAliases); + } + + if (name === "Record" && isBuiltIn(name, environment)) { + const value = unwrapped.typeArguments?.params[1] ?? null; + return value === null ? [] : [{ type: value, substitutions }]; + } + + if ((name === "Pick" || name === "Omit") && isBuiltIn(name, environment)) { + const source = unwrapped.typeArguments?.params[0]; + return source === undefined + ? [] + : dictionaryValueTypes(source, environment, substitutions, resolvingAliases); + } + + const alias = environment.aliases.get(name); + if (alias === undefined || resolvingAliases.has(name)) return []; + const nextSubstitutions = aliasSubstitution(alias, unwrapped, substitutions); + if (nextSubstitutions === null) return []; + const nextResolving = new Set(resolvingAliases); + nextResolving.add(name); + return dictionaryValueTypes(alias.typeAnnotation, environment, nextSubstitutions, nextResolving); +} + +export function classifyUnsafeDictionaryValue( + valueType: ESTree.TSType, + environment: TypeEnvironment, +): UnsafeDictionary | null { + const unsafeValue = unsafeDirectValue(valueType, environment, new Map(), new Set()); + return unsafeValue === null ? null : { kind: "unsafe-dictionary", unsafeValue }; +} + +export function classifyUnsafeDictionary( + type: ESTree.TSType, + environment: TypeEnvironment, +): UnsafeDictionary | null { + for (const valueType of dictionaryValueTypes(type, environment, new Map(), new Set())) { + const unsafeValue = unsafeDirectValue( + valueType.type, + environment, + valueType.substitutions, + new Set(), + ); + if (unsafeValue !== null) return { kind: "unsafe-dictionary", unsafeValue }; + } + return null; +} + +function resolvesToDictionary( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): boolean { + return dictionaryValueTypes(type, environment, substitutions, resolvingAliases).length > 0; +} + +export function classifyWideningTarget( + type: ESTree.TSType, + environment: TypeEnvironment, +): WideningTarget | null { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type === "TSUnknownKeyword") return { kind: "unknown" }; + if (unwrapped.type === "TSObjectKeyword") return { kind: "object" }; + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.some((member) => member.type === "TSIndexSignature") + ? { kind: "open dictionary" } + : unwrapped.members.length > 0 + ? { kind: "anonymous object" } + : null; + } + if (unwrapped.type === "TSMappedType") return { kind: "open dictionary" }; + if (unwrapped.type !== "TSTypeReference") return null; + const name = typeReferenceName(unwrapped); + if (name === null) return null; + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined ? null : classifyWideningTarget(wrapped, environment); + } + if (name === "Record" && isBuiltIn(name, environment)) return { kind: "open dictionary" }; + const alias = environment.aliases.get(name); + if (alias === undefined) return null; + if ((alias.typeParameters?.params.length ?? 0) > 0) { + const substitutions = aliasSubstitution(alias, unwrapped, new Map()); + return substitutions !== null && + resolvesToDictionary(alias.typeAnnotation, environment, substitutions, new Set([name])) + ? { kind: "generic container" } + : null; + } + const substitutions = aliasSubstitution(alias, unwrapped, new Map()); + if (substitutions === null) return null; + const resolved = classifyAliasBroadTarget( + alias.typeAnnotation, + environment, + substitutions, + new Set([name]), + ); + return resolved; +} + +function isBroadMappedKey( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, +): boolean { + const unwrapped = unwrapTransparentType(type); + if ( + unwrapped.type === "TSStringKeyword" || + unwrapped.type === "TSNumberKeyword" || + unwrapped.type === "TSSymbolKeyword" + ) { + return true; + } + if (unwrapped.type === "TSUnionType") { + return unwrapped.types.every((member) => + isBroadMappedKey(member, environment, substitutions), + ); + } + if (unwrapped.type !== "TSTypeReference") return false; + const name = typeReferenceName(unwrapped); + if (name === null) return false; + const substitution = substitutions.get(name); + if (substitution !== undefined && !isUnappliedReferenceTo(substitution, name)) { + return isBroadMappedKey(substitution, environment, substitutions); + } + return name === "PropertyKey" && isBuiltIn(name, environment); +} + +function classifyAliasBroadTarget( + type: ESTree.TSType, + environment: TypeEnvironment, + substitutions: TypeAliasEnvironment, + resolvingAliases: ReadonlySet, +): WideningTarget | null { + const unwrapped = unwrapTransparentType(type); + if (unwrapped.type === "TSUnknownKeyword") return { kind: "unknown" }; + if (unwrapped.type === "TSObjectKeyword") return { kind: "object" }; + if (unwrapped.type === "TSTypeLiteral") { + return unwrapped.members.some((member) => member.type === "TSIndexSignature") + ? { kind: "open dictionary" } + : null; + } + if (unwrapped.type === "TSMappedType") { + return isBroadMappedKey(unwrapped.constraint, environment, substitutions) + ? { kind: "open dictionary" } + : null; + } + if (unwrapped.type !== "TSTypeReference") return null; + const name = typeReferenceName(unwrapped); + if (name === null) return null; + const substitution = substitutions.get(name); + if (substitution !== undefined) { + return isUnappliedReferenceTo(substitution, name) + ? null + : classifyAliasBroadTarget( + substitution, + environment, + substitutions, + resolvingAliases, + ); + } + if (TRANSPARENT_WRAPPERS.has(name) && isBuiltIn(name, environment)) { + const wrapped = unwrapped.typeArguments?.params[0]; + return wrapped === undefined + ? null + : classifyAliasBroadTarget(wrapped, environment, substitutions, resolvingAliases); + } + if (name === "Record" && isBuiltIn(name, environment)) { + return { kind: "open dictionary" }; + } + const alias = environment.aliases.get(name); + if (alias === undefined || resolvingAliases.has(name)) return null; + const nextSubstitutions = aliasSubstitution(alias, unwrapped, substitutions); + if (nextSubstitutions === null) return null; + const nextResolving = new Set(resolvingAliases); + nextResolving.add(name); + return classifyAliasBroadTarget( + alias.typeAnnotation, + environment, + nextSubstitutions, + nextResolving, + ); +} + +export function isPopulatedObjectExpression(expression: ESTree.Expression): boolean { + let current = expression; + while ( + current.type === "ParenthesizedExpression" || + current.type === "TSAsExpression" || + current.type === "TSTypeAssertion" || + current.type === "TSNonNullExpression" + ) { + current = current.expression; + } + return current.type === "ObjectExpression" && current.properties.length > 0; +} + +export function isKnownEvidenceExpression(expression: ESTree.Expression): boolean { + let current = expression; + while ( + current.type === "ParenthesizedExpression" || + current.type === "TSAsExpression" || + current.type === "TSTypeAssertion" || + current.type === "TSNonNullExpression" || + current.type === "TSSatisfiesExpression" + ) { + current = current.expression; + } + if (current.type === "ObjectExpression") return true; + return ( + current.type === "ArrayExpression" || + current.type === "ArrowFunctionExpression" || + current.type === "ClassExpression" || + current.type === "FunctionExpression" || + current.type === "NewExpression" || + current.type === "Literal" || + current.type === "TemplateLiteral" || + current.type === "UnaryExpression" + ); +} diff --git a/tools/oxlint/anti-slop/shared/lexical-type-parameters.ts b/tools/oxlint/anti-slop/shared/lexical-type-parameters.ts new file mode 100644 index 00000000..7cdb18c9 --- /dev/null +++ b/tools/oxlint/anti-slop/shared/lexical-type-parameters.ts @@ -0,0 +1,61 @@ +import type { ESTree } from "@oxlint/plugins"; + +type VisitorKeys = Readonly>; + +function isNode(value: unknown): value is ESTree.Node { + return ( + typeof value === "object" && + value !== null && + "type" in value && + typeof value.type === "string" + ); +} + +function collectInferTypeParameterNames( + node: ESTree.Node, + visitorKeys: VisitorKeys, + names: Set, +): void { + if (node.type === "TSInferType") names.add(node.typeParameter.name.name); + const record = node as unknown as Readonly>; + for (const key of visitorKeys[node.type] ?? []) { + const value = record[key]; + if (isNode(value)) { + collectInferTypeParameterNames(value, visitorKeys, names); + continue; + } + if (!Array.isArray(value)) continue; + for (const child of value) { + if (isNode(child)) collectInferTypeParameterNames(child, visitorKeys, names); + } + } +} + +/** Collect type binders that are in scope at a node and can shadow module aliases. */ +export function lexicalTypeParameterNames( + node: ESTree.Node, + visitorKeys: VisitorKeys, +): ReadonlySet { + const names = new Set(); + let descendant: ESTree.Node = node; + let current: ESTree.Node | null = node; + while (current !== null && current.type !== "Program") { + if ("typeParameters" in current) { + for (const parameter of current.typeParameters?.params ?? []) { + names.add(parameter.name.name); + } + } + if ( + current.type === "TSMappedType" && + (descendant === current.nameType || descendant === current.typeAnnotation) + ) { + names.add(current.key.name); + } + if (current.type === "TSConditionalType" && descendant === current.trueType) { + collectInferTypeParameterNames(current.extendsType, visitorKeys, names); + } + descendant = current; + current = current.parent; + } + return names; +} diff --git a/tools/oxlint/anti-slop/shared/reflect-method.ts b/tools/oxlint/anti-slop/shared/reflect-method.ts new file mode 100644 index 00000000..39bc218c --- /dev/null +++ b/tools/oxlint/anti-slop/shared/reflect-method.ts @@ -0,0 +1,35 @@ +import type { ESTree, Scope, SourceCode, Variable } from "@oxlint/plugins"; + +function resolveVariable( + sourceCode: SourceCode, + identifier: ESTree.IdentifierReference, +): Variable | null { + let scope: Scope | null = sourceCode.getScope(identifier); + while (scope !== null) { + const variable = scope.set.get(identifier.name); + if (variable !== undefined) return variable; + scope = scope.upper; + } + return null; +} + +function isGlobalReflect(sourceCode: SourceCode, expression: ESTree.Expression): boolean { + if (expression.type !== "Identifier" || expression.name !== "Reflect") return false; + if (sourceCode.isGlobalReference(expression)) return true; + const variable = resolveVariable(sourceCode, expression); + return variable === null || variable.defs.length === 0; +} + +/** Reports whether a call target names one method on the global Reflect object. */ +export function isGlobalReflectMethodCall( + sourceCode: SourceCode, + callee: ESTree.Expression, + methodName: string, +): boolean { + if (!("property" in callee) || !("object" in callee) || !("computed" in callee)) return false; + if (!isGlobalReflect(sourceCode, callee.object)) return false; + const property = callee.property; + return callee.computed + ? property.type === "Literal" && property.value === methodName + : property.type === "Identifier" && property.name === methodName; +} From 21152ec120b963605220d06055273c4c74c1c7c9 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 15:55:49 +0300 Subject: [PATCH 10/17] chore(vscode): update changelog for integrations settings page --- packages/vscode/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/vscode/CHANGELOG.md b/packages/vscode/CHANGELOG.md index 44f43241..f0bfc64e 100644 --- a/packages/vscode/CHANGELOG.md +++ b/packages/vscode/CHANGELOG.md @@ -1,5 +1,6 @@ ## [Unreleased] +- **Settings/Integrations:** a new Integrations settings page lists Claude Code, Command Code, and Cursor plugins with install, update, setup, and remove actions, plus Discord and Telegram Coming soon placeholders. - Chat: new chats no longer start against a deleted last worktree directory; they fall back to the active project instead of saving the first message and never starting. - Chat: opening a busy subagent in the context panel now shows its history instead of only the working-status line (thanks to @makeittech). - Projects: project names now match the folder name exactly, so `.ssh` and `opencode-claude` are no longer shown as `.Ssh` and `Opencode Claude`; names you renamed yourself are kept. From 97691fc4ac7a19d7aa8aab4a83f6983b315b5cc0 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 16:34:05 +0300 Subject: [PATCH 11/17] chore(scripts): raise default active batch limit to 10 --- scripts/anti-slop.mjs | 2 +- scripts/react-doctor.mjs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/anti-slop.mjs b/scripts/anti-slop.mjs index ca91c8e1..197f11f1 100644 --- a/scripts/anti-slop.mjs +++ b/scripts/anti-slop.mjs @@ -16,7 +16,7 @@ import { const PIPELINE = "as"; // Resolved before command dispatch so every command shares one claims location. const { runsDir: RUNS_DIR, shared: SHARED_CLAIMS } = resolveRunsDir(parseArgs(process.argv.slice(2))["claims-dir"]); -const DEFAULT_MAX_ACTIVE = 3; +const DEFAULT_MAX_ACTIVE = 10; const DEFAULT_CLAIM_TTL_DAYS = 3; // Rules ordered by how mechanical and behavior-safe their fixes are. Higher diff --git a/scripts/react-doctor.mjs b/scripts/react-doctor.mjs index 5ed108b5..acaf11ff 100644 --- a/scripts/react-doctor.mjs +++ b/scripts/react-doctor.mjs @@ -20,7 +20,7 @@ const REACT_DOCTOR_VERSION = "0.9.12"; const PIPELINE = "rd"; // Resolved before command dispatch so every command shares one claims location. const { runsDir: RUNS_DIR, shared: SHARED_CLAIMS } = resolveRunsDir(parseArgs(process.argv.slice(2))["claims-dir"]); -const DEFAULT_MAX_ACTIVE = 3; +const DEFAULT_MAX_ACTIVE = 10; const DEFAULT_CLAIM_TTL_DAYS = 3; const PRIORITY_RULES = new Map([ From b178f75eff6f9ea2049f9276e9fdd992ebccd90d Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 17:10:31 +0300 Subject: [PATCH 12/17] docs: add maintenance review workflow --- .opencode/commands/maintenance-review.md | 133 +++++++++++++++++++++++ 1 file changed, 133 insertions(+) create mode 100644 .opencode/commands/maintenance-review.md diff --git a/.opencode/commands/maintenance-review.md b/.opencode/commands/maintenance-review.md new file mode 100644 index 00000000..6e41c132 --- /dev/null +++ b/.opencode/commands/maintenance-review.md @@ -0,0 +1,133 @@ +--- +description: Deeply review every open maintenance PR and fix it to completion, not by commenting +agent: build +--- + +You are working in the OpenChamber repository. + +Goal: take every open automated maintenance pull request and bring it to a state where a human reviewer would merge it without a single objection. You are the intelligence layer between unattended batch tasks and the repository owner. The batch tasks optimize for a metric; you optimize for the code being right. + +You do not leave review comments. You do the work. A finding you notice and do not fix is a failure of this task. + +## Scope of the run + +In scope: every open pull request whose head branch starts with `anti-slop/` or `react-doctor/`. + +Find them: + +`gh pr list --state open --search "head:anti-slop/" --json number,title,headRefName,url` + +`gh pr list --state open --search "head:react-doctor/" --json number,title,headRefName,url` + +Work through them one at a time, oldest first. Finish a PR completely before starting the next. Do not interleave. + +This task ends only when every PR in the list has been reviewed, fixed, validated, pushed, and its description updated. Do not stop at the first one. Do not stop because a PR looks acceptable at a glance; that judgement comes after reading the diff, not before. + +## Before you start + +Verify the worktree is clean: + +`git status --porcelain` + +If the output is not empty, stop immediately and report it. Do not stash, reset, or discard anything. + +Read `AGENTS.md`, and read `.opencode/commands/as-fixes.md` in full, including the sections "What a good fix looks like" and "Hard prohibitions". Those describe the standard the anti-slop PRs were supposed to meet. Your job includes verifying they actually met it. + +Load every project skill matching the code you end up touching, exactly as `AGENTS.md` requires. These PRs reach into sync, stores, UI, runtime, and CLI code, and the applicable skill is determined by what you change, not by the fact that this is maintenance work. + +## Working on one PR + +Check out the branch and bring it up to date with `main`: + +`gh pr checkout ` + +`git merge origin/main` + +If the merge conflicts, resolve it correctly by reading both sides. Never resolve a conflict by taking one side wholesale to save time. + +Then read the entire diff against `main`, not just the changed lines: + +`git diff origin/main...HEAD` + +For every file in the diff, open the file itself and read the surrounding code. These PRs change type contracts and component structure, so a line that looks correct in isolation is frequently wrong in context. + +## What you are looking for + +Treat the PR body's claims as unverified. Re-run the checks yourself; do not trust reported results. + +Correctness of the change itself: +- Did the change alter runtime behavior? Effect cleanup, hook dependencies, component extraction, conditional object spreads, and added parsing all can. Decide whether the new behavior is right, not merely whether it is different. +- Does a removed or reordered object key change what gets serialized to an API, persisted to disk, or merged over defaults? A key that used to be absent and is now present as `undefined` is a real change. +- Was dead code removed that is actually referenced somewhere the batch task did not search, including dynamic imports, string-keyed lookups, generated assets, and other packages? +- Did a type contract change without every call site being updated? Search for each changed symbol across the workspace. +- Did an extracted component lose state, memoization, ref forwarding, or a stable identity that the original had? + +Honesty of the change: +- Is any `// SAFETY:` comment vague, generic, or untrue? A comment must name the check that already ran. If it does not, either delete the assertion by fixing the contract, or write the truthful comment. +- Was a type laundered rather than fixed? Look for invented primitive unions, `any`, new assertions, hand-written type predicates that merely relocate a rejected `typeof` check, or deleted fields and tests. +- Was a lint rule disabled, downgraded, ignored, or suppressed inline anywhere in the diff? Revert that and fix the underlying code. +- Was a new dependency, schema library, utility module, or architectural pattern introduced under the cover of cleanup? Remove it and solve the problem within existing precedent. + +Quality of the result: +- Does the new code read like the code around it, in naming, structure, and comment density? +- Are the new names accurate, or do they describe the refactor instead of the domain? +- Is the change complete, or did the batch task fix eight of eleven findings in a file and leave three arbitrary ones behind? + +## Fixing + +Fix everything you find, on the PR branch, as additional commits. You are explicitly permitted to go beyond the batch's original file scope when correctness requires it: update call sites, correct an upstream contract, add a missing test, or finish an incomplete refactor. + +Two boundaries on that freedom: + +1. Do not touch files that another open maintenance PR modifies. Check with `gh pr diff --name-only` for the other open PRs in this run. If a correct fix genuinely requires such a file, make the change in whichever PR already owns that file, and note the cross-PR dependency in both descriptions. +2. Do not turn a maintenance PR into a feature or a redesign. If you conclude the batch's approach was wrong at the root, revert that part of the diff rather than building on it, and explain the revert in the PR body. A smaller correct PR beats a larger clever one. + +Do not disable, downgrade, or ignore lint rules. Do not add `any`, widen a type, or add an assertion to make a check pass. Do not edit `oxlint.config.ts`, `tools/oxlint/anti-slop/`, `CHANGELOG.md`, package versions, or release metadata. + +If a batch left findings unfixed and the PR body called them skipped, evaluate each one yourself. Fix the ones that are fixable within a correct, reviewable change. Keep a skip only when you can articulate why fixing it would be wrong here, not merely hard. + +## Validating each PR + +Re-run the pipeline's own check for the batch, using the run id from the PR body when it is present: + +`bun run deslop -- check-batch --run ` for anti-slop PRs + +`bun run doctor -- check-batch --run ` for React Doctor PRs + +If the run directory no longer exists, skip that command and say so; it is a convenience, not the source of truth. + +Then, for every package the final diff touches, run its own checks: + +`bun run --cwd packages/ type-check` + +`bun run --cwd packages/ lint` + +`bun run --cwd packages/ test` + +Run `bunx oxlint ` on the files in the diff and confirm you have not increased anti-slop findings anywhere. + +For surfaces TypeScript does not cover, such as server JavaScript, CLI JavaScript, or Electron main-process helpers, run the focused tests for that surface. Static checks do not prove those correct. + +If a check fails for a reason unrelated to this PR, verify that claim by checking the same command on `main` before dismissing it, and report the result either way. + +## Delivering each PR + +- Commit your fixes with concise messages describing what was actually wrong. +- Push to the PR branch. Never force-push. +- Update the PR description so it describes the final state: what the batch did, what you corrected and why, every behavior change with the decision you made, every remaining skipped finding with its reason, and the exact validation commands you ran with their results. +- Preserve any content the repository owner added to the description by hand, including screenshots. Read the live description before editing it and merge your changes into it rather than overwriting. +- Add one PR comment summarizing your review pass, so the history shows what was examined and what was changed. +- Do not merge, do not close, do not approve, and do not request review. +- Do not release the batch claim. The batch stays claimed until its PR is merged or closed. + +Then move to the next PR. + +## Finishing the run + +When every PR has been handled, return to `main` and pull: + +`git checkout main && git pull` + +Report, per PR: number, title, what was wrong, what you fixed, what you deliberately left alone and why, validation results, and your assessment of whether it is now ready to merge. State plainly if any PR is not ready and what blocks it. + +If you found nothing wrong in a PR, say that explicitly and describe what you checked to reach that conclusion. That is a valid outcome, but only after real inspection. From 58c190f0b1a1e92a1e66d85ac03f659b0624f392 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 17:30:45 +0300 Subject: [PATCH 13/17] fix(web): use Vitest timers in PR status tests --- packages/web/server/lib/github/pr-status.test.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/web/server/lib/github/pr-status.test.js b/packages/web/server/lib/github/pr-status.test.js index bbde6c78..eca83611 100644 --- a/packages/web/server/lib/github/pr-status.test.js +++ b/packages/web/server/lib/github/pr-status.test.js @@ -1,4 +1,4 @@ -import { afterEach, beforeEach, describe, expect, mock, setSystemTime, test } from 'bun:test'; +import { afterEach, beforeEach, describe, expect, mock, test, vi } from 'bun:test'; const listMock = mock(async () => ({ data: [] })); @@ -63,7 +63,7 @@ describe('findBranchPrCandidates', () => { }); afterEach(() => { - setSystemTime(); + vi.useRealTimers(); }); test('an open PR wins and no history lookup is spent', async () => { @@ -156,7 +156,8 @@ describe('findBranchPrCandidates', () => { // Past the "no history" expiry, but far short of the found-record one. The // shared open list is re-fetched; the history answer is not re-queried. - setSystemTime(new Date(startedAt + 30 * 60 * 1000)); + vi.useFakeTimers(); + vi.setSystemTime(new Date(startedAt + 30 * 60 * 1000)); const { historical } = await call({ force: false }); expect(historical?.number).toBe(12); @@ -171,7 +172,8 @@ describe('findBranchPrCandidates', () => { await call(); const callsAfterFirst = listMock.mock.calls.length; - setSystemTime(new Date(startedAt + 30 * 60 * 1000)); + vi.useFakeTimers(); + vi.setSystemTime(new Date(startedAt + 30 * 60 * 1000)); await call({ force: false }); expect(listMock.mock.calls.some((entry) => entry[0]?.state === 'all')).toBe(true); From 80150aaf0dd8c19f22c29b272bee6f259d4d54fb Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 17:31:46 +0300 Subject: [PATCH 14/17] chore: increase default max active claims to 20 --- scripts/anti-slop.mjs | 2 +- scripts/react-doctor.mjs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/scripts/anti-slop.mjs b/scripts/anti-slop.mjs index 197f11f1..ff7041f4 100644 --- a/scripts/anti-slop.mjs +++ b/scripts/anti-slop.mjs @@ -16,7 +16,7 @@ import { const PIPELINE = "as"; // Resolved before command dispatch so every command shares one claims location. const { runsDir: RUNS_DIR, shared: SHARED_CLAIMS } = resolveRunsDir(parseArgs(process.argv.slice(2))["claims-dir"]); -const DEFAULT_MAX_ACTIVE = 10; +const DEFAULT_MAX_ACTIVE = 20; const DEFAULT_CLAIM_TTL_DAYS = 3; // Rules ordered by how mechanical and behavior-safe their fixes are. Higher diff --git a/scripts/react-doctor.mjs b/scripts/react-doctor.mjs index acaf11ff..f3846d72 100644 --- a/scripts/react-doctor.mjs +++ b/scripts/react-doctor.mjs @@ -20,7 +20,7 @@ const REACT_DOCTOR_VERSION = "0.9.12"; const PIPELINE = "rd"; // Resolved before command dispatch so every command shares one claims location. const { runsDir: RUNS_DIR, shared: SHARED_CLAIMS } = resolveRunsDir(parseArgs(process.argv.slice(2))["claims-dir"]); -const DEFAULT_MAX_ACTIVE = 10; +const DEFAULT_MAX_ACTIVE = 20; const DEFAULT_CLAIM_TTL_DAYS = 3; const PRIORITY_RULES = new Map([ From bfa0f9ee2a7eaaba309fa93a01ad11f407e10744 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 18:33:45 +0300 Subject: [PATCH 15/17] docs: require PR template and complete-file batches in maintenance flows Maintenance task commands now fill .github/PULL_REQUEST_TEMPLATE.md section by section instead of inventing their own headings, and follow-up tasks keep the description true for the final HEAD while preserving hand-added content. Raise the anti-slop batch window to 60-120 findings and require each selected file to be finished: remaining findings need an individual specific reason, shared root causes count once, and difficulty alone no longer justifies a skip. A half-fixed file otherwise returns as a second pull request over the same code. Add the maintenance-review command, which reviews every open anti-slop and react-doctor pull request and fixes the findings directly rather than commenting, without merging or approving. --- .opencode/commands/as-fixes.md | 40 +++++++++++++++++------- .opencode/commands/as-follow-up.md | 1 + .opencode/commands/maintenance-review.md | 2 +- .opencode/commands/rd-fixes.md | 26 +++++++++------ .opencode/commands/rd-follow-up.md | 1 + scripts/anti-slop.mjs | 8 ++--- 6 files changed, 52 insertions(+), 26 deletions(-) diff --git a/.opencode/commands/as-fixes.md b/.opencode/commands/as-fixes.md index cdf6f494..fb9152b6 100644 --- a/.opencode/commands/as-fixes.md +++ b/.opencode/commands/as-fixes.md @@ -17,7 +17,7 @@ If the output is not empty, stop immediately and report that the worktree has un Then run: -`bun run deslop -- next-batch --min-issues 25 --max-issues 60` +`bun run deslop -- next-batch --min-issues 60 --max-issues 120` Use the command output as the source of truth for this task scope. @@ -282,7 +282,20 @@ Before moving to the next finding, check all four: Handle findings deliberately instead of skipping them: for parsing work, add the smallest schema that covers the fields actually used; for contract changes, follow call sites with search and update them; for tests, prefer real seams over widened fixtures. -Skip a finding only when the fix would require broad architectural changes, unclear behavior changes, or changes outside the selected batch scope. If skipped, mention it in the PR body. +## Finish the file + +A selected file is finished when it has zero anti-slop findings for the enabled rules, or when every remaining finding has an individual, specific reason to stay. + +This matters beyond tidiness. A file left half-fixed will be selected again by a later batch, producing a second pull request over the same file, with its own template, its own review, and its own merge. Every finding you defer costs the repository owner a future review cycle. Treat "I fixed the easy half" as an incomplete task, not a delivery. + +So, before you consider a selected file done: + +- Re-run `bun run deslop -- file ` and read what is left. +- If findings remain, they must be the genuinely hard ones, and you must be able to explain each one specifically. "Requires a broader refactor" is only acceptable when you name the refactor, the module boundary it crosses, and why doing it here would make the change unreviewable. +- A group of findings sharing one root cause counts as one reason, and that root cause is usually worth fixing. If eleven findings in a file all come from one untyped parser, fixing that parser is the point of the batch, not a reason to skip. +- Leaving more than roughly a quarter of a file's findings behind means you have not finished. Either finish them or explain, per group, why the file was a bad selection in the first place. + +Skip a finding only when the fix would require unclear behavior changes, or a change so large it would stop the pull request from being reviewable. Difficulty alone is not a reason. If skipped, give the specific reason in the PR body under `## Non-goals`. Hard prohibitions. Each of these makes the lint output greener while making the code worse, and each is grounds for rejecting the whole PR: - Do not disable, downgrade, or ignore anti-slop rules, in configuration or with inline comments. @@ -323,17 +336,20 @@ Validation and delivery: - Create exactly one PR with `gh pr create` using the exact printed `PR title`. - After the PR is created, switch back to `main` and pull the latest remote changes again. -PR requirements: +PR requirements. The repository has a mandatory pull request template at `.github/PULL_REQUEST_TEMPLATE.md`, and `AGENTS.md` requires it to be completed with concrete evidence for the final PR HEAD. Read the template and `CONTRIBUTING.md` before writing the description. Use every template heading, in the template's order, and do not invent replacement headings. Fill each section as follows. + - Use the exact printed `PR title`. -- Include the `Run ID`, `Batch name`, and `Branch name`. -- Include selected files. -- Include findings fixed according to `check-batch`. -- Include remaining findings in selected files. -- Include validation results for `check-batch` and every package-scoped type-check, lint, and test command you ran, naming the packages. -- Include a `Manual testing recommendations` section with focused checks for the changed behavior, based on the selected files and actual edits. Type-contract changes can alter runtime behavior at call sites, so name the affected surfaces concretely. -- Include any skipped findings and why. -- Include any `// SAFETY:` comment you added, with the invariant it documents. -- Include every parsing decision you introduced: what schema was added, and what now happens when input fails to parse. Reviewers must be able to see where behavior changed without reading the whole diff. +- `## Intent`: state that this is an unattended maintenance batch, name the `Run ID`, `Batch name`, and `Branch name`, and say what behavior changes. When nothing observable changes, say so explicitly rather than leaving it implied. +- `## Non-goals`: the findings left unfixed in the selected files, findings elsewhere in the repository, and any refactor you deliberately did not start. Give the reason for each, not just the count. +- `## Affected surfaces`: the packages, runtimes, user-visible states, and persisted or external contracts the diff reaches. Name every runtime the changed code runs in, and explain why an apparently applicable runtime is unaffected. +- `## Repository guidance`: fill the table. List the `AGENTS.md` rules you followed, every project skill that matched the change, required skill references you read, and the nearest `README.md` or `DOCUMENTATION.md` for the touched modules. For each row explain why it applies and how the change complies. Do not list filenames without explanation. +- `## Validation`: fill the table with the exact commands you ran and their results, including `check-batch` and every package-scoped type-check, lint, and test command, naming the packages. Record failures honestly, including pre-existing failures unrelated to this PR, and say which checks you did not run. Do not claim runtime behavior from type-check or lint alone. +- `## Visual evidence`: these PRs usually have no visible change, so explain concretely why the diff cannot affect rendered behavior. If anything user-visible did change, attach before/after evidence for the affected states. +- `## Risks and failure behavior`: cover what breaks if a change is wrong, how to roll it back, and any compatibility, data, performance, or cross-runtime concern. This is where every behavior-affecting decision belongs: each parsing decision you introduced and what now happens on invalid input, each `// SAFETY:` comment you added with the invariant it documents, and any change to whether an object key is present. State "None identified" only with a concrete reason. + +Add a `## Manual testing recommendations` section after the template sections, with focused checks for the changed behavior, based on the selected files and actual edits. Type-contract changes can alter runtime behavior at call sites, so name the affected surfaces concretely. + +Also state, inside `## Intent`, the selected files and how many findings `check-batch` reports as fixed and remaining. Constraints: - Keep the PR small and reviewable. diff --git a/.opencode/commands/as-follow-up.md b/.opencode/commands/as-follow-up.md index 51b62b9f..6e1c6faf 100644 --- a/.opencode/commands/as-follow-up.md +++ b/.opencode/commands/as-follow-up.md @@ -54,6 +54,7 @@ Delivery: - Reply to addressed review comments using `gh`. - For each specific review comment you addressed, reply with what was changed and the follow-up commit hash. - If the feedback was a general PR comment, add one general PR comment summarizing what was addressed, commit hashes, and validation results. +- Update the PR description so it stays true for the final HEAD: refresh `## Validation` with the checks you re-ran, and move any new behavior change into `## Risks and failure behavior`. Keep every heading of `.github/PULL_REQUEST_TEMPLATE.md` intact, and preserve content the repository owner added by hand, including screenshots. Read the live description before editing and merge into it rather than overwriting. - If a comment is intentionally not addressed, reply with a concise reason. - Do not release the batch while its PR is still open and awaiting review. The claim is what keeps parallel batches off these files. - Release the batch only once its PR has been merged or closed: `bun run deslop -- release --run `. diff --git a/.opencode/commands/maintenance-review.md b/.opencode/commands/maintenance-review.md index 6e41c132..74eddeea 100644 --- a/.opencode/commands/maintenance-review.md +++ b/.opencode/commands/maintenance-review.md @@ -114,7 +114,7 @@ If a check fails for a reason unrelated to this PR, verify that claim by checkin - Commit your fixes with concise messages describing what was actually wrong. - Push to the PR branch. Never force-push. -- Update the PR description so it describes the final state: what the batch did, what you corrected and why, every behavior change with the decision you made, every remaining skipped finding with its reason, and the exact validation commands you ran with their results. +- Update the PR description so it describes the final state, using every heading of `.github/PULL_REQUEST_TEMPLATE.md` in the template's order. `## Intent` covers what the batch did and what you corrected; `## Non-goals` covers what you deliberately left alone; `## Affected surfaces` must reflect the final diff, including files you added beyond the batch scope; `## Repository guidance` must list the rules, skills, and module documentation that applied to your own edits, not only the batch's; `## Validation` must contain the exact commands you re-ran and their results; `## Risks and failure behavior` must carry every behavior change you accepted or introduced. A description that still describes only the batch's original work is incomplete. - Preserve any content the repository owner added to the description by hand, including screenshots. Read the live description before editing it and merge your changes into it rather than overwriting. - Add one PR comment summarizing your review pass, so the history shows what was examined and what was changed. - Do not merge, do not close, do not approve, and do not request review. diff --git a/.opencode/commands/rd-fixes.md b/.opencode/commands/rd-fixes.md index bc026e08..b610a4d0 100644 --- a/.opencode/commands/rd-fixes.md +++ b/.opencode/commands/rd-fixes.md @@ -33,7 +33,10 @@ Workflow: - Fix as many diagnostics as practical in the selected files. Your default should be to fix selected diagnostics, not to skip them. - Prefer direct, behavior-preserving fixes: missing effect cleanup, mutable effect dependencies, accessibility issues with semantic fixes, local performance improvements, Tailwind shorthand replacements, component extraction when the boundary is clear, dead-code removal after verifying no references, and reducer or derived-state cleanup when the state relationship is local and clear. - Handle larger diagnostics deliberately instead of skipping them: for component splits, extract the smallest coherent subcomponent that reduces the diagnostic while preserving props/state flow; for dead code, verify references with search before deleting exports, types, or files; for state architecture issues, prefer the smallest local reducer or derived-state simplification that preserves behavior; for render-function extraction, extract only stable render helpers that do not depend on large implicit closure state, or pass explicit props; for behavior-sensitive diagnostics, read the surrounding code first and preserve existing runtime behavior. -- Skip a diagnostic only when the fix would require broad architectural changes, unclear behavior changes, or changes outside the selected batch scope. If skipped, mention it in the PR body. +- Finish each selected file. A file is finished when it has zero React Doctor diagnostics, or when every remaining diagnostic has an individual, specific reason to stay. A half-fixed file will be selected again later and cost a second pull request, a second review, and a second merge over the same code. +- Before considering a file done, re-run `bun run doctor -- file ` and read what is left. Leaving more than roughly a quarter of a file's diagnostics behind means you have not finished. +- A group of diagnostics sharing one root cause counts as one reason, and that root cause is usually worth fixing rather than deferring. +- Skip a diagnostic only when the fix would require unclear behavior changes, or a change so large it would stop the pull request from being reviewable. Difficulty alone is not a reason. If skipped, give the specific reason in the PR body under `## Non-goals`. - Do not suppress React Doctor diagnostics unless there is a clear false positive. - If a listed diagnostic requires changes outside the selected files, make only the minimal required supporting change. Do not expand the cleanup scope. @@ -59,15 +62,20 @@ Validation and delivery: - Create exactly one PR with `gh pr create` using the exact printed `PR title`. - After the PR is created, switch back to `main` and pull the latest remote changes again. -PR requirements: +PR requirements. The repository has a mandatory pull request template at `.github/PULL_REQUEST_TEMPLATE.md`, and `AGENTS.md` requires it to be completed with concrete evidence for the final PR HEAD. Read the template and `CONTRIBUTING.md` before writing the description. Use every template heading, in the template's order, and do not invent replacement headings. Fill each section as follows. + - Use the exact printed `PR title`. -- Include the `Run ID`, `Batch name`, and `Branch name`. -- Include selected files. -- Include diagnostics fixed according to `check-batch`. -- Include remaining diagnostics in selected files. -- Include validation results for every package-scoped type-check, lint, and test command you ran, naming the packages. -- Include a `Manual testing recommendations` section with focused checks for the changed behavior. Base it on the selected files and actual edits, for example checking affected dropdowns, keyboard navigation, model/agent selection, settings controls, or mobile/desktop variants. -- Include any skipped diagnostics and why. +- `## Intent`: state that this is an unattended maintenance batch, name the `Run ID`, `Batch name`, and `Branch name`, and say what behavior changes. When nothing observable changes, say so explicitly rather than leaving it implied. +- `## Non-goals`: the diagnostics left unfixed in the selected files, diagnostics elsewhere in the repository, and any refactor you deliberately did not start. Give the reason for each, not just the count. +- `## Affected surfaces`: the packages, runtimes, user-visible states, and persisted or external contracts the diff reaches. Name every runtime the changed code runs in, and explain why an apparently applicable runtime is unaffected. +- `## Repository guidance`: fill the table. List the `AGENTS.md` rules you followed, every project skill that matched the change, required skill references you read, and the nearest `README.md` or `DOCUMENTATION.md` for the touched modules. For each row explain why it applies and how the change complies. Do not list filenames without explanation. +- `## Validation`: fill the table with the exact commands you ran and their results, including `check-batch` and every package-scoped type-check, lint, and test command, naming the packages. Record failures honestly, including pre-existing failures unrelated to this PR, and say which checks you did not run. Do not claim runtime behavior from type-check or lint alone. +- `## Visual evidence`: these PRs usually have no visible change, so explain concretely why the diff cannot affect rendered behavior. If anything user-visible did change, attach before/after evidence for the affected states. +- `## Risks and failure behavior`: cover what breaks if a change is wrong, how to roll it back, and any compatibility, data, performance, or cross-runtime concern. State "None identified" only with a concrete reason. + +Add a `## Manual testing recommendations` section after the template sections, with focused checks for the changed behavior. Base it on the selected files and actual edits, for example checking affected dropdowns, keyboard navigation, model or agent selection, settings controls, and mobile or desktop variants. + +Also state, inside `## Intent`, the selected files and how many diagnostics `check-batch` reports as fixed and remaining. Constraints: - Keep the PR small and reviewable. diff --git a/.opencode/commands/rd-follow-up.md b/.opencode/commands/rd-follow-up.md index 6bd75325..478a03a7 100644 --- a/.opencode/commands/rd-follow-up.md +++ b/.opencode/commands/rd-follow-up.md @@ -51,6 +51,7 @@ Delivery: - Reply to addressed review comments using `gh`. - For each specific review comment you addressed, reply with what was changed and the follow-up commit hash. - If the feedback was a general PR comment, add one general PR comment summarizing what was addressed, commit hashes, and validation results. +- Update the PR description so it stays true for the final HEAD: refresh `## Validation` with the checks you re-ran, and move any new behavior change into `## Risks and failure behavior`. Keep every heading of `.github/PULL_REQUEST_TEMPLATE.md` intact, and preserve content the repository owner added by hand, including screenshots. Read the live description before editing and merge into it rather than overwriting. - If a comment is intentionally not addressed, reply with a concise reason. - Do not release the batch while its PR is still open and awaiting review. The claim is what keeps parallel batches off these files. - Release the batch only once its PR has been merged or closed: `bun run doctor -- release --run `. diff --git a/scripts/anti-slop.mjs b/scripts/anti-slop.mjs index ff7041f4..465686c1 100644 --- a/scripts/anti-slop.mjs +++ b/scripts/anti-slop.mjs @@ -46,7 +46,7 @@ const NOISY_RULES = new Set(["no-runtime-typeof", "require-safety-comment-for-ty function usage(exitCode = 0) { const out = exitCode === 0 ? console.log : console.error; out(`Usage: - bun run deslop -- next-batch [--min-issues 25] [--max-issues 60] [--max-files 4] + bun run deslop -- next-batch [--min-issues 60] [--max-issues 120] [--max-files 4] [--max-active ${DEFAULT_MAX_ACTIVE}] [--claim-ttl ${DEFAULT_CLAIM_TTL_DAYS}] [--include-noisy] bun run deslop -- check-batch --run bun run deslop -- active [--claim-ttl ${DEFAULT_CLAIM_TTL_DAYS}] @@ -62,7 +62,7 @@ pipeline. Claims are shared across clones by default. A batch stays active until it is released. Examples: - bun run deslop -- next-batch --min-issues 25 --max-issues 60 + bun run deslop -- next-batch --min-issues 60 --max-issues 120 bun run deslop -- file packages/ui/src/lib/settings/metadata.ts bun run deslop -- check-batch --run 2026-08-16T10-12-44Z bun run deslop -- release --run 2026-08-16T10-12-44Z`); @@ -311,8 +311,8 @@ function printReportHeader(report) { } function commandNextBatch(args) { - const minIssues = asPositiveInt(args["min-issues"], 25, "min-issues"); - const maxIssues = asPositiveInt(args["max-issues"], 60, "max-issues"); + const minIssues = asPositiveInt(args["min-issues"], 60, "min-issues"); + const maxIssues = asPositiveInt(args["max-issues"], 120, "max-issues"); const maxFiles = asPositiveInt(args["max-files"], 4, "max-files"); if (minIssues > maxIssues) throw new Error("--min-issues cannot be greater than --max-issues."); const maxActive = asPositiveInt(args["max-active"], DEFAULT_MAX_ACTIVE, "max-active"); From 7ef6441bf389e34bd968cd3b3bd6c5c4af5293d7 Mon Sep 17 00:00:00 2001 From: Bohdan Triapitsyn Date: Sun, 16 Aug 2026 19:21:35 +0300 Subject: [PATCH 16/17] Reduce anti-slop findings in Persistence (#2953) * chore(ui): reduce persistence anti-slop findings * test(ui): cover fallback settings response * fix(ui): preserve usage model group contract --- packages/ui/src/lib/persistence.test.ts | 41 +++++++++++++++++++++++++ packages/ui/src/lib/persistence.ts | 31 +++++++++---------- 2 files changed, 55 insertions(+), 17 deletions(-) diff --git a/packages/ui/src/lib/persistence.test.ts b/packages/ui/src/lib/persistence.test.ts index 38057e9c..bd59bcfe 100644 --- a/packages/ui/src/lib/persistence.test.ts +++ b/packages/ui/src/lib/persistence.test.ts @@ -241,6 +241,47 @@ describe('updateDesktopSettings', () => { } }); + test('sanitizes a successful fallback settings response before applying it', async () => { + const previousFetch = globalThis.fetch; + const fallbackFetch: typeof fetch = async () => new Response(JSON.stringify({ terminalShell: 'zsh' }), { + headers: { 'Content-Type': 'application/json' }, + }); + try { + globalThis.fetch = fallbackFetch; + useUIStore.getState().setTerminalShell('fish'); + + await updateDesktopSettings({ terminalShell: 'zsh' }); + + expect(useUIStore.getState().terminalShell).toBe('zsh'); + expect(getSettingsSaveState()).toBe('idle'); + } finally { + globalThis.fetch = previousFetch; + } + }); + + test('reports an error without applying a malformed fallback settings response', async () => { + const previousFetch = globalThis.fetch; + const fallbackFetch: typeof fetch = async () => new Response(JSON.stringify('ok'), { + headers: { 'Content-Type': 'application/json' }, + }); + const states: string[] = []; + const unsubscribe = subscribeToSettingsSaveState(() => { + states.push(getSettingsSaveState()); + }); + try { + globalThis.fetch = fallbackFetch; + useUIStore.getState().setTerminalShell('fish'); + + await updateDesktopSettings({ terminalShell: 'zsh' }); + + expect(useUIStore.getState().terminalShell).toBe('fish'); + expect(states).toEqual(['saving', 'error']); + } finally { + unsubscribe(); + globalThis.fetch = previousFetch; + } + }); + test('drains a pending save to the previous runtime and ignores its stale response', async () => { switchRuntimeEndpoint({ apiBaseUrl: 'https://settings-a.example', runtimeKey: 'settings-a' }); const saveResult = deferred(); diff --git a/packages/ui/src/lib/persistence.ts b/packages/ui/src/lib/persistence.ts index c02fc88d..c53c4c4c 100644 --- a/packages/ui/src/lib/persistence.ts +++ b/packages/ui/src/lib/persistence.ts @@ -130,7 +130,7 @@ const persistToLocalStorage = (settings: DesktopSettings) => { if (Array.isArray(settings.projects) && settings.projects.length > 0) { const collapsed = settings.projects - .filter((project) => (project as unknown as { sidebarCollapsed?: boolean }).sidebarCollapsed === true) + .filter((project) => project.sidebarCollapsed === true) .map((project) => project.id) .filter((id): id is string => typeof id === 'string' && id.length > 0); if (collapsed.length > 0) { @@ -273,13 +273,14 @@ const sanitizeSkillCatalogs = (value: unknown): DesktopSettings['skillCatalogs'] if (seen.has(id)) continue; seen.add(id); - result.push({ + const catalog: NonNullable[number] = { id, label, source, - ...(subpath ? { subpath } : {}), - ...(gitIdentityId ? { gitIdentityId } : {}), - }); + }; + if (subpath) catalog.subpath = subpath; + if (gitIdentityId) catalog.gitIdentityId = gitIdentityId; + result.push(catalog); } return result; @@ -393,7 +394,7 @@ const sanitizeProjects = (value: unknown): DesktopSettings['projects'] | undefin project.icon = candidate.icon.trim(); } if (candidate.iconImage === null) { - (project as unknown as Record).iconImage = null; + project.iconImage = null; } else if (candidate.iconImage && typeof candidate.iconImage === 'object') { const iconImage = candidate.iconImage as Record; const mime = typeof iconImage.mime === 'string' ? iconImage.mime.trim() : ''; @@ -404,18 +405,18 @@ const sanitizeProjects = (value: unknown): DesktopSettings['projects'] | undefin ? iconImage.source : null; if (mime && updatedAt > 0 && source) { - (project as unknown as Record).iconImage = { mime, updatedAt, source }; + project.iconImage = { mime, updatedAt, source }; } } if (typeof candidate.color === 'string' && candidate.color.trim().length > 0) { project.color = candidate.color.trim(); } if (candidate.iconBackground === null) { - (project as unknown as Record).iconBackground = null; + project.iconBackground = null; } else { const iconBackground = normalizeIconBackground(candidate.iconBackground); if (iconBackground) { - (project as unknown as Record).iconBackground = iconBackground; + project.iconBackground = iconBackground; } } if (typeof candidate.addedAt === 'number' && Number.isFinite(candidate.addedAt) && candidate.addedAt >= 0) { @@ -429,7 +430,7 @@ const sanitizeProjects = (value: unknown): DesktopSettings['projects'] | undefin project.lastOpenedAt = candidate.lastOpenedAt; } if (typeof candidate.sidebarCollapsed === 'boolean') { - (project as unknown as Record).sidebarCollapsed = candidate.sidebarCollapsed; + project.sidebarCollapsed = candidate.sidebarCollapsed; } result.push(project); } @@ -507,7 +508,7 @@ const sanitizeModelRefs = (value: unknown, limit: number): Array<{ providerID: s }; const getPersistApi = (): PersistApi | undefined => { - const candidate = (useUIStore as unknown as { persist?: PersistApi }).persist; + const candidate = useUIStore.persist; if (candidate && typeof candidate === 'object') { return candidate; } @@ -1325,11 +1326,7 @@ const sanitizeWebSettings = (payload: unknown): DesktopSettings | null => { for (const [providerId, config] of Object.entries(candidate.usageModelGroups)) { if (config && typeof config === 'object') { const typedConfig = config as Record; - const providerConfig: { - customGroups?: Array<{id: string; label: string; models: string[]; order: number}>; - modelAssignments?: Record; - renamedGroups?: Record; - } = {}; + const providerConfig: NonNullable[string] = {}; // Parse customGroups if (Array.isArray(typedConfig.customGroups)) { @@ -1875,7 +1872,7 @@ async function _flushSettingsUpdate(): Promise { return; } - const updated = (await response.json().catch(() => null)) as DesktopSettings | null; + const updated = sanitizeWebSettings(await response.json().catch(() => null)); if (!isSettingsRuntimeContextCurrent(context)) return; if (updated) { applyDesktopUiPreferences(updated); From 1c76dbefe40f320e9bb318be4b592711b5e2c161 Mon Sep 17 00:00:00 2001 From: Serhii Dziupin Date: Mon, 17 Aug 2026 14:24:39 +0300 Subject: [PATCH 17/17] fix(chat): defer composer value writeback during IME composition (Fixes #2527) (#2691) * fix(chat): defer composer value writeback during IME composition The controlled-writeback effect compared the value prop against the CodeMirror document and, on mismatch, dispatched a wholesale replacement with the caret forced to the end. While the browser composes (pinyin, kana, hangul) the uncommitted text lives in the DOM, not in the document, so the mismatch is expected and the dispatch interrupted the IME session and jumped the cursor. Skip the writeback while the view is composing, using CodeMirror's public compositionStarted getter; the composition commits through its own pipeline and reports via onChange. Fixes #2527 * fix(chat): preserve external composer writes during IME * fix(chat): restore composition-wide writeback guard --------- Co-authored-by: Bohdan Triapitsyn --- .../chat/composer/editor/ComposerEditor.tsx | 4 +++ .../writebackCompositionGuard.test.ts | 28 +++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 packages/ui/src/components/chat/composer/editor/__tests__/writebackCompositionGuard.test.ts diff --git a/packages/ui/src/components/chat/composer/editor/ComposerEditor.tsx b/packages/ui/src/components/chat/composer/editor/ComposerEditor.tsx index 41a2825f..c248a53f 100644 --- a/packages/ui/src/components/chat/composer/editor/ComposerEditor.tsx +++ b/packages/ui/src/components/chat/composer/editor/ComposerEditor.tsx @@ -344,6 +344,10 @@ export const ComposerEditor = React.forwardRef { + const start = composerEditorSource.indexOf('// Controlled value:'); + expect(start).toBeGreaterThan(-1); + const end = composerEditorSource.indexOf('}, [value]);', start); + expect(end).toBeGreaterThan(start); + return composerEditorSource.slice(start, end); +}; + +describe('composer value writeback composition guard (issue #2527)', () => { + test('checks equality, then composition, before dispatching', () => { + const effect = writebackEffect(); + const equalityCheck = effect.indexOf('if (current === value) return;'); + const compositionGuard = effect.indexOf('if (view.compositionStarted) return;'); + const dispatch = effect.indexOf('view.dispatch({'); + + expect(equalityCheck).toBeGreaterThan(-1); + expect(compositionGuard).toBeGreaterThan(equalityCheck); + expect(dispatch).toBeGreaterThan(compositionGuard); + }); +});