feat(mutils): Add session-id persistence across Auto Compact#19
feat(mutils): Add session-id persistence across Auto Compact#19
Conversation
- Add session-id-persist.ts hook to save/restore session-id - Add shared patterns.ts for regex pattern reuse - Update generate.ts to use shared patterns - Update ccs-handoff.ts to show session-id for handoff - Update SKILL.md with persistence documentation The hook captures session-id generation via PostToolUse[Bash] and restores it on SessionStart when source is "compact" or "resume". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR introduces workspace-id management capabilities to replace session-id terminology, adds a TanStack Query plugin with comprehensive documentation, enhances the CCS handoff skill with user session ID extraction, and implements workspace-id persistence via SQLite. Version bumped to 0.13.0 for mutils plugin. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CLI as generate.ts
participant Validation as patterns.ts
participant FileSystem as File System
participant DB as MutilsDB
User->>CLI: provide feature-name
CLI->>Validation: isValidFeatureName()
alt Invalid feature-name
Validation-->>CLI: false
CLI-->>User: error & usage
else Valid feature-name
Validation-->>CLI: true
CLI->>CLI: compute workspaceId<br/>(yyyymmdd-HHmm-featureName)
CLI-->>User: output workspaceId to stdout
CLI->>FileSystem: locate git root
FileSystem-->>CLI: git root path
CLI->>FileSystem: check .agents directory
alt .agents exists
CLI->>DB: open MutilsDB
DB-->>CLI: connection
CLI->>DB: ensure workspace_mappings table
DB-->>CLI: table ready
CLI->>DB: upsert mapping<br/>(workspace_id, feature_name, created_at)
DB-->>CLI: record persisted
else .agents not found
CLI->>CLI: silently skip DB write
end
end
sequenceDiagram
participant Session as Claude Session
participant Hook as workspace-id-persist.ts
participant FileSystem as File System
participant DB as MutilsDB
participant Context as Additional Context
Session->>Hook: SessionStart event
Hook->>FileSystem: verify git root
alt Git root not found
FileSystem-->>Hook: error
Hook-->>Session: exit gracefully
else Git root found
Hook->>FileSystem: check .agents directory
alt .agents not found
FileSystem-->>Hook: not found
Hook-->>Session: exit gracefully
else .agents exists
Hook->>Hook: determine session source<br/>(default: startup)
alt Not compact/resume session
Hook-->>Session: exit gracefully
else Compact or resume
Hook->>DB: getLatestWorkspaceMapping()
alt Mapping found
DB-->>Hook: workspace_id, feature_name
Hook->>Context: format restoration context
Context-->>Hook: detailed message with path
Hook-->>Session: return JSON with<br/>additionalContext
Session->>Session: restore workspace
else No mapping found
DB-->>Hook: null
Hook-->>Session: exit gracefully
end
end
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugins/mutils/hooks/session-id-persist.ts (1)
140-185: Consider adding type safety fortool_response.The
tool_responseis cast tostring(line 144), but the cc-hooks-ts type is generic. Iftool_responseis unexpectedly not a string (e.g., structured output), this could cause runtime issues.The current code is safe in practice since
parseSessionIdhandles non-matching strings gracefully by returningnull, but adding a type check would be more defensive.♻️ Optional: Add explicit type check
if (eventType === "PostToolUse" && context.input.tool_name === "Bash") { // Type assertion for Bash tool input/output const toolInput = context.input.tool_input as { command?: string }; - const toolResponse = context.input.tool_response as string; + const toolResponse = context.input.tool_response; + if (typeof toolResponse !== "string") { + logger.debug("tool_response is not a string, skipping"); + return context.success({}); + } const command = toolInput.command ?? "";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/hooks/session-id-persist.ts` around lines 140 - 185, The code currently casts context.input.tool_response to string (toolResponse) before parsing; add a runtime type check to avoid runtime errors if tool_response is non-string: check typeof context.input.tool_response === "string" (or if it's an object, serialize with JSON.stringify) before assigning toolResponse, log a warning via logger.warn and return context.success({}) if it's neither string nor serializable, then use the validated/serialized toolResponse with parseSessionId, saveSessionMapping, and the existing return payload; reference the symbols context.input.tool_response, toolResponse, parseSessionId, isGenerateTsCommand, and saveSessionMapping to locate and update the logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plugins/mutils/hooks/session-id-persist.ts`:
- Around line 140-185: The code currently casts context.input.tool_response to
string (toolResponse) before parsing; add a runtime type check to avoid runtime
errors if tool_response is non-string: check typeof context.input.tool_response
=== "string" (or if it's an object, serialize with JSON.stringify) before
assigning toolResponse, log a warning via logger.warn and return
context.success({}) if it's neither string nor serializable, then use the
validated/serialized toolResponse with parseSessionId, saveSessionMapping, and
the existing return payload; reference the symbols context.input.tool_response,
toolResponse, parseSessionId, isGenerateTsCommand, and saveSessionMapping to
locate and update the logic.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.claude-plugin/marketplace.jsonplugins/mutils/hooks/hooks.jsonplugins/mutils/hooks/session-id-persist.tsplugins/mutils/plugin.jsonplugins/mutils/skills/ccs-handoff/ccs-handoff.tsplugins/mutils/skills/session-id/SKILL.mdplugins/mutils/skills/session-id/generate.tsplugins/mutils/skills/session-id/patterns.ts
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. The implementation follows the project's conventions:
CI checks passed:
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Move command pattern check before git/.agents checks in PostToolUse to avoid unnecessary filesystem operations for non-matching commands - Remove technical implementation details from SKILL.md Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
plugins/mutils/hooks/session-id-persist.ts (1)
139-150: Extract duplicated git/.agents guard into a shared helper.The repository eligibility check is duplicated in two branches. A small helper (e.g.,
resolveEligibleGitRoot) would reduce divergence risk and make future condition changes safer.Also applies to: 188-199
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/hooks/session-id-persist.ts` around lines 139 - 150, Extract the duplicated git/.agents guard into a shared helper (e.g., resolveEligibleGitRoot) that encapsulates the logic using findGitRoot(cwd) and existsSync(path.join(gitRoot, ".agents")); replace both occurrences (the block around findGitRoot/agentsDir and the other block at lines ~188-199) by calling resolveEligibleGitRoot(cwd) and returning context.success({}) when it returns null or throwing/handling as the original blocks did; ensure the helper returns the gitRoot string when eligible and null otherwise and update any local variables (agentsDir, gitRoot) to use the helper's result so behavior remains identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/mutils/hooks/session-id-persist.ts`:
- Around line 130-133: Normalize and type-check tool_response before
parsing/storing session-id: ensure the variable context.input.tool_response is
checked for null/undefined and is coerced to a string (e.g., String(...) or via
typeof check), then trimmed of whitespace/newlines before any parsing or
assignment; update usages around the variables tool_input, tool_response and
command (the code that reads tool_input.command and later reads tool_response at
the session-id persistence/restoration code paths) to use the normalized/trimmed
value and guard against non-string payloads (skip persistence or log/handle
invalid types) so trailing newlines or non-string outputs cannot produce or
store an invalid session-id.
---
Nitpick comments:
In `@plugins/mutils/hooks/session-id-persist.ts`:
- Around line 139-150: Extract the duplicated git/.agents guard into a shared
helper (e.g., resolveEligibleGitRoot) that encapsulates the logic using
findGitRoot(cwd) and existsSync(path.join(gitRoot, ".agents")); replace both
occurrences (the block around findGitRoot/agentsDir and the other block at lines
~188-199) by calling resolveEligibleGitRoot(cwd) and returning
context.success({}) when it returns null or throwing/handling as the original
blocks did; ensure the helper returns the gitRoot string when eligible and null
otherwise and update any local variables (agentsDir, gitRoot) to use the
helper's result so behavior remains identical.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
plugins/mutils/hooks/session-id-persist.tsplugins/mutils/skills/session-id/SKILL.md
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/mutils/skills/session-id/SKILL.md
| const toolInput = context.input.tool_input as { command?: string }; | ||
| const toolResponse = context.input.tool_response as string; | ||
| const command = toolInput.command ?? ""; | ||
|
|
There was a problem hiding this comment.
Normalize and type-check tool_response before parsing/storing session-id.
Line 131 force-casts tool_response and Lines 156/169 use it raw. If Bash output includes trailing newlines or non-string payloads, parsing/persistence can fail or store an invalid session-id value for restoration.
🔧 Proposed fix
- const toolResponse = context.input.tool_response as string;
+ const toolResponseRaw = context.input.tool_response;
+ if (typeof toolResponseRaw !== "string") {
+ logger.warn("Skipping: Bash tool_response is not a string");
+ return context.success({});
+ }
+ const toolResponse = toolResponseRaw.trim();
+ if (toolResponse.length === 0) {
+ logger.warn("Skipping: Bash tool_response is empty");
+ return context.success({});
+ }Also applies to: 155-171, 179-180
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/mutils/hooks/session-id-persist.ts` around lines 130 - 133, Normalize
and type-check tool_response before parsing/storing session-id: ensure the
variable context.input.tool_response is checked for null/undefined and is
coerced to a string (e.g., String(...) or via typeof check), then trimmed of
whitespace/newlines before any parsing or assignment; update usages around the
variables tool_input, tool_response and command (the code that reads
tool_input.command and later reads tool_response at the session-id
persistence/restoration code paths) to use the normalized/trimmed value and
guard against non-string payloads (skip persistence or log/handle invalid types)
so trailing newlines or non-string outputs cannot produce or store an invalid
session-id.
Code reviewFound 2 issues:
claude-code-plugin/plugins/mutils/hooks/session-id-persist.ts Lines 66 to 77 in f5f474a
🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
- Preserve id in upsert to prevent auto-increment on update - Add PostToolUse to AGENTS.md hooks list Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/tanstack-query/skills/tanstack-query/references/error-handling.md`:
- Around line 94-95: Update the documentation examples to reflect v5 defaults
and make error access type-safe: change any statement that claims the default
error type is unknown to state it is Error (unless overridden), correct the
description of retry behavior to note v5 will retry 4xx if the queryFn throws
and that status-based skipping requires a custom retry function (refer to the
retry option), and modify the example using throwOnError / error.status so it
either demonstrates a custom error class (e.g., ApiError with a status property)
or shows type narrowing (e.g., axios.isAxiosError or instanceof checks) before
accessing error.status to avoid unsafe property access.
In `@plugins/tanstack-query/skills/tanstack-query/references/testing.md`:
- Around line 373-377: The import statement incorrectly includes a non-existent
named export "debug" from '@testing-library/react'; remove "debug" from the
import (leave "screen") in the import line that currently reads "import {
screen, debug } ..." and ensure any calls use screen.debug() (as already
present) and no other references to the removed symbol remain in the file.
In `@plugins/tanstack-query/skills/tanstack-query/references/transformations.md`:
- Around line 123-124: Update the sentence about select to clarify its actual
re-run semantics: explain that TanStack Query v5's select callback only
re-executes when the query's data changes or when the selector function
reference changes, and note that inline selector functions appear to run every
render only because their reference changes each render; recommend using stable
selectors (extracting the selector function or wrapping it in useCallback) to
avoid unnecessary re-runs. Reference the select callback and
useCallback/selector extraction as the recommended fixes.
In `@plugins/tanstack-query/skills/tanstack-query/SKILL.md`:
- Around line 185-189: The optimistic-update onMutate implementation can crash
when the ['todos'] cache is empty because setQueryData's updater receives
undefined; update the updater passed to queryClient.setQueryData in onMutate
(the function currently using old) to handle undefined safely (e.g., treat old
as an empty array) and preserve the previous value returned from
queryClient.getQueryData(['todos']) into previousTodos so rollback still works;
modify the updater used in queryClient.setQueryData to default old to an empty
array before spreading and adding newTodo.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
plugins/mutils/AGENTS.mdplugins/mutils/hooks/session-id-persist.tsplugins/tanstack-query/plugin.jsonplugins/tanstack-query/skills/tanstack-query/SKILL.mdplugins/tanstack-query/skills/tanstack-query/references/error-handling.mdplugins/tanstack-query/skills/tanstack-query/references/mutations.mdplugins/tanstack-query/skills/tanstack-query/references/query-keys.mdplugins/tanstack-query/skills/tanstack-query/references/render-optimization.mdplugins/tanstack-query/skills/tanstack-query/references/testing.mdplugins/tanstack-query/skills/tanstack-query/references/transformations.md
✅ Files skipped from review due to trivial changes (2)
- plugins/tanstack-query/skills/tanstack-query/references/query-keys.md
- plugins/tanstack-query/skills/tanstack-query/references/mutations.md
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/mutils/hooks/session-id-persist.ts
| throwOnError: (error) => error.status >= 500, // Only throw server errors | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack Query v5 useQuery default error type TypeScript unknown
💡 Result:
In TanStack Query v5, useQuery’s default error type is Error (not unknown). If you’re seeing unknown, it’s usually because you (or a dependency) globally set the default error type to unknown via module augmentation. [1][2]
1) If you want the default error type to be Error again
Remove/adjust any augmentation like this:
import '@tanstack/react-query'
declare module '@tanstack/react-query' {
interface Register {
defaultError: unknown
}
}That exact pattern is documented as the way to force unknown (so call sites must narrow). [1]
2) If you do want unknown, narrow at the call site
const { error, isError } = useQuery({ queryKey: ['x'], queryFn })
if (isError && error instanceof Error) {
console.log(error.message)
}3) If you want a custom global error type (e.g. AxiosError)
import '@tanstack/react-query'
import type { AxiosError } from 'axios'
declare module '@tanstack/react-query' {
interface Register {
defaultError: AxiosError
}
}This is the recommended v5 approach to avoid having to pass TError generics everywhere. [1]
Sources: TanStack Query v5 TypeScript docs (default error type + global Register.defaultError) [1], v5 migration guide (error default changed to Error) [2].
🌐 Web query:
TanStack Query v5 default retry logic 4xx HTTP status code behavior
💡 Result:
In TanStack Query v5, the default retry behavior does not special‑case HTTP status codes (including 4xx). A query is retried purely based on whether your queryFn throws and whether the retry policy allows another attempt. By default, it retries 3 times (client-side; server-side rendering defaults to 0). [1], [2]
So for 4xx responses:
- If your
queryFn(or HTTP client like axios) throws for a 4xx, TanStack Query will retry it up to 3 times by default. [1] - TanStack Query intentionally doesn’t assume 4xx should never be retried, because some APIs/systems (e.g., eventual consistency) can legitimately return a transient 404. You’re expected to encode your own “don’t retry 4xx” rule if you want it. [3]
Typical “don’t retry 4xx” setup:
const queryClient = new QueryClient({
defaultOptions: {
queries: {
retry: (failureCount, error: any) => {
const status = error?.response?.status // axios-style
if (status >= 400 && status < 500) return false
return failureCount < 3
},
},
},
})🌐 Web query:
TanStack Query v5 throwOnError error.status TypeScript safe handling
💡 Result:
In TanStack Query v5, error is typed as Error by default (not unknown), so error.status is not type-safe unless your queryFn throws an error type that actually has status (or you narrow it first). [1][2]
1) Throw a typed error that includes status (recommended)
export class ApiError extends Error {
constructor(
public status: number,
message: string,
public body?: unknown,
) {
super(message)
this.name = 'ApiError'
}
}
async function fetchJson<T>(url: string): Promise<T> {
const res = await fetch(url)
const body = await res.json().catch(() => undefined)
if (!res.ok) throw new ApiError(res.status, `HTTP ${res.status}`, body)
return body as T
}
// Usage
useQuery({
queryKey: ['user'],
queryFn: () => fetchJson<User>('/api/user'),
throwOnError: (err) => err instanceof ApiError && err.status >= 500,
})throwOnError can be boolean or a function (error, query) => boolean, so you can decide “throw to the nearest ErrorBoundary” only for certain statuses. [3]
2) If you use Axios: narrow before reading status
TanStack recommends type narrowing for specific error subclasses (like AxiosError). [2]
import axios from 'axios'
throwOnError: (err) =>
axios.isAxiosError(err) && (err.response?.status ?? 0) === 4013) (Optional) Make this globally type-safe
If you want compile-time forcing of narrowing everywhere, set defaultError: unknown via module augmentation; or set it to your ApiError if all your queryFns throw that type. [2]
import '@tanstack/react-query'
declare module '@tanstack/react-query' {
interface Register {
defaultError: unknown
}
}Key point: throwOnError doesn’t “add” a status; it only controls whether the caught error is thrown to an Error Boundary vs returned in query state—so you must model/narrow the error type yourself to safely access status. [3][1][2]
Sources: [1] Migrating to v5 (throwOnError rename + default Error) (tanstack.com), [2] TypeScript docs (error typing + narrowing + Register) (tanstack.com), [3] useQuery option type for throwOnError (tanstack.com)
Citations:
- 1: https://tanstack.com/query/v5/docs/react/guides/migrating-to-v5?utm_source=openai
- 2: https://tanstack.com/query/v5/docs/framework/react/typescript?utm_source=openai
- 3: https://tanstack.com/query/v2/docs/react/reference/useQuery?utm_source=openai
🏁 Script executed:
cat -n plugins/tanstack-query/skills/tanstack-query/references/error-handling.md | sed -n '90,100p;125,135p;175,190p'Repository: masseater/claude-code-plugin
Length of output: 975
Fix inaccurate v5 behavior defaults in error handling documentation.
Line 128 incorrectly states errors are unknown by default; in v5, the default error type is Error (unless globally overridden via module augmentation).
Lines 180–183 misrepresent the default retry behavior. v5 does not skip 4xx errors by default—it retries them if your queryFn throws. Status-based retry logic requires an explicit custom retry function.
Line 94's error.status access is not type-safe with the default Error type. The example needs either a custom error class with a status property or explicit type narrowing to safely access status codes.
Suggested corrections
At line 128:
-By default, errors are `unknown`:
+By default in v5, `error` is typed as `Error`:At lines 180–183:
- Retries 3 times by default
- Exponential backoff
-- No retry on 4xx errors (client errors)
+- Status-based retry logic requires a custom `retry` function; by default, 4xx responses are retried if thrownAt line 94, add context showing proper error handling (e.g., custom ApiError class or type narrowing like axios.isAxiosError()).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/tanstack-query/skills/tanstack-query/references/error-handling.md`
around lines 94 - 95, Update the documentation examples to reflect v5 defaults
and make error access type-safe: change any statement that claims the default
error type is unknown to state it is Error (unless overridden), correct the
description of retry behavior to note v5 will retry 4xx if the queryFn throws
and that status-based skipping requires a custom retry function (refer to the
retry option), and modify the example using throwOnError / error.status so it
either demonstrates a custom error class (e.g., ApiError with a status property)
or shows type narrowing (e.g., axios.isAxiosError or instanceof checks) before
accessing error.status to avoid unsafe property access.
| import { screen, debug } from '@testing-library/react' | ||
|
|
||
| renderWithQueryClient(<TodoList />) | ||
| screen.debug() // Print DOM | ||
| ``` |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@testing-library/react debug function named export vs screen.debug
💡 Result:
- There is no
debugnamed export from@testing-library/react. In React Testing Library,debugis a method on the object returned byrender()(the “render result”). [2] screen.debug()comes from DOM Testing Library (re-exported by React Testing Library) and is a convenience method that logsprettyDOM()fordocument.bodyby default, or for a specific element / array of elements you pass in. [1]
Practical differences
-
render(...).debug()/{debug} = render(...)- Logs
prettyDOM(baseElement)for that render call (wherebaseElementdefaults todocument.body, but can be customized viarenderoptions). [2] - Useful if you have multiple renders/containers and want the debug output tied to a specific render result.
- Logs
-
screen.debug()- Logs what’s in
document.body(or the element(s) you pass). [1] - Recommended by the RTL docs over
render’sdebug. [2]
- Logs what’s in
Typical usage
import { render, screen } from '@testing-library/react'
render(<App />)
screen.debug() // recommendedRemove unused debug import from Line 373.
@testing-library/react does not export a named debug function. The debug method exists only on the render result object. The code correctly uses screen.debug() (the recommended approach), so the named import should be removed.
Suggested fix
-import { screen, debug } from '@testing-library/react'
+import { screen } from '@testing-library/react'🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/tanstack-query/skills/tanstack-query/references/testing.md` around
lines 373 - 377, The import statement incorrectly includes a non-existent named
export "debug" from '@testing-library/react'; remove "debug" from the import
(leave "screen") in the import line that currently reads "import { screen, debug
} ..." and ensure any calls use screen.debug() (as already present) and no other
references to the removed symbol remain in the file.
| The select function runs on every render. Memoize if expensive: | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
TanStack Query v5 useQuery select function execution behavior memoization
💡 Result:
In TanStack Query v5, useQuery({ select }) behaves like a memoized per-observer selector:
- When
selectexecutes: it re-runs only if (a) the query’s cacheddatachanged or (b) theselectfunction reference changed (e.g., you pass an inline arrow each render). To avoid re-running due to (b), wrap it inuseCallbackor use a stable function reference. [1] - It is observer-specific:
selectis intentionally applied peruseQueryobserver so different components can derive different shapes from the same cached query data; therefore it must be pure / side-effect free (don’t putsetState, toasts, etc. in it). [2] - Structural sharing still helps: React Query’s structural sharing is applied to the result of
selectas well, so if the selected output is effectively unchanged, React Query can keep references stable and avoid unnecessary updates. [3] - Common pitfall: an inline
selectcan make it look like “selectruns every render”; that’s because its function identity changes every render unless memoized. [1][4]
Sources: Render Optimizations (memoization rules + examples) [1], maintainer explanation of per-observer/pure select [2], maintainer confirmation that structural sharing applies to select output [3], discussion noting inline select can run every render unless memoized [4].
References
- https://tanstack.com/query/latest/docs/framework/react/guides/render-optimizations
- v5 Roadmap 🗺 TanStack/query#4252
- select: does structural sharing apply to the result of select? (stable between renders if shape doesn't change, even if not memoized). TanStack/query#7280
- Re-rendering Issue with Select function When Using TanStack Query and Formik TanStack/query#8557
🌐 Web query:
TanStack Query select function runs on render vs data changes
💡 Result:
In TanStack Query (React Query), select is memoized per QueryObserver and will only re-run when:
- the fetched/cached
datachanges, or - the
selectfunction reference changes (referential identity). [1][3]
So:
- If you write
select: (data) => ...inline, it’s a new function every render, soselectwill run on every render even if the underlyingdatadidn’t change. [1][3] - If you pass a stable function (extract it or wrap in
useCallback),selectwon’t re-run on re-renders unlessdatachanges. [1] selectruns per component instance / peruseQuerycall (per observer). Multiple components using the same query will each run their ownselect. [3]
Fix (stable selector):
const selectTodoCount = (data: Todo[]) => data.length
useQuery({ queryKey: ['todos'], queryFn: fetchTodos, select: selectTodoCount })or:
const selectTodoCount = useCallback((data: Todo[]) => data.length, [])[1]
Sources: TanStack Query render optimizations guide (covers select + memoization rules) [1], TkDodo deep-dive on selectors (why inline selectors re-run; per-observer caching) [3], maintainer confirmation on GitHub discussions [2].
References: [1] (tanstack.com) [2] (github.com) [3] (tkdodo.eu)
Citations:
- 1: https://tanstack.com/query/latest/docs/framework/react/guides/render-optimizations?utm_source=openai
- 2: useQuerySelector instead of `select` option in useQuery TanStack/query#3353
- 3: https://tkdodo.eu/blog/react-query-selectors-supercharged?utm_source=openai
Clarify select execution semantics in Line 123.
The original statement "The select function runs on every render" is misleading. TanStack Query v5's select only re-runs when the query data changes or the selector function reference changes. Inline selectors appear to run every render only because they're a new function each render; stable selectors (extracted functions or wrapped in useCallback) won't re-run unless data actually changes. Update the doc to clarify this distinction:
Suggested doc tweak
-The select function runs on every render. Memoize if expensive:
+Inline select functions may run on each render due to function identity changes.
+Use a stable/memoized selector for expensive transforms:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/tanstack-query/skills/tanstack-query/references/transformations.md`
around lines 123 - 124, Update the sentence about select to clarify its actual
re-run semantics: explain that TanStack Query v5's select callback only
re-executes when the query's data changes or when the selector function
reference changes, and note that inline selector functions appear to run every
render only because their reference changes each render; recommend using stable
selectors (extracting the selector function or wrapping it in useCallback) to
avoid unnecessary re-runs. Reference the select callback and
useCallback/selector extraction as the recommended fixes.
| onMutate: async (newTodo) => { | ||
| await queryClient.cancelQueries({ queryKey: ['todos'] }) | ||
| const previousTodos = queryClient.getQueryData(['todos']) | ||
| queryClient.setQueryData(['todos'], (old) => [...old, newTodo]) | ||
| return { previousTodos } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "SKILL.md" -path "*tanstack-query*" | head -5Repository: masseater/claude-code-plugin
Length of output: 127
🌐 Web query:
TanStack Query v5 queryClient.setQueryData updater function oldData undefined when no cache entry
💡 Result:
In TanStack Query v5 this is expected:
queryClient.setQueryData(queryKey, updater)calls your updater with the current cached data, so if there’s no cache entry yet,oldDatawill beundefined. Also, if the query does not exist, it will be created. [1]- If you don’t want to create a new cache entry when none exists, bail out by returning
undefinedwhenoldDataisundefined(v5 explicitly documents this). [1]
Example:
queryClient.setQueryData(['todo', id], (old) => {
if (!old) return undefined // don't create cache entry
return { ...old, done: true }
})If you want to check first, queryClient.getQueryData(queryKey) returns undefined when the query doesn’t exist. [1]
Sources: [1] TanStack Query v5 QueryClient reference (setQueryData / getQueryData).
🏁 Script executed:
cat -n ./plugins/tanstack-query/skills/tanstack-query/SKILL.md | sed -n '180,200p'Repository: masseater/claude-code-plugin
Length of output: 820
Handle old being undefined in optimistic-update snippet.
Line 188 will crash if the ['todos'] query has no cached value yet. When setQueryData is called with no prior cache entry, the updater receives undefined, and spreading it throws a TypeError. Use a default parameter to safely handle this:
Suggested fix
- queryClient.setQueryData(['todos'], (old) => [...old, newTodo])
+ queryClient.setQueryData(['todos'], (old = []) => [...old, newTodo])🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/tanstack-query/skills/tanstack-query/SKILL.md` around lines 185 -
189, The optimistic-update onMutate implementation can crash when the ['todos']
cache is empty because setQueryData's updater receives undefined; update the
updater passed to queryClient.setQueryData in onMutate (the function currently
using old) to handle undefined safely (e.g., treat old as an empty array) and
preserve the previous value returned from queryClient.getQueryData(['todos'])
into previousTodos so rollback still works; modify the updater used in
queryClient.setQueryData to default old to an empty array before spreading and
adding newTodo.
Rename session-id skill to workspace-id to avoid confusion with Claude Code's internal session_id. This clarifies the purpose: workspace-id identifies a work context directory, not a session. Changes: - Rename skills/session-id/ → skills/workspace-id/ - Rename hooks/session-id-persist.ts → hooks/workspace-id-persist.ts - Update all references: session-id → workspace-id - Update directory path: .agents/sessions/ → .agents/workspaces/ - Update documentation and rules Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
plugins/mutils/hooks/workspace-id-persist.ts (1)
131-133:⚠️ Potential issue | 🟠 MajorNormalize and type-check Bash payloads before parsing/persisting workspace-id.
Line 131-133 force-casts both
tool_inputandtool_response. A non-string or newline-suffixedtool_responsecan fail parse/persist or store an invalid value.🔧 Suggested fix
- // Type assertion for Bash tool input/output - const toolInput = context.input.tool_input as { command?: string }; - const toolResponse = context.input.tool_response as string; - const command = toolInput.command ?? ""; + const toolInputRaw = context.input.tool_input; + const command = + typeof toolInputRaw === "object" && + toolInputRaw !== null && + "command" in toolInputRaw && + typeof (toolInputRaw as { command?: unknown }).command === "string" + ? ((toolInputRaw as { command: string }).command ?? "") + : ""; + + const toolResponseRaw = context.input.tool_response; + if (typeof toolResponseRaw !== "string") { + logger.warn("Skipping: Bash tool_response is not a string"); + return context.success({}); + } + const toolResponse = toolResponseRaw.trim(); + if (toolResponse.length === 0) { + logger.warn("Skipping: Bash tool_response is empty"); + return context.success({}); + }Also applies to: 157-171
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/hooks/workspace-id-persist.ts` around lines 131 - 133, The code currently force-casts context.input.tool_input and context.input.tool_response into toolInput/toolResponse and uses toolInput.command (see variables toolInput, toolResponse, command), which can crash or store invalid values; update the code to first assert types and normalize strings: check typeof context.input.tool_response === "string" then trim() to remove trailing newlines (and reject empty strings), ensure context.input.tool_input is an object and that tool_input.command is a string before assigning command (fallback to empty string only after those checks), and apply the same guards/normalization to the parsing/persist logic later (the same block referenced around the later parse/persist code) so only validated, trimmed strings are parsed and persisted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/mutils/skills/ccs-handoff/ccs-handoff.ts`:
- Around line 457-493: The extractUserSessionId function currently picks the
most-recent workspace by mtime which can be incorrect; update
extractUserSessionId to first attempt to resolve the workspace id from the
persisted Claude session mapping (the mapping saved by
plugins/mutils/hooks/workspace-id-persist.ts), and only if that lookup returns
null/undefined fall back to the existing mtime-based selection and
parseWorkspaceId validation; keep the existing parseWorkspaceId usage and return
shape (fullId, featureName) unchanged and ensure any IO/lookup errors are
handled the same way as the current existence checks.
- Around line 450-455: The handoff output and instruction text still reference
"session-id" and the .agents/sessions/... path; update all occurrences to the
workspace model (e.g., "workspace-id" and .agents/workspaces/...) and adjust
types/identifiers accordingly—rename the UserSessionId concept to
UserWorkspaceId (or equivalent) and update any usages of fullId/featureName that
construct or display the path so the instructions point to
.agents/workspaces/<workspace-id> instead of .agents/sessions/<session-id>;
ensure the handoff messages generated in ccs-handoff.ts (the functions/variables
that build the handoff output around lines ~579-596) are changed to use
workspace naming consistently.
In `@plugins/mutils/skills/workspace-id/SKILL.md`:
- Around line 40-46: Two unlabeled fenced code blocks in SKILL.md (the directory
tree block starting with ".agents/workspaces/" and the "Workspace ID:
20260302-1430-doc-engine" block) must be labeled to satisfy markdownlint MD040;
update each triple-backtick fence to include a language token such as text
(i.e., change ``` to ```text) for the blocks that contain the directory listing
and the workspace/files listing so the linter recognizes them as code fences.
---
Duplicate comments:
In `@plugins/mutils/hooks/workspace-id-persist.ts`:
- Around line 131-133: The code currently force-casts context.input.tool_input
and context.input.tool_response into toolInput/toolResponse and uses
toolInput.command (see variables toolInput, toolResponse, command), which can
crash or store invalid values; update the code to first assert types and
normalize strings: check typeof context.input.tool_response === "string" then
trim() to remove trailing newlines (and reject empty strings), ensure
context.input.tool_input is an object and that tool_input.command is a string
before assigning command (fallback to empty string only after those checks), and
apply the same guards/normalization to the parsing/persist logic later (the same
block referenced around the later parse/persist code) so only validated, trimmed
strings are parsed and persisted.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.claude/rules/ai-generated/project-context.mdplugins/mutils/hooks/hooks.jsonplugins/mutils/hooks/workspace-id-persist.tsplugins/mutils/skills/ccs-handoff/ccs-handoff.tsplugins/mutils/skills/workspace-id/SKILL.mdplugins/mutils/skills/workspace-id/generate.tsplugins/mutils/skills/workspace-id/patterns.ts
| // --- Session ID extraction from .agents/sessions/ --- | ||
|
|
||
| interface UserSessionId { | ||
| fullId: string; | ||
| featureName: string; | ||
| } |
There was a problem hiding this comment.
Complete the session→workspace rename in handoff output and instructions.
Line 580/583/588/595 and Line 592 still refer to session-id and .agents/sessions/..., which conflicts with the new workspace model and points users to the wrong directory.
🔧 Suggested fix
-// --- Session ID extraction from .agents/sessions/ ---
+// --- Workspace ID extraction from .agents/workspaces/ ---
@@
- // User Session ID (for handoff)
- lines.push("## User Session ID");
+ // Workspace ID (for handoff)
+ lines.push("## Workspace ID");
@@
- lines.push(`**Session ID**: \`${userSessionId.fullId}\``);
+ lines.push(`**Workspace ID**: \`${userSessionId.fullId}\``);
@@
- lines.push(
- "To continue using this session-id, create the session directory:",
- );
+ lines.push(
+ "To continue using this workspace-id, create the workspace directory:",
+ );
@@
- lines.push(`mkdir -p .agents/sessions/${userSessionId.fullId}`);
+ lines.push(`mkdir -p .agents/workspaces/${userSessionId.fullId}`);
@@
- lines.push("No user session-id found in .agents/sessions/");
+ lines.push("No workspace-id found in .agents/workspaces/");Also applies to: 579-596
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/mutils/skills/ccs-handoff/ccs-handoff.ts` around lines 450 - 455, The
handoff output and instruction text still reference "session-id" and the
.agents/sessions/... path; update all occurrences to the workspace model (e.g.,
"workspace-id" and .agents/workspaces/...) and adjust types/identifiers
accordingly—rename the UserSessionId concept to UserWorkspaceId (or equivalent)
and update any usages of fullId/featureName that construct or display the path
so the instructions point to .agents/workspaces/<workspace-id> instead of
.agents/sessions/<session-id>; ensure the handoff messages generated in
ccs-handoff.ts (the functions/variables that build the handoff output around
lines ~579-596) are changed to use workspace naming consistently.
| function extractUserSessionId(projectPath: string): UserSessionId | null { | ||
| const workspacesDir = path.join(projectPath, ".agents", "workspaces"); | ||
| if (!fs.existsSync(workspacesDir)) { | ||
| return null; | ||
| } | ||
|
|
||
| // Get the most recently modified workspace directory | ||
| const entries = fs | ||
| .readdirSync(workspacesDir, { withFileTypes: true }) | ||
| .filter((d) => d.isDirectory()) | ||
| .map((d) => { | ||
| const fullPath = path.join(workspacesDir, d.name); | ||
| const stat = fs.statSync(fullPath); | ||
| return { name: d.name, mtime: stat.mtimeMs }; | ||
| }) | ||
| .sort((a, b) => b.mtime - a.mtime); | ||
|
|
||
| if (entries.length === 0) { | ||
| return null; | ||
| } | ||
|
|
||
| const workspaceId = entries[0]?.name; | ||
| if (!workspaceId) { | ||
| return null; | ||
| } | ||
|
|
||
| // Validate workspace-id format using shared pattern | ||
| const parsed = parseWorkspaceId(workspaceId); | ||
| if (!parsed) { | ||
| return null; | ||
| } | ||
|
|
||
| return { | ||
| fullId: workspaceId, | ||
| featureName: parsed.featureName, | ||
| }; | ||
| } |
There was a problem hiding this comment.
Avoid using “latest modified workspace directory” as the session mapping source.
Selecting entries[0] by mtime can associate the wrong workspace-id when multiple workspaces exist for the same project path. For handoff accuracy, resolve workspace-id by the current Claude session identifier first (the mapping persisted by plugins/mutils/hooks/workspace-id-persist.ts), and only use mtime as an explicit fallback.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/mutils/skills/ccs-handoff/ccs-handoff.ts` around lines 457 - 493, The
extractUserSessionId function currently picks the most-recent workspace by mtime
which can be incorrect; update extractUserSessionId to first attempt to resolve
the workspace id from the persisted Claude session mapping (the mapping saved by
plugins/mutils/hooks/workspace-id-persist.ts), and only if that lookup returns
null/undefined fall back to the existing mtime-based selection and
parseWorkspaceId validation; keep the existing parseWorkspaceId usage and return
shape (fullId, featureName) unchanged and ensure any IO/lookup errors are
handled the same way as the current existence checks.
- Remove PostToolUse hook - no longer needed - generate.ts now saves directly to DB - workspace-id-persist.ts only handles SessionStart for restore - Simpler architecture: just get latest workspace-id on restore Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
plugins/mutils/hooks/workspace-id-persist.ts (2)
69-114: Consider validating.agents/workspaces/exists before advertising the path.Right now you only check
.agentsexists. If.agents/workspacesis missing, the restored “Workspace directory: …” line may be misleading.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/hooks/workspace-id-persist.ts` around lines 69 - 114, Validate the actual workspace path before advertising it: after obtaining mapping from getLatestWorkspaceMapping and before using mapping.workspace_id in the returned context.json, build the workspaceDir path (using agentsDir or gitRoot + ".agents/workspaces/<mapping.workspace_id>"), check that it exists (existsSync or equivalent), and if it doesn't exist log/debug a warning and return context.success({}) (or adjust the message to not claim the directory exists). Update the message sent in context.json to use the confirmed workspaceDir variable (or omit the directory line when missing) so mapping.workspace_id is only advertised when the physical .agents/workspaces/<id> directory is present.
36-49: Make “latest mapping” selection independent ofidsemantics; improve error logging.Two concerns here:
ORDER BY id DESCmakes “latest” depend on howidbehaves under upserts/replacements; ordering bycreated_at DESC, id DESCis more robust.logger.error(\... ${error}`)` loses stack/context; prefer structured logging.Also,
table.findAll("1=1 ORDER BY ... LIMIT 1", [])is a bit opaque unlessfindAllexplicitly supports ORDER/LIMIT in that argument.Proposed refactor
function getLatestWorkspaceMapping(cwd: string): WorkspaceMapping | null { try { using db = MutilsDB.open(cwd); const table = db.table<WorkspaceMapping>( "workspace_mappings", WORKSPACE_MAPPINGS_SCHEMA, ); - // Get the most recent entry - const all = table.findAll("1=1 ORDER BY id DESC LIMIT 1", []); + // Get the most recent entry (prefer created_at; id as a tie-breaker) + const all = table.findAll( + "1=1 ORDER BY created_at DESC, id DESC LIMIT 1", + [], + ); return all[0] ?? null; } catch (error) { - logger.error(`Failed to get workspace mapping: ${error}`); + logger.error("Failed to get workspace mapping", { error }); return null; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/hooks/workspace-id-persist.ts` around lines 36 - 49, getLatestWorkspaceMapping currently orders only by id and logs errors as a string; change the selection to order by created_at DESC, id DESC to reliably pick the most recent mapping (ensure WORKSPACE_MAPPINGS_SCHEMA includes a created_at timestamp), and if the DB API supports explicit order/limit params use those instead of embedding SQL in findAll; also replace string interpolation in logger.error with structured logging (e.g., pass the error object/metadata to logger.error) so the stack and context are preserved when catching exceptions in getLatestWorkspaceMapping and when calling table.findAll on the "workspace_mappings" table.plugins/mutils/skills/workspace-id/generate.ts (1)
21-26: Confirm minute-level time granularity and local-time semantics are intentional.
YYYYMMDD-HHMM-...uses local time and can collide if re-run within the same minute for the same feature-name (you’ll get the sameworkspaceId). If collisions/timezones matter, consider documenting this or switching to UTC / adding seconds.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plugins/mutils/skills/workspace-id/generate.ts` around lines 21 - 26, The current workspaceId generation (variables: now, pad, yyyymmdd, HHmm, workspaceId) uses local time truncated to minute granularity which can produce identical IDs if rerun within the same minute and varies by timezone; update the generation to use a higher-resolution or UTC-based timestamp (e.g., use UTC getters like getUTCFullYear/getUTCMonth/getUTCDate/getUTCHours/getUTCMinutes and/or append seconds via pad(now.getSeconds()) or milliseconds via Date.now()) or explicitly document that local-minute granularity is intended—modify the yyyymmdd/HHmm construction or replace it with an ISO/epoch-derived component to avoid collisions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@plugins/mutils/skills/workspace-id/generate.ts`:
- Around line 31-60: The current upsert into the "workspace_mappings" table uses
table.upsert which triggers INSERT OR REPLACE and causes the row id to be
regenerated; fix by first querying the table for an existing row with the same
workspace_id (use the same table reference for "workspace_mappings"), if found
capture its id and then include that id in the object passed to table.upsert({
id, workspace_id: workspaceId, feature_name: featureName, created_at: timestamp
}) so the existing row is updated instead of replaced; also remove the explicit
UNIQUE index creation for workspace_id since the column declaration workspace_id
TEXT NOT NULL UNIQUE already creates an implicit index.
---
Nitpick comments:
In `@plugins/mutils/hooks/workspace-id-persist.ts`:
- Around line 69-114: Validate the actual workspace path before advertising it:
after obtaining mapping from getLatestWorkspaceMapping and before using
mapping.workspace_id in the returned context.json, build the workspaceDir path
(using agentsDir or gitRoot + ".agents/workspaces/<mapping.workspace_id>"),
check that it exists (existsSync or equivalent), and if it doesn't exist
log/debug a warning and return context.success({}) (or adjust the message to not
claim the directory exists). Update the message sent in context.json to use the
confirmed workspaceDir variable (or omit the directory line when missing) so
mapping.workspace_id is only advertised when the physical
.agents/workspaces/<id> directory is present.
- Around line 36-49: getLatestWorkspaceMapping currently orders only by id and
logs errors as a string; change the selection to order by created_at DESC, id
DESC to reliably pick the most recent mapping (ensure WORKSPACE_MAPPINGS_SCHEMA
includes a created_at timestamp), and if the DB API supports explicit
order/limit params use those instead of embedding SQL in findAll; also replace
string interpolation in logger.error with structured logging (e.g., pass the
error object/metadata to logger.error) so the stack and context are preserved
when catching exceptions in getLatestWorkspaceMapping and when calling
table.findAll on the "workspace_mappings" table.
In `@plugins/mutils/skills/workspace-id/generate.ts`:
- Around line 21-26: The current workspaceId generation (variables: now, pad,
yyyymmdd, HHmm, workspaceId) uses local time truncated to minute granularity
which can produce identical IDs if rerun within the same minute and varies by
timezone; update the generation to use a higher-resolution or UTC-based
timestamp (e.g., use UTC getters like
getUTCFullYear/getUTCMonth/getUTCDate/getUTCHours/getUTCMinutes and/or append
seconds via pad(now.getSeconds()) or milliseconds via Date.now()) or explicitly
document that local-minute granularity is intended—modify the yyyymmdd/HHmm
construction or replace it with an ISO/epoch-derived component to avoid
collisions.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/mutils/hooks/hooks.jsonplugins/mutils/hooks/workspace-id-persist.tsplugins/mutils/skills/workspace-id/generate.ts
| if (gitRoot) { | ||
| const agentsDir = path.join(gitRoot, ".agents"); | ||
| if (existsSync(agentsDir)) { | ||
| try { | ||
| using db = MutilsDB.open(gitRoot); | ||
| const table = db.table<{ | ||
| workspace_id: string; | ||
| feature_name: string; | ||
| created_at: string; | ||
| }>("workspace_mappings", { | ||
| columns: ` | ||
| id INTEGER PRIMARY KEY AUTOINCREMENT, | ||
| workspace_id TEXT NOT NULL UNIQUE, | ||
| feature_name TEXT NOT NULL, | ||
| created_at TEXT NOT NULL | ||
| `, | ||
| indexes: [ | ||
| "CREATE UNIQUE INDEX IF NOT EXISTS idx_workspace_mappings_workspace_id ON workspace_mappings(workspace_id)", | ||
| ], | ||
| }); | ||
|
|
||
| const timestamp = now.toISOString(); | ||
| table.upsert({ | ||
| workspace_id: workspaceId, | ||
| feature_name: featureName, | ||
| created_at: timestamp, | ||
| }); | ||
| } catch { | ||
| // Fail silently - workspace-id generation should still work | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find MutilsDB/MutilsTable implementation =="
find . -type f -name "*.ts" -o -name "*.tsx" | xargs rg -l "MutilsDB|MutilsTable" | head -20
echo
echo "== Locate the main MutilsDB file =="
fd -e ts -e tsx -e js -e jsx | xargs rg -l "class MutilsDB|export.*MutilsDB" 2>/dev/null | head -10Repository: masseater/claude-code-plugin
Length of output: 393
🏁 Script executed:
#!/bin/bash
echo "== Check the file being reviewed =="
head -70 plugins/mutils/skills/workspace-id/generate.ts | tail -50Repository: masseater/claude-code-plugin
Length of output: 1538
🏁 Script executed:
#!/bin/bash
echo "== Find the MutilsDB module/implementation =="
find . -path "*/packages/*" -name "*.ts" | xargs rg -l "MutilsDB" 2>/dev/null | head -20Repository: masseater/claude-code-plugin
Length of output: 190
🏁 Script executed:
#!/bin/bash
echo "== Read the db.ts file to understand MutilsDB and upsert implementation =="
wc -l packages/cc-plugin-lib/src/db.tsRepository: masseater/claude-code-plugin
Length of output: 183
🏁 Script executed:
#!/bin/bash
echo "== Read db.ts =="
cat -n packages/cc-plugin-lib/src/db.tsRepository: masseater/claude-code-plugin
Length of output: 3765
🏁 Script executed:
#!/bin/bash
echo "== Check if bun run check and typecheck work =="
cd . && bun run check plugins/mutils/skills/workspace-id/generate.ts 2>&1 | head -100Repository: masseater/claude-code-plugin
Length of output: 161
🏁 Script executed:
#!/bin/bash
echo "== Check bun run typecheck =="
cd . && bun run typecheck 2>&1 | grep -A 5 "workspace-id/generate.ts" || echo "No typecheck errors for this file"Repository: masseater/claude-code-plugin
Length of output: 135
🏁 Script executed:
#!/bin/bash
echo "== Search for other upsert calls in the codebase =="
rg -n "\.upsert\(" --type ts --type tsxRepository: masseater/claude-code-plugin
Length of output: 168
🏁 Script executed:
#!/bin/bash
echo "== Check if there are any tests or issues related to workspace-id churning =="
rg -i "churn|workspace.*id|id.*auto" plugins/mutils/ --type ts --type tsx | head -20Repository: masseater/claude-code-plugin
Length of output: 194
🏁 Script executed:
#!/bin/bash
echo "== Search for upsert calls with different approach =="
rg -n "\.upsert\(" plugins/ packages/Repository: masseater/claude-code-plugin
Length of output: 261
🏁 Script executed:
#!/bin/bash
echo "== Check the skill file guidelines - look for resource references =="
cat plugins/mutils/skills/workspace-id/generate.ts | grep -n "@\|reference\|resource" || echo "No resource references found"Repository: masseater/claude-code-plugin
Length of output: 210
🏁 Script executed:
#!/bin/bash
echo "== Check the entire generate.ts file for context =="
cat -n plugins/mutils/skills/workspace-id/generate.tsRepository: masseater/claude-code-plugin
Length of output: 2391
Preserve id on upsert conflict to prevent id regeneration.
The upsert() method in packages/cc-plugin-lib/src/db.ts uses INSERT OR REPLACE, which deletes and reinserts the row on conflict, generating a new id each time (line 55 of db.ts). This causes unexpected id-ordering semantics when the same workspace_id is upserted multiple times.
Additionally, the explicit UNIQUE index on workspace_id (line 48) is redundant—SQLite automatically creates an implicit index for TEXT NOT NULL UNIQUE constraints (line 43).
To fix: Query for the existing row by workspace_id, retrieve its id, and include the id in the upsert call to preserve it across updates.
Proposed fix (preserve id on conflict)
- const table = db.table<{
- workspace_id: string;
- feature_name: string;
- created_at: string;
- }>("workspace_mappings", {
+ type WorkspaceMappingRow = {
+ id?: number;
+ workspace_id: string;
+ feature_name: string;
+ created_at: string;
+ };
+
+ const table = db.table<WorkspaceMappingRow>("workspace_mappings", {
columns: `
id INTEGER PRIMARY KEY AUTOINCREMENT,
workspace_id TEXT NOT NULL UNIQUE,
feature_name TEXT NOT NULL,
created_at TEXT NOT NULL
`,
indexes: [
"CREATE UNIQUE INDEX IF NOT EXISTS idx_workspace_mappings_workspace_id ON workspace_mappings(workspace_id)",
],
});
const timestamp = now.toISOString();
- table.upsert({
- workspace_id: workspaceId,
- feature_name: featureName,
- created_at: timestamp,
- });
+ const existing = table.findAll("workspace_id = ?", [workspaceId])[0];
+ if (existing?.id) {
+ table.upsert({
+ id: existing.id,
+ workspace_id: workspaceId,
+ feature_name: featureName,
+ created_at: timestamp,
+ });
+ } else {
+ table.upsert({
+ workspace_id: workspaceId,
+ feature_name: featureName,
+ created_at: timestamp,
+ });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@plugins/mutils/skills/workspace-id/generate.ts` around lines 31 - 60, The
current upsert into the "workspace_mappings" table uses table.upsert which
triggers INSERT OR REPLACE and causes the row id to be regenerated; fix by first
querying the table for an existing row with the same workspace_id (use the same
table reference for "workspace_mappings"), if found capture its id and then
include that id in the object passed to table.upsert({ id, workspace_id:
workspaceId, feature_name: featureName, created_at: timestamp }) so the existing
row is updated instead of replaced; also remove the explicit UNIQUE index
creation for workspace_id since the column declaration workspace_id TEXT NOT
NULL UNIQUE already creates an implicit index.
|
Superseded by #20 - renamed session-id to workspace-id and simplified architecture |
1 participant
Summary
Test plan
generate.tsto create a session-id.agents/mutils/mutils.db/compactand verify session-id is restored viaadditionalContextccs-handoffshows session-id in output🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores