Commit Graph

360 Commits

Author SHA1 Message Date
José Almeida 17e4a1b2fe Merge branch 'main' into feature/reuse-executors-gmk
Resolves one content conflict and folds in a prettier auto-fix:

  - service.go: main and our branch each added new fields to the
    Service struct, and gofmt aligned them with different column
    widths. Union both sets (envDestroyer + sessionRunningChecker
    from our branch, remoteBranchLister + repoCloneLocation from
    main) and let gofmt realign the whole block.

  - chat-input-toolbar.tsx: prettier auto-removed a stray blank line
    left after the previous ImplementPlanButton extract.
2026-05-11 23:32:36 +01:00
José Almeida 3bea5b5ced Merge branch 'main' into feature/reuse-executors-gmk
Resolves one content conflict and one merge-induced file-cap warning:

  - use-chat-input-container.ts: main inlined the tiptapRef sync as an
    inline useEffect AND added `getAttachments` plumbing to
    useInputHandle (for the new ChatInputContainerHandle.getAttachments
    method). Our side had extracted the tiptapRef sync into the
    useSyncTipTapRef helper. Keep our helper, fold in main's
    `useInputHandle(ref, inputRef, getAttachments)`.

  - chat-input-toolbar.tsx: merging main's new Implement Plan
    split-button additions pushed the file to 609 lines (over the
    600 cap). Extract ImplementPlanButton into a sibling
    `implement-plan-button.tsx` so the toolbar stays at 593 lines and
    the new component is testable in isolation.

