Skip to content

feat(comments): reintroduce comments popover with in-memory draft store#1415

Merged
arnestrickmann merged 9 commits intogeneralaction:mainfrom
jschwxrz:emdash/feat-reintroduce-comments-popover-4tz
Mar 11, 2026
Merged

feat(comments): reintroduce comments popover with in-memory draft store#1415
arnestrickmann merged 9 commits intogeneralaction:mainfrom
jschwxrz:emdash/feat-reintroduce-comments-popover-4tz

Conversation

@jschwxrz
Copy link
Collaborator

@jschwxrz jschwxrz commented Mar 11, 2026

summary

  • replaced db backed comment system with a lightweight in-memory DraftCommentsStore that scopes comments per task+worktree path, so multi-agent variants don't bleed into each other
  • comments are now injected directly into agent terminal input as XML when the user submits a message

changes:

  • new DraftCommentsStore with useSyncExternalStore integration, 200-comment-per-scope cap, and proper scope key isolation via buildCommentScopeKey(taskId, worktreePath)
  • useCommentInjection hook syncs draft comments into PendingInjectionManager, which TerminalSessionManager picks up on submit
  • MultiAgentTask builds per-variant comment payloads and only consumes comments after successful PTY injection
  • extracted slashCommand.ts (detects /help, i/model style inputs to skip comment injection) and terminalInjection.ts (payload construction with bracketed paste for non-Claude agents)
  • moved stripAnsi to @shared/text/stripAnsi so both main and renderer can use it
  • removed old lineCommentsIpc.ts, usePendingInjection.ts, useTaskCommentCounts.ts, shared/lineComments.ts, and ~90 lines of electron-api type defs

fixes #1388

@vercel
Copy link

vercel bot commented Mar 11, 2026

@jschwxrz is attempting to deploy a commit to the General Action Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 11, 2026

Greptile Summary

This PR replaces the database-backed line comments system with a lightweight in-memory DraftCommentsStore scoped per taskId + worktreePath, and wires comments into agent terminals as XML via a new useCommentInjection hook and PendingInjectionManager singleton. The change removes ~90 lines of IPC/schema/DB code and gains multi-agent isolation and zero-persistence comment drafts.

Key changes:

  • DraftCommentsStore provides a useSyncExternalStore-compatible store with a 200-comment cap and proper scope-key isolation
  • useCommentInjection syncs formatted comments into PendingInjectionManager so TerminalSessionManager can intercept the next non-slash-command Enter press and inject the XML comment block
  • MultiAgentTask builds per-variant payloads via handleRunAll and only consumes comments after confirmed PTY injection
  • stripAnsi is extracted to @shared/text/stripAnsi with a composable options API, replacing multiple ad-hoc inline regexes

Notable issue: useCommentInjection uses hasPendingRef to decide whether to clear PendingInjectionManager, but this ref is per-hook-instance while PendingInjectionManager is a global singleton. In MultiAgentTask, switching tabs changes activeVariantPath, which causes the hook to see 0 comments for the new scope and call pendingInjectionManager.clear() — erasing comments that were pending for the previous variant's terminal. The hook also lacks an unmount cleanup, so if a component unmounts while comments are pending, stale comments persist in the singleton and can be injected into a subsequently mounted task's terminal.

Confidence Score: 3/5

  • Safe to merge for single-task flows; multi-agent tab-switching has a verified comment-leakage bug before this lands
  • The core architecture (DraftCommentsStore, formatCommentsForAgent, slashCommand guard, terminalInjection, stripAnsi refactor) is clean and well-tested. The single-agent ChatInterface path is safe. The risk area is useCommentInjection's interaction with the PendingInjectionManager singleton: (1) tab switching in MultiAgentTask incorrectly clears the previous variant's pending comments, and (2) there is no unmount cleanup, allowing stale comments to leak across tasks. Both issues are logic bugs in the newly introduced hook, not in the store or formatting layers.
  • src/renderer/hooks/useCommentInjection.ts requires the most attention — the missing scope tracking and unmount cleanup need to be addressed before multi-agent comment injection is reliable.

Important Files Changed

