Spec: Claude Code session topic tags in cmd-p switcher#1087
Spec: Claude Code session topic tags in cmd-p switcher#1087apollow wants to merge 2 commits intomanaflow-ai:mainfrom
Conversation
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@apollow is attempting to deploy a commit to the Manaflow Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughA design specification document is introduced for implementing topic tags from Claude Code session contexts within Cmd-P session search. The spec outlines data structures, persistence mechanisms via socket commands, tag indexing, extraction pipelines, gating configuration, and UI presentation details. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 1 file
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="docs/specs/cmd-p-session-tags.md">
<violation number="1" location="docs/specs/cmd-p-session-tags.md:92">
P1: The pseudocode computes `tagTokens` but never appends them to `contextKeywords`, so actual tag content would not be searchable. Only the static labels `"tag"`, `"topic"`, `"claude"` are added. Add `contextKeywords.append(contentsOf: tagTokens)` before or after the static-label append.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Greptile SummaryThis PR adds a spec for surfacing Claude Code session topics as searchable tags in the cmd-p workspace switcher. The design is well-structured with thoughtful defaults (opt-in, namespaced tag sources, PII sanitization, caps on tag storage), but the spec has one critical logic defect and several underspecified areas that would produce a broken or ambiguous implementation. Key findings:
Not ready to implement — the spec contains a critical logic defect that would silently ship a non-functional feature, plus several underspecified areas. Confidence Score: 2/5
Sequence DiagramsequenceDiagram
participant Claude as Claude Code
participant Hook as cmux claude-hook CLI
participant Store as claude-hook-sessions.json
participant Socket as cmux Socket
participant WS as Workspace Model (tagsBySource)
participant Indexer as CommandPaletteSwitcherSearchIndexer
participant UI as Cmd-P Switcher
Claude->>Hook: notification (stdin JSON)
Hook->>Store: upsert lastSubtitle / lastBody
Hook->>Hook: check cmdPSessionTags setting
alt setting enabled
Hook->>Hook: extract + sanitize topic tags
Hook->>Socket: workspace.set_tags <id> --source claude:<sessionId> [tags]
Socket->>WS: update tagsBySource["claude:<sessionId>"]
end
Claude->>Hook: stop event
Hook->>Socket: workspace.clear_tags <id> --source claude:<sessionId>
Socket->>WS: delete tagsBySource["claude:<sessionId>"]
Note over WS: Manual tags via CLI<br/>always stored under "manual" source
WS-->>Indexer: workspace.searchTags (flattened)
Indexer-->>UI: keywords include tag tokens + ["tag","topic","claude"]
UI-->>UI: user types "open claw" → workspace surfaces
Last reviewed commit: 83482c2 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| ```swift | ||
| let tagTokens = metadata.tags.flatMap { tagTokensForSearch($0) } | ||
| // Split on delimiters like branches, so "open-claw" matches "open", "claw", "open-claw" | ||
| if !tagTokens.isEmpty { | ||
| contextKeywords.append(contentsOf: ["tag", "topic", "claude"]) | ||
| } | ||
| ``` |
There was a problem hiding this comment.
tagTokens is computed but never appended to the returned keywords, so searching for a specific topic like "open claw" would not surface the workspace.
The code computes tagTokens from the metadata but only uses the non-empty check to add generic labels ["tag", "topic", "claude"]. The actual tag content is never included in the keyword list.
The return statement needs to include tagTokens:
return contextKeywords + directoryTokens + branchTokens + portTokens + tagTokensWithout this, the indexer would silently pass all wiring tests while never making topics searchable — the core purpose of the feature would be broken.
There was a problem hiding this comment.
Addressed in 01c749f — added the missing append and return line.
| - **Check setting**: skip tag extraction if `cmdPSessionTags` is disabled | ||
| - Extract topic keywords from `summary.subtitle` and `summary.body` (simple word tokenization, filter stopwords, keep meaningful tokens) | ||
| - **Sanitize**: strip tokens that look like secrets, file paths, UUIDs, emails, or other PII patterns before persisting | ||
| - Send `workspace.set_tags --source claude:<sessionId>` with the extracted tags |
There was a problem hiding this comment.
sessionId is embedded directly in the socket command string without validation, creating a command-injection risk.
The spec shows workspace.set_tags --source claude:<sessionId> where sessionId comes from Claude's hook stdin JSON via parsedInput.sessionId. If this contains spaces, -- prefixes, or other shell/command-parsing characters, the resulting command string can be malformed or hijacked by the socket command parser.
The spec should require that sessionId values are validated/sanitized before being embedded in the source key — for example, restricting to alphanumeric characters and hyphens, with a maximum length. This is especially critical because the session ID originates from external tool output.
There was a problem hiding this comment.
Addressed in 01c749f — spec now requires sessionId validation against ^[a-zA-Z0-9_-]{1,128}$ before embedding in source key. Malformed IDs skip tag push silently.
| ```swift | ||
| return CommandPaletteSwitcherSearchMetadata( | ||
| directories: directories, | ||
| branches: branches, | ||
| ports: ports, | ||
| tags: workspace.searchTags // NEW — gated on cmdPSessionTags setting for display | ||
| ) | ||
| ``` |
There was a problem hiding this comment.
The comment // gated on cmdPSessionTags setting for display is ambiguous and conflicts with how gating is described elsewhere in the spec.
The Settings section states that when disabled, "No tags are extracted from Claude notifications" — implying extraction-level gating. But this comment suggests gating only affects display, not extraction. There are two possible interpretations:
- Gating at extraction (section 6): Claude hooks skip tag extraction when disabled, so
tagsBySourcestays empty for Claude-sourced tags. - Gating at metadata construction (section 4):
workspace.searchTagsis conditionally passed based on the setting, even if tags were previously extracted.
The spec should clarify which is the single source of truth, and explain whether double-gating is needed (e.g., to handle users who had the feature enabled, accumulated tags, then disabled it).
There was a problem hiding this comment.
Addressed in 01c749f — removed ambiguous comment. Spec now clearly states: single gate at extraction time (section 6). Metadata construction always passes tags through, ensuring manual tags work regardless of setting.
| ### Edge Cases | ||
|
|
||
| - **Multiple Claude sessions per workspace**: each session writes to its own `claude:<sessionId>` source — no conflicts | ||
| - **Session ends**: only that session's tags are cleared; manual and other session tags persist | ||
| - **Tag overflow**: caps enforced per source (20 tags, 100 chars each); oldest dropped if exceeded | ||
| - **No Claude session**: tags field is empty, no UI change | ||
| - **Feature disabled** (default): no automatic tag extraction, socket commands still work for manual use | ||
| - **Sensitive data**: tag extraction sanitizes secrets/PII patterns before persisting |
There was a problem hiding this comment.
The edge cases section does not address what happens when the feature is disabled after use.
Since tagsBySource is persisted in workspace snapshots, if a user enables the feature, accumulates Claude-sourced tags, and then disables it, those stale tags remain in the model and workspace snapshots. The expected behavior is unspecified:
- Should toggling the feature off clear previously accumulated Claude-sourced tags automatically?
- If tags persist, should the gating behavior (whether at extraction or metadata construction) prevent them from surfacing in cmd-p search?
- Are stale tags acceptable, or should there be a migration/cleanup path when the setting is toggled?
The spec should define the intended behavior so users and implementations agree on what happens after disabling the feature.
There was a problem hiding this comment.
Addressed in 01c749f — edge cases now define stale tag behavior: Claude-sourced tags are not auto-purged on toggle-off (they decay as sessions end). Users can run cmux workspace clear-tags for immediate cleanup.
docs/specs/cmd-p-session-tags.md
Outdated
| let tagTokens = metadata.tags.flatMap { tagTokensForSearch($0) } | ||
| // Split on delimiters like branches, so "open-claw" matches "open", "claw", "open-claw" |
There was a problem hiding this comment.
tagTokensForSearch is referenced but never defined.
The snippet calls metadata.tags.flatMap { tagTokensForSearch($0) } but this function doesn't exist in the codebase (the existing helpers are directoryTokensForSearch, branchTokensForSearch, portTokensForSearch). The spec should define:
- The function signature:
func tagTokensForSearch(_ tag: String) -> [String] - Tokenization rules: should it split on
metadataDelimitersthe same waybranchTokensForSearchdoes? - Whether the original unsplit tag should be included as a keyword (important for multi-word phrases like "open claw")
- Whether it should be
detail-aware
Without this definition, the tag tokenization behavior is underspecified.
There was a problem hiding this comment.
Addressed in 01c749f — full tagTokensForSearch definition added with signature, tokenization rules (splits on metadataDelimiters, includes original multi-word tag), and documented as not detail-aware.
…essionId validation, clarify gating, define stale cleanup Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
docs/specs/cmd-p-session-tags.md (1)
65-67: Make tag ordering explicit instead of relying on dictionary flattening.
tagsBySource.values.flatMap { $0 }does not encode the priority order described later, so footnote/search ordering will depend on map iteration rather than the spec. Consider definingsearchTagsfrom an explicit source precedence list to keep display and indexing deterministic.Also applies to: 183-187
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/specs/cmd-p-session-tags.md` around lines 65 - 67, The current searchTags computed property uses tagsBySource.values.flatMap { $0 } which relies on dictionary iteration order; change searchTags to build its array by iterating an explicit source-precedence list (e.g., let sourcePrecedence = [...]) and appending/flatMapping tagsBySource[source] in that order so ordering is deterministic; update the same pattern elsewhere where tagsBySource.values.flatMap { $0 } appears (the other occurrence that controls footnote/search ordering) to use the same explicit precedence-driven construction.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/specs/cmd-p-session-tags.md`:
- Around line 135-136: The spec currently only gates Claude tag extraction via
cmdPSessionTags, but previously persisted Claude tags still surface through
workspace.searchTags, metadata construction, the search indexer, and rendering
(footnotes); update the behavior so Claude-sourced tags are also suppressed when
cmdPSessionTags is false by either (A) filtering out or hiding tags with source
"claude"/the relevant source key at read/index/render time (where
workspace.searchTags, metadata construction, and the search indexer consume
tags), or (B) explicitly purging/hiding persisted Claude tags when
cmdPSessionTags is disabled; ensure the change references cmdPSessionTags,
tagsBySource, workspace.searchTags, and the search indexer/rendering paths and
documents the chosen gating strategy.
- Around line 92-100: The spec currently promises preserved multi-word tags but
the extractor behavior is underspecified; update the documentation around
tagTokensForSearch (which trims, splits by metadataDelimiters, and calls
uniqueNormalizedPreservingOrder) to explicitly state whether Claude-extracted
tags can be multi-word or are always single-tokenized, and if multi-word phrases
must be preserved ensure the extractor output format is described (e.g., emit
full phrase tags before tokenization) so tagTokensForSearch will index the
phrase as the first element; alternatively, narrow the guarantee to token-based
matching and adjust the wording wherever similar claims appear (also update the
equivalent paragraphs referenced at lines ~150-153) to avoid promising phrase
matching that the extractor may not supply.
- Around line 137-146: The CLI examples contradict the socket API
ownership/namespace rule: update the docs for the workspace.set_tags and
workspace.clear_tags commands so that the CLI explicitly requires --source in
every example (e.g., "workspace.set_tags <workspace-id> --source <source-key>
[...]" and "workspace.clear_tags <workspace-id> --source <source-key>") and
clarify that these operate only on the given source; alternatively, if you
intend a global/administrator operation, add and document a distinct command
(e.g., workspace.clear_all_tags <workspace-id> --admin) and mark it as
admin-only — update the description lines that reference these commands to state
the chosen contract (per-source required or separate admin-clear) and ensure the
“callers declare ownership” rule is reflected verbatim for workspace.set_tags
and workspace.clear_tags.
---
Nitpick comments:
In `@docs/specs/cmd-p-session-tags.md`:
- Around line 65-67: The current searchTags computed property uses
tagsBySource.values.flatMap { $0 } which relies on dictionary iteration order;
change searchTags to build its array by iterating an explicit source-precedence
list (e.g., let sourcePrecedence = [...]) and appending/flatMapping
tagsBySource[source] in that order so ordering is deterministic; update the same
pattern elsewhere where tagsBySource.values.flatMap { $0 } appears (the other
occurrence that controls footnote/search ordering) to use the same explicit
precedence-driven construction.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 11ebc8ff-2655-4578-94dc-bd4df8160539
📒 Files selected for processing (1)
docs/specs/cmd-p-session-tags.md
| /// Tokenizes a single tag string into searchable keywords. | ||
| /// Includes the original tag (for multi-word phrase matching like "open claw") | ||
| /// plus individual components split on standard delimiters. | ||
| /// Not detail-aware — tags are always fully tokenized regardless of workspace/surface context. | ||
| private static func tagTokensForSearch(_ rawTag: String) -> [String] { | ||
| let trimmed = rawTag.trimmingCharacters(in: .whitespacesAndNewlines) | ||
| guard !trimmed.isEmpty else { return [] } | ||
| let components = trimmed.components(separatedBy: metadataDelimiters).filter { !$0.isEmpty } | ||
| return uniqueNormalizedPreservingOrder([trimmed] + components) |
There was a problem hiding this comment.
Phrase matching is underspecified for Claude-extracted tags.
The indexer is designed to preserve multi-word tags, but the extractor is described as “simple word tokenization.” If Claude-generated tags are only single tokens, the "open claw" example is no longer guaranteed by the data model. The spec should define how phrase tags are formed before indexing, or narrow the promised behavior to token-based matching.
Also applies to: 150-153
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/specs/cmd-p-session-tags.md` around lines 92 - 100, The spec currently
promises preserved multi-word tags but the extractor behavior is underspecified;
update the documentation around tagTokensForSearch (which trims, splits by
metadataDelimiters, and calls uniqueNormalizedPreservingOrder) to explicitly
state whether Claude-extracted tags can be multi-word or are always
single-tokenized, and if multi-word phrases must be preserved ensure the
extractor output format is described (e.g., emit full phrase tags before
tokenization) so tagTokensForSearch will index the phrase as the first element;
alternatively, narrow the guarantee to token-based matching and adjust the
wording wherever similar claims appear (also update the equivalent paragraphs
referenced at lines ~150-153) to avoid promising phrase matching that the
extractor may not supply.
| **Gating strategy**: The single gate is at **extraction time** (section 6). When `cmdPSessionTags` is disabled, no Claude-sourced tags are extracted or pushed, so `tagsBySource` stays empty for Claude sources. The metadata construction and search indexer always include whatever tags exist — this ensures manual tags (which bypass the setting) always work, and avoids double-gating complexity. | ||
|
|
There was a problem hiding this comment.
Disabled mode still leaks previously persisted Claude tags.
This gating model only stops new extraction. Previously stored Claude tags would still flow through workspace.searchTags, get indexed, and render as footnotes, so they are not actually “inert” after the setting is turned off. The spec should either gate Claude-sourced tags at read/index/render time as well, or explicitly purge/hide them when cmdPSessionTags is disabled.
Also applies to: 195-196
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/specs/cmd-p-session-tags.md` around lines 135 - 136, The spec currently
only gates Claude tag extraction via cmdPSessionTags, but previously persisted
Claude tags still surface through workspace.searchTags, metadata construction,
the search indexer, and rendering (footnotes); update the behavior so
Claude-sourced tags are also suppressed when cmdPSessionTags is false by either
(A) filtering out or hiding tags with source "claude"/the relevant source key at
read/index/render time (where workspace.searchTags, metadata construction, and
the search indexer consume tags), or (B) explicitly purging/hiding persisted
Claude tags when cmdPSessionTags is disabled; ensure the change references
cmdPSessionTags, tagsBySource, workspace.searchTags, and the search
indexer/rendering paths and documents the chosen gating strategy.
| #### 5. Socket command: `workspace.set_tags` | ||
|
|
||
| ```text | ||
| workspace.set_tags <workspace-id> --source <source-key> ["tag1","tag2","tag3"] | ||
| workspace.clear_tags <workspace-id> --source <source-key> | ||
| ``` | ||
|
|
||
| - Replaces tags **for the given source only** — other sources are untouched | ||
| - `--source` is required; callers declare ownership | ||
| - Off-main parsing, `DispatchQueue.main.async` for the model update (per socket threading policy) |
There was a problem hiding this comment.
CLI ownership semantics contradict the namespacing model.
The socket API requires --source, but the CLI examples omit it and clear-tags 1 reads like a global delete. That undermines the “callers declare ownership” rule and makes it unclear whether manual commands operate on manual only or on every source. Please make the CLI contract explicit: require --source everywhere, or define a separate admin-only “clear all tags” command.
Also applies to: 158-165, 196-196
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/specs/cmd-p-session-tags.md` around lines 137 - 146, The CLI examples
contradict the socket API ownership/namespace rule: update the docs for the
workspace.set_tags and workspace.clear_tags commands so that the CLI explicitly
requires --source in every example (e.g., "workspace.set_tags <workspace-id>
--source <source-key> [...]" and "workspace.clear_tags <workspace-id> --source
<source-key>") and clarify that these operate only on the given source;
alternatively, if you intend a global/administrator operation, add and document
a distinct command (e.g., workspace.clear_all_tags <workspace-id> --admin) and
mark it as admin-only — update the description lines that reference these
commands to state the chosen contract (per-source required or separate
admin-clear) and ensure the “callers declare ownership” rule is reflected
verbatim for workspace.set_tags and workspace.clear_tags.
1 participant
Summary
Changes from v1 (#1086)
cmdPSessionTagssetting, users opt in explicitlyclaude:<sessionId>,manual) so Claude hooks never overwrite manual tagsDetails
See
docs/specs/cmd-p-session-tags.mdfor full spec including data flow, code changes, edge cases, and UI mockup.Test plan
Supersedes #1086
Generated with Claude Code
Summary by cubic
Spec to make Claude Code session topics searchable in cmd‑p by indexing topic tags from Claude hook notifications into workspace search metadata. Opt‑in via
cmdPSessionTags, with namespaced and sanitized tags to avoid conflicts and protect sensitive data.tagstoCommandPaletteSwitcherSearchMetadata; index as search keywords and show as faint footnotes in cmd‑p.claude:<sessionId>,manual) with caps; validatesessionId(^[a-zA-Z0-9_-]{1,128}$); persist in workspace snapshot.workspace.set_tags,workspace.clear_tags, andcmux workspace set-tags;--sourcerequired for ownership.cmdPSessionTagsis off, no Claude tags are pushed; manual tags always work. Cleanup: clear a session’s tags on stop; disabling doesn’t auto‑purge—usecmux workspace clear-tagsif needed.Written for commit 01c749f. Summary will update on new commits.
Summary by CodeRabbit