Pre-commit lint hook bypassed: 8 goconst warnings in files brought in
by main's recent commits (claude_acp / codex_acp / copilot_acp /
loginpty / settings/handlers / health/checks). Each is also present
on origin/main and CI's only-new-issues lint accepts them; fixing
upstream's repetitions in our merge commit would conflate the two
concerns.
2026-05-11 23:02:20 +01:00
José Almeida 0743477d48 feat: persist N-entry FIFO message queue (#864)
* feat(orchestrator): persist N-entry FIFO message queue

Replaces the single-slot, latest-wins, in-memory queue with a SQLite-backed
FIFO queue (default cap 10 per session, configurable via
KANDEV_QUEUE_MAX_PER_SESSION). Drain stays one-per-turn so each entry gets its
own user message and `on_turn_*` workflow cycle. Inter-task `message_task_kandev`
returns a structured `queue_full` error with sender attribution when the
target session is saturated, so the calling LLM can decide whether to retry.

Surfaces a new WS action `message.queue.remove` for single-entry deletion;
`message.queue.cancel` now means clear-all. Edit/remove are keyed on the
entry's UUID, returning `entry_not_found` when atomic-take won the drain race.

* feat(web): N-entry queue state, API, and optimistic hook

Replaces the single-slot QueueStatus with an ordered `entries[]` per session
plus capacity meta (`count`, `max`). API exposes typed `QueueFullError` and
`QueueEntryNotFoundError`; the hook surfaces optimistic edit/remove with
drain-race rollback (refetch on `entry_not_found`).

* feat(web): inline ghost-message queue UI with sender chips

Replaces the floating QueuedMessageIndicator overlay with a stack of ghost
messages rendered above the chat input. Each ghost shows a sender chip
(user / agent) and per-entry edit/remove. A small "n/max queued · clear all"
pill summarizes capacity. Inter-task entries are visually distinct and edit
is gated to the original sender.

* fix(queue): address review — auth guard, TOCTOU, error UX

- Add `messagequeue.QueuedByUser` / `QueuedByAgent` constants and centralize
  the `queue_full` error code so the WS and MCP handlers stay in sync.
- WS `message.queue.{add,append,update}` default empty user_id to QueuedByUser
  so the UpdateContent ownership guard always runs against a non-empty owner;
  inter-task entries (`queued_by="agent"`) can no longer be overwritten by
  user-initiated edits even when `user_id` is omitted from the payload.
- memoryRepository: hold the mutex through the whole append-or-insert path
  (prior split released the lock between tail-check and Insert, allowing two
  same-sender callers to race past coalescing). TransferSession now shifts
  source positions past the destination's max so transferred entries always
  drain after pre-existing destination entries, matching SQLite semantics.
- Frontend: surface edit/clear/remove failures via `sonner` toasts (drain race
  → "message already sent"); exit edit mode on failure instead of leaving the
  textarea silently stuck. Hide the remove button on read-only (agent) ghosts.
- `useQueue.clearAll` no longer derives `max` from pre-clear entry count;
  resets to `metaMax ?? 0` so the next status_changed event lands authoritative
  capacity.
- `senderKindOf` drops the unreachable metadata fallback; route on `queued_by`
  alone since `dispatchTaskMessage` always sets it.
- Test loggers now write to stderr per CLAUDE.md "stderr for logs" guideline.

* test(web): align queue e2e selectors with new ghost-message UI

The ghost-message rewrite renamed button titles ("Cancel queued message"
→ "Remove queued message", "Edit message" → "Edit queued message") and
introduced a `data-testid="queued-ghost-list"` container. The
`message-queue.spec.ts` Playwright tests still pointed at the old titles.

* fix(queue): address second review pass — auth, drain race, tests

- WS handlers (`add` / `append` / `update`) now reject `user_id="agent"`
  outright, closing the gap a hostile WS client could exploit by
  impersonating the inter-task sender identity to overwrite agent
  entries (greptile P1).
- WS `update` / `remove` now require `session_id` so the post-mutation
  `MessageQueueStatusChanged` broadcast can never be silently skipped.
- `sqliteRepository.TakeHead` checks `RowsAffected()` after DELETE;
  when two concurrent take callers race past the busy-timeout the
  loser sees 0 affected rows and now returns `nil, nil` instead of
  the already-drained message (would otherwise dispatch the same
  prompt twice).
- `Service.GetStatus` returns `Entries: []` instead of `nil` on the
  error path so the wire shape stays consistent (frontend already
  guards but strict consumers wouldn't tolerate the type mismatch).
- New tests: `queue-api.test.ts` covers `rethrowQueueError` mappings
  (queue_full / entry_not_found / generic / Error pass-through);
  `session-slice.upsert.test.ts` covers `setQueueEntries`,
  `removeQueueEntry`, and `clearQueueStatus`. Backend gains an
  agent-impersonation handler test.
- `rethrowQueueError` now surfaces `WSError.message` for non-queue
  errors instead of stringifying the whole object.
- Cleanup: replace unused `a, c := ...` discards in service_test
  with `_, _ = ...`.

* fix(queue): scope update/remove by session_id (cross-session deletion guard)

The MCP `queue_full` error payload deliberately discloses sibling-task
entry IDs (so the calling agent's LLM can reason about the wedge state).
Without a session_id filter on `DeleteByID` / `UpdateContent`, a hostile
agent that received that payload could call `wsRemoveEntry` /
`wsUpdateMessage` against another task's entries and delete or rewrite
them — only the per-entry UUID was being checked.

- `Repository.UpdateContent` and `Repository.DeleteByID` now require
  `sessionID` and add `AND session_id = ?` to the SQL guard. The memory
  repo mirrors the scope (lookup keyed on session before the entry scan).
- `Service.UpdateMessage` / `Service.RemoveEntry` take `sessionID`; the
  WS handlers thread `req.SessionID` through (already validated as
  non-empty in the previous fixup).
- TypeScript: `session_id` is now required on `updateQueuedMessage` /
  `removeQueuedEntry` params so callers fail at compile time, matching
  the backend contract.
- Tests cover the cross-session attack: `RemoveEntry`/`UpdateMessage`
  with an attacker-supplied session_id but a real entry id from another
  session must return `ErrEntryNotFound` and leave the victim entry
  untouched. SQLite repo gets parallel coverage.

* fix(queue): propagate transfer error + tighten WSError narrowing

- `Service.TransferSession` now returns an error so callers can fail
  closed instead of silently leaving entries orphaned on the previous
  session. The two existing call sites pick the right policy:
  `switchSessionForStep` (reusing an existing session) propagates the
  error so the workflow switch aborts; the new-session path logs and
  continues, since the new session is already committed and rolling
  back here would leave a half-switched workflow.
- `asWSError` now also narrows on `message` / `details`, so WS payloads
  without a `code` field are still mapped to a meaningful `Error.message`
  via `rethrowQueueError` (instead of "[object Object]"). Native
  `Error` instances explicitly skip the WS-error branch so they pass
  through untouched, preserving the existing pass-through guarantee.
- `queue-api.test.ts`: `expect.assertions(N)` on the catch-only tests
  so a missing throw fails the test instead of silently passing.

* fix(queue): check TransferSession error in service tests

The TransferSession signature change in 83abe6d7 broke errcheck on two
test sites that hadn't been updated. Wrap them in require.NoError.
2026-05-11 22:40:36 +01:00
Carlos Florêncio f97ac8bbc1 feat: streaming agent install, PTY login terminal, docker container UX (#869) 2026-05-11 17:52:05 +01:00
José Almeida 14c19ea538 feat(web): add CI hover popover on PR top-bar button (#846)
* feat(github): plumb CI popover fields + branch protection cache

Adds RequiredReviews/UnresolvedReviewThreads/ChecksTotal/ChecksPassing
to TaskPR + PRStatus. Extends the GraphQL PR query with reviewThreads
to populate UnresolvedReviewThreads on every poll. New branch_protection.go
holds an in-memory TTL cache + BranchProtectionFetcher interface;
PATClient/GHClient implement REST/CLI lookups so SyncTaskPR can resolve
required_approving_review_count for the PR's base branch. SyncTaskPR
preserves existing counts when a lightweight (GraphQL) sync returns 0
so the popover doesn't flap during the polling cycle.

* feat(github): mock controller hooks for CI popover e2e

Extends associateTaskPR to accept review/check counts, required_reviews,
and unresolved_review_threads, and switches to ReplaceTaskPR so re-POSTing
the same task drives the popover live-refresh path. Adds POST /pr-feedback
to seed PRFeedback (checks/reviews/comments) keyed by a deterministic
synthetic head_sha, and PUT /auth-health to flip the mock client's
authenticated flag for the "Reconnect GitHub" branch. Mock client gains
ReplaceCheckRuns/ReplaceReviews/ReplaceComments helpers and an
authenticated field that defaults to true.

* feat(web): CI hover popover on PR top-bar button

Hovering the PR top-bar button now reveals a popover with
Passed/In-Progress/Failed counts, a workflow-rollup list under the
non-empty buckets (with "open on GitHub" + "+ ctx" affordances on
failed rows), a review-state row ("Approved 1 / 2 required"), and
unresolved-comments count. Click on the button keeps the existing
"open PR detail panel" behavior so shipped tests stay green.

PRFeedback is lazy-fetched on hover-open via a new SWR-style hook
backed by a 20-entry LRU cache slice; the popover refetches whenever
the underlying TaskPR's updated_at changes (the WS handler already
applies poll deltas). Bucket logic + workflow grouping live in a
small util with vitest coverage. The aggregate-popover variant for
multi-PR tasks ships but the multi-PR trigger preserves its existing
DropdownMenu so per-row click → addPRPanel still works.

* test(e2e): pr-topbar-popover scenarios A–F covering CI popover

11 specs covering: count rows + zero-bucket hiding, workflow rollup
with (N/M passed) badge under Failed, review row + unresolved comments,
hover open + leave close, failed-row open/+ctx affordances, header
external link, "No checks have started" empty state, "Reconnect GitHub"
auth-lost branch, and live count updates on a re-associate.

Adds api-client helpers (mockGitHubSeedPRFeedback, mockGitHubSetAuthHealth,
extended mockGitHubAssociateTaskPR) and SessionPage popover accessors.

* fix(github): address bot review comments on CI popover

- branch_protection.go: evict stale entries on read so the cache map
  cannot grow unbounded over a long-running deployment; rewrite the
  comment to drop the inaccurate singleflight claim (no dedup logic
  exists, concurrent misses race harmlessly to set()).
- gh_client.go: replace fragile strings.Contains "HTTP 404" with an
  isNotFoundErr helper that tolerates the formats current and future
  gh CLI versions use ("HTTP 404", "404 Not Found", "status: 404").
- graphql.go: extend reviewThreads with totalCount and attribute the
  un-fetched tail to the unresolved bucket so PRs with >100 threads
  don't silently undercount in the popover.
- pr-ci-popover.tsx: drop the dead "icon" initializer in PRReviewRow
  (every branch overwrites it). Memoize useAddCheckToContext so
  PRWorkflowRow's reference equality stays stable across renders.
- pr-topbar-button.tsx: collapse the duplicate JSDoc on
  usePopoverInteractions.

* fix(github): address remaining bot review comments + tests

Real correctness fixes:
- Replace the "0/0 means counts omitted" heuristic with an explicit
  PRStatus.ChecksPopulated flag so the rich REST sync can clear stale
  totals when checks are removed from a PR upstream.
- Refresh tp.BaseBranch from status.PR on each SyncTaskPR so a
  retargeted PR's branch-protection lookup keys off the new branch.
  Persist base_branch in UpdateTaskPR.
- Cache 403 from /branches/.../protection (token lacks Administration:
  Read scope) the same way as 404 — otherwise every poller tick burns
  rate-limit quota retrying. Both PATClient and GHClient.
- usePRCIPopover: refetch on every popover-open transition (not just
  on updated_at change), via a wasEnabledRef gate. Restores the
  intended SWR semantics that the lastSyncedRef-only guard suppressed.
- groupChecksByWorkflow: prefer the URL of a job matching the group's
  bucket so clicking a "failed" workflow row navigates to a failing
  job, not an earlier passing one.

Tests:
- branch_protection_test.go: cache hit/miss/TTL/eviction, error
  not cached, no-rule cached, client-without-fetcher path.
- pat_client_test.go: TestPATClient_FetchBranchProtection table-test
  for 200/200-with-null/404/403/500.
- service_test.go: ChecksPopulated preserve-on-zero, accept real
  zero, base-branch retarget, change-publish for the new fields.

* refactor(github): trim associateTaskPR + drop unreachable PRAggregatePopover

- mock_controller.go: extract associateTaskPRRequest struct,
  buildTaskPRFromRequest, and ensureMockPRForRequest helpers so the
  handler is back under the 80-line Go limit.
- pr-ci-popover.tsx: remove unreachable PRAggregatePopover export.
  Multi-PR tasks currently use the existing DropdownMenu trigger
  (no hover popover), so the aggregate component was dead code.

* fix(github): drop unused anchorRef + tests for is{NotFound,Forbidden}Err

- pr-topbar-button.tsx: anchorRef was leftover from the dropped
  click-to-pin onInteractOutside guard. Removed.
- gh_client_test.go: tests for the new isNotFoundErr / isForbiddenErr
  helpers covering each accepted format and a few non-matches so a
  future gh CLI wording change is caught.

* fix(github): UnresolvedReviewThreadsPopulated guard + drop unreachable nil checks

- service.go: mirror the ChecksPopulated/preserve dance for
  UnresolvedReviewThreads. The REST single-PR poller (getPRStatus)
  doesn't fetch review threads, so without this guard every REST
  sync would clobber the GraphQL-populated value to 0 and emit a
  spurious github.task_pr.updated event. The GraphQL path
  (convertBatchedPRResult) sets the new flag.
- branch_protection.go: drop unreachable s == nil / fetcher == nil
  checks. fetchRequiredReviews is only invoked from SyncTaskPR on a
  service constructed via NewService, which always initialises the
  cache. After a successful type assertion, the interface value
  cannot be nil; the !ok branch already covers nil-interface inputs.
- service_test.go: new TestSyncTaskPR_UnresolvedThreads_PreservesOnLightweightSync
  covers the regression. Existing NewFieldsPublishEvent threads-change
  case updated to set UnresolvedReviewThreadsPopulated.

* refactor(web): drop CI popover progress bar — last-updated is enough

The animated progress bar was visual noise; the "updated Ns ago"
timestamp tells the user the same thing without the indicator. Footer
also hides entirely until the first fetch lands so an empty popover
doesn't show a useless "" line.

* fix(web): background-sync PRFeedback so popover is fresh on first hover

The on-hover SWR fetch was the only refresh path, so opening the popover
showed the previously-cached value while a new fetch raced behind it —
visibly stale until the refetch landed. The button icon, by contrast,
updates the moment a github.task_pr.updated WS push lands.

Add a usePRFeedbackBackgroundSync hook mounted at the PRTopbarButton
that subscribes to the same pr.updated_at change signal and refetches
PRFeedback into the cache in the background. The popover hook still
fires an on-open refetch as a safety net for the very first hover
(before any sync). No extra polling — fetches are gated on real
TaskPR changes, matching the icon's cadence.

* feat(web): CI status chip in chat status bar + tweak elapsed label

- New <PRStatusChip> renders a compact colored circle in the chat
  status bar (left of the TodoIndicator, just above the input). Hover
  opens the same PRCIPopover anchored to the chip's top edge so the
  card expands upward — matching the user's "open bottom-up" ask. The
  chip mounts the background-sync hook so the popover is fresh on
  first hover even when the top-bar PR button isn't visible.
- elapsedLabel: "just now" → "updated just now" so the footer reads
  the same regardless of how recently the cache was refreshed.

* fix(web): chip color was missing — use static class map for Tailwind JIT

\`getPRStatusColor().replace("text-", "bg-")\` produced class strings at
runtime, which Tailwind's content scanner doesn't pick up — the chip
rendered with no background. Replace with a static lookup so each
literal `bg-*` class appears in the source and gets compiled into the
bundle.

* fix(web): chip uses currentColor SVG so the dot actually renders

The previous bg-* approach relied on Tailwind compiling the literal
class strings. Even with a static map some palette entries (notably
bg-muted-foreground) didn't surface as a usable utility, so the chip
remained transparent. Apply getPRStatusColor's text-* class to the
button wrapper and fill an SVG circle with currentColor — the text-*
utilities are already compiled (PRTaskIcon uses them) so the dot
is guaranteed to render in the right color.

* fix(web): redesign PR status chip — clipboard icon + status glyph

- Move chip to right of TodoIndicator (was left).
- Render as "[clipboard icon] [status glyph]" instead of a bare circle:
  passed → green CircleCheck, failed → red CircleX, in_progress →
  yellow Loader2 (spinning), merged/neutral → muted dot. Status icons
  reuse already-compiled text-* utilities so colors render reliably.

* fix(github): ReviewCount = approved-reviewer count + ReviewCountsPopulated guard

Two bugs feeding the popover's "Approved (0)" misreport:

- Semantic: REST path set ReviewCount to len(reviews), which double-counts
  authors who left multiple reviews and includes COMMENTED rows. Replace
  with countApprovedReviewers — distinct authors whose latest review
  state is APPROVED. GraphQL converter gets the parallel
  countApprovedReviewerNodes helper and also populates PendingReviewCount
  from ReviewRequests.TotalCount (was unset, so the chip's "(K requested)"
  suffix was always missing).

- Clobber: GraphQL polls used to write ReviewCount/PendingReviewCount=0
  on every cycle because the converter never set them. Add
  ReviewCountsPopulated to PRStatus and gate SyncTaskPR's writes on it
  the same way ChecksPopulated already does, so a partial sync path
  preserves the persisted values instead of resetting them to zero.

* fix(web): popover nitpicks (icon, review row, indent, badge)

- Chip icon: ClipboardCheck → ListCheck.
- PRReviewRow always renders (drop the early-return). Fresh PRs with
  no approvals + no protection rule now show "Awaiting review" instead
  of vanishing.
- Indent workflow rows under the In Progress / Failed group headers
  (pl-5) so the hierarchy reads more clearly.
- In-progress workflow badge: "0/1 ran" → "1 running". The old phrasing
  hid whether the job had even started.

* feat(web): stacked pass-rate progress bar in CI popover

Mirrors TodoIndicator's progress bar at the top of the CI section, but
split into three coloured segments:
  green (passed) | yellow (in_progress) | red (failed)
sized proportionally to each bucket's count. The label reads
"Pass rate · K/N (P%)" so the user gets the same at-a-glance summary
TODOs shows. Skipped/cancelled-but-not-failed jobs aren't counted —
the bar always reflects the actionable picture.

* fix(web): right-align review counts + drop chip hover bg

- Move approval/required counts to the right edge of the review row
  ("Approved" on the left, "1 / 2" on the right), mirroring the
  bucket-row layout. The "(K requested)" suffix becomes a "· K requested"
  trailing fragment on the right side.
- PRStatusChip: drop hover:bg-accent/50 so the trigger doesn't grow a
  background plate when the popover is open.

* fix(web): switch CI chip icon to IconChecklist

* fix(github): clear bot review-comment backlog

Real correctness fixes:
- pr-status-chip: pending checks / pending review now beat
  checks_state === "success" in chipStatus precedence — a PR with
  CI green but reviewers outstanding is no longer rendered as
  "passed".
- branch_protection: drop the unreachable Reset() method.
- pr-topbar-button: stop double-mounting usePRFeedbackBackgroundSync;
  the chip already covers it.
- use-pr-ci-popover: dedupe key combines prFeedbackKey + updated_at
  so switching tasks with same-timestamp PRs still refetches.
- use-pr-ci-popover: drop stale comment about the removed progress bar.

Refactor:
- client_helpers + graphql: extract reviewSample +
  latestReviewStateByAuthor helper so the three near-identical
  per-reviewer dedup loops share one implementation.

Tests:
- countApprovedReviewers + reduceReviewSummary unit tests.
- TestSyncTaskPR_ReviewCounts_PreservesOnLightweightSync +
  AcceptsRealZero mirror the existing ChecksPopulated guards.
- groupChecksByWorkflow distinct-URL test verifies the failed-job
  URL-selection fix.
- branch_protection TTL tests now use testing/synctest — instant
  virtual time, no real-sleep flake on CI.

* fix(web): clicking the CI chip no longer closes the popover

HoverCard treats the trigger button as outside the content's bounding
box, so a click on the chip fired Radix's onPointerDownOutside and
auto-closed the popover. Add a triggerRef + onPointerDownOutside guard
on HoverCardContent that filters out clicks landing on the trigger —
clicking the chip is now a no-op while the popover stays open via hover.
2026-05-11 16:55:58 +01:00
José Almeida 1733014721 feat(web): prettify Kandev MCP tool titles in chat (#858)
* feat(web): prettify Kandev MCP tool titles in chat

Render `create_task_kandev` as "Kandev: Create Task" instead of the
raw underscored tool name. Frontend-only display transform via a
regex-gated helper so non-Kandev tool names (Read, Bash, other MCP
servers, Claude-supplied human titles) pass through unchanged.

* fix(web): update e2e assertions for prettified Kandev tool titles + trim pass-through

- E2E `config-management.spec.ts` asserted raw `*_kandev` names in chat;
  update to the new "Kandev: ..." display so Shard 6/10 passes.
- `prettifyToolTitle` now returns `trimmed` instead of `raw` on the
  non-match path so whitespace handling is symmetric across both
  branches (Greptile review).

* fix(web): expand collapsed tool-call groups in e2e config-management assertions

Once a turn completes, consecutive tool-call messages collapse into a
"{N} tool call(s)" group header, hiding the individual prettified titles
the test asserts against. The previous tests passed by lucky substring
match against the script directives in the user prompt; once
prettification removed `*_kandev` from the chat surface that
coincidence broke. Add an `expandToolCallGroups` helper that clicks
each group header before running the title visibility assertions.

* refactor(web): trim ACRONYMS to only entries used by current Kandev tool catalog

Per "no speculative code" guideline (Claude review): only `mcp` is
produced by the current 29-tool catalog. Remove the unused
id/url/api/ui/vs/ide/cli entries; add new ones when a catalog tool
first needs them.

* fix(web): strip MCP namespace prefix when prettifying Kandev tool titles

Codex passes the tool name as `<server>/<tool>` (e.g.
`kandev/get_task_plan_kandev`), so the bare-suffix regex didn't match
and the raw underscored name leaked through to the chat. Split on
`/` or `__` and prettify the trailing segment, covering Codex's
`server/tool` and Claude's `mcp__server__tool` naming alike.
2026-05-10 18:50:32 +01:00
Luan Martins 1066ac5b16 feat: extend implement plan button with fresh-agent path and server-side plan mode (#832)
* feat: extend implement plan button with fresh-agent path and server-side plan mode

- Split-button: primary click = same-session, chevron = fresh agent with clean context window
- Forward draft attachments and context files on implement (adds getAttachments() to handle)
- Authoritatively clear plan_mode via new session.set_plan_mode WS endpoint, not a message flag
- Add on_exit:disable_plan_mode to plan-and-build.yml plan step (mirrors feature-dev.yml)
- Drop localStorage plan-mode rehydration; server session metadata is the single source of truth
- Delete dead DocumentPanel component (no callers)
- Add SetSessionPlanModeByID on orchestrator Service; wire wsSetPlanMode handler

* test: fix store isolation in useImplementFresh test + extract duplicate strings

- Fix test isolation: reset store directly and clear mocks before rendering
- Extract sessionId, appFilePath, appFileName to constants to fix ESLint duplicate-string warnings

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

* test(e2e): cover implement-plan fresh-agent split-button path

Adds Playwright coverage for the new chevron menu on the violet Implement
button: asserts a brand-new TaskSession is launched on the same task with
the planning session's agent profile inherited, and that the planning
session is left running in parallel. Second spec verifies the split-button
is gated behind plan mode.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: defer plan-mode exit and surface error toast on implement failures

Move handlePlanModeChange(false) into the message.add success callback in
useImplementPlan so a failed send leaves the plan layout intact for retry.
Add error toast to useImplementFresh catch so launchSession failures are
visible to the user instead of silently logged.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

* fix: guard fresh-agent launch on planning session executor_id

TaskSession.executor_id is optional, so the fresh-agent path could call
launchSession with an undefined executor and silently inherit the
workspace default instead of the planning session's executor. Add the
field to the early-return guard so the hook no-ops when the planning
session is missing either an agent profile or an executor.

* feat: mark fresh-agent session as primary and focus it

When useImplementFresh launches a new implementation session, the fresh
session is now automatically:
1. Set as primary via session.set_primary WS endpoint
2. Focused/selected as the active session in the UI

This improves UX by switching the focus to the fresh implementation context
immediately rather than keeping the planning session active.

Addresses comment in PR #832 from carlosflorencio.

- Extract setupFreshSession helper to keep complexity under 15
- Add tests for primary session marking and UI focus
- Add test for graceful handling of set_primary failures
- Update JSDoc to document the new behavior
- Extract test constants to avoid duplicate string warnings

* fix(test): replace self-referential test constants

`const TASK_ID = TASK_ID;` (and SESS_PLAN, SESS_FRESH) caused a TDZ
ReferenceError at module load, so vitest reported `(0 test)` and none of
the new fresh-agent coverage actually ran. Replace with literal values
so the 13 tests in this file execute again.

* fix: drop strict executor_id guard and skip composer clear on missing session_id

Two related issues:

1. e2e regression — the planning session in the FE store does not always
   carry executor_id (the field is omitempty on the backend DTO and isn't
   populated for fresh task sessions in the WS push path). The earlier
   guard required it before calling launchSession, so the fresh-agent
   path silently no-op'd in real flows. Drop executor_id from the guard
   and only forward it when set; backend resolves it from the agent
   profile / workspace default when omitted.

2. PR review (Greptile P1) — when launchSession resolves without a
   session_id, the previous flow still cleared the composer and draft
   even though no fresh session was activated. Bail out before the
   clear so the user keeps their typed text and can retry.

Tests updated:
- replace the no-op-on-missing-executor test with one that asserts the
  hook still launches and omits the key from the payload
- add coverage for the no-session_id branch keeping the composer intact

---------

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-10 17:30:24 +01:00
José Almeida 21202ef437 fix(web): mobile task switcher sheet skeleton while snapshot loads (#860)
* fix(web): show skeleton in mobile task switcher sheet while snapshot loads

When the user opens the mobile task switcher inside a task page before the
workflow snapshot has hydrated kanban, the sheet rendered "No tasks yet."
instead of a loading state — and stayed that way if the fetch silently
failed.

useWorkflowSnapshot now drives kanban.isLoading, useTasks exposes it as
{ tasks, isLoading }, and the sheet wires that into TaskSwitcher's existing
skeleton. Fetch errors are surfaced via console.warn so an indefinite empty
list isn't silent anymore.

* fix(web): guard snapshot finally on cancel and drop redundant subscription

Skip the workflow-snapshot fetch's finally block when the effect was superseded by a newer workflowId — otherwise the old fetch's settle would clear the loading flag the new effect just set, briefly flashing the empty-state UI mid-switch.

Also drop the redundant kanbanIsLoading subscription in useSheetData (useTasks already encapsulates it) and make the setState mock in use-tasks.test.ts actually mutate state so useWorkflowSnapshot's side effects are exercised by the test.

* fix(web): scope snapshot finally clear to the effect that raised the flag

If kanban already matched the requested workflowId, the effect skipped raising isLoading=true — but its finally still cleared it on settle, collapsing a skeleton a concurrent caller (e.g. workspace switch) had set independently. Track whether this effect raised the flag and only clear in that case.

Also collapse the multi-line useTasks comment per CLAUDE.md style.

* fix(web): suppress superseded snapshot fetch warnings

Skip the catch-handler console.warn when the effect has been cancelled — a stale fetch shouldn't surface noise after a newer workflow has taken over.

* test(web): flush microtasks explicitly in race test

waitFor on a never-true assertion resolves immediately, hiding the intent. Use explicit Promise.resolve() flushes to land the superseded fetch's .then/.finally chain before asserting.

* style(web): collapse multi-line comments per CLAUDE.md
2026-05-10 17:02:40 +01:00
José Almeida 3c9b835ba1 docs(web): correct isPreparingEnvironment JSDoc after STARTING re-block
The 9389ac9 fix restored isStarting as the editor disabled gate.
Update the JSDoc on isPreparingEnvironment to match: it now drives
the "agent still being set up" submit-disabled tooltip only — the
disabled state itself is gated on the broader isStarting so e2e
Cmd+Enter doesn't race the not-yet-ready agent.
2026-05-10 15:31:19 +01:00
José Almeida 9389ac9c79 fix(web): keep editor disabled during STARTING so quick-chat sends don't race
My previous fix made submit no longer block on isStarting, with the
goal of keeping local quick-chat sessions from silently dropping
messages. The trace from CI showed it traded one failure for another:
the editor became contenteditable while the session was still
STARTING, the e2e suite pressed Cmd+Enter immediately, and the
backend rejected with "Failed to send message to agent" because the
agent process wasn't ready yet.

Restore the original semantics — STARTING blocks both editor and
submit — but keep `isPreparingEnvironment` as the narrower trigger for
the "agent still being set up" tooltip. Net behaviour:
  - STARTING transition: editor disabled, no tooltip (signal is the
    disabled state itself)
  - container/sandbox prepare: editor disabled + tooltip explaining why
  - quick-chat sessions: editor stays disabled until RUNNING, no race
2026-05-10 15:19:50 +01:00
José Almeida 7851052634 fix(web): only block submit during real env prepare, not every STARTING
The "agent still being set up" submit gate was triggering on every
session that briefly transitions through STARTING — including local
quick-chat, which has no prepare phase and has its agent live within
~100ms. The eager-init quick-chat e2e tests (chat/quick-chat.spec.ts
and chat/message-queue.spec.ts) wait for the editor to become
editable, then press Cmd+Enter. The race made wrappedSubmit a no-op
exactly when the test was firing — message dropped, agent never saw
it, downstream assertions failed.

Split the merged isStarting flag in useSessionState: keep the broad
flag (still used for the workspace-prep loading UI), and expose
isPreparingEnvironment as a separate signal that's only true while
prepareProgress.status === "preparing". Thread it through chat-input-
area → chat-input-container → useChatInputContainer and gate the
submit-disabled tooltip on it instead of the broad isStarting.

Net: Docker / Sprites bootstrap still surfaces the "agent still being
set up" message and disables submit; quick-chat / local sessions stop
silently dropping in-flight messages.
2026-05-10 15:00:41 +01:00
José Almeida 462862f5e7 Merge branch 'main' into feature/reuse-executors-gmk
Resolves conflicts from concurrent main work:
  - helpers.go: keep our 4-arg RegisterDockerRoutes (with task-title
    provider) and bring in main's Slack route registration block.
  - service_turns_test.go: keep both lifecycle and events imports.
  - kanban-card.tsx: take main's slim version — main extracted
    KanbanCardContextMenu / KanbanCardShell / kanban-card-content into
    sibling files which the new tree depends on. Our PR's executor
    visualization (RemoteCloudTooltip) lives in kanban-card-content.
  - passthrough-terminal.tsx + use-passthrough-terminal.ts: keep our
    onDisconnected callback alongside main's manualInputRouting and
    onWsReady (mobile keyboard input routing). Adopted main's
    teardownWebSocket helper and dropped the now-unused
    cleanupTerminalConnection.
  - event_handlers_linear.go: switch to existing
    models.MetaKey{Agent,Executor}ProfileID constants so the merged
    diff stays under the goconst threshold.
2026-05-10 14:01:13 +01:00
Carlos Florêncio 40c89cd380 fix(web): preserve dockview state across task and plan-mode switches (#855) 2026-05-09 11:59:16 +01:00
Carlos Florêncio 9664e13f07 fix(web): keep "All Workflows" filter on task nav and workflow.created (#854) 2026-05-08 21:23:48 +01:00
Carlos Florêncio dfcaa24be3 feat: allow subtasks to target a sibling repository (#852) 2026-05-08 19:05:10 +01:00
José Almeida cf3dbc9582 fix(web): keep command palette selection on first result (#845)
* fix(web): keep command palette selection on first result

* test(web): fix launch session mock response shape

* refactor(web): address command palette review feedback

* fix(web): align command panel selection setter type
2026-05-08 16:32:06 +01:00
Carlos Florêncio 0d3fce25b4 fix(web): stop dockview layout corruption when switching between maximized / sessionless tasks (#838) 2026-05-07 18:43:07 +01:00
José Almeida 9ed3e64a91 test(web): hoist test constants above vi.mock factories
Move SESSION_ID and friends above the vi.mock factories so the mock
closures don't depend on vitest hoisting magic capturing TDZ-undefined
locals. The previous layout worked because factories aren't invoked
until the module under test is imported (after const initialization),
but a refactor that imports earlier would break the closure silently.
2026-05-07 11:17:24 +01:00
José Almeida c0b75864ba refactor(web): split executor-settings-button by concern
Slice the 471-line button file into three modules with single
concerns: the popover shell stays in `executor-settings-button.tsx`
(168 lines, just trigger + popover layout + icon/status dot),
`executor-environment-info.tsx` owns the env detail rows + status
badge, and `hooks/domains/session/use-task-environment.ts` owns the
fetch/poll/reset lifecycle alongside other session hooks.
2026-05-07 00:49:43 +01:00
José Almeida 1f9ead5dc5 refactor(web): lift prepare-summary reducer + isFallbackNoticeStep into lib/prepare
Move the popover's prepare-summary reducer (and the
`isFallbackNoticeStep` helper duplicated word-for-word in
`prepare-progress.tsx`) into `apps/web/lib/prepare/summarize.ts`. The
hook moves to `hooks/domains/session/use-prepare-summary.ts` to match
the project's domain-organized hook layout.

The popover's PrepareSummary type and the chat panel's EffectiveStatus
serve different UIs, so the panel keeps its own `deriveStatus` — only
the literal duplicate (`isFallbackNoticeStep`) is shared.
2026-05-07 00:44:44 +01:00
José Almeida 948743600e refactor(cleanup): drop dead code, dedupe predicates, unify token injection
Tier 1 of the post-review cleanup. Each item is independently safe; bundling
because they're all small, mechanical, and lint-passing.

- fix(clone): unify injectTokenIntoURL across Docker and Sprites — Sprites
  used to ignore GH_TOKEN entirely (a user with only GH_TOKEN set got 401
  on Sprites clone but worked on Docker). Promote one
  `injectGitHubTokenIntoCloneURL` helper, accept both env vars
  (GITHUB_TOKEN wins, matching Docker), and use the documented
  `https://x-access-token:T@github.com/` form on both surfaces.
- fix(executor-popover): swap the <span tabIndex role="button"> trigger
  for <Button variant="ghost"> so Enter/Space activation, focus rings on
  Safari, and disabled semantics come for free.
- test(docker): rewrite TestDockerStopInstance_PreservesSessionDirOnPlainStop
  to drive the real DockerExecutor.StopInstance path with ContainerID=""
  (skips the docker daemon, exercises the kandev-side cleanup branch).
  Previous version only iterated shouldRunExecutorCleanup strings — would
  pass even if StopInstance always cleaned up. Now table-driven over
  preserve+destructive reasons with on-disk asserts.
- refactor(orchestrator): drop dead `_ = arg` parameters from persist*
  helpers (persistResumeState, persistModelSwitchState, persistLaunchState).
  Brings signatures from 9→6 args, under the funlen lint cap.
- refactor(orchestrator): collapse byte-identical isContainerizedExecutor
  ≡ isCloneBasedExecutorType predicate into one.
- refactor(lifecycle): drop dead getMetadataBool, FileUploader.RunCommand,
  expandMountSource {home} arm (no production agent ships {home} in
  Mounts; codex_acp_test asserts it), dockerTaskBranch one-line wrapper.
2026-05-07 00:29:12 +01:00
José Almeida ac7b35a497 fix(web): move executor info to the left top bar, hover-to-open, no border
The executor section is informational, not a button — readiness state +
sandbox/container/branch fields. Move it to the left top bar (next to
the integrations menu) where it sits alongside the other contextual
info, drop the outline-button styling, and switch from click-to-open to
hover-to-open via HoverCard with 150ms open / 250ms close so the cursor
can travel to the destructive Reset button without flickering.

The trigger is a borderless icon-with-status-dot span; tabIndex keeps
it keyboard-accessible. Reset still works — it opens the existing
confirmation dialog independently of the hover card.

Tests: pointerEnter instead of click for the hover-to-open path.
2026-05-06 23:43:52 +01:00
José Almeida f8dd01b5a9 fix(web): drop the redundant Environment-ready row + tighten popover top
The READY badge next to the executor name already conveys ready-state;
the dedicated "Environment ready · 12s" row above the executor info
just added vertical noise. Skip rendering for the ready phase (other
phases that surface non-redundant info — preparing, resuming, failed —
still render). Drop top padding on the executor info section now that
nothing sits above it on the happy path.
2026-05-06 23:36:01 +01:00
José Almeida 12956f1e9c fix(web): tighten executor popover bottom padding too
The previous tighten only fixed the top half — pb under the field rows
plus the Reset-row's full p-2 still left a generous gap below the last
"Sprite ..." line. Same fix applies whether the popover is showing
Docker or Sprites.

EnvironmentInfo: pt-2 pb-1.5 (was py-2) so the bottom field hugs the
divider, and the Reset-row goes from p-2 to px-2 py-1.5 so the button
sits closer to the text above.
2026-05-06 23:24:58 +01:00
José Almeida 8673449af8 fix(web): tighten executor popover padding under the env header
The Sprites/Docker info block under the Environment-ready row had too
much vertical breathing room — the header-to-fields gap and the
inter-row spacing piled up and the popover felt airy. Trim the section
to match the snug rhythm of the rest of the popover (px-3 py-2,
1.5-unit gap before the field list, 1-unit gap between rows). Also
unify the Environment-ready row from p-3 to px-3 py-2 so the dividers
between sections sit at the same vertical cadence.
2026-05-06 23:11:50 +01:00
Carlos Florêncio 7642b984c6 feat(web): show grid spinner on agent tab while session is working (#836) 2026-05-06 20:25:34 +01:00
José Almeida 7ec639bf2f feat(web): mobile parity (session/terminal/repo pickers + multi-terminal) (#840)
* refactor(web): extract shared use-session-actions hook from SessionTab

Move WS-level session lifecycle handlers (set primary, stop, resume,
delete) into hooks/domains/session/use-session-actions so the same logic
can power both the dockview tab context menu and upcoming mobile
controls. Dockview-specific panel removal stays local in SessionTab via
the onDeleted callback.

* feat(web): humanize task and session state labels

Mobile task sheet was rendering raw enum values like "IN_PROGRESS".
Add formatTaskStateLabel + formatTaskSessionStateLabel helpers and use
them in the mobile task sheet badge. Session state badge in the new
mobile sessions picker also reuses the formatter.

* feat(web): mobile session/terminal/repo pickers + multi-terminal support

Address parity gaps on mobile by introducing per-pane header pills that
open bottom-sheet pickers, scoped to the relevant view rather than
overloading the left task sheet.

Pickers:
- Sessions pill in chat header → bottom sheet with launch / set-primary
  / stop / resume / delete (with confirm). Pill shows agent label +
  idx/total badge.
- Terminals pill in terminal header → bottom sheet with add / close
  (with confirm) / switch. Replaces an earlier chip-strip prototype.
- Repos pill on the top bar (multi-repo only) → bottom sheet listing
  task repositories; switching jumps to a session on that repo.

Each pill renders with a chevron-down icon + count badge so the
trigger reads as a button, not a status badge.

Multi-terminal:
- New useMobileTerminals hook wraps useTerminals and auto-creates a
  first shell when the env reports zero.
- MobileTerminalPane multi-mounts PassthroughTerminal instances keyed
  by terminalId so switching preserves scrollback. Active xterm
  registers itself in a small mobile-active-terminal sender registry;
  the existing key-bar reroutes input through xterm.paste() so Ctrl /
  ^C / arrows reach whichever terminal is currently visible, not just
  the per-session default shell.
- Skip the WebGL renderer addon on mobile (new disableWebgl prop on
  PassthroughTerminal). The WebGL atlas miscomputed glyph scaling
  after the desktop→mobile responsive switch and rendered text at
  several times the intended font size; the canvas renderer is correct
  and fast enough for shell use.

Refactors:
- Left sheet reverts to a single-purpose task list (the sectioned-hub
  variant was rolled back in favour of per-pane pickers).
- session-task-switcher-sheet-hooks extracted from the sheet to keep
  it under the file-size lint cap.

* fix(web): address review feedback on mobile pickers + multi-terminal

Bots flagged a real first-mount bug and a handful of smaller items;
addressed each:

- mobile-terminal-pane: track xterm in state instead of a ref so the
  active-sender registration effect re-runs once xterm is ready. With
  the ref, the effect ran once on mount, found xtermRef.current === null
  while PassthroughTerminal was still initializing (notably under the
  ResizeObserver fallback path when the container starts at 0×0), and
  silently never registered — leaving the key-bar wired to the default
  shell instead of the visible terminal.
- send-shell-input: wrap the activeSender call in try/catch so a stale
  registry entry (xterm GC'd mid-frame) falls back to the WS path
  instead of dropping the keystroke and silently consuming modifiers.
  Added tests for the activeSender routing branch (success, modifier
  consumption, fallback on throw).
- mobile-sessions-section: remove the dead `showCloseConfirm ? "Delete" : "Delete"`
  ternary and the unused prop on SessionActionsMenu.
- use-mobile-terminals: clear the auto-create guard when addTerminal()
  rejects so the user gets a retry instead of an empty terminal list.
- mobile-repo-pill: sort taskRepos by position before falling back to
  the first repo, so the pill label matches the picker's ordering when
  no session is active yet.
- session-task-switcher-sheet-hooks: catch loadTaskSessionsForTask
  rejection in handleSelectTask (was an unhandled rejection that left
  the sheet open on backend errors), and stabilize useCallback deps in
  handleWorkspaceChange by spreading the SheetNavOptions fields so a
  fresh literal each render doesn't defeat memoization. Extracted
  selectTaskWithoutPrimarySession to keep useSheetActions under the
  function-size lint cap.

* fix(web): preserve task.repositories in sheet-hooks kanban mappers

mapSnapshotToKanban (workspace switch) and handleTaskCreated (newly
created task) only carried the first repository_id, dropping the full
TaskRepository array. The mobile repo picker (useTaskRepoCount +
MobileReposSection) reads task.repositories.length, so it silently
disappeared after a workspace switch and for freshly created
multi-repo tasks. Map the array with the same shape the SSR mapper
uses (id, repository_id, base_branch, checkout_branch, position).

Also address a small nit: wrap addTerminal in an arrow at the New
terminal button so the click MouseEvent isn't passed as the (unused)
first argument.

* fix(web): de-dup mobile terminal auto-create + add state-label tests + repo-tap toast

- use-mobile-terminals: move the auto-create-first-shell guard from
  per-instance refs to a module-level Set keyed by environmentId.
  MobileTerminalPane mounts useMobileTerminals three times for the
  same session (the pane itself, the picker pill via
  useActiveTerminalPillLabel, and the picker sheet content via
  MobileTerminalsList — Vaul keeps it mounted while closed). With per-
  instance guards, all three effects ran on first commit and each
  fired createUserShell, racing into up to three shells. The shared
  Set ensures only the first effect to observe an empty list creates,
  with retry-on-failure preserved (delete + next render re-checks).
- mobile-repos-section: tapping a repo with no sessions now toasts a
  hint to open the session picker, instead of silently doing nothing.
- state-labels: add tests covering known mappings, null/undefined
  fallbacks, and unknown-state passthrough for both formatters.

* fix(web): single useMobileTerminals via context + tests + drop dead export

Address two follow-up review items:

- Lift useMobileTerminals to a single MobileTerminalsProvider owned by
  MobileTerminalPane. The picker pill and the picker sheet's list now
  read state from useMobileTerminalsContext instead of calling
  useMobileTerminals themselves. The previous setup had three
  independent useState-backed lists that drifted out of sync after
  addTerminal/removeTerminal — the pane could show a stale active
  terminal and the pill count could lag until a server WS push
  reconciled them. The module-level guard (added in 9488bb0c) fixed
  the creation race; this fix ensures the live list stays consistent
  too.
- Drop the dead `subscribeActiveTerminalSender` export from the mobile
  active-terminal registry. The keybar reads via getActiveTerminalSender;
  notify/subscribe was speculative scaffolding with no production
  consumer. Test trimmed accordingly.
- Add use-mobile-terminals.test.ts covering the auto-create logic
  (initial create, skip when shells exist, skip while loading,
  multiple instances trigger only one creation, retry after failure).
  Module-level guard exposes a __resetAutoCreatedEnvironmentsForTest
  helper so tests start from a clean slate.

* test(web): capture binary AttachAddon frames in mobile keybar e2e helper

The mobile-terminal-keybar Playwright spec asserted on captured
shell.input WS frames. After this PR, the on-screen mobile terminal is
a PassthroughTerminal with its own AttachAddon WebSocket, and the
keybar routes input through xterm.paste() — bytes go on the binary
terminal WS, not the kandev gateway's shell.input action. The capture
helper missed those frames and every keybar-asserts-on-frame test
failed (Ctrl+L, Shift+Tab, Esc, arrows, symbols, etc.).

Extend attachShellInputCapture to also decode binary terminal frames
(skipping the resize tag 0x01 prefix used by useSendResize) so the
existing assertions on `data` continue to work regardless of which
transport carried them.

* fix(web): keybar control chars + session pill propagation + terminal close ordering

Round-3 review feedback. Several real bugs that the previous rounds'
mocks hadn't caught:

- mobile-terminal-pane: switch from xterm.paste(data) to
  xterm.input(data, true). paste() wraps the payload in bracketed-paste
  escapes (\\e[200~…\\e[201~) under bash/zsh/fish, so every control
  byte from the keybar (^C, arrow keys, Esc, ^D, Tab) was landing in
  the shell as literal pasted text instead of as a control signal.
- session-mobile-layout: TaskChatPanel and the agent PassthroughTerminal
  were rendering with the URL-derived `sessionId` prop instead of the
  store-driven `effectiveSessionId`. The session picker would update
  the top bar and terminal pane (which already read effectiveSessionId)
  but the chat content and passthrough agent stayed on the old session
  until a navigation refresh. Both consumers now follow
  effectiveSessionId; the now-unused `sessionId` prop is removed from
  MobileChatPanelContent + MobilePanelArea.
- mobile-terminals-section: rework handleConfirmClose. Stop the remote
  shell first (and only mutate the picker on success), advance
  setRightPanelActiveTab to a remaining terminal when the closed one
  was active so terminalTabValue doesn't dangle, and call
  releaseAutoCreatedEnvironment when terminals drop to 0 so the pane
  recreates a default shell on next render instead of getting stuck
  on "Starting terminal…". Extracted as a useTerminalCloseHandler hook
  to keep the list under the function-size lint cap.
- session-task-switcher-sheet-hooks: handleTaskCreated now merges
  session-derived fields (primarySessionId, sessionCount, etc.) from
  the existing kanban entry, with meta.taskSessionId as the priority
  source — so an "edit" call doesn't wipe the existing session and a
  "create with session" call sets primarySessionId correctly.
  Extracted into buildKanbanTaskUpsert + mergeSessionFields +
  mapTaskRepositories helpers.
- session-task-switcher-sheet-hooks: mapSnapshotToKanban now carries
  optional step fields (allow_manual_move, prompt, is_start_step,
  show_in_command_panel, agent_profile_id) so the kanban store keeps
  step capabilities after a workspace switch.
- use-mobile-terminals: add releaseAutoCreatedEnvironment(envId) — a
  runtime API to clear the module-level guard for one env, used by the
  picker when the user closes the last terminal.
- use-mobile-terminals.test: extract SESSION_ID constant.

* fix(web): restore mobile keybar modifier transforms with manual input routing

Round-3 swapped mobile from ShellTerminal → PassthroughTerminal, which
made AttachAddon auto-send xterm.onData straight to the WebSocket.
That bypassed sendShellInput's modifier transform — so OS-keyboard
input on mobile (e.g. tap Ctrl on the key-bar, then press "l") sent
the raw "l" instead of "\\x0c" (form feed). Multiple e2e keybar tests
caught this; my prior xterm.input(data, true) attempt only fixed the
key-bar tap path, not OS-keyboard typing.

Add `manualInputRouting` + `onWsReady` props to PassthroughTerminal
(plumbed through useWebSocketConnection / startReconnectLoop /
connectWebSocket). When `manualInputRouting` is true, AttachAddon is
constructed with `bidirectional: false` so it only handles PTY → screen,
and the consumer is responsible for forwarding xterm.onData itself.

Mobile TerminalSlot now:
- passes manualInputRouting=true and an onWsReady callback that
  registers an active-terminal sender writing raw bytes to that
  terminal's dedicated WS (replacing the prior xterm.input registry).
- attaches its own xterm.onData listener that calls sendShellInput,
  which applies the key-bar's sticky Ctrl/Shift transforms before
  forwarding via the registered sender.

Net effect: both key-bar taps AND OS-keyboard typing flow through
sendShellInput on mobile, so modifiers apply uniformly. Desktop
unaffected (default bidirectional AttachAddon path unchanged).

Also extracted teardownWebSocket into ws-reconnect to keep the
use-passthrough-terminal file under the line-count cap after the
new options.

* fix(web): tighten mobile sender registration + Set Primary + launch return

- mobile-sessions-section: drop the borrowed isSessionStoppable guard
  on the "Set as Primary" item. The backend session.set_primary action
  has no such restriction, so a COMPLETED or FAILED session was being
  silently grayed out. Disable only when isPrimary or state isn't
  loaded yet.
- mobile-terminal-pane: register the active key-bar sender only while
  the dedicated WebSocket is OPEN. Previously the sender went into the
  registry as soon as the tab became active, so during connect /
  reconnect it would silently no-op instead of letting sendShellInput
  fall through to its WS path. Track ws in state, drop it on close,
  and gate registration on readyState === OPEN.
- session-task-switcher-sheet-hooks: early-return after a successful
  launchSession so we don't redundantly call setActiveTask after
  setActiveSession. Mirrors the existing "sessions exist" branch.
2026-05-06 19:05:21 +01:00
João Salavisa aa6023e421 feat(web): per-task color indicator in sidebar (#835)
* feat(web): per-task color indicator in sidebar (localStorage)

Adds a color marker to tasks in the left task list for better visual
identification. Right-click a task → Color submenu picks one of seven
colors (or None to clear). Color renders as a 3px bar on the left edge
of the row and persists in localStorage.

* fix(web): address review feedback on task color indicator

- Filter storage events to TASK_COLORS_STORAGE_KEY so cross-tab writes
  to unrelated keys no longer wake every TaskItem subscriber.
- Cache the parsed color map at module level, invalidated on writes and
  cross-tab storage events. Avoids re-parsing localStorage on every
  getSnapshot call (N tasks × N renders).
- Drop redundant typeof string check on Object.entries keys.
- Dim the colored bar to opacity-60 when not selected so colored rows
  remain distinguishable from colored-and-selected rows at the left edge.

* refactor(web): remove test-only export from task-colors

Drop the __resetTaskColorsCacheForTests escape hatch from the
production module. Tests now invalidate the cache via a synthesized
cross-tab `storage` event, exercising the same listener path the
production code uses for real cross-tab updates.

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
2026-05-06 19:05:19 +01:00
José Almeida ce49d6dfec feat(web): show "Resuming session" feedback in popover during reconnect
The bottom Resume button on the chat panel was leaving users staring at
nothing for the 5-30s reconnect window: prepare-progress events are
intentionally suppressed on resume (the workspace is already there), so
the popover sat in the idle phase even while session.state was STARTING.

Synthesize a "resuming" phase from session state when no prepare events
are flowing, rendered as a spinner row in the popover with copy
"Resuming session — Reconnecting to the existing environment…". The
button icon also flips to the spinner so the toolbar advertises the
in-flight work without the user having to open the popover.

Extract the prepare-status reducer + section component into their own
file so the main popover module stays under the 600-line ESLint cap.
2026-05-05 23:14:28 +01:00
José Almeida d35e601d42 test(docker): cover stop + resume → same container, branch preserved
Adds a Docker e2e test that exercises the user-visible stop-then-resume
flow: launch session, stop it via WS session.stop, launch a fresh
session on the same task, and assert the same task environment +
container ID are reused and the kandev feature branch is preserved.

Also nudge the executor popover padding: when the prepare-status
section is rendered above the Sprites/Docker info block, the cumulative
top+bottom paddings looked too generous around "Sprites sandbox" /
"Local Docker". Tighten both sections vertically without affecting the
horizontal rhythm or the bottom action row.
2026-05-05 23:05:22 +01:00
João Salavisa 9fe2cbd874 fix: source commit pushed status from git remote, not PR commits (#833)
* fix: source commit pushed status from git remote, not PR commits

Commits showed as "not pushed" whenever the branch had no open PR,
since the UI derived pushed status by matching local SHAs against
the PR's commit list. Sourcing it from the upstream tracking ref in
agentctl's GetLog keeps the answer correct without a PR, after
rebases, and across multi-repo workspaces.

* fix: treat PR-matched commits as pushed alongside the git remote signal

mergeCommits now flags a commit as pushed when EITHER the backend's
git-sourced flag is true OR a PR commit matches by SHA. The git signal
fixes the original "no PR = unpushed" bug; the PR signal preserves the
"PR exists ⇒ commit is on remote" assumption that the E2E mocks rely
on (and that holds in practice, since GitHub PRs require pushed
branches). Also corrects the validation-path comment in
markPushedCommits to describe the actual non-branch fall-through.

* perf: bound rev-list walk in markPushedCommits to result length

Without -n the walk was unbounded — a branch with many local-only
commits would traverse full unpushed history per GetLog call. Capping
at len(commits) keeps the two walks proportional. rev-list walks
newest-first like GetLog, so the cap covers every commit we mark.

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
2026-05-05 22:09:02 +01:00
João Salavisa 5e3248a66b feat(web): pin tasks and drag-to-reorder in sidebar (#829)
Co-authored-by: Kandev Agent <agent@kandev.dev>
2026-05-05 22:06:24 +01:00
José Almeida 4282ba262f feat(web): unify executor surfaces in the right-side popover
The desktop task top bar carried a small cloud icon (RemoteCloudTooltip)
in the breadcrumb cluster while the right-side Executor popover showed
the same kind of info — two surfaces for one concept. Remove the cloud
icon from the desktop top bar; kanban cards, task list rows, and the
mobile top bar keep theirs since they have no popover.

The popover button now picks its icon by executor type (cloud for
sprites, container for docker, server for local PC) using a shared
getExecutorStatusIcon helper. While the prepare progress is preparing
the icon swaps to a spinner so the user can see at a glance that work
is in flight.

The popover content gains a Preparing-environment section that streams
"Step N of M: <step>", an amber inline note when the missing-sandbox
fallback is in progress, an Environment-ready row with duration on
success, and a failed-step summary on failure. Polling promotes to the
active 3s cadence while preparing so the icon flips to ready without
the user opening the popover.
2026-05-05 21:20:05 +01:00
José Almeida bce9fd266a feat: multi-question support for ask_user_question_kandev (#828)
* feat(backend): multi-question support for ask_user_question_kandev

Reshape the MCP tool from a single prompt+options pair to a questions[]
array (1-4 entries, single-choice each, custom text fallback). The
clarification HTTP handler gates the response until every question has an
answer, fans out one chat message per question (sharing pending_id, with
question_index/question_total in metadata), and returns a JSON map keyed
by question id back to the agent. The orchestrator resume prompt now
formats multi-line Q1/A1, Q2/A2 summaries when the bundle has more than
one question. Mock-agent gets a clarification-multi scenario and
ask_user_question_test.go covers the new schema + map response. No
back-compat shim — local-first tool, single-PR breaking change.

* feat(web): refresh clarification overlay with multi-question + modern UI

Each per-question chat message gets its own card with a "Question N of M"
progress chip, a ✓ Answered badge once committed, and shadcn-aligned
option buttons with kbd hints. A new useClarificationGroup hook coordinates
the per-question commits (decision A) and posts every answer in a single
batch once the user finishes the bundle; X rejects the whole group. The
single-question case keeps the 1-9 / Esc keyboard shortcuts. Playwright
e2e adds 6 multi-question scenarios (rendering, full happy path, partial
state, mixed option/custom, skip-rejects-bundle, sibling-state) on top of
the existing single-question regression suite.

* fix(web): clarification carousel UX with stepper navigation + race fix

Replaces the vertically stacked card layout with a horizontal stepper:
numbered circle dots up top show step state (active/answered/pending),
the single active question card sits in the middle, and Back / Next /
Submit buttons sit at the bottom. Picking an option auto-advances to the
next step (single-question case auto-submits, no explicit Submit button).
The stepper steps are clickable so users can revise prior answers.
Keyboard shortcuts: 1-9 for option pick, ArrowLeft/Right for navigation,
Esc for skip.

Also fixes a race that surfaced as a 400 BadRequest on the
/api/v1/clarification/:id/respond endpoint when the user clicked an
option before all N messages of a multi-question bundle had streamed in:
findPendingClarificationGroup now gates on metadata.question_total and
returns an empty array until the full bundle has arrived.

Playwright suite expanded from 6 to 12 multi-question scenarios:
stepper rendering, auto-advance, Back, stepper jump, Submit gating,
happy path, mixed custom-text, revise-via-jump, skip-from-mid-step,
number-key shortcuts, Esc skip, first-step Back disabled.

* fix(clarification): address CodeRabbit/Greptile/Claude review comments

Backend:
- Single source of truth for question validation: rename/export
  NormalizeAndValidateQuestions and call it from both the HTTP and the
  WebSocket-side MCP handlers, so duplicate question ids and bad option
  counts can't slip through either path.
- httpRespond now validates that every answer references a real question
  id from the bundle and that no id is answered twice (regression tests
  added).
- questionsFromMessages reorders by metadata.question_index so the
  reconstructed bundle preserves the original question order.
- adapters.CreateClarificationRequestMessages rolls back already-created
  messages when a later write in the bundle fails.
- extractQuestionAnswers always returns a JSON map (even on rejection,
  with an envelope { rejected, reject_reason }) and simplifyAnswer
  preserves both selected_option and custom_text when both are present.
- MCP tool description: rewrite the response example to match the actual
  output shape; add minItems / maxItems to the questions and options
  arrays in the JSON Schema.
- Test helper: drop the local contains() shim, use strings.Contains.

Frontend:
- useMemo([props]) was a no-op (props is always a fresh ref); split into
  [message, messages] so it actually memoizes.
- Drop the dead "expired" SubmitState branch — the backend never returns
  410 from this endpoint.
- Remove the asymmetric option-list dimming: the agent now receives both
  selected_option and custom_text together, so a half-grey state was
  misleading.
- New tests: pending-clarification.test.ts covers
  findPendingClarificationGroup including the question_total gate, and
  use-clarification-group.test.ts covers the hook's record/submit/skip
  flows and the override-merge path used by the single-question case.

* fix(web): treat clarification 409 as success so duplicate submit doesn't stick the overlay

* fix(clarification): unblock agent on persistence failure + nil-guard rejected branch

- Add Store.CancelRequest(pendingID) and call it from both the WebSocket
  MCP handler and the HTTP create-request handler when message creation
  fails. Without it, the agent's WaitForResponse parked for the full
  2-hour MCP timeout while no clarification cards ever reached the user.
  Both handlers now return an explicit error response on failure.
- applyAnswersToMessages now guards h.repo == nil in the rejected branch
  (parity with sibling expectedQuestionIDs).
- extractQuestionAnswers stubs for unanswered questions in a rejected
  bundle now carry { answered: false, rejected: true } so agents reading
  per-question payloads see the rejection without inspecting the
  envelope.
- Hook test: added 409-as-ok regression coverage for both submit and
  skip paths so the previous fix can't drift.
- Store test: covers CancelRequest unblocking a parked WaitForResponse.

* fix: drop unused e2e helper + use channel sync in CancelRequest test

* refactor(web): drop dead 'message' prop from clarification overlay

Both call sites pass 'messages' (the bundle group), so the legacy
single-message prop was unused. Tightening the prop type also lets the
useMemo depend on a single value.

* fix(web): ignore modifier-keyed events in clarification carousel shortcuts

Bare 1..9 / ArrowLeft / ArrowRight / Esc still drive the carousel, but
Cmd/Ctrl+1..9 (browser tab switch) and Alt+ArrowLeft (browser back) now
pass through as the user expects.
2026-05-05 19:14:57 +01:00
José Almeida c64e835612 feat: allow moving tasks across workflows (#822)
* feat: allow moving tasks across workflows

* fix: address PR review feedback

* fix: address remaining PR review feedback

* fix: remove unreachable move state event

* fix: map task move conflicts correctly

* fix: address PR review nitpicks

* fix: use neutral task move error message
2026-05-05 14:13:43 +01:00
José Almeida 3e676e3bb2 feat(web): double-click tab to toggle maximize (#823)
* feat(web): double-click tab to toggle maximize

Adds a dblclick gesture on dockview tabs that toggles maximize/restore on
the containing group, mirroring the existing maximize button. Preview tabs
keep their promote-to-pin shortcut: first dblclick pins, then dblclick
maximizes. Sidebar group is excluded.

* fix(web): add cursor-pointer to tab wrappers for consistency

* fix(web): add cursor-pointer to plan-tab wrapper

* fix(web): make tab dblclick maximize resilient to layout rebuild

Refactor the dblclick-to-maximize handler so it reads the panel's group ID
and store state at call time instead of from a render-time closure, and
guards against firing while the dockview store is mid-layout-restore.

This addresses oscillation when double-clicking a session tab on a task
that has multiple sessions: the maximize call now stops event propagation,
respects isRestoringLayout, and always uses the live api.group.id, so it
remains correct even after dockview rebuilds groups via fromJSON.
2026-05-05 13:55:46 +01:00
José Almeida 001ecf9582 fix(docker): use container icon for docker tasks 2026-05-04 21:11:22 +01:00
José Almeida 3b07d72a91 fix(docker): stabilize container reuse and feedback 2026-05-04 21:11:22 +01:00
José Almeida 6c045f9a07 fix(docker): reuse task containers across sessions 2026-05-04 21:10:32 +01:00
José Almeida 6cd50ab841 fix: address PR review feedback (CI, CodeRabbit, Greptile, Claude)
Backend:
- service_task_environments: fail closed when checkAnySessionRunning
  errors so a transient DB hiccup can't bypass the running-session
  guard and tear down a live container/sandbox; teardown is now truly
  best-effort with errors.Join across container/sandbox/worktree, so a
  failed Docker call doesn't permanently orphan the worktree on retry.
  Adds tests for the running-check error path and the multi-resource
  best-effort path.
- executor_execute: propagate GetTaskEnvironmentByTaskID errors instead
  of silently launching a fresh environment that orphans existing
  containers/worktrees; gate environment reuse on matching ExecutorType
  so a profile switch doesn't pass stale PreviousExecutionID/container
  IDs to the wrong backend.
- task_environment_handlers: reject malformed JSON on POST
  /tasks/:id/environment/reset (still allow empty body) so a destructive
  endpoint doesn't silently proceed with a bad payload.
- worktree (PushEnvironmentBranch): bound git push with a 30s context
  timeout and disable interactive prompts via GIT_TERMINAL_PROMPT=0
  so a stalled remote can't hang the request thread.
- manager_environment: explicitly call ensureClient() before
  ContainerMgr() in DestroyContainer/GetContainerLiveStatus so a missing
  Docker daemon surfaces as "docker unavailable" rather than the
  generic "container manager not initialized".
- container: WorkingDir always uses the in-container /workspace path
  rather than the host workspace path (Docker would happily start in an
  unrelated directory in host bind-mount mode); bootstrap now logs the
  prepare-script exit code so failures show up in container logs.
- docker/client: CreateContainerInteractive honors ContainerConfig.Entrypoint
  so the same config struct produces consistent behavior on both creation
  paths.

Frontend:
- executor-settings-button: extract useTaskEnvironment hook (component
  shrinks below the 100-line limit); only treat 404 as empty env (other
  errors no longer wipe a valid environment); track "loaded once" via a
  ref so loadEnv doesn't depend on env state — that dep was creating a
  new function identity on every successful poll, cancelling the
  3-second interval and turning the popover into a tight loop hammering
  /environment/live; copy-shell command uses sh instead of bash so it
  works on user-built minimal images.
- task-reset-env-confirm-dialog: AlertDialogAction onClick calls
  e.preventDefault() so Radix's auto-close doesn't dismiss the dialog
  before the async reset completes (loading spinner now visible; close
  is parent-controlled via onOpenChange).
- remote-credentials-card: track env method in selectedIds the same way
  files/gh_cli_token are tracked, and let initialChoice match either
  selectedIds OR envSecretId. First-time env-credential setup for an
  agent that exposes both files+env was broken: clicking the env radio
  never updated state, choice re-derived to "none", and the secret
  picker disappeared before the user could pick anything.
- e2e/executor-reuse: select the acknowledge checkbox by label, not by
  positional last(), so adding another checkbox to the dialog can't
  silently target the wrong control.

Format pass on prettier-flagged files; gofmt fixed two whitespace
nits in lifecycle files.
2026-05-04 21:09:39 +01:00
José Almeida 2779fb24aa feat(docker): make mock-agent runnable inside Docker for E2E
Lay the backend groundwork so the existing mock-agent can run inside
local_docker containers — needed for real-Docker E2E tests that don't
require Anthropic/OpenAI API keys.

Container builder bind-mounts a host mock-agent binary at
/usr/local/bin/mock-agent when KANDEV_MOCK_AGENT_LINUX_BINARY is set
(or build/mock-agent-linux-amd64 sits next to the kandev binary), the
same pattern already used for agentctl. MockAgent.BuildCommand always
resolves via PATH so the same command works on host (E2E PATH includes
bin/) and in containers (mounted at /usr/local/bin/mock-agent).

Mock now declares non-nil empty RemoteAuth so it counts as a
remote-capable agent. The remoteauth catalog includes specs even with
zero methods, and the frontend gating helper treats an empty-method
spec as "no credentials needed" — letting mock-agent pass the
configure-on-executor check that hides uncatalogued agents on remote
executors.

repositoryCloneURL falls back to the local origin remote URL when the
repo has no provider info, so local-only repos with a real remote
(including file:// remotes used in E2E) work on remote executors that
clone inside the container.

Makefile: build-mock-agent-linux target mirrors build-agentctl-linux.
2026-05-04 21:09:39 +01:00
José Almeida cbfff9bc8a feat(web): gate agent profiles by executor profile credentials
Filter the agent profile dropdown in both the Create Task dialog and the
New Session dialog to only show agents that are actually configured for
the selected executor profile.

For local/worktree executors, no gating (agents need no extra creds).
For Docker/Sprites profiles, an agent is "configured" iff one of its
remote-auth methods has a value in `remote_credentials` or a non-null
secret in `remote_auth_secrets`. Agents without an entry in the remote
auth catalog (Copilot, Amp, OpenCode, TUI, mock) cannot carry creds on
remote executors and are blocked.

Spec IDs match `AgentProfileOption.agent_name` (the registry-type string
like "claude-acp"); `agent_id` is a DB UUID and unrelated to the catalog.

The Create Task footer surfaces a "No agent profile is configured for X"
tooltip and disables submit when the selected executor profile has no
compatible agents. The New Session dialog auto-swaps to the first
compatible agent and shows the same banner inline.

`useRemoteAuthSpecs` returns a `loaded` flag so callers don't filter
during the initial fetch (would otherwise hide everything).
2026-05-04 21:09:39 +01:00
José Almeida f169d25b72 fix(executor-profile): live Configured badge + session/load resilience + live container status
Three independent fixes that all surface together while iterating on the
Docker executor flow:

- Remote Credentials card: the per-agent "Configured" / "Not Configured"
  badge was driven by useState that only seeded once on mount. Picking a
  secret in the dropdown updated the parent's `agentEnvVars` map but the
  local `choice` never moved to `"env"`, so the badge stayed grey until
  the page was reloaded. Derive `choice` straight from props every render
  so the badge tracks live state.

- ACP session/load: when the agent process restarted between turns
  (common with claude-agent-acp because its session map is in-memory),
  the host's session/load call failed with JSON-RPC -32002 "Resource not
  found" and the entire launch errored out. Treat that as the same kind
  of "session is gone" signal as -32601 / capability=false and fall back
  to session/new. Worst case the agent loses prior context; best case
  the user just keeps typing.

- Executor Settings popover: dropped Execution ID and Environment ID
  rows (debug-only noise users don't need), kept Container ID + the
  copyable `docker exec -it … bash` line, added a live status badge
  with tones for running / exited / paused / dead / error / missing,
  and a 3-second polling interval so the badge follows the real
  container state. New `GET /api/v1/tasks/:id/environment/live`
  endpoint plumbs Docker `inspect` data through the lifecycle manager
  → task service → handler. ContainerLiveStatus mirror type at the
  service layer keeps lifecycle types out of the public surface.
2026-05-04 21:08:51 +01:00
José Almeida 4228f92aef feat(web): add executor settings popover with reset environment action
A new button on the task top bar opens a popover showing the active
TaskEnvironment — executor type, status, container/sprite/worktree
identifiers, and a copy-ready 'docker exec' command for local Docker.
The popover hosts the destructive Reset Environment action so users have
one obvious place to inspect and manage the environment without
cluttering the New Session dialog.

The reset confirmation dialog requires explicit acknowledgement that
uncommitted changes will be lost. When the environment has a host-side
worktree it offers an opt-in 'push branch before resetting' checkbox so
committed work can be pushed first; the backend aborts the reset on
push failure.

- task-environment-api.ts: fetchTaskEnvironment + resetTaskEnvironment
- executor-settings-button.tsx: popover trigger + identity panel
- task-reset-env-confirm-dialog.tsx: destructive confirm with push opt-in
- task-top-bar.tsx: wire the new button into the right-side controls
- e2e: replace fresh-env-toggle test with reset-from-popover flow
2026-05-04 21:08:51 +01:00
José Almeida a4e0fc0452 feat: attribute cross-task agent messages with a sender badge (#819)
* feat(orchestrator): carry sender_task metadata through user message paths

Add WithSenderTask builder to UserMessageMeta and a metadata-aware
QueueMessageWithMetadata variant on the message queue. The drain path
in event_handlers_agent merges queued metadata into the recorded user
message so attribution survives the queue → DB transition.

* feat(mcp): attribute message_task_kandev calls to the sending agent

The MCP server inside agentctl now injects sender_task_id and
sender_session_id from its own server context. The backend handler
validates the sender (rejecting missing IDs, self-messages, and unknown
tasks), looks up the sender title, wraps the prompt in a <kandev-system>
attribution block, and propagates structured metadata across all three
delivery paths (queue, created-session, record+prompt). The wrapped
content lets the receiving agent identify the peer both live and on ACP
session resume; the existing sysprompt strip pipeline hides the wrapper
from the UI.

* feat(web): render sender-task badge on cross-task agent messages

User messages whose metadata carries sender_task_id now render a
clickable purple badge above the bubble, linking to the source task.
The badge live-resolves the sender's title from the kanban store and
falls back to a greyed snapshot when the task isn't loaded
(cross-workspace, deleted, etc.). Adds a non-subscribing useTaskById
selector so badges don't fan out WS subscriptions per message.

* test(e2e): cover cross-task agent message attribution

Eight scenarios driven through the mock-agent's MCP script directive:
queue path, prompt path, self-message rejection, unknown-target error,
badge link target, live title re-resolution after rename, kandev-system
wrapper preservation in raw_content, and prompt bodies containing their
own kandev-system tags. Adds an updateTaskTitle helper to ApiClient.

* fix: address CodeRabbit + Greptile P1 findings on PR #819

- requeueMessage now forwards queuedMsg.Metadata via QueueMessageWithMetadata
  so sender attribution survives transient delivery failures and retries.
- CREATED-session branch in handleMessageTask records the user message AFTER
  StartCreatedSession succeeds, so a launch failure no longer leaves an
  orphaned attributed chat row.
- SenderTaskBadge: collapse identical displayTitle/fullTitle locals.
- E2E: extract waitForTargetIdle helper; replace the 1.5s raw sleep in the
  collision-check test with the same poll the rest of the suite uses.

* fix: address Claude review suggestions on PR #819

- wrapAgentMessage strips </kandev-system> from the embedded sender title
  before formatting. Without this, a title containing the closing tag
  would short-circuit the non-greedy strip regex and leak the attribution
  tail into the visible UI bubble. Metadata snapshot keeps the original
  title so the badge still displays it.
- Add a regression test covering the malicious-title case.
- Document the deliberate cross-workspace messaging policy on the sender
  lookup so future readers don't add a workspace-scope check (Q13 of the
  feature plan locked this as intentional).
2026-05-04 19:30:10 +01:00
Carlos Florêncio cb0feea650 fix(web): dedup topbar PR button against auto-shown PR panel (#812) 2026-05-04 19:20:37 +01:00
Carlos Florêncio c042050469 fix: stabilize task switch flow and session recovery (#818) 2026-05-04 18:53:33 +01:00
João Salavisa 5c5e7bf6a7 fix(changes-panel): scope commits/diff to live branch divergence; flatten single-repo (#816)
* fix(changes-panel): scope commits/diff to live branch divergence; flatten single-repo

Two bugs in the COMMITS panel made it surface commits and file changes
that were already on main:

1. The commit list and the cumulative diff both anchored to a stale base.
   Commit list used `git merge-base HEAD <local-branch>` and the file diff
   used the session's stored base SHA captured once at session start. On
   long-lived worktrees the local main ref lags far behind origin/main, so
   the merge-base lands on an old SHA; once the agent merges main back in,
   the stored base is also stale. Both end up sweeping in dozens of main
   commits as if they were the branch's work.

2. `git log <base>..HEAD` follows both parents of a merge commit by
   default, surfacing main commits brought in via merges from main.

Fixes:
- agentctl computes merge-base against `origin/<target_branch>` (with a
  local-ref fallback for unfetched/local-only branches), and uses that
  for both the commit log and the cumulative diff. Mirrors GitHub's
  three-dot PR diff semantics — anchor to live divergence with the
  upstream, not a session-start snapshot.
- `git log` now passes `--first-parent` so only the branch's own line of
  work plus its merge commits show up; commits brought in via merges from
  main are excluded.

Frontend: drop the per-repo "REPOSITORY" sub-header in the COMMITS panel
for single-repo workspaces. CommitsRepoGroup gains a `showHeader` prop;
when the workspace has one untagged group the section renders flat with
the per-repo Push / PR (and Stage all / Commit / Unstage all) buttons
lifted into the section header. Multi-repo rendering unchanged.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: address PR review feedback

- Scope --first-parent to the divergence-range path only (Greptile P1).
  The flag is wrong for open-ended history calls — future activity
  widgets etc. need the full graph.
- Log empty merge-base case in cumulative diff (Greptile P2). Silent
  fallback to stored base was invisible during diagnostics.
- Rename TestRunGitCumulativeDiffForRepo_PrefersOriginRef →
  TestComputeMergeBase_CorrectAnchorForCumulativeDiff (Claude). Test
  exercises computeMergeBase, not the runGitCumulativeDiffForRepo
  conditional; new name reflects what it actually proves.
- Add TestGetLog_NoBaseReturnsFullGraph to lock in the --first-parent
  scoping.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

* fix: address PR review round 2

- Drop dead ActionButtonsSection from changes-panel-timeline.tsx (unused
  export, no consumers anywhere). Brings the file from 640 → 516 lines,
  comfortably under the 600-line TS limit Claude flagged.
- Extract isSingleRepoGroup helper into lib/group-by-repo.ts and use it
  in all three sites (CommitsSection, FileListBody, FileListSection) so
  single-repo detection has one source of truth.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
2026-05-04 17:28:04 +01:00
João Salavisa a77df053a6 refactor(web): drop redundant VCS split button from task top bar (#804)
* refactor(web): drop redundant VCS split button from task top bar

All git operations (commit, push, pull, rebase, merge, create PR,
force push) are reachable from the Changes panel — its timeline
exposes stage/commit and the per-section action row provides
push/force-push/create-PR, while the panel header carries the
pull/rebase/merge dropdown. The session command palette mirrors
the same set for keyboard users. The expanded review dialog keeps
its own VcsSplitButton because the changes timeline is hidden
behind the modal there.

Also removes the now-orphaned 'required' overflow flag (the VCS
item was its only caller) and the e2e helper + spec that targeted
the removed primary-action button.

* test(e2e): drop task-topbar-overflow spec relying on removed VCS button

The 1100px viewport test forced overflow by counting on the VCS button's
width; with VcsSplitButton gone the scenario no longer holds. Overflow
priority logic is already covered by topbar-action-overflow.test.ts.

Co-authored-by: João Salavisa <jcfs@users.noreply.github.com>

---------

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
Co-authored-by: João Salavisa <jcfs@users.noreply.github.com>
2026-05-04 00:53:21 +01:00
José Almeida b2dcd29818 fix(web): tab session bugs — primary star drift + tab close re-creation (#800)
* fix(web): preserve primarySessionId across kanban.update broadcasts

The kanban.update WS handler rebuilt kanban.tasks without
primarySessionId / primarySessionState, wiping values set by
task.updated. After any board refresh the primary star vanished and
selectTaskWithLayout fell into the slower async fallback path. Move
task building inside setState so existing values are preserved in both
single and multi snapshots.

* fix(web): prevent deleted session panel from being re-created

handleDelete called removeTaskSession before containerApi.removePanel,
which let useAutoSessionTab re-run with effectiveSessionId still
pointing at the deleted session and re-create its panel. The
setupChatPanelSafetyNet had a matching gap — it re-added a panel for
activeSessionId without checking the session still existed.

Switch the active session (or clear it) before removeTaskSession so
effectiveSessionId updates first. Add guards to useAutoSessionTab and
the safety net so neither creates a panel for a session that is no
longer in the store.

* test(web): add session tab management regression suite

Cover the three bugs fixed in this branch end-to-end: deleting a
non-active session keeps it gone, deleting the active session switches
focus to the remainder, deleting then immediately switching tasks does
not resurrect the tab, and the primary star survives both a
kanban.update broadcast (triggered via apiClient.moveTask) and a
task-switch round trip.

* fix(web): hand off deletion to newest remaining session, document multi-snapshot fallback

- session-tab.tsx: sort remaining sessions by started_at DESC and
  pick the newest, instead of relying on the unspecified store ordering.
- kanban.ts: comment the multi-snapshot primarySessionId fallback so
  the double lookup does not look like redundancy.

Both per Claude review feedback on #800.

* fix(web): preserve explicit-null primary, abort delete on WS failure

- kanban.ts: switch the multi-snapshot fallback from `??` to an
  explicit `=== undefined` check. An explicitly-cleared null primary
  must NOT be replaced by a stale snapshot value, only an absent key.
- session-tab.tsx: wsAction now returns a boolean; handleDelete bails
  out before mutating the store / dockview when the backend
  session.delete request fails. Prevents client/server desync from a
  transient WS failure.
- kanban.test.ts: add regression covering the null-vs-undefined merge.

Per CodeRabbit and Greptile review on #800.

* style(web): drop redundant .slice() in session deletion sort chain

filter() already returns a fresh array, so the subsequent sort() does
not mutate shared store state. Per Greptile review on #800.
2026-05-03 17:32:29 +01:00