Filename Overview
src/renderer/hooks/useCommentInjection.ts New hook that bridges DraftCommentsStore to PendingInjectionManager; has a logic bug where tab switching in MultiAgentTask clears the wrong scope's pending comments, and missing unmount cleanup leaks comments into subsequent tasks.
src/renderer/lib/DraftCommentsStore.ts Well-designed in-memory comment store with useSyncExternalStore integration, 200-comment cap, and proper scope isolation; minor API issue where add() returns an ID even when the comment wasn't stored at capacity.
src/renderer/lib/PendingInjectionManager.ts Global singleton for coordinating comment injection timing between React hooks and the class-based TerminalSessionManager; the singleton design creates cross-task contamination risk when multiple tasks are simultaneously mounted.
src/renderer/components/MultiAgentTask.tsx handleRunAll correctly builds per-variant comment payloads and consumes comments only after successful PTY injection; properly isolates variants via buildCommentScopeKey.
src/renderer/terminal/TerminalSessionManager.ts Correctly gates comment injection behind providerId check, slash-command detection, and hasUserMessage guard; only triggers injection for real user messages, not bare Enter presses.
src/renderer/lib/terminalInjection.ts Payload construction correctly uses raw multiline for Claude and bracketed paste for other providers; inputData is only trailing-newline-stripped and may carry residual ANSI sequences.
src/shared/text/stripAnsi.ts Clean, option-driven stripAnsi implementation replacing multiple ad-hoc inline regexes; correctly handles CSI, OSC-BEL, OSC-ST, and other escapes with composable options.
src/renderer/lib/formatCommentsForAgent.ts Straightforward XML formatter grouping comments by file; correctly sorts by line number and supports optional intro and leading-newline options.
src/renderer/lib/slashCommand.ts Correctly classifies /commands and TUI-style i/model prefixed inputs to skip comment injection; regex SINGLE_PREFIX_TUI_SLASH_RE requires at least 3 chars after the slash which covers real commands.
src/renderer/hooks/useLineComments.ts Clean React hook wrapping DraftCommentsStore with useSyncExternalStore; correctly memoizes callbacks and filters by file path for the diff-view use case.
src/renderer/components/CommentsPopover.tsx Simple, clean popover UI for displaying and deleting draft comments; reads from TaskScopeContext and correctly groups by file path.
src/renderer/terminal/submitCapture.ts Correctly reconstructs submitted text from keystroke stream by handling backspace, Ctrl-U, and Enter; uses shared stripAnsi with private CSI params enabled.
src/renderer/components/ChatInterface.tsx Correctly replaced useTaskComments with useCommentInjection and removed the old comment prepending logic from issue context builders.

Sequence Diagram

sequenceDiagram
    participant User
    participant DiffView
    participant DraftCommentsStore
    participant useCommentInjection
    participant PendingInjectionManager
    participant TerminalSessionManager
    participant PTY

    User->>DiffView: Click gutter + icon
    DiffView->>DraftCommentsStore: add(scopeKey, comment)
    DraftCommentsStore-->>useCommentInjection: notify subscribers

    useCommentInjection->>useCommentInjection: formatCommentsForAgent(comments)
    useCommentInjection->>PendingInjectionManager: setPending(formattedXML)

    User->>TerminalSessionManager: type message + Enter
    TerminalSessionManager->>TerminalSessionManager: consumeSubmittedInput(data)
    TerminalSessionManager->>PendingInjectionManager: getPending()
    PendingInjectionManager-->>TerminalSessionManager: formattedXML

    TerminalSessionManager->>TerminalSessionManager: isSlashCommandInput? → no
    TerminalSessionManager->>TerminalSessionManager: buildCommentInjectionPayload(providerId, input, formattedXML)
    TerminalSessionManager->>PTY: ptyInput(payload)
    TerminalSessionManager->>PendingInjectionManager: markUsed()
    PendingInjectionManager->>useCommentInjection: onInjectionUsed callback
    useCommentInjection->>DraftCommentsStore: consumeAll(scopeKey)

    Note over DraftCommentsStore: Comments cleared from store

    Note over PendingInjectionManager: MultiAgent path differs — handleRunAll injects per-variant directly via ptyInput, then calls consumeAll manually
Loading

Last reviewed commit: 63b906a

@arnestrickmann arnestrickmann merged commit 8a6596b into generalaction:main Mar 11, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: CommentsPopover UI removed during refactor — no way to send inline diff comments to agent

2 participants