Skip to content

fix: branch dropdown, diff base branch, and direct mode auto-checkout#33

Closed
ASRagab wants to merge 2 commits into
johannesjo:mainfrom
ASRagab:main
Closed

fix: branch dropdown, diff base branch, and direct mode auto-checkout#33
ASRagab wants to merge 2 commits into
johannesjo:mainfrom
ASRagab:main

Conversation

@ASRagab
Copy link
Copy Markdown
Contributor

@ASRagab ASRagab commented Mar 25, 2026

Summary

  • Branch dropdown: Fixed branch list disappearing on dialog re-open by merging two racing SolidJS createEffect blocks into one. Added loading spinner and retry+toast error handling.
  • Diff base branch: Fixed diff viewer computing diffs against main instead of per-task base branch. Normalized empty-string baseBranch to undefined at the IPC boundary, task creation, and legacy state load.
  • Direct mode auto-checkout: Replaced "Please checkout X first" blocking warning with automatic git checkout when creating a Direct mode task on a non-current branch. Shows git's error message if checkout fails (e.g., dirty working tree).

Test plan

  • Open new task dialog, verify branch dropdown populates with spinner
  • Close and reopen dialog — branches should persist (not reset to empty)
  • Rapid close/reopen 3-4x — branches populate every time
  • Create a task with base branch set to develop — diff should show against develop, not main
  • Restart app with legacy state containing baseBranch: '' — should auto-normalize
  • Select Direct mode, pick a non-current branch, click Create Task — should auto-checkout
  • Select Direct mode with dirty working tree on wrong branch — should show git error
  • npm run typecheck passes
  • npx vitest run — all 76 tests pass

🤖 Generated with Claude Code

ASRagab and others added 2 commits March 24, 2026 22:23
@ASRagab @claude
fix: branch dropdown, diff base branch, and direct mode auto-checkout
8de6bd7 
Three bug fixes for the task creation and diff review workflows:

1. Branch dropdown: merge two racing SolidJS createEffect blocks into one
   effect with loading spinner and retry+toast error handling (DROP-01, DROP-02)

2. Diff base branch: normalize empty-string baseBranch to undefined at the
   IPC boundary (register.ts), task creation (tasks.ts), and legacy state
   load (persistence.ts) so diffs use the correct per-task base branch
   instead of silently falling back to main (DIFF-01, DIFF-02, DIFF-03)

3. Direct mode auto-checkout: replace "Please checkout X first" warning with
   automatic git checkout when creating a Direct mode task on a non-current
   branch (CHECKOUT-01)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ASRagab
Bump version from 1.1.1 to 1.2.0
22dcdee
@ASRagab
Copy link
Copy Markdown
Contributor Author

ASRagab commented Mar 25, 2026

Recreating with a clean branch — the original included unrelated fork changes.

@ASRagab ASRagab closed this Mar 25, 2026
brooksc added a commit to brooksc/parallel-code that referenced this pull request May 10, 2026
@brooksc @claude
fix(coordinator): three post-review bug fixes + TODOS additions
a447645 
- coordinator.ts: setTaskControl('coordinator') now resolves idle waiters
  with { reason: 'idle' } instead of { reason: 'human_control' }
- coordinator.ts: cleanupTask resolves pending idle waiters with
  { reason: 'exited' } before deleting them, preventing leaked promises
- preload.cjs: add mcp_stale_url_warning to ALLOWED_CHANNELS allowlist
- coordinator.test.ts: update test expectation to match corrected reason
- TODOS.md: add johannesjo#32 (sync file I/O in createTask) and johannesjo#33 (no restart
  integration tests) from code review

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
brooksc added a commit to brooksc/parallel-code that referenced this pull request May 11, 2026
@brooksc @claude
fix(security): two-class token, mcpReady gate, propagateSkipPermissio…
fd7a79a 
…ns, input validation

Security:
- Generate separate subtaskToken for sub-task MCP configs; coordinator keeps full token
- Sub-task token restricted to POST /api/tasks/:id/done only (403 on all other routes)
- WebSocket rejects subtask tokens; classifyCandidate/classifyToken replace safeCompare
- Remove skipPermissions from MCP/REST create_task (coordinator propagates it directly)

Correctness:
- mcpReady flag gates TerminalView auto-spawn for coordinator and coordinated tasks
- MCP_TaskHydrated IPC event sets mcpReady after successful hydration on restore
- propagateSkipPermissions persisted/restored on coordinator tasks
- Coordinator and sub-tasks wait for mcpReady before spawning PTY

