batch session fixes#32
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements unified project navigation and completes the dashboard v2 migration. Projects now display directly in the main dashboard sidebar with agents nested underneath, replacing the separate bridge interface. The update includes shadow agent routing support, improved multi-line message parsing, and comprehensive mobile responsive styling.
Key Changes
- Unified navigation: Projects appear in sidebar with nested agents, eliminating the separate bridge interface
- Dashboard v2 is now the default, completing the migration from v1
- Shadow agent message routing with configurable trigger support
- Enhanced message parser handling for multi-line content with
>>>delimiters
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/wrapper/parser.ts | Enhanced regex for multi-line fenced messages, auto-closes incomplete blocks |
| src/wrapper/parser.test.ts | Comprehensive tests for new parser edge cases |
| src/wrapper/client.ts | Added bindAsShadow/unbindAsShadow methods for shadow agent pairing |
| src/protocol/types.ts | Added shadow agent types and message type definitions |
| src/hooks/types.ts | New file defining hook system interfaces (HookContext, HookResult) |
| src/hooks/index.ts | Entry point exporting hook types |
| src/dashboard/types/index.ts | Added optional team field to Agent interface |
| src/dashboard/react-components/layout/Sidebar.tsx | Unified project/agent navigation with conditional rendering |
| src/dashboard/react-components/layout/Header.tsx | Added mobile menu button |
| src/dashboard/react-components/SpawnModal.tsx | Added team assignment field |
| src/dashboard/react-components/ProjectList.tsx | New component for hierarchical project/agent display |
| src/dashboard/react-components/CommandPalette.tsx | Added project navigation commands |
| src/dashboard/react-components/App.tsx | Integrated project state, mobile sidebar, multi-line composer |
| src/dashboard/react-components/AgentCard.tsx | Added displayNameOverride prop for team prefix stripping |
| src/dashboard/app/globals.css | 670 lines of mobile responsive styles |
| src/dashboard-server/server.ts | Mark spawned agents with team metadata |
| src/daemon/server.ts | Shadow bind/unbind message handling, processing state callback |
| src/daemon/router.ts | Shadow relationship tracking and message routing |
| src/daemon/connection.ts | Heartbeat exemption during active processing |
| src/daemon/auth.ts | New authentication module with peer credentials and team config |
| src/cli/index.ts | Teams.json auto-spawn support with --spawn flag |
| src/bridge/teams-config.ts | Teams configuration loader for multi-project setup |
| .beads/issues.jsonl | Updated issue statuses for completed migration tasks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| // Use ss command to get socket peer info (requires net-tools) | ||
| // This is a workaround - proper implementation needs native bindings | ||
| const result = execSync(`ss -xp 2>/dev/null | grep -E "^u_str.*,pid=" | head -1`, { |
There was a problem hiding this comment.
Using execSync with shell commands introduces command injection risks. Consider using native Node.js APIs or a dedicated library for socket credential extraction instead of shell commands.
Comment on lines
+215
to
+220
| const idx = shadows.findIndex(s => s.shadowAgent === shadowAgent); | ||
| if (idx !== -1) { | ||
| shadows.splice(idx, 1); | ||
| } | ||
| if (shadows.length === 0) { | ||
| this.shadowsByPrimary.delete(primaryAgent); |
There was a problem hiding this comment.
Using findIndex followed by splice is inefficient for removing items. Consider using filter() to create a new array without the target element, which is more performant and functional.
Suggested change
| const idx = shadows.findIndex(s => s.shadowAgent === shadowAgent); | |
| if (idx !== -1) { | |
| shadows.splice(idx, 1); | |
| } | |
| if (shadows.length === 0) { | |
| this.shadowsByPrimary.delete(primaryAgent); | |
| const updatedShadows = shadows.filter(s => s.shadowAgent !== shadowAgent); | |
| if (updatedShadows.length === 0) { | |
| this.shadowsByPrimary.delete(primaryAgent); | |
| } else { | |
| this.shadowsByPrimary.set(primaryAgent, updatedShadows); |
| if (isExpanded && expandedTeams.size === 0 && teams.length > 0) { | ||
| setExpandedTeams(new Set(teams.map((t) => t.name))); | ||
| } | ||
| }, [isExpanded, teams]); |
There was a problem hiding this comment.
Missing expandedTeams in the dependency array. This effect depends on expandedTeams.size but doesn't include expandedTeams in dependencies, which could cause stale closure issues.
Suggested change
| }, [isExpanded, teams]); | |
| }, [isExpanded, teams, expandedTeams]); |
| const FENCE_END = /^(?:\s*)?>>>/; | ||
| // FENCE_END is lenient - matches >>> at start OR end of line regardless of surrounding content | ||
| // This handles both ">>>" on its own line and "content>>>" at end of message | ||
| const FENCE_END = /^(?:\s*)?>>>|>>>\s*$/; |
There was a problem hiding this comment.
The regex pattern combines two distinct cases without explanation. Consider splitting into two separate regex constants (FENCE_END_START and FENCE_END_LINE) with descriptive names for better maintainability.
Suggested change
| const FENCE_END = /^(?:\s*)?>>>|>>>\s*$/; | |
| const FENCE_END_START = /^(?:\s*)?>>>/; | |
| const FENCE_END_LINE = />>>\s*$/; | |
| const FENCE_END = new RegExp(`${FENCE_END_START.source}|${FENCE_END_LINE.source}`); |
Comment on lines
+100
to
+104
| const projectList: Project[] = result.data.servers.map((server) => ({ | ||
| id: server.id, | ||
| path: server.url, | ||
| name: server.name || server.url.split('/').pop(), | ||
| agents: result.data!.agents.filter((a) => a.server === server.id), |
There was a problem hiding this comment.
Using non-null assertion (result.data!) inside the map callback is unsafe. The outer check for result.data doesn't guarantee it remains defined during the map operation if the callback references it from closure.
Suggested change
| const projectList: Project[] = result.data.servers.map((server) => ({ | |
| id: server.id, | |
| path: server.url, | |
| name: server.name || server.url.split('/').pop(), | |
| agents: result.data!.agents.filter((a) => a.server === server.id), | |
| const { servers, agents } = result.data; | |
| const projectList: Project[] = servers.map((server) => ({ | |
| id: server.id, | |
| path: server.url, | |
| name: server.name || server.url.split('/').pop(), | |
| agents: agents.filter((a) => a.server === server.id), |
- auth.ts: Remove execSync shell command to avoid command injection risk - router.ts: Use filter() instead of findIndex + splice for efficiency - ProjectList.tsx: Add expandedTeams to useEffect dependency array - parser.ts: Split FENCE_END regex into separate named constants - App.tsx: Destructure result.data to avoid non-null assertion in closure 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Replaced invisible dark blue with vibrant emerald green accent - Added gradient-filled bar chart icon for visual interest - Added subtle top highlight glow effect - Added animated pulse indicator to draw attention - Enhanced hover state with glow and lift effect The button now stands out clearly in the dark sidebar while maintaining visual harmony with the overall dashboard design. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Hide breadcrumb in AgentCard when displayNameOverride is provided (team header already provides context) - Improved stripTeamPrefix to handle more patterns: - Team prefix with dash separator (frontend-lead → Lead) - Team prefix with underscore separator - Multi-segment names (shows last segment) - Added capitalizeWords helper for proper formatting Agents in team groups now show just their role name (e.g., "Lead") instead of the full breadcrumb path (e.g., "Frontend > Lead"). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Icon: - Replaced generic X-in-circle with power/terminate symbol - Distinctive open-arc design with vertical stem Full button (.release-btn): - Dark crimson gradient background with depth - Subtle CRT scanline texture overlay - Warning glow on hover with red halo effect - Icon pulses when hovered to signal danger - Scale transform on interaction Compact button (.release-btn-compact): - Starts transparent, fades in on card hover - Transforms to danger state with border glow on button hover - Pulsing scale animation on icon - Drop shadow glow effect The button now feels like a deliberate control panel element rather than a generic action button. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Removed overly complex gradients and animations - Now uses same solid background style as spawn button - Simplified icon back to clean stroke-based bars - Consistent visual weight with other footer buttons 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added metrics-link styles to globals.css where other sidebar styles live - Removed duplicate styles from Sidebar.tsx (sidebarStyles not being used) - Button now matches spawn button with proper background and colors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added MetricsIcon to Header component - Metrics now appears as icon button in top-right header - Removed metrics link from sidebar footer - Added anchor button styles in globals.css 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- Added .release-btn industrial control panel styles - Added .release-btn-compact minimal danger state styles - Added keyframe animations (killPulse, killPulseCompact) - Added .agent-actions and .agent-compact-actions containers Styles were previously in AgentCard.tsx's agentCardStyles export which wasn't being loaded. Now in globals.css where they work. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
khaliqgant
added a commit
that referenced
this pull request
Apr 1, 2026
template-resolver.ts: shell-escape interpolated variables (CRITICAL #1) broker_tests.rs: uncomment and wire up 5 real tests (CRITICAL #2) worker_tests.rs: uncomment and wire up 5 real tests (CRITICAL #3) worker.rs: log bypass-flag injection, add .. path traversal rejection (CRITICAL #4, #7) verification.ts: export stripInjectedTaskEcho, add path traversal guard (CRITICAL #5) runner.ts: remove duplicate stripInjectedTaskEcho, add ENV_ALLOWLIST filtering (HIGH #17) channel-messenger.ts: add secret scrubbing, hoist regex constants (MEDIUM #27, #28) process-spawner.ts: add settled guard for race condition (MEDIUM #23) step-executor.ts: add sideEffects to callback type, deprecate alias (HIGH #15, #16) index.ts: export StepExecutor directly (MEDIUM #29) workflows/refactor/*.ts: replace hardcoded paths, remove --no-verify (HIGH #8-11) broker.rs: move is_pid_alive to canonical location (HIGH #14) cost/tracker.ts: add restrictive file permissions (MEDIUM #30) cost/pricing.ts: add last-verified date (MEDIUM #31) verification.test.ts: 9 new tests for exported helpers (MEDIUM #32) Co-Authored-By: My Senior Dev <dev@myseniordev.com>
khaliqgant
added a commit
that referenced
this pull request
Apr 6, 2026
…#675) * refactor: TDD decomposition of runner.ts + main.rs with extracted modules Extracted 5 modules from runner.ts (6,878 lines): - verification.ts (143 lines) - template-resolver.ts (87 lines) - channel-messenger.ts (151 lines) - step-executor.ts (571 lines) - process-spawner.ts (96 lines) Added characterization tests for all extracted modules. Extracted broker.rs and worker.rs from main.rs. Bug fixes: - Restore stripInjectedTaskEcho in verification.ts - Guard agent.release() against broker 400 race condition - Fix run-summary-table test for new table format - Export normalizeModel for correct pricing resolution - Fix --wave argument parsing in run-refactor.ts - ESM imports in all workflow files * fix: address 10 review finding(s) tracker.ts: resolveModel now uses normalizeModel for alias resolution (pre-existing fix verified) run-refactor.ts: --wave parsing with proper validation (pre-existing fix verified) step-executor.ts: signal-killed processes now correctly treated as failures channel-messenger.ts: replaced ReDoS-vulnerable regex with iterative indexOf stripping runner.ts: eliminated shell injection by using direct git spawn with argument arrays process-spawner.ts: fixed SIGKILL fallback timer leak by storing and clearing reference Co-Authored-By: My Senior Dev <dev@myseniordev.com> * Revert "chore: gitignore .trajectories/ (automated run artifacts) (#676)" (#677) This reverts commit 07a8dc0. * refactor: TDD decomposition of runner.ts + main.rs with extracted modules Extracted 5 modules from runner.ts (6,878 lines): - verification.ts (143 lines) - template-resolver.ts (87 lines) - channel-messenger.ts (151 lines) - step-executor.ts (571 lines) - process-spawner.ts (96 lines) Added characterization tests for all extracted modules. Extracted broker.rs and worker.rs from main.rs. Bug fixes: - Restore stripInjectedTaskEcho in verification.ts - Guard agent.release() against broker 400 race condition - Fix run-summary-table test for new table format - Export normalizeModel for correct pricing resolution - Fix --wave argument parsing in run-refactor.ts - ESM imports in all workflow files * trajectories correction again * pre commit is executable * remove tracked workflows * fix: address 36 review findings across Rust and TypeScript modules template-resolver.ts: shell-escape interpolated variables (CRITICAL #1) broker_tests.rs: uncomment and wire up 5 real tests (CRITICAL #2) worker_tests.rs: uncomment and wire up 5 real tests (CRITICAL #3) worker.rs: log bypass-flag injection, add .. path traversal rejection (CRITICAL #4, #7) verification.ts: export stripInjectedTaskEcho, add path traversal guard (CRITICAL #5) runner.ts: remove duplicate stripInjectedTaskEcho, add ENV_ALLOWLIST filtering (HIGH #17) channel-messenger.ts: add secret scrubbing, hoist regex constants (MEDIUM #27, #28) process-spawner.ts: add settled guard for race condition (MEDIUM #23) step-executor.ts: add sideEffects to callback type, deprecate alias (HIGH #15, #16) index.ts: export StepExecutor directly (MEDIUM #29) workflows/refactor/*.ts: replace hardcoded paths, remove --no-verify (HIGH #8-11) broker.rs: move is_pid_alive to canonical location (HIGH #14) cost/tracker.ts: add restrictive file permissions (MEDIUM #30) cost/pricing.ts: add last-verified date (MEDIUM #31) verification.test.ts: 9 new tests for exported helpers (MEDIUM #32) Co-Authored-By: My Senior Dev <dev@myseniordev.com> * style: auto-format Rust code with cargo fmt * minor clean * fix: reinstate deleted workflow files into workflows/ci/ Moved fix-mcp-spawn.yaml, add-swift-sdk.ts, and cli-observability.ts into workflows/ci/ to clearly distinguish them as CI test suite workflows. Updated .gitignore to allow workflows/ci/ and workflows/refactor/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address remaining Devin review findings and fix failing test - Fix tracker test: expect mode: 0o700 in mkdirSync assertion - Use Object.hasOwn() instead of `in` operator to avoid prototype chain false positives - Use Promise.allSettled to preserve partial output on process timeout - Apply path containment check for absolute paths in checkFileExists Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * fix: address new Devin review findings — StepExecutor name collision and cwd trailing slash - Rename StepExecutor interface in runner.ts to RunnerStepExecutor to avoid shadowing the StepExecutor class export in the barrel index - Normalize cwd with path.resolve() in checkFileExists to handle trailing slashes Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Changes
New Features
src/dashboard/react-components/ProjectList.tsx- Project listing with nested agents in sidebarsrc/bridge/teams-config.ts- Teams configuration loader for multi-project setupsrc/daemon/auth.ts- Authentication module for daemon connectionssrc/daemon/router.ts- Message routing with shadow agent supportsrc/hooks/types.ts- Type definitions for hooks APIImprovements
Bug Fixes
Test plan
c77a656 fix lint🤖 Generated with Claude Code