Reliability:
- remoteServerPendingStop flag remembers StopRemoteServer during active coordinator
- Fire-and-forget IPC calls in task close/deregister paths awaited with .catch(warn)
- .mcp.json always written chmod 0o600; merge preserves other servers; cleanup removes only parallel-code key
- hydrateTask validates mcpConfigPath against exact expected paths (no path traversal)
- baseBranch validated in coordinator.createTask (non-empty, no leading dash)
- StartMCPServer validates renderer-supplied paths/IDs before any file I/O
- registerCoordinator called with options object (fixes signature mismatch)

Tests:
- coordinator-scoping.test.ts: subtask token restricted to signal_done (7 tests)
- coordinator.test.ts: two-class token propagation, setMCPServerInfo 5-arg form (25 call sites)
- TODOs johannesjo#33-43 added/updated for remaining test and fix coverage

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
brooksc added a commit to brooksc/parallel-code that referenced this pull request May 11, 2026
@brooksc @claude
test(coordinator): add missing tests for johannesjo#33, johannesjo#36, …
cd66d32 
…johannesjo#39

- johannesjo#33: restart round-trip — hydrateTask rewrites config with new subtaskToken
  (not coordinator token) when setMCPServerInfo has already run; and
  waitForIdle resolves after agent output fires post-hydration
- johannesjo#36: mcpConfigPath directory scoping — path traversal rejected, wrong-dir
  rejected, correct host tmpdir accepted, Docker dirname(serverPath) accepted,
  Docker wrong-dir rejected
- johannesjo#39: per-task close isolation — closing task-1 deletes only its config file;
  task-2 config and task entry remain untouched

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
brooksc added a commit to brooksc/parallel-code that referenced this pull request May 14, 2026
@brooksc @claude
fix(coordinator): three post-review bug fixes + TODOS additions
a6b71a0 
- coordinator.ts: setTaskControl('coordinator') now resolves idle waiters
  with { reason: 'idle' } instead of { reason: 'human_control' }
- coordinator.ts: cleanupTask resolves pending idle waiters with
  { reason: 'exited' } before deleting them, preventing leaked promises
- preload.cjs: add mcp_stale_url_warning to ALLOWED_CHANNELS allowlist
- coordinator.test.ts: update test expectation to match corrected reason
- TODOS.md: add johannesjo#32 (sync file I/O in createTask) and johannesjo#33 (no restart
  integration tests) from code review

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
brooksc added a commit to brooksc/parallel-code that referenced this pull request May 14, 2026
@brooksc @claude
fix(security): two-class token, mcpReady gate, propagateSkipPermissio…
e84cb35 
…ns, input validation

Security:
- Generate separate subtaskToken for sub-task MCP configs; coordinator keeps full token
- Sub-task token restricted to POST /api/tasks/:id/done only (403 on all other routes)
- WebSocket rejects subtask tokens; classifyCandidate/classifyToken replace safeCompare
- Remove skipPermissions from MCP/REST create_task (coordinator propagates it directly)

Correctness:
- mcpReady flag gates TerminalView auto-spawn for coordinator and coordinated tasks
- MCP_TaskHydrated IPC event sets mcpReady after successful hydration on restore
- propagateSkipPermissions persisted/restored on coordinator tasks
- Coordinator and sub-tasks wait for mcpReady before spawning PTY

Reliability:
- remoteServerPendingStop flag remembers StopRemoteServer during active coordinator
- Fire-and-forget IPC calls in task close/deregister paths awaited with .catch(warn)
- .mcp.json always written chmod 0o600; merge preserves other servers; cleanup removes only parallel-code key
- hydrateTask validates mcpConfigPath against exact expected paths (no path traversal)
- baseBranch validated in coordinator.createTask (non-empty, no leading dash)
- StartMCPServer validates renderer-supplied paths/IDs before any file I/O
- registerCoordinator called with options object (fixes signature mismatch)

Tests:
- coordinator-scoping.test.ts: subtask token restricted to signal_done (7 tests)
- coordinator.test.ts: two-class token propagation, setMCPServerInfo 5-arg form (25 call sites)
- TODOs johannesjo#33-43 added/updated for remaining test and fix coverage

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
brooksc added a commit to brooksc/parallel-code that referenced this pull request May 14, 2026
@brooksc @claude
test(coordinator): add missing tests for johannesjo#33, johannesjo#36, …
70d5d27 
…johannesjo#39

- johannesjo#33: restart round-trip — hydrateTask rewrites config with new subtaskToken
  (not coordinator token) when setMCPServerInfo has already run; and
  waitForIdle resolves after agent output fires post-hydration
- johannesjo#36: mcpConfigPath directory scoping — path traversal rejected, wrong-dir
  rejected, correct host tmpdir accepted, Docker dirname(serverPath) accepted,
  Docker wrong-dir rejected
- johannesjo#39: per-task close isolation — closing task-1 deletes only its config file;
  task-2 config and task entry remain untouched

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@ASRagab