mirror of
https://github.com/coleam00/Archon
synced 2026-04-21 13:37:41 +00:00
Add unified isolation environment architecture (Phase 2.5) (#92)
Introduces work-centric isolation model where orchestrator is the single source of truth for all isolation decisions. Adapters now pass IsolationHints instead of managing worktrees directly. Key changes: - Add isolation_environments table for workflow-based isolation tracking - Add validateAndResolveIsolation to orchestrator as single decision point - Add cleanup-service for unified event-driven cleanup handling - Simplify GitHub adapter to delegate isolation to orchestrator - Add IsolationHints type for adapter-to-orchestrator communication - Support automatic worktree sharing between linked issues and PRs
This commit is contained in:
Rasmus Widing
GitHub
parent
52669f52a8
commit
e24a4a8f4b
21 changed files with 4352 additions and 1349 deletions
1602
.agents/plans/completed/phase-2.5-unified-isolation.plan.md
Normal file
1602
.agents/plans/completed/phase-2.5-unified-isolation.plan.md
Normal file
File diff suppressed because it is too large
Load diff
1
.gitignore
vendored
1
.gitignore
vendored
|
|
@ -37,6 +37,7 @@ api_sessions/
|
|||
.agents/rca-reports/
|
||||
.claude/PRPs/features
|
||||
.agents/pr-reviews
|
||||
.agents/implementation-reports
|
||||
|
||||
# Local workspace
|
||||
workspace/
|
||||
|
|
|
|||
|
|
@ -476,17 +476,17 @@ export interface IIsolationProvider {
|
|||
```typescript
|
||||
interface IsolationRequest {
|
||||
codebaseId: string;
|
||||
canonicalRepoPath: string; // Main repo path, never a worktree
|
||||
canonicalRepoPath: string; // Main repo path, never a worktree
|
||||
workflowType: 'issue' | 'pr' | 'review' | 'thread' | 'task';
|
||||
identifier: string; // "42", "feature-auth", etc.
|
||||
prBranch?: string; // For PR adoption
|
||||
prSha?: string; // For reproducible PR reviews
|
||||
identifier: string; // "42", "feature-auth", etc.
|
||||
prBranch?: string; // For PR adoption
|
||||
prSha?: string; // For reproducible PR reviews
|
||||
}
|
||||
|
||||
interface IsolatedEnvironment {
|
||||
id: string; // Worktree path (for worktree provider)
|
||||
id: string; // Worktree path (for worktree provider)
|
||||
provider: 'worktree' | 'container' | 'vm' | 'remote';
|
||||
workingPath: string; // Where AI should work
|
||||
workingPath: string; // Where AI should work
|
||||
branchName?: string;
|
||||
status: 'active' | 'suspended' | 'destroyed';
|
||||
createdAt: Date;
|
||||
|
|
@ -517,13 +517,13 @@ export class WorktreeProvider implements IIsolationProvider {
|
|||
|
||||
### Branch Naming Convention
|
||||
|
||||
| Workflow | Identifier | Generated Branch |
|
||||
|----------|------------|------------------|
|
||||
| issue | `"42"` | `issue-42` |
|
||||
| pr | `"123"` | `pr-123` |
|
||||
| pr + SHA | `"123"` | `pr-123-review` |
|
||||
| task | `"my-feature"` | `task-my-feature` |
|
||||
| thread | `"C123:ts.123"` | `thread-a1b2c3d4` (8-char hash) |
|
||||
| Workflow | Identifier | Generated Branch |
|
||||
| -------- | --------------- | ------------------------------- |
|
||||
| issue | `"42"` | `issue-42` |
|
||||
| pr | `"123"` | `pr-123` |
|
||||
| pr + SHA | `"123"` | `pr-123-review` |
|
||||
| task | `"my-feature"` | `task-my-feature` |
|
||||
| thread | `"C123:ts.123"` | `thread-a1b2c3d4` (8-char hash) |
|
||||
|
||||
### Storage Location
|
||||
|
||||
|
|
@ -533,6 +533,7 @@ DOCKER: /workspace/worktrees/<project>/<branch>/ ← FIXED, no override
|
|||
```
|
||||
|
||||
**Logic in `getWorktreeBase()`:**
|
||||
|
||||
1. Docker detected? → `/workspace/worktrees` (always, no override)
|
||||
2. `WORKTREE_BASE` set? → use it (local only)
|
||||
3. Default → `~/tmp/worktrees`
|
||||
|
|
@ -602,6 +603,7 @@ remote_agent_conversations
|
|||
```
|
||||
|
||||
**Lookup pattern:**
|
||||
|
||||
```typescript
|
||||
const envId = conversation.isolation_env_id ?? conversation.worktree_path;
|
||||
```
|
||||
|
|
|
|||
File diff suppressed because it is too large
Load diff
|
|
@ -1,5 +1,7 @@
|
|||
# Worktree Orchestration
|
||||
|
||||
> **Note**: This document describes the current architecture. See `docs/worktree-orchestration-research.md` for the planned unified architecture (Phase 2.5+) which centralizes all isolation logic in the orchestrator.
|
||||
|
||||
## Storage Location
|
||||
|
||||
```
|
||||
|
|
@ -8,6 +10,7 @@ DOCKER: /workspace/worktrees/<project>/<branch>/ ← FIXED, no override
|
|||
```
|
||||
|
||||
Detection order in `getWorktreeBase()`:
|
||||
|
||||
```
|
||||
1. isDocker? → /workspace/worktrees (ALWAYS)
|
||||
2. WORKTREE_BASE set? → use it (local only)
|
||||
|
|
@ -52,21 +55,21 @@ Detection order in `getWorktreeBase()`:
|
|||
```typescript
|
||||
interface IsolationRequest {
|
||||
codebaseId: string;
|
||||
canonicalRepoPath: string; // Main repo, never a worktree
|
||||
canonicalRepoPath: string; // Main repo, never a worktree
|
||||
workflowType: 'issue' | 'pr' | 'review' | 'thread' | 'task';
|
||||
identifier: string;
|
||||
prBranch?: string; // For PR adoption
|
||||
prSha?: string; // For reproducible reviews
|
||||
prBranch?: string; // For PR adoption
|
||||
prSha?: string; // For reproducible reviews
|
||||
}
|
||||
```
|
||||
|
||||
| Workflow | Identifier | Branch Name |
|
||||
|----------|------------|-------------|
|
||||
| issue | `"42"` | `issue-42` |
|
||||
| pr | `"123"` | `pr-123` |
|
||||
| pr + SHA | `"123"` | `pr-123-review` |
|
||||
| task | `"my-feature"` | `task-my-feature` |
|
||||
| thread | `"C123:ts.123"` | `thread-a1b2c3d4` (hash) |
|
||||
| Workflow | Identifier | Branch Name |
|
||||
| -------- | --------------- | ------------------------ |
|
||||
| issue | `"42"` | `issue-42` |
|
||||
| pr | `"123"` | `pr-123` |
|
||||
| pr + SHA | `"123"` | `pr-123-review` |
|
||||
| task | `"my-feature"` | `task-my-feature` |
|
||||
| thread | `"C123:ts.123"` | `thread-a1b2c3d4` (hash) |
|
||||
|
||||
## Creation Flow
|
||||
|
||||
|
|
@ -225,6 +228,7 @@ conversations
|
|||
```
|
||||
|
||||
Lookup pattern:
|
||||
|
||||
```typescript
|
||||
const envId = conversation.isolation_env_id ?? conversation.worktree_path;
|
||||
```
|
||||
|
|
@ -234,6 +238,7 @@ const envId = conversation.isolation_env_id ?? conversation.worktree_path;
|
|||
The worktree-manager Claude Code skill uses `~/.claude/worktree-registry.json`.
|
||||
|
||||
**Adoption scenarios:**
|
||||
|
||||
1. **Path match**: Skill created worktree at expected path → adopted
|
||||
2. **Branch match**: Skill created worktree for PR's branch → adopted
|
||||
|
||||
|
|
@ -253,11 +258,96 @@ App checks: findWorktreeByBranch("feature/auth")
|
|||
|
||||
## Key Files
|
||||
|
||||
| File | Purpose |
|
||||
|------|---------|
|
||||
| `src/isolation/types.ts` | `IIsolationProvider`, `IsolationRequest`, `IsolatedEnvironment` |
|
||||
| `src/isolation/providers/worktree.ts` | `WorktreeProvider` implementation |
|
||||
| `src/isolation/index.ts` | `getIsolationProvider()` factory |
|
||||
| `src/utils/git.ts` | `getWorktreeBase()`, `listWorktrees()`, low-level git ops |
|
||||
| `src/adapters/github.ts` | Webhook handling, `cleanupPRWorktree()` |
|
||||
| `src/handlers/command-handler.ts` | `/worktree` command handling |
|
||||
| File | Purpose |
|
||||
| ------------------------------------- | --------------------------------------------------------------- |
|
||||
| `src/isolation/types.ts` | `IIsolationProvider`, `IsolationRequest`, `IsolatedEnvironment` |
|
||||
| `src/isolation/providers/worktree.ts` | `WorktreeProvider` implementation |
|
||||
| `src/isolation/index.ts` | `getIsolationProvider()` factory |
|
||||
| `src/utils/git.ts` | `getWorktreeBase()`, `listWorktrees()`, low-level git ops |
|
||||
| `src/adapters/github.ts` | Webhook handling, `cleanupPRWorktree()` |
|
||||
| `src/handlers/command-handler.ts` | `/worktree` command handling |
|
||||
|
||||
---
|
||||
|
||||
## Planned Architecture (Phase 2.5+)
|
||||
|
||||
The current architecture has isolation logic split between the GitHub adapter and orchestrator. Phase 2.5 will unify all isolation in the orchestrator.
|
||||
|
||||
### Target Architecture
|
||||
|
||||
```
|
||||
┌─────────────────────────────────────────────────────────────────────────┐
|
||||
│ ALL ADAPTERS (Thin) │
|
||||
│ GitHub, Slack, Discord, Telegram │
|
||||
├─────────────────────────────────────────────────────────────────────────┤
|
||||
│ ✓ Parse platform events │
|
||||
│ ✓ Detect @mentions │
|
||||
│ ✓ Build context + IsolationHints │
|
||||
│ ✓ Call handleMessage(platform, convId, message, context, hints) │
|
||||
│ ✓ Trigger cleanup events (GitHub only: close/merge) │
|
||||
│ ✗ NO worktree creation │
|
||||
│ ✗ NO isolation UX messages │
|
||||
└───────────────────────────────────┬─────────────────────────────────────┘
|
||||
│
|
||||
▼
|
||||
┌─────────────────────────────────────────────────────────────────────────┐
|
||||
│ ORCHESTRATOR (Authority) │
|
||||
├─────────────────────────────────────────────────────────────────────────┤
|
||||
│ validateAndResolveIsolation(): │
|
||||
│ 1. Validate existing isolation (cwd exists?) │
|
||||
│ 2. Check for reuse (same workflow_type + workflow_id) │
|
||||
│ 3. Check linked issues for sharing │
|
||||
│ 4. Check for skill adoption (findWorktreeByBranch) │
|
||||
│ 5. Create new if needed │
|
||||
│ 6. Send UX message │
|
||||
│ 7. Update database │
|
||||
└───────────────────────────────────┬─────────────────────────────────────┘
|
||||
│
|
||||
┌───────────────┴───────────────┐
|
||||
▼ ▼
|
||||
┌───────────────────────────────┐ ┌───────────────────────────────────┐
|
||||
│ ISOLATION PROVIDER │ │ CLEANUP SERVICE │
|
||||
│ (WorktreeProvider) │ │ src/services/cleanup-service.ts │
|
||||
├───────────────────────────────┤ ├───────────────────────────────────┤
|
||||
│ create() → IsolatedEnv │ │ onConversationClosed() │
|
||||
│ destroy() │ │ runScheduledCleanup() │
|
||||
│ get() / list() │ │ isBranchMerged() - git-first │
|
||||
│ adopt() │ │ hasUncommittedChanges() │
|
||||
└───────────────────────────────┘ └───────────────────────────────────┘
|
||||
```
|
||||
|
||||
### New Database Schema
|
||||
|
||||
```sql
|
||||
-- Work-centric isolation (independent lifecycle)
|
||||
CREATE TABLE remote_agent_isolation_environments (
|
||||
id UUID PRIMARY KEY,
|
||||
codebase_id UUID REFERENCES remote_agent_codebases(id),
|
||||
workflow_type TEXT NOT NULL, -- 'issue', 'pr', 'thread', 'task'
|
||||
workflow_id TEXT NOT NULL, -- '42', 'thread-abc123'
|
||||
provider TEXT DEFAULT 'worktree',
|
||||
working_path TEXT NOT NULL,
|
||||
branch_name TEXT NOT NULL,
|
||||
status TEXT DEFAULT 'active',
|
||||
created_at TIMESTAMP DEFAULT NOW(),
|
||||
created_by_platform TEXT,
|
||||
metadata JSONB DEFAULT '{}',
|
||||
UNIQUE (codebase_id, workflow_type, workflow_id)
|
||||
);
|
||||
|
||||
-- Conversations link to environments (many-to-one)
|
||||
ALTER TABLE remote_agent_conversations
|
||||
ADD COLUMN isolation_env_id UUID REFERENCES remote_agent_isolation_environments(id);
|
||||
```
|
||||
|
||||
### Implementation Phases
|
||||
|
||||
| Phase | Description | Status |
|
||||
| ----- | ------------------------------------------- | ------- |
|
||||
| 2.5 | Unified Isolation Architecture | Planned |
|
||||
| 3A | Force-Thread Response Model (Slack/Discord) | Planned |
|
||||
| 3C | Git-Based Cleanup Scheduler | Planned |
|
||||
| 3D | Limits and User Feedback | Planned |
|
||||
| 4 | Drop Legacy Columns | Planned |
|
||||
|
||||
See `docs/worktree-orchestration-research.md` for detailed implementation plans.
|
||||
|
|
|
|||
70
migrations/006_isolation_environments.sql
Normal file
70
migrations/006_isolation_environments.sql
Normal file
|
|
@ -0,0 +1,70 @@
|
|||
-- Work-centric isolation environments
|
||||
-- Version: 6.0
|
||||
-- Description: Independent isolation entities with workflow identity
|
||||
|
||||
CREATE TABLE IF NOT EXISTS remote_agent_isolation_environments (
|
||||
id UUID PRIMARY KEY DEFAULT gen_random_uuid(),
|
||||
codebase_id UUID NOT NULL REFERENCES remote_agent_codebases(id) ON DELETE CASCADE,
|
||||
|
||||
-- Workflow identification (what work this is for)
|
||||
workflow_type TEXT NOT NULL, -- 'issue', 'pr', 'review', 'thread', 'task'
|
||||
workflow_id TEXT NOT NULL, -- '42', 'pr-99', 'thread-abc123'
|
||||
|
||||
-- Implementation details
|
||||
provider TEXT NOT NULL DEFAULT 'worktree',
|
||||
working_path TEXT NOT NULL, -- Actual filesystem path
|
||||
branch_name TEXT NOT NULL, -- Git branch name
|
||||
|
||||
-- Lifecycle
|
||||
status TEXT NOT NULL DEFAULT 'active', -- 'active', 'destroyed'
|
||||
created_at TIMESTAMP WITH TIME ZONE DEFAULT NOW(),
|
||||
created_by_platform TEXT, -- 'github', 'slack', etc.
|
||||
|
||||
-- Cross-reference metadata (for linking)
|
||||
metadata JSONB DEFAULT '{}',
|
||||
|
||||
CONSTRAINT unique_workflow UNIQUE (codebase_id, workflow_type, workflow_id)
|
||||
);
|
||||
|
||||
-- Indexes for common queries
|
||||
CREATE INDEX IF NOT EXISTS idx_isolation_env_codebase
|
||||
ON remote_agent_isolation_environments(codebase_id);
|
||||
CREATE INDEX IF NOT EXISTS idx_isolation_env_status
|
||||
ON remote_agent_isolation_environments(status);
|
||||
CREATE INDEX IF NOT EXISTS idx_isolation_env_workflow
|
||||
ON remote_agent_isolation_environments(workflow_type, workflow_id);
|
||||
|
||||
-- Rename old column to legacy (for migration)
|
||||
-- Note: This will fail if column doesn't exist or is already renamed - that's OK
|
||||
DO $$
|
||||
BEGIN
|
||||
IF EXISTS (
|
||||
SELECT 1 FROM information_schema.columns
|
||||
WHERE table_name = 'remote_agent_conversations'
|
||||
AND column_name = 'isolation_env_id'
|
||||
AND data_type = 'character varying'
|
||||
) THEN
|
||||
ALTER TABLE remote_agent_conversations
|
||||
RENAME COLUMN isolation_env_id TO isolation_env_id_legacy;
|
||||
END IF;
|
||||
END $$;
|
||||
|
||||
-- Add new UUID FK column
|
||||
ALTER TABLE remote_agent_conversations
|
||||
ADD COLUMN IF NOT EXISTS isolation_env_id UUID
|
||||
REFERENCES remote_agent_isolation_environments(id) ON DELETE SET NULL;
|
||||
|
||||
-- Add last_activity_at for staleness detection
|
||||
ALTER TABLE remote_agent_conversations
|
||||
ADD COLUMN IF NOT EXISTS last_activity_at TIMESTAMP WITH TIME ZONE DEFAULT NOW();
|
||||
|
||||
-- Create index for FK lookups
|
||||
CREATE INDEX IF NOT EXISTS idx_conversations_isolation_env_id
|
||||
ON remote_agent_conversations(isolation_env_id);
|
||||
|
||||
COMMENT ON TABLE remote_agent_isolation_environments IS
|
||||
'Work-centric isolated environments with independent lifecycle';
|
||||
COMMENT ON COLUMN remote_agent_isolation_environments.workflow_type IS
|
||||
'Type of work: issue, pr, review, thread, task';
|
||||
COMMENT ON COLUMN remote_agent_isolation_environments.workflow_id IS
|
||||
'Identifier for the work (issue number, PR number, thread hash, etc.)';
|
||||
|
|
@ -210,7 +210,8 @@ describe('GitHubAdapter', () => {
|
|||
});
|
||||
|
||||
const crypto = require('crypto');
|
||||
const signature = 'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
const signature =
|
||||
'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
|
||||
await adapter.handleWebhook(payload, signature);
|
||||
|
||||
|
|
@ -290,7 +291,8 @@ describe('GitHubAdapter', () => {
|
|||
});
|
||||
|
||||
const crypto = require('crypto');
|
||||
const signature = 'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
const signature =
|
||||
'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
|
||||
await adapter.handleWebhook(payload, signature);
|
||||
|
||||
|
|
@ -373,7 +375,8 @@ describe('GitHubAdapter', () => {
|
|||
});
|
||||
|
||||
const crypto = require('crypto');
|
||||
const signature = 'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
const signature =
|
||||
'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
|
||||
await adapter.handleWebhook(payload, signature);
|
||||
|
||||
|
|
@ -447,7 +450,8 @@ describe('GitHubAdapter', () => {
|
|||
});
|
||||
|
||||
const crypto = require('crypto');
|
||||
const signature = 'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
const signature =
|
||||
'sha256=' + crypto.createHmac('sha256', 'test-secret').update(payload).digest('hex');
|
||||
|
||||
await adapter.handleWebhook(payload, signature);
|
||||
|
||||
|
|
|
|||
|
|
@ -4,20 +4,19 @@
|
|||
*/
|
||||
import { Octokit } from '@octokit/rest';
|
||||
import { createHmac, timingSafeEqual } from 'crypto';
|
||||
import { IPlatformAdapter } from '../types';
|
||||
import { IPlatformAdapter, IsolationHints } from '../types';
|
||||
import { handleMessage } from '../orchestrator/orchestrator';
|
||||
import { classifyAndFormatError } from '../utils/error-formatter';
|
||||
import * as db from '../db/conversations';
|
||||
import * as codebaseDb from '../db/codebases';
|
||||
import * as sessionDb from '../db/sessions';
|
||||
import { exec } from 'child_process';
|
||||
import { promisify } from 'util';
|
||||
import { readdir, access } from 'fs/promises';
|
||||
import { join, resolve } from 'path';
|
||||
import { parseAllowedUsers, isGitHubUserAuthorized } from '../utils/github-auth';
|
||||
import { isWorktreePath } from '../utils/git';
|
||||
import { getIsolationProvider } from '../isolation';
|
||||
import { getLinkedIssueNumbers } from '../utils/github-graphql';
|
||||
import { onConversationClosed } from '../services/cleanup-service';
|
||||
import { isWorktreePath } from '../utils/git';
|
||||
|
||||
const execAsync = promisify(exec);
|
||||
|
||||
|
|
@ -439,80 +438,19 @@ export class GitHubAdapter implements IPlatformAdapter {
|
|||
|
||||
/**
|
||||
* Clean up worktree when an issue/PR is closed
|
||||
* Handles shared worktrees: only removes if no other conversations reference it
|
||||
* Delegates to cleanup service for unified handling
|
||||
*/
|
||||
private async cleanupWorktree(owner: string, repo: string, number: number): Promise<void> {
|
||||
const conversationId = this.buildConversationId(owner, repo, number);
|
||||
const conversation = await db.getConversationByPlatformId('github', conversationId);
|
||||
console.log(`[GitHub] Cleaning up isolation for ${conversationId}`);
|
||||
|
||||
// Check both old and new isolation fields for backwards compatibility
|
||||
if (!conversation) {
|
||||
console.log(`[GitHub] No conversation to cleanup for ${conversationId}`);
|
||||
return;
|
||||
}
|
||||
|
||||
const isolationEnvId = conversation.isolation_env_id ?? conversation.worktree_path;
|
||||
if (!isolationEnvId) {
|
||||
console.log(`[GitHub] No worktree to cleanup for ${conversationId}`);
|
||||
return;
|
||||
}
|
||||
|
||||
// Deactivate any active session for this conversation
|
||||
// This prevents resume attempts with a stale cwd
|
||||
const activeSession = await sessionDb.getActiveSession(conversation.id);
|
||||
if (activeSession) {
|
||||
await sessionDb.deactivateSession(activeSession.id);
|
||||
console.log(`[GitHub] Deactivated session ${activeSession.id} for worktree cleanup`);
|
||||
}
|
||||
|
||||
const { codebase } = await this.getOrCreateCodebaseForRepo(owner, repo);
|
||||
|
||||
// Clear isolation references from THIS conversation first
|
||||
// This must happen before checking for other users of the worktree
|
||||
await db.updateConversation(conversation.id, {
|
||||
worktree_path: null,
|
||||
isolation_env_id: null,
|
||||
isolation_provider: null,
|
||||
cwd: codebase.default_cwd,
|
||||
});
|
||||
|
||||
// Check if any OTHER conversations still use this worktree (shared worktree case)
|
||||
// Check both old and new fields for backwards compatibility
|
||||
const otherConvByWorktree = await db.getConversationByWorktreePath(isolationEnvId);
|
||||
const otherConvByEnvId = await db.getConversationByIsolationEnvId(isolationEnvId);
|
||||
const otherConv = otherConvByWorktree ?? otherConvByEnvId;
|
||||
if (otherConv) {
|
||||
console.log(
|
||||
`[GitHub] Keeping worktree ${isolationEnvId}, still used by ${otherConv.platform_conversation_id}`
|
||||
);
|
||||
console.log(`[GitHub] Cleanup complete for ${conversationId}`);
|
||||
return;
|
||||
}
|
||||
|
||||
// No other conversations use this worktree - safe to remove via provider
|
||||
const provider = getIsolationProvider();
|
||||
try {
|
||||
await provider.destroy(isolationEnvId);
|
||||
console.log(`[GitHub] Removed worktree: ${isolationEnvId}`);
|
||||
await onConversationClosed('github', conversationId);
|
||||
console.log(`[GitHub] Cleanup complete for ${conversationId}`);
|
||||
} catch (error) {
|
||||
const err = error as Error;
|
||||
// Handle already-deleted worktree gracefully (e.g., manual cleanup)
|
||||
if (err.message.includes('is not a working tree')) {
|
||||
console.log(`[GitHub] Worktree already removed: ${isolationEnvId}`);
|
||||
} else {
|
||||
console.error('[GitHub] Failed to remove worktree:', error);
|
||||
// Notify user about orphaned worktree (likely has uncommitted changes)
|
||||
const hasUncommittedChanges = err.message.includes('contains modified or untracked files');
|
||||
if (hasUncommittedChanges) {
|
||||
await this.sendMessage(
|
||||
conversationId,
|
||||
`Warning: Could not remove worktree at \`${isolationEnvId}\` because it contains uncommitted changes. You may want to manually commit or discard these changes.`
|
||||
);
|
||||
}
|
||||
}
|
||||
console.error(`[GitHub] Cleanup failed for ${conversationId}:`, err.message);
|
||||
}
|
||||
|
||||
console.log(`[GitHub] Cleanup complete for ${conversationId}`);
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -626,130 +564,40 @@ ${userComment}`;
|
|||
await this.autoDetectAndLoadCommands(repoPath, codebase.id);
|
||||
}
|
||||
|
||||
// 10. Create worktree for this issue/PR (if conversation doesn't have one)
|
||||
let worktreePath: string | null = null;
|
||||
let prHeadBranch: string | undefined;
|
||||
let prHeadSha: string | undefined;
|
||||
// Detect PR: either pull_request event, or issue_comment on a PR (indicated by issue.pull_request or pullRequest)
|
||||
// 10. Gather isolation hints for orchestrator
|
||||
// The orchestrator now handles all isolation decisions
|
||||
const isPR = eventType === 'pull_request' || !!pullRequest || !!issue?.pull_request;
|
||||
const existingIsolation = existingConv.isolation_env_id ?? existingConv.worktree_path;
|
||||
if (!existingIsolation) {
|
||||
// For PRs: Check if this PR is linked to an existing issue with a worktree
|
||||
if (isPR) {
|
||||
const linkedIssues = await getLinkedIssueNumbers(owner, repo, number);
|
||||
|
||||
for (const issueNum of linkedIssues) {
|
||||
// Check if the linked issue has a worktree we can reuse
|
||||
const issueConvId = this.buildConversationId(owner, repo, issueNum);
|
||||
const issueConv = await db.getConversationByPlatformId('github', issueConvId);
|
||||
// Build isolation hints for orchestrator
|
||||
const isolationHints: IsolationHints = {
|
||||
workflowType: isPR ? 'pr' : 'issue',
|
||||
workflowId: String(number),
|
||||
};
|
||||
|
||||
const issueIsolation = issueConv?.isolation_env_id ?? issueConv?.worktree_path;
|
||||
if (issueIsolation) {
|
||||
// Reuse the issue's worktree
|
||||
worktreePath = issueIsolation;
|
||||
console.log(
|
||||
`[GitHub] PR #${String(number)} linked to issue #${String(issueNum)}, sharing worktree: ${worktreePath}`
|
||||
);
|
||||
|
||||
// Send user feedback about shared worktree
|
||||
await this.sendMessage(
|
||||
conversationId,
|
||||
`Reusing worktree from issue #${String(issueNum)}`
|
||||
);
|
||||
|
||||
// Update this conversation to use the shared worktree
|
||||
await db.updateConversation(existingConv.id, {
|
||||
codebase_id: codebase.id,
|
||||
cwd: worktreePath,
|
||||
worktree_path: worktreePath,
|
||||
isolation_env_id: issueConv?.isolation_env_id ?? worktreePath,
|
||||
isolation_provider: issueConv?.isolation_provider ?? 'worktree',
|
||||
});
|
||||
break; // Use first found worktree
|
||||
}
|
||||
}
|
||||
// For PRs: get linked issues and branch info
|
||||
if (isPR) {
|
||||
// Get linked issues for worktree sharing
|
||||
const linkedIssues = await getLinkedIssueNumbers(owner, repo, number);
|
||||
if (linkedIssues.length > 0) {
|
||||
isolationHints.linkedIssues = linkedIssues;
|
||||
console.log(`[GitHub] PR #${String(number)} linked to issues: ${linkedIssues.join(', ')}`);
|
||||
}
|
||||
|
||||
// If no shared worktree found, create new one via provider
|
||||
if (!worktreePath) {
|
||||
try {
|
||||
// For PRs, fetch the head branch name and SHA from GitHub API
|
||||
if (isPR) {
|
||||
try {
|
||||
const { data: prData } = await this.octokit.rest.pulls.get({
|
||||
owner,
|
||||
repo,
|
||||
pull_number: number,
|
||||
});
|
||||
prHeadBranch = prData.head.ref;
|
||||
prHeadSha = prData.head.sha;
|
||||
console.log(
|
||||
`[GitHub] PR #${String(number)} head branch: ${prHeadBranch}, SHA: ${prHeadSha}`
|
||||
);
|
||||
} catch (error) {
|
||||
console.warn(
|
||||
'[GitHub] Failed to fetch PR head branch, will create new branch instead:',
|
||||
error
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Use isolation provider for worktree creation
|
||||
const provider = getIsolationProvider();
|
||||
const env = await provider.create({
|
||||
codebaseId: codebase.id,
|
||||
canonicalRepoPath: repoPath,
|
||||
workflowType: isPR ? 'pr' : 'issue',
|
||||
identifier: String(number),
|
||||
prBranch: prHeadBranch,
|
||||
prSha: prHeadSha,
|
||||
description: `GitHub ${isPR ? 'PR' : 'issue'} #${String(number)}`,
|
||||
});
|
||||
|
||||
worktreePath = env.workingPath;
|
||||
console.log(`[GitHub] Created worktree: ${worktreePath}`);
|
||||
|
||||
// Send user feedback about isolation (single line, not verbose)
|
||||
if (isPR && prHeadBranch && prHeadSha) {
|
||||
const shortSha = prHeadSha.substring(0, 7);
|
||||
await this.sendMessage(
|
||||
conversationId,
|
||||
`Reviewing PR at commit \`${shortSha}\` (branch: \`${prHeadBranch}\`)`
|
||||
);
|
||||
} else {
|
||||
const branchName = isPR ? `pr-${String(number)}` : `issue-${String(number)}`;
|
||||
await this.sendMessage(
|
||||
conversationId,
|
||||
`Working in isolated branch \`${branchName}\``
|
||||
);
|
||||
}
|
||||
|
||||
// Update conversation with isolation info (both old and new fields for compatibility)
|
||||
await db.updateConversation(existingConv.id, {
|
||||
codebase_id: codebase.id,
|
||||
cwd: worktreePath,
|
||||
worktree_path: worktreePath,
|
||||
isolation_env_id: env.id,
|
||||
isolation_provider: env.provider,
|
||||
});
|
||||
} catch (error) {
|
||||
const err = error as Error;
|
||||
console.error('[GitHub] Failed to create worktree:', error);
|
||||
const branchName = isPR ? `pr-${String(number)}` : `issue-${String(number)}`;
|
||||
await this.sendMessage(
|
||||
conversationId,
|
||||
`Failed to create isolated worktree for branch \`${branchName}\`. This may be due to a branch name conflict or filesystem issue.\n\nError: ${err.message}\n\nPlease resolve the issue and try again.`
|
||||
);
|
||||
return; // Don't continue without isolation
|
||||
}
|
||||
// Fetch PR head branch and SHA for isolation
|
||||
try {
|
||||
const { data: prData } = await this.octokit.rest.pulls.get({
|
||||
owner,
|
||||
repo,
|
||||
pull_number: number,
|
||||
});
|
||||
isolationHints.prBranch = prData.head.ref;
|
||||
isolationHints.prSha = prData.head.sha;
|
||||
console.log(
|
||||
`[GitHub] PR #${String(number)} head: ${prData.head.ref}@${prData.head.sha.substring(0, 7)}`
|
||||
);
|
||||
} catch (error) {
|
||||
console.warn('[GitHub] Failed to fetch PR head info:', error);
|
||||
}
|
||||
} else {
|
||||
// Conversation already has isolation, use it
|
||||
worktreePath = existingIsolation;
|
||||
|
||||
// Send user feedback about reusing existing worktree
|
||||
const branchName = isPR ? `pr-${String(number)}` : `issue-${String(number)}`;
|
||||
await this.sendMessage(conversationId, `Reusing worktree \`${branchName}\``);
|
||||
}
|
||||
|
||||
// 11. Build message with context
|
||||
|
|
@ -758,38 +606,27 @@ ${userComment}`;
|
|||
let contextToAppend: string | undefined;
|
||||
|
||||
// IMPORTANT: Slash commands must be processed deterministically (not by AI)
|
||||
// Extract only the first line if it's a slash command
|
||||
const isSlashCommand = strippedComment.trim().startsWith('/');
|
||||
const isCommandInvoke = strippedComment.trim().startsWith('/command-invoke');
|
||||
|
||||
if (isSlashCommand) {
|
||||
// For slash commands, use only the first line to avoid mixing commands with instructions
|
||||
const firstLine = strippedComment.split('\n')[0].trim();
|
||||
finalMessage = firstLine;
|
||||
console.log(`[GitHub] Processing slash command: ${firstLine}`);
|
||||
// For slash commands, use only the first line
|
||||
finalMessage = strippedComment.split('\n')[0].trim();
|
||||
console.log(`[GitHub] Processing slash command: ${finalMessage}`);
|
||||
|
||||
// For /command-invoke, pass just the issue/PR number (not full description)
|
||||
// This avoids tempting the AI to implement before planning
|
||||
if (isCommandInvoke) {
|
||||
const activeSession = await sessionDb.getActiveSession(existingConv.id);
|
||||
const isFirstCommandInvoke = !activeSession;
|
||||
|
||||
if (isFirstCommandInvoke) {
|
||||
console.log('[GitHub] Adding issue/PR reference for first /command-invoke');
|
||||
if (eventType === 'issue' && issue) {
|
||||
contextToAppend = `GitHub Issue #${String(issue.number)}: "${issue.title}"\nUse 'gh issue view ${String(issue.number)}' for full details if needed.`;
|
||||
} else if (eventType === 'issue_comment' && issue) {
|
||||
contextToAppend = `GitHub Issue #${String(issue.number)}: "${issue.title}"\nUse 'gh issue view ${String(issue.number)}' for full details if needed.`;
|
||||
} else if (eventType === 'pull_request' && pullRequest) {
|
||||
contextToAppend = `GitHub Pull Request #${String(pullRequest.number)}: "${pullRequest.title}"\nUse 'gh pr view ${String(pullRequest.number)}' for full details if needed.`;
|
||||
} else if (eventType === 'issue_comment' && pullRequest) {
|
||||
contextToAppend = `GitHub Pull Request #${String(pullRequest.number)}: "${pullRequest.title}"\nUse 'gh pr view ${String(pullRequest.number)}' for full details if needed.`;
|
||||
}
|
||||
// Add issue/PR reference context
|
||||
if (eventType === 'issue' && issue) {
|
||||
contextToAppend = `GitHub Issue #${String(issue.number)}: "${issue.title}"\nUse 'gh issue view ${String(issue.number)}' for full details if needed.`;
|
||||
} else if (eventType === 'pull_request' && pullRequest) {
|
||||
contextToAppend = `GitHub Pull Request #${String(pullRequest.number)}: "${pullRequest.title}"\nUse 'gh pr view ${String(pullRequest.number)}' for full details if needed.`;
|
||||
} else if (eventType === 'issue_comment') {
|
||||
if (pullRequest) {
|
||||
contextToAppend = `GitHub Pull Request #${String(pullRequest.number)}: "${pullRequest.title}"\nUse 'gh pr view ${String(pullRequest.number)}' for full details if needed.`;
|
||||
} else if (issue) {
|
||||
contextToAppend = `GitHub Issue #${String(issue.number)}: "${issue.title}"\nUse 'gh issue view ${String(issue.number)}' for full details if needed.`;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// For non-command messages, always add issue/PR context
|
||||
// Router needs this context to understand what the user is asking about
|
||||
// For non-command messages, add rich context
|
||||
if (eventType === 'issue' && issue) {
|
||||
finalMessage = this.buildIssueContext(issue, strippedComment);
|
||||
} else if (eventType === 'issue_comment' && issue) {
|
||||
|
|
@ -801,35 +638,17 @@ ${userComment}`;
|
|||
}
|
||||
}
|
||||
|
||||
// Add worktree context if working in an isolated branch
|
||||
if (worktreePath) {
|
||||
// Use the actual PR head branch name if available, otherwise use the worktree naming convention
|
||||
const branchName =
|
||||
isPR && prHeadBranch
|
||||
? prHeadBranch
|
||||
: isPR
|
||||
? `pr-${String(number)}`
|
||||
: `issue-${String(number)}`;
|
||||
let worktreeContext: string;
|
||||
|
||||
if (isPR && prHeadBranch && prHeadSha) {
|
||||
// SHA-based PR review - show commit SHA for reproducibility
|
||||
const shortSha = prHeadSha.substring(0, 7);
|
||||
worktreeContext = `\n\n[Working in isolated worktree at commit ${shortSha} (PR #${String(number)} branch: ${branchName}). This is the exact commit that triggered the review for reproducibility.]`;
|
||||
} else if (isPR && prHeadBranch) {
|
||||
// Branch-based PR review (fallback)
|
||||
worktreeContext = `\n\n[Working in isolated worktree with PR branch: ${branchName}. This is the actual PR branch with all changes.]`;
|
||||
} else {
|
||||
// Issue workflow
|
||||
worktreeContext = `\n\n[Working in isolated branch: ${branchName}. When done, changes can be committed and pushed, then a PR can be created from this branch.]`;
|
||||
}
|
||||
|
||||
contextToAppend = contextToAppend ? contextToAppend + worktreeContext : worktreeContext;
|
||||
}
|
||||
|
||||
// 12. Route to orchestrator
|
||||
// 12. Route to orchestrator with isolation hints
|
||||
try {
|
||||
await handleMessage(this, conversationId, finalMessage, contextToAppend);
|
||||
await handleMessage(
|
||||
this,
|
||||
conversationId,
|
||||
finalMessage,
|
||||
contextToAppend,
|
||||
undefined, // threadContext
|
||||
undefined, // parentConversationId
|
||||
isolationHints
|
||||
);
|
||||
} catch (error) {
|
||||
const err = error as Error;
|
||||
console.error('[GitHub] Message handling error:', error);
|
||||
|
|
|
|||
|
|
@ -138,15 +138,33 @@ export async function updateConversation(
|
|||
}
|
||||
|
||||
/**
|
||||
* Find a conversation by isolation environment ID
|
||||
* Find a conversation by isolation environment ID (legacy - single result)
|
||||
* Used for provider-based lookup and shared environment detection
|
||||
*/
|
||||
export async function getConversationByIsolationEnvId(
|
||||
envId: string
|
||||
): Promise<Conversation | null> {
|
||||
export async function getConversationByIsolationEnvId(envId: string): Promise<Conversation | null> {
|
||||
const result = await pool.query<Conversation>(
|
||||
'SELECT * FROM remote_agent_conversations WHERE isolation_env_id = $1 LIMIT 1',
|
||||
[envId]
|
||||
);
|
||||
return result.rows[0] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find all conversations using a specific isolation environment (new UUID model)
|
||||
*/
|
||||
export async function getConversationsByIsolationEnvId(envId: string): Promise<Conversation[]> {
|
||||
const result = await pool.query<Conversation>(
|
||||
'SELECT * FROM remote_agent_conversations WHERE isolation_env_id = $1',
|
||||
[envId]
|
||||
);
|
||||
return result.rows;
|
||||
}
|
||||
|
||||
/**
|
||||
* Update last_activity_at for staleness tracking
|
||||
*/
|
||||
export async function touchConversation(id: string): Promise<void> {
|
||||
await pool.query('UPDATE remote_agent_conversations SET last_activity_at = NOW() WHERE id = $1', [
|
||||
id,
|
||||
]);
|
||||
}
|
||||
|
|
|
|||
237
src/db/isolation-environments.test.ts
Normal file
237
src/db/isolation-environments.test.ts
Normal file
|
|
@ -0,0 +1,237 @@
|
|||
import { mock, describe, test, expect, beforeEach } from 'bun:test';
|
||||
import { createQueryResult } from '../test/mocks/database';
|
||||
import { IsolationEnvironmentRow } from '../types';
|
||||
|
||||
const mockQuery = mock(() => Promise.resolve(createQueryResult([])));
|
||||
|
||||
mock.module('./connection', () => ({
|
||||
pool: {
|
||||
query: mockQuery,
|
||||
},
|
||||
}));
|
||||
|
||||
import {
|
||||
getById,
|
||||
findByWorkflow,
|
||||
listByCodebase,
|
||||
create,
|
||||
updateStatus,
|
||||
updateMetadata,
|
||||
countByCodebase,
|
||||
getConversationsUsingEnv,
|
||||
} from './isolation-environments';
|
||||
|
||||
describe('isolation-environments', () => {
|
||||
beforeEach(() => {
|
||||
mockQuery.mockClear();
|
||||
});
|
||||
|
||||
const sampleEnv: IsolationEnvironmentRow = {
|
||||
id: 'env-123',
|
||||
codebase_id: 'codebase-456',
|
||||
workflow_type: 'issue',
|
||||
workflow_id: '42',
|
||||
provider: 'worktree',
|
||||
working_path: '/workspace/worktrees/project/issue-42',
|
||||
branch_name: 'issue-42',
|
||||
status: 'active',
|
||||
created_at: new Date(),
|
||||
created_by_platform: 'github',
|
||||
metadata: {},
|
||||
};
|
||||
|
||||
describe('getById', () => {
|
||||
test('returns environment when found', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([sampleEnv]));
|
||||
|
||||
const result = await getById('env-123');
|
||||
|
||||
expect(result).toEqual(sampleEnv);
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
'SELECT * FROM remote_agent_isolation_environments WHERE id = $1',
|
||||
['env-123']
|
||||
);
|
||||
});
|
||||
|
||||
test('returns null when not found', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([]));
|
||||
|
||||
const result = await getById('nonexistent');
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('findByWorkflow', () => {
|
||||
test('finds active environment by workflow identity', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([sampleEnv]));
|
||||
|
||||
const result = await findByWorkflow('codebase-456', 'issue', '42');
|
||||
|
||||
expect(result).toEqual(sampleEnv);
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
expect.stringContaining('workflow_type = $2 AND workflow_id = $3'),
|
||||
['codebase-456', 'issue', '42']
|
||||
);
|
||||
});
|
||||
|
||||
test('returns null when no matching active environment', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([]));
|
||||
|
||||
const result = await findByWorkflow('codebase-456', 'issue', '99');
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('listByCodebase', () => {
|
||||
test('returns all active environments for codebase', async () => {
|
||||
const envs = [sampleEnv, { ...sampleEnv, id: 'env-456', workflow_id: '43' }];
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult(envs));
|
||||
|
||||
const result = await listByCodebase('codebase-456');
|
||||
|
||||
expect(result).toEqual(envs);
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
expect.stringContaining('codebase_id = $1 AND status'),
|
||||
['codebase-456']
|
||||
);
|
||||
});
|
||||
|
||||
test('returns empty array when no environments', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([]));
|
||||
|
||||
const result = await listByCodebase('empty-codebase');
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('create', () => {
|
||||
test('creates new environment with defaults', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([sampleEnv]));
|
||||
|
||||
const result = await create({
|
||||
codebase_id: 'codebase-456',
|
||||
workflow_type: 'issue',
|
||||
workflow_id: '42',
|
||||
working_path: '/workspace/worktrees/project/issue-42',
|
||||
branch_name: 'issue-42',
|
||||
});
|
||||
|
||||
expect(result).toEqual(sampleEnv);
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
expect.stringContaining('INSERT INTO remote_agent_isolation_environments'),
|
||||
expect.arrayContaining(['codebase-456', 'issue', '42', 'worktree'])
|
||||
);
|
||||
});
|
||||
|
||||
test('creates environment with custom provider and metadata', async () => {
|
||||
const customEnv = { ...sampleEnv, provider: 'container', metadata: { custom: true } };
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([customEnv]));
|
||||
|
||||
await create({
|
||||
codebase_id: 'codebase-456',
|
||||
workflow_type: 'issue',
|
||||
workflow_id: '42',
|
||||
provider: 'container',
|
||||
working_path: '/workspace/worktrees/project/issue-42',
|
||||
branch_name: 'issue-42',
|
||||
created_by_platform: 'slack',
|
||||
metadata: { custom: true },
|
||||
});
|
||||
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
expect.stringContaining('INSERT INTO remote_agent_isolation_environments'),
|
||||
[
|
||||
'codebase-456',
|
||||
'issue',
|
||||
'42',
|
||||
'container',
|
||||
'/workspace/worktrees/project/issue-42',
|
||||
'issue-42',
|
||||
'slack',
|
||||
'{"custom":true}',
|
||||
]
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('updateStatus', () => {
|
||||
test('updates status to destroyed', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([], 1));
|
||||
|
||||
await updateStatus('env-123', 'destroyed');
|
||||
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
'UPDATE remote_agent_isolation_environments SET status = $1 WHERE id = $2',
|
||||
['destroyed', 'env-123']
|
||||
);
|
||||
});
|
||||
|
||||
test('updates status to active', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([], 1));
|
||||
|
||||
await updateStatus('env-123', 'active');
|
||||
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
'UPDATE remote_agent_isolation_environments SET status = $1 WHERE id = $2',
|
||||
['active', 'env-123']
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe('updateMetadata', () => {
|
||||
test('merges metadata with existing', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([], 1));
|
||||
|
||||
await updateMetadata('env-123', { pr_number: 42 });
|
||||
|
||||
expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('metadata = metadata ||'), [
|
||||
'{"pr_number":42}',
|
||||
'env-123',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('countByCodebase', () => {
|
||||
test('returns count of active environments', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([{ count: '5' }]));
|
||||
|
||||
const result = await countByCodebase('codebase-456');
|
||||
|
||||
expect(result).toBe(5);
|
||||
expect(mockQuery).toHaveBeenCalledWith(expect.stringContaining('COUNT(*)'), ['codebase-456']);
|
||||
});
|
||||
|
||||
test('returns 0 when no environments', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([{ count: '0' }]));
|
||||
|
||||
const result = await countByCodebase('empty-codebase');
|
||||
|
||||
expect(result).toBe(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getConversationsUsingEnv', () => {
|
||||
test('returns conversation IDs using the environment', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([{ id: 'conv-1' }, { id: 'conv-2' }]));
|
||||
|
||||
const result = await getConversationsUsingEnv('env-123');
|
||||
|
||||
expect(result).toEqual(['conv-1', 'conv-2']);
|
||||
expect(mockQuery).toHaveBeenCalledWith(
|
||||
'SELECT id FROM remote_agent_conversations WHERE isolation_env_id = $1',
|
||||
['env-123']
|
||||
);
|
||||
});
|
||||
|
||||
test('returns empty array when no conversations use env', async () => {
|
||||
mockQuery.mockResolvedValueOnce(createQueryResult([]));
|
||||
|
||||
const result = await getConversationsUsingEnv('unused-env');
|
||||
|
||||
expect(result).toEqual([]);
|
||||
});
|
||||
});
|
||||
});
|
||||
140
src/db/isolation-environments.ts
Normal file
140
src/db/isolation-environments.ts
Normal file
|
|
@ -0,0 +1,140 @@
|
|||
/**
|
||||
* Database operations for isolation environments
|
||||
*/
|
||||
import { pool } from './connection';
|
||||
import { IsolationEnvironmentRow } from '../types';
|
||||
|
||||
/**
|
||||
* Get an isolation environment by UUID
|
||||
*/
|
||||
export async function getById(id: string): Promise<IsolationEnvironmentRow | null> {
|
||||
const result = await pool.query<IsolationEnvironmentRow>(
|
||||
'SELECT * FROM remote_agent_isolation_environments WHERE id = $1',
|
||||
[id]
|
||||
);
|
||||
return result.rows[0] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find an isolation environment by workflow identity
|
||||
*/
|
||||
export async function findByWorkflow(
|
||||
codebaseId: string,
|
||||
workflowType: string,
|
||||
workflowId: string
|
||||
): Promise<IsolationEnvironmentRow | null> {
|
||||
const result = await pool.query<IsolationEnvironmentRow>(
|
||||
`SELECT * FROM remote_agent_isolation_environments
|
||||
WHERE codebase_id = $1 AND workflow_type = $2 AND workflow_id = $3 AND status = 'active'`,
|
||||
[codebaseId, workflowType, workflowId]
|
||||
);
|
||||
return result.rows[0] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Find all active environments for a codebase
|
||||
*/
|
||||
export async function listByCodebase(codebaseId: string): Promise<IsolationEnvironmentRow[]> {
|
||||
const result = await pool.query<IsolationEnvironmentRow>(
|
||||
`SELECT * FROM remote_agent_isolation_environments
|
||||
WHERE codebase_id = $1 AND status = 'active'
|
||||
ORDER BY created_at DESC`,
|
||||
[codebaseId]
|
||||
);
|
||||
return result.rows;
|
||||
}
|
||||
|
||||
/**
|
||||
* Create a new isolation environment
|
||||
*/
|
||||
export async function create(env: {
|
||||
codebase_id: string;
|
||||
workflow_type: string;
|
||||
workflow_id: string;
|
||||
provider?: string;
|
||||
working_path: string;
|
||||
branch_name: string;
|
||||
created_by_platform?: string;
|
||||
metadata?: Record<string, unknown>;
|
||||
}): Promise<IsolationEnvironmentRow> {
|
||||
const result = await pool.query<IsolationEnvironmentRow>(
|
||||
`INSERT INTO remote_agent_isolation_environments
|
||||
(codebase_id, workflow_type, workflow_id, provider, working_path, branch_name, created_by_platform, metadata)
|
||||
VALUES ($1, $2, $3, $4, $5, $6, $7, $8)
|
||||
RETURNING *`,
|
||||
[
|
||||
env.codebase_id,
|
||||
env.workflow_type,
|
||||
env.workflow_id,
|
||||
env.provider ?? 'worktree',
|
||||
env.working_path,
|
||||
env.branch_name,
|
||||
env.created_by_platform ?? null,
|
||||
JSON.stringify(env.metadata ?? {}),
|
||||
]
|
||||
);
|
||||
return result.rows[0];
|
||||
}
|
||||
|
||||
/**
|
||||
* Update environment status
|
||||
*/
|
||||
export async function updateStatus(id: string, status: 'active' | 'destroyed'): Promise<void> {
|
||||
await pool.query('UPDATE remote_agent_isolation_environments SET status = $1 WHERE id = $2', [
|
||||
status,
|
||||
id,
|
||||
]);
|
||||
}
|
||||
|
||||
/**
|
||||
* Update environment metadata (merge with existing)
|
||||
*/
|
||||
export async function updateMetadata(id: string, metadata: Record<string, unknown>): Promise<void> {
|
||||
await pool.query(
|
||||
`UPDATE remote_agent_isolation_environments
|
||||
SET metadata = metadata || $1::jsonb
|
||||
WHERE id = $2`,
|
||||
[JSON.stringify(metadata), id]
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Find environments by related issue (from metadata)
|
||||
*/
|
||||
export async function findByRelatedIssue(
|
||||
codebaseId: string,
|
||||
issueNumber: number
|
||||
): Promise<IsolationEnvironmentRow | null> {
|
||||
const result = await pool.query<IsolationEnvironmentRow>(
|
||||
`SELECT * FROM remote_agent_isolation_environments
|
||||
WHERE codebase_id = $1
|
||||
AND status = 'active'
|
||||
AND metadata->'related_issues' ? $2
|
||||
LIMIT 1`,
|
||||
[codebaseId, String(issueNumber)]
|
||||
);
|
||||
return result.rows[0] ?? null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Count active environments for a codebase (for limit checks)
|
||||
*/
|
||||
export async function countByCodebase(codebaseId: string): Promise<number> {
|
||||
const result = await pool.query<{ count: string }>(
|
||||
`SELECT COUNT(*) as count FROM remote_agent_isolation_environments
|
||||
WHERE codebase_id = $1 AND status = 'active'`,
|
||||
[codebaseId]
|
||||
);
|
||||
return parseInt(result.rows[0]?.count ?? '0', 10);
|
||||
}
|
||||
|
||||
/**
|
||||
* Find conversations using an isolation environment
|
||||
*/
|
||||
export async function getConversationsUsingEnv(envId: string): Promise<string[]> {
|
||||
const result = await pool.query<{ id: string }>(
|
||||
'SELECT id FROM remote_agent_conversations WHERE isolation_env_id = $1',
|
||||
[envId]
|
||||
);
|
||||
return result.rows.map(r => r.id);
|
||||
}
|
||||
|
|
@ -852,14 +852,21 @@ describe('CommandHandler', () => {
|
|||
expect(result.success).toBe(true);
|
||||
expect(result.message).toContain('Repository cloned successfully');
|
||||
expect(result.message).toContain('✓ Loaded 2 commands');
|
||||
const updateCall = mockUpdateCodebaseCommands.mock.calls[0] as [string, Record<string, { path: string; description: string }>];
|
||||
const updateCall = mockUpdateCodebaseCommands.mock.calls[0] as [
|
||||
string,
|
||||
Record<string, { path: string; description: string }>,
|
||||
];
|
||||
expect(updateCall[0]).toBe('cb-new');
|
||||
const commands = updateCall[1];
|
||||
expect(commands['test-command']).toBeDefined();
|
||||
expect(commands['another-command']).toBeDefined();
|
||||
// Check path ends with expected relative path (platform-agnostic)
|
||||
expect(commands['test-command'].path).toMatch(/\.claude[\\\/]commands[\\\/]test-command\.md$/);
|
||||
expect(commands['another-command'].path).toMatch(/\.claude[\\\/]commands[\\\/]another-command\.md$/);
|
||||
expect(commands['test-command'].path).toMatch(
|
||||
/\.claude[\\\/]commands[\\\/]test-command\.md$/
|
||||
);
|
||||
expect(commands['another-command'].path).toMatch(
|
||||
/\.claude[\\\/]commands[\\\/]another-command\.md$/
|
||||
);
|
||||
expect(commands['test-command'].description).toBe('From .claude/commands');
|
||||
expect(commands['another-command'].description).toBe('From .claude/commands');
|
||||
});
|
||||
|
|
@ -886,7 +893,10 @@ describe('CommandHandler', () => {
|
|||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.message).toContain('✓ Loaded 1 commands');
|
||||
const updateCall = mockUpdateCodebaseCommands.mock.calls[0] as [string, Record<string, { path: string; description: string }>];
|
||||
const updateCall = mockUpdateCodebaseCommands.mock.calls[0] as [
|
||||
string,
|
||||
Record<string, { path: string; description: string }>,
|
||||
];
|
||||
expect(updateCall[0]).toBe('cb-new');
|
||||
const commands = updateCall[1];
|
||||
expect(commands.rca).toBeDefined();
|
||||
|
|
@ -1023,7 +1033,10 @@ describe('CommandHandler', () => {
|
|||
|
||||
// Verify commands were updated
|
||||
expect(mockUpdateCodebaseCommands).toHaveBeenCalled();
|
||||
const updateCall = mockUpdateCodebaseCommands.mock.calls[0] as [string, Record<string, { path: string; description: string }>];
|
||||
const updateCall = mockUpdateCodebaseCommands.mock.calls[0] as [
|
||||
string,
|
||||
Record<string, { path: string; description: string }>,
|
||||
];
|
||||
const commands = updateCall[1];
|
||||
|
||||
// Should preserve existing command
|
||||
|
|
|
|||
|
|
@ -28,7 +28,7 @@ describe('WorktreeProvider', () => {
|
|||
worktreeExistsSpy.mockResolvedValue(false);
|
||||
listWorktreesSpy.mockResolvedValue([]);
|
||||
findWorktreeByBranchSpy.mockResolvedValue(null);
|
||||
getCanonicalRepoPathSpy.mockImplementation(async (path) => path);
|
||||
getCanonicalRepoPathSpy.mockImplementation(async path => path);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
|
|
@ -150,7 +150,15 @@ describe('WorktreeProvider', () => {
|
|||
// Verify git worktree add was called with -b flag
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', '/workspace/repo', 'worktree', 'add', expect.any(String), '-b', 'issue-42']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'-b',
|
||||
'issue-42',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
});
|
||||
|
|
@ -176,14 +184,28 @@ describe('WorktreeProvider', () => {
|
|||
// Verify worktree add with SHA
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', '/workspace/repo', 'worktree', 'add', expect.any(String), 'abc123def456']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'abc123def456',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
|
||||
// Verify checkout -b for tracking branch
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', expect.any(String), 'checkout', '-b', 'pr-42-review', 'abc123def456']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
expect.any(String),
|
||||
'checkout',
|
||||
'-b',
|
||||
'pr-42-review',
|
||||
'abc123def456',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
});
|
||||
|
|
@ -201,14 +223,27 @@ describe('WorktreeProvider', () => {
|
|||
// Verify fetch with PR ref and local branch creation
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', '/workspace/repo', 'fetch', 'origin', 'pull/42/head:pr-42-review']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'fetch',
|
||||
'origin',
|
||||
'pull/42/head:pr-42-review',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
|
||||
// Verify worktree add with the local branch
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', '/workspace/repo', 'worktree', 'add', expect.any(String), 'pr-42-review']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'pr-42-review',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
});
|
||||
|
|
@ -262,7 +297,9 @@ describe('WorktreeProvider', () => {
|
|||
callCount++;
|
||||
// First worktree add call fails (branch exists)
|
||||
if (callCount === 1 && args.includes('-b')) {
|
||||
const error = new Error('fatal: A branch named issue-42 already exists.') as Error & { stderr?: string };
|
||||
const error = new Error('fatal: A branch named issue-42 already exists.') as Error & {
|
||||
stderr?: string;
|
||||
};
|
||||
error.stderr = 'fatal: A branch named issue-42 already exists.';
|
||||
throw error;
|
||||
}
|
||||
|
|
@ -274,14 +311,29 @@ describe('WorktreeProvider', () => {
|
|||
// Verify first call attempted new branch
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', '/workspace/repo', 'worktree', 'add', expect.any(String), '-b', 'issue-42']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'-b',
|
||||
'issue-42',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
|
||||
// Verify second call used existing branch
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', '/workspace/repo', 'worktree', 'add', expect.any(String), 'issue-42']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'issue-42',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
});
|
||||
|
|
@ -301,7 +353,9 @@ describe('WorktreeProvider', () => {
|
|||
return { stdout: '', stderr: '' };
|
||||
});
|
||||
|
||||
await expect(provider.create(request)).rejects.toThrow('Failed to create worktree for PR #42');
|
||||
await expect(provider.create(request)).rejects.toThrow(
|
||||
'Failed to create worktree for PR #42'
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
|
|
@ -331,7 +385,14 @@ describe('WorktreeProvider', () => {
|
|||
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', '/workspace/repo', 'worktree', 'remove', '--force', worktreePath]),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'worktree',
|
||||
'remove',
|
||||
'--force',
|
||||
worktreePath,
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -278,9 +278,13 @@ export class WorktreeProvider implements IIsolationProvider {
|
|||
});
|
||||
|
||||
// Create worktree at the specific SHA
|
||||
await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, request.prSha], {
|
||||
timeout: 30000,
|
||||
});
|
||||
await execFileAsync(
|
||||
'git',
|
||||
['-C', repoPath, 'worktree', 'add', worktreePath, request.prSha],
|
||||
{
|
||||
timeout: 30000,
|
||||
}
|
||||
);
|
||||
|
||||
// Create a local tracking branch so it's not detached HEAD
|
||||
await execFileAsync(
|
||||
|
|
@ -325,9 +329,13 @@ export class WorktreeProvider implements IIsolationProvider {
|
|||
): Promise<void> {
|
||||
try {
|
||||
// Try to create with new branch
|
||||
await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, '-b', branchName], {
|
||||
timeout: 30000,
|
||||
});
|
||||
await execFileAsync(
|
||||
'git',
|
||||
['-C', repoPath, 'worktree', 'add', worktreePath, '-b', branchName],
|
||||
{
|
||||
timeout: 30000,
|
||||
}
|
||||
);
|
||||
} catch (error) {
|
||||
const err = error as Error & { stderr?: string };
|
||||
// Branch already exists - use existing branch
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import { join } from 'path';
|
|||
// Setup mocks before importing the module under test
|
||||
const mockGetOrCreateConversation = mock(() => Promise.resolve(null));
|
||||
const mockUpdateConversation = mock(() => Promise.resolve());
|
||||
const mockTouchConversation = mock(() => Promise.resolve());
|
||||
const mockGetCodebase = mock(() => Promise.resolve(null));
|
||||
const mockGetActiveSession = mock(() => Promise.resolve(null));
|
||||
const mockCreateSession = mock(() => Promise.resolve(null));
|
||||
|
|
@ -20,11 +21,57 @@ const mockParseCommand = mock((message: string) => {
|
|||
});
|
||||
const mockGetAssistantClient = mock(() => null);
|
||||
const mockReadFile = mock(() => Promise.resolve(''));
|
||||
const mockAccess = mock(() => Promise.resolve());
|
||||
|
||||
// Isolation environment mocks
|
||||
const mockIsolationEnvGetById = mock(() => Promise.resolve(null));
|
||||
const mockIsolationEnvFindByWorkflow = mock(() => Promise.resolve(null));
|
||||
const mockIsolationEnvCreate = mock(() => Promise.resolve(null));
|
||||
const mockIsolationEnvUpdateStatus = mock(() => Promise.resolve());
|
||||
|
||||
// Git utils mocks
|
||||
const mockWorktreeExists = mock(() => Promise.resolve(false));
|
||||
const mockFindWorktreeByBranch = mock(() => Promise.resolve(null));
|
||||
const mockGetCanonicalRepoPath = mock((path: string) => Promise.resolve(path));
|
||||
const mockExecFileAsync = mock(() => Promise.resolve({ stdout: 'main', stderr: '' }));
|
||||
|
||||
// Isolation provider mock
|
||||
const mockIsolationProviderCreate = mock(() =>
|
||||
Promise.resolve({
|
||||
id: 'env-123',
|
||||
provider: 'worktree',
|
||||
workingPath: '/workspace/worktrees/test/thread-abc',
|
||||
branchName: 'thread-abc',
|
||||
status: 'active',
|
||||
createdAt: new Date(),
|
||||
metadata: {},
|
||||
})
|
||||
);
|
||||
const mockGetIsolationProvider = mock(() => ({
|
||||
create: mockIsolationProviderCreate,
|
||||
}));
|
||||
|
||||
mock.module('../db/conversations', () => ({
|
||||
getOrCreateConversation: mockGetOrCreateConversation,
|
||||
updateConversation: mockUpdateConversation,
|
||||
touchConversation: mockTouchConversation,
|
||||
}));
|
||||
|
||||
mock.module('../db/isolation-environments', () => ({
|
||||
getById: mockIsolationEnvGetById,
|
||||
findByWorkflow: mockIsolationEnvFindByWorkflow,
|
||||
create: mockIsolationEnvCreate,
|
||||
updateStatus: mockIsolationEnvUpdateStatus,
|
||||
}));
|
||||
|
||||
mock.module('../utils/git', () => ({
|
||||
worktreeExists: mockWorktreeExists,
|
||||
findWorktreeByBranch: mockFindWorktreeByBranch,
|
||||
getCanonicalRepoPath: mockGetCanonicalRepoPath,
|
||||
execFileAsync: mockExecFileAsync,
|
||||
}));
|
||||
|
||||
mock.module('../isolation', () => ({
|
||||
getIsolationProvider: mockGetIsolationProvider,
|
||||
}));
|
||||
|
||||
mock.module('../db/codebases', () => ({
|
||||
|
|
@ -54,7 +101,6 @@ mock.module('../clients/factory', () => ({
|
|||
|
||||
mock.module('fs/promises', () => ({
|
||||
readFile: mockReadFile,
|
||||
access: mockAccess,
|
||||
}));
|
||||
|
||||
import { handleMessage } from './orchestrator';
|
||||
|
|
@ -84,7 +130,11 @@ describe('orchestrator', () => {
|
|||
ai_assistant_type: 'claude',
|
||||
codebase_id: 'codebase-789',
|
||||
cwd: '/workspace/project',
|
||||
worktree_path: null,
|
||||
worktree_path: '/workspace/project', // Simulate existing worktree
|
||||
isolation_env_id_legacy: null,
|
||||
isolation_env_id: 'env-existing', // Simulate existing isolation
|
||||
isolation_provider: 'worktree',
|
||||
last_activity_at: null,
|
||||
created_at: new Date(),
|
||||
updated_at: new Date(),
|
||||
};
|
||||
|
|
@ -126,6 +176,7 @@ describe('orchestrator', () => {
|
|||
platform = new MockPlatformAdapter();
|
||||
mockGetOrCreateConversation.mockClear();
|
||||
mockUpdateConversation.mockClear();
|
||||
mockTouchConversation.mockClear();
|
||||
mockGetCodebase.mockClear();
|
||||
mockGetActiveSession.mockClear();
|
||||
mockCreateSession.mockClear();
|
||||
|
|
@ -137,10 +188,21 @@ describe('orchestrator', () => {
|
|||
mockParseCommand.mockClear();
|
||||
mockGetAssistantClient.mockClear();
|
||||
mockReadFile.mockClear();
|
||||
mockAccess.mockClear();
|
||||
mockClient.sendQuery.mockClear();
|
||||
mockClient.getType.mockClear();
|
||||
|
||||
// New isolation mocks
|
||||
mockIsolationEnvGetById.mockClear();
|
||||
mockIsolationEnvFindByWorkflow.mockClear();
|
||||
mockIsolationEnvCreate.mockClear();
|
||||
mockIsolationEnvUpdateStatus.mockClear();
|
||||
mockWorktreeExists.mockClear();
|
||||
mockFindWorktreeByBranch.mockClear();
|
||||
mockGetCanonicalRepoPath.mockClear();
|
||||
mockExecFileAsync.mockClear();
|
||||
mockIsolationProviderCreate.mockClear();
|
||||
mockGetIsolationProvider.mockClear();
|
||||
|
||||
// Default mocks
|
||||
mockGetOrCreateConversation.mockResolvedValue(mockConversation);
|
||||
mockGetCodebase.mockResolvedValue(mockCodebase);
|
||||
|
|
@ -148,11 +210,42 @@ describe('orchestrator', () => {
|
|||
mockCreateSession.mockResolvedValue(mockSession);
|
||||
mockGetTemplate.mockResolvedValue(null); // No templates by default
|
||||
mockGetAssistantClient.mockReturnValue(mockClient);
|
||||
mockAccess.mockResolvedValue(undefined); // Path exists by default
|
||||
mockParseCommand.mockImplementation((message: string) => {
|
||||
const parts = message.split(/\s+/);
|
||||
return { command: parts[0].substring(1), args: parts.slice(1) };
|
||||
});
|
||||
|
||||
// Default isolation mocks - simulate existing isolation env
|
||||
mockIsolationEnvGetById.mockResolvedValue({
|
||||
id: 'env-existing',
|
||||
codebase_id: 'codebase-789',
|
||||
workflow_type: 'thread',
|
||||
workflow_id: 'chat-456',
|
||||
provider: 'worktree',
|
||||
working_path: '/workspace/project',
|
||||
branch_name: 'thread-chat-456',
|
||||
status: 'active',
|
||||
created_at: new Date(),
|
||||
created_by_platform: 'telegram',
|
||||
metadata: {},
|
||||
});
|
||||
mockIsolationEnvFindByWorkflow.mockResolvedValue(null);
|
||||
mockIsolationEnvCreate.mockResolvedValue({
|
||||
id: 'env-new',
|
||||
codebase_id: 'codebase-789',
|
||||
workflow_type: 'thread',
|
||||
workflow_id: 'chat-456',
|
||||
provider: 'worktree',
|
||||
working_path: '/workspace/worktrees/test/thread-chat-456',
|
||||
branch_name: 'thread-chat-456',
|
||||
status: 'active',
|
||||
created_at: new Date(),
|
||||
created_by_platform: 'telegram',
|
||||
metadata: {},
|
||||
});
|
||||
mockWorktreeExists.mockResolvedValue(true); // Existing worktree valid
|
||||
mockGetCanonicalRepoPath.mockImplementation((path: string) => Promise.resolve(path));
|
||||
mockExecFileAsync.mockResolvedValue({ stdout: 'main', stderr: '' });
|
||||
});
|
||||
|
||||
describe('slash commands (non-invoke)', () => {
|
||||
|
|
@ -538,11 +631,34 @@ describe('orchestrator', () => {
|
|||
);
|
||||
});
|
||||
|
||||
test('falls back to codebase default_cwd', async () => {
|
||||
test('falls back to codebase default_cwd when no isolation env', async () => {
|
||||
// Conversation without isolation, will get auto-created
|
||||
mockGetOrCreateConversation.mockResolvedValue({
|
||||
...mockConversation,
|
||||
isolation_env_id: null,
|
||||
worktree_path: null,
|
||||
cwd: null,
|
||||
});
|
||||
|
||||
// No isolation env in DB, no existing worktree
|
||||
mockIsolationEnvGetById.mockResolvedValue(null);
|
||||
mockWorktreeExists.mockResolvedValue(false);
|
||||
|
||||
// Auto-create will be triggered, returns a new env
|
||||
mockIsolationEnvCreate.mockResolvedValue({
|
||||
id: 'env-auto-created',
|
||||
codebase_id: 'codebase-789',
|
||||
workflow_type: 'thread',
|
||||
workflow_id: 'chat-456',
|
||||
provider: 'worktree',
|
||||
working_path: '/workspace/test-project/worktrees/thread-chat-456',
|
||||
branch_name: 'thread-chat-456',
|
||||
status: 'active',
|
||||
created_at: new Date(),
|
||||
created_by_platform: 'telegram',
|
||||
metadata: {},
|
||||
});
|
||||
|
||||
mockParseCommand.mockReturnValue({ command: 'command-invoke', args: ['plan'] });
|
||||
mockReadFile.mockResolvedValue('Plan command');
|
||||
mockClient.sendQuery.mockImplementation(async function* () {
|
||||
|
|
@ -551,20 +667,38 @@ describe('orchestrator', () => {
|
|||
|
||||
await handleMessage(platform, 'chat-456', '/command-invoke plan');
|
||||
|
||||
// Uses the auto-created worktree path
|
||||
expect(mockClient.sendQuery).toHaveBeenCalledWith(
|
||||
wrapCommandForExecution('plan', 'Plan command'),
|
||||
'/workspace/test-project', // codebase.default_cwd
|
||||
'claude-session-xyz' // Uses existing session's ID
|
||||
'/workspace/test-project/worktrees/thread-chat-456', // From auto-created env
|
||||
'claude-session-xyz'
|
||||
);
|
||||
});
|
||||
|
||||
test('uses isolation_env_id over worktree_path and cwd', async () => {
|
||||
test('uses isolation_env_id (UUID) to look up working path', async () => {
|
||||
// conversation has a UUID isolation_env_id
|
||||
mockGetOrCreateConversation.mockResolvedValue({
|
||||
...mockConversation,
|
||||
isolation_env_id: '/workspace/isolation-env',
|
||||
isolation_env_id: 'env-priority',
|
||||
worktree_path: '/workspace/old-worktree',
|
||||
cwd: '/workspace/project',
|
||||
});
|
||||
|
||||
// Mock the env lookup to return a specific working_path
|
||||
mockIsolationEnvGetById.mockResolvedValue({
|
||||
id: 'env-priority',
|
||||
codebase_id: 'codebase-789',
|
||||
workflow_type: 'thread',
|
||||
workflow_id: 'chat-456',
|
||||
provider: 'worktree',
|
||||
working_path: '/workspace/isolation-env', // This is the path to use
|
||||
branch_name: 'thread-chat-456',
|
||||
status: 'active',
|
||||
created_at: new Date(),
|
||||
created_by_platform: 'telegram',
|
||||
metadata: {},
|
||||
});
|
||||
|
||||
mockParseCommand.mockReturnValue({ command: 'command-invoke', args: ['plan'] });
|
||||
mockReadFile.mockResolvedValue('Plan command');
|
||||
mockClient.sendQuery.mockImplementation(async function* () {
|
||||
|
|
@ -573,9 +707,10 @@ describe('orchestrator', () => {
|
|||
|
||||
await handleMessage(platform, 'chat-456', '/command-invoke plan');
|
||||
|
||||
// Env lookup is used, working_path from env takes priority
|
||||
expect(mockClient.sendQuery).toHaveBeenCalledWith(
|
||||
wrapCommandForExecution('plan', 'Plan command'),
|
||||
'/workspace/isolation-env', // isolation_env_id takes priority
|
||||
'/workspace/isolation-env', // working_path from isolation env
|
||||
'claude-session-xyz'
|
||||
);
|
||||
});
|
||||
|
|
@ -604,64 +739,32 @@ describe('orchestrator', () => {
|
|||
});
|
||||
|
||||
describe('stale worktree handling', () => {
|
||||
test('should deactivate session and clear worktree when cwd does not exist', async () => {
|
||||
// Setup: conversation with worktree_path that doesn't exist
|
||||
const conversationWithStaleWorktree = {
|
||||
test('should clear isolation fields when isolation_env_id points to non-existent path', async () => {
|
||||
// Setup: conversation with isolation_env_id pointing to stale env
|
||||
const conversationWithStaleIsolation = {
|
||||
...mockConversation,
|
||||
isolation_env_id: 'env-stale',
|
||||
isolation_provider: 'worktree',
|
||||
worktree_path: '/nonexistent/worktree/path',
|
||||
cwd: '/nonexistent/worktree/path',
|
||||
};
|
||||
mockGetOrCreateConversation.mockResolvedValue(conversationWithStaleWorktree);
|
||||
|
||||
// Mock fs.access to throw (path doesn't exist)
|
||||
mockAccess.mockRejectedValueOnce(new Error('ENOENT: no such file or directory'));
|
||||
|
||||
// Mock active session
|
||||
mockGetActiveSession.mockResolvedValue({
|
||||
...mockSession,
|
||||
id: 'stale-session-id',
|
||||
});
|
||||
|
||||
mockParseCommand.mockReturnValue({ command: 'command-invoke', args: ['plan'] });
|
||||
mockReadFile.mockResolvedValue('Plan command');
|
||||
mockClient.sendQuery.mockImplementation(async function* () {
|
||||
yield { type: 'result', sessionId: 'session-id' };
|
||||
});
|
||||
|
||||
await handleMessage(platform, 'chat-456', '/command-invoke plan');
|
||||
|
||||
// Verify session was deactivated
|
||||
expect(mockDeactivateSession).toHaveBeenCalledWith('stale-session-id');
|
||||
|
||||
// Verify worktree_path was cleared
|
||||
expect(mockUpdateConversation).toHaveBeenCalledWith(
|
||||
'conv-123',
|
||||
expect.objectContaining({
|
||||
worktree_path: null,
|
||||
cwd: '/workspace/test-project', // Falls back to codebase default_cwd
|
||||
})
|
||||
);
|
||||
});
|
||||
|
||||
test('should clear all isolation fields when isolation_env_id is stale', async () => {
|
||||
// Setup: conversation with isolation_env_id that doesn't exist
|
||||
const conversationWithStaleIsolation = {
|
||||
...mockConversation,
|
||||
isolation_env_id: '/nonexistent/isolation/path',
|
||||
isolation_provider: 'worktree',
|
||||
worktree_path: null,
|
||||
cwd: '/workspace/project',
|
||||
};
|
||||
mockGetOrCreateConversation.mockResolvedValue(conversationWithStaleIsolation);
|
||||
|
||||
// Mock fs.access to throw (path doesn't exist)
|
||||
mockAccess.mockRejectedValueOnce(new Error('ENOENT: no such file or directory'));
|
||||
|
||||
// Mock active session
|
||||
mockGetActiveSession.mockResolvedValue({
|
||||
...mockSession,
|
||||
id: 'stale-session-id',
|
||||
// Mock: env exists in DB but path doesn't exist on disk
|
||||
mockIsolationEnvGetById.mockResolvedValue({
|
||||
id: 'env-stale',
|
||||
codebase_id: 'codebase-789',
|
||||
workflow_type: 'thread',
|
||||
workflow_id: 'chat-456',
|
||||
provider: 'worktree',
|
||||
working_path: '/nonexistent/worktree/path',
|
||||
branch_name: 'thread-chat-456',
|
||||
status: 'active',
|
||||
created_at: new Date(),
|
||||
created_by_platform: 'telegram',
|
||||
metadata: {},
|
||||
});
|
||||
mockWorktreeExists.mockResolvedValue(false); // Path doesn't exist
|
||||
|
||||
mockParseCommand.mockReturnValue({ command: 'command-invoke', args: ['plan'] });
|
||||
mockReadFile.mockResolvedValue('Plan command');
|
||||
|
|
@ -671,58 +774,22 @@ describe('orchestrator', () => {
|
|||
|
||||
await handleMessage(platform, 'chat-456', '/command-invoke plan');
|
||||
|
||||
// Verify all isolation fields are cleared
|
||||
// Verify isolation fields are cleared
|
||||
expect(mockUpdateConversation).toHaveBeenCalledWith(
|
||||
'conv-123',
|
||||
expect.objectContaining({
|
||||
worktree_path: null,
|
||||
isolation_env_id: null,
|
||||
isolation_provider: null,
|
||||
cwd: '/workspace/test-project',
|
||||
})
|
||||
);
|
||||
|
||||
// Verify env marked as destroyed
|
||||
expect(mockIsolationEnvUpdateStatus).toHaveBeenCalledWith('env-stale', 'destroyed');
|
||||
});
|
||||
|
||||
test('should use default cwd when worktree path is stale', async () => {
|
||||
// Setup: conversation with worktree_path that doesn't exist
|
||||
const conversationWithStaleWorktree = {
|
||||
...mockConversation,
|
||||
worktree_path: '/nonexistent/worktree/path',
|
||||
cwd: '/nonexistent/worktree/path',
|
||||
};
|
||||
mockGetOrCreateConversation.mockResolvedValue(conversationWithStaleWorktree);
|
||||
|
||||
// Mock fs.access to throw (path doesn't exist)
|
||||
mockAccess.mockRejectedValueOnce(new Error('ENOENT'));
|
||||
|
||||
// Create a new session without assistant_session_id (simulating fresh start)
|
||||
const freshSession = {
|
||||
...mockSession,
|
||||
assistant_session_id: null,
|
||||
};
|
||||
mockCreateSession.mockResolvedValue(freshSession);
|
||||
|
||||
mockParseCommand.mockReturnValue({ command: 'command-invoke', args: ['plan'] });
|
||||
mockReadFile.mockResolvedValue('Plan command');
|
||||
mockClient.sendQuery.mockImplementation(async function* () {
|
||||
yield { type: 'result', sessionId: 'session-id' };
|
||||
});
|
||||
|
||||
await handleMessage(platform, 'chat-456', '/command-invoke plan');
|
||||
|
||||
// Verify AI client was called with the fallback cwd
|
||||
expect(mockClient.sendQuery).toHaveBeenCalledWith(
|
||||
wrapCommandForExecution('plan', 'Plan command'),
|
||||
'/workspace/test-project', // Falls back to codebase default_cwd
|
||||
undefined // New session created (assistant_session_id is null -> undefined)
|
||||
);
|
||||
});
|
||||
|
||||
test('should not deactivate session if cwd exists', async () => {
|
||||
// Mock fs.access to succeed (path exists)
|
||||
mockAccess.mockResolvedValue(undefined);
|
||||
|
||||
// Mock active session
|
||||
test('should not clear isolation if path exists', async () => {
|
||||
// Default setup: valid isolation env
|
||||
mockGetActiveSession.mockResolvedValue(mockSession);
|
||||
|
||||
mockParseCommand.mockReturnValue({ command: 'command-invoke', args: ['plan'] });
|
||||
|
|
@ -733,24 +800,17 @@ describe('orchestrator', () => {
|
|||
|
||||
await handleMessage(platform, 'chat-456', '/command-invoke plan');
|
||||
|
||||
// Verify session was NOT deactivated (for stale worktree reason)
|
||||
// Note: It might be deactivated for plan→execute transition, but not for stale worktree
|
||||
// Verify session was NOT deactivated (isolation is valid)
|
||||
// updateConversation should NOT be called with null isolation fields
|
||||
expect(mockUpdateConversation).not.toHaveBeenCalledWith(
|
||||
expect.any(String),
|
||||
expect.objectContaining({ worktree_path: null })
|
||||
expect.objectContaining({ worktree_path: null, isolation_env_id: null })
|
||||
);
|
||||
});
|
||||
|
||||
test('should handle conversation without worktree_path gracefully', async () => {
|
||||
// Conversation without worktree_path
|
||||
mockGetOrCreateConversation.mockResolvedValue({
|
||||
...mockConversation,
|
||||
worktree_path: null,
|
||||
cwd: '/workspace/project',
|
||||
});
|
||||
|
||||
// cwd exists
|
||||
mockAccess.mockResolvedValue(undefined);
|
||||
test('should use existing valid isolation environment', async () => {
|
||||
// Default mocks already set up valid isolation
|
||||
mockGetActiveSession.mockResolvedValue(mockSession);
|
||||
|
||||
mockParseCommand.mockReturnValue({ command: 'command-invoke', args: ['plan'] });
|
||||
mockReadFile.mockResolvedValue('Plan command');
|
||||
|
|
@ -760,10 +820,10 @@ describe('orchestrator', () => {
|
|||
|
||||
await handleMessage(platform, 'chat-456', '/command-invoke plan');
|
||||
|
||||
// Should work normally, no worktree cleanup needed
|
||||
// Should work normally with existing isolation
|
||||
expect(mockClient.sendQuery).toHaveBeenCalledWith(
|
||||
wrapCommandForExecution('plan', 'Plan command'),
|
||||
'/workspace/project',
|
||||
'/workspace/project', // From existing env working_path
|
||||
'claude-session-xyz'
|
||||
);
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,18 +2,274 @@
|
|||
* Orchestrator - Main conversation handler
|
||||
* Routes slash commands and AI messages appropriately
|
||||
*/
|
||||
import { readFile, access } from 'fs/promises';
|
||||
import { readFile } from 'fs/promises';
|
||||
import { join } from 'path';
|
||||
import { IPlatformAdapter } from '../types';
|
||||
import {
|
||||
IPlatformAdapter,
|
||||
IsolationHints,
|
||||
IsolationEnvironmentRow,
|
||||
Conversation,
|
||||
Codebase,
|
||||
} from '../types';
|
||||
import * as db from '../db/conversations';
|
||||
import * as codebaseDb from '../db/codebases';
|
||||
import * as sessionDb from '../db/sessions';
|
||||
import * as templateDb from '../db/command-templates';
|
||||
import * as isolationEnvDb from '../db/isolation-environments';
|
||||
import * as commandHandler from '../handlers/command-handler';
|
||||
import { formatToolCall } from '../utils/tool-formatter';
|
||||
import { substituteVariables } from '../utils/variable-substitution';
|
||||
import { classifyAndFormatError } from '../utils/error-formatter';
|
||||
import { getAssistantClient } from '../clients/factory';
|
||||
import { getIsolationProvider } from '../isolation';
|
||||
import {
|
||||
worktreeExists,
|
||||
findWorktreeByBranch,
|
||||
getCanonicalRepoPath,
|
||||
execFileAsync,
|
||||
} from '../utils/git';
|
||||
|
||||
/**
|
||||
* Validate existing isolation and create new if needed
|
||||
* This is the single source of truth for isolation decisions
|
||||
*/
|
||||
async function validateAndResolveIsolation(
|
||||
conversation: Conversation,
|
||||
codebase: Codebase | null,
|
||||
platform: IPlatformAdapter,
|
||||
conversationId: string,
|
||||
hints?: IsolationHints
|
||||
): Promise<{ cwd: string; env: IsolationEnvironmentRow | null; isNew: boolean }> {
|
||||
// 1. Check existing isolation reference (new UUID model)
|
||||
if (conversation.isolation_env_id) {
|
||||
const env = await isolationEnvDb.getById(conversation.isolation_env_id);
|
||||
|
||||
if (env && (await worktreeExists(env.working_path))) {
|
||||
// Valid - use it
|
||||
return { cwd: env.working_path, env, isNew: false };
|
||||
}
|
||||
|
||||
// Stale reference - clean up
|
||||
console.warn(`[Orchestrator] Stale isolation: ${conversation.isolation_env_id}`);
|
||||
await db.updateConversation(conversation.id, {
|
||||
isolation_env_id: null,
|
||||
worktree_path: null,
|
||||
isolation_provider: null,
|
||||
});
|
||||
|
||||
if (env) {
|
||||
await isolationEnvDb.updateStatus(env.id, 'destroyed');
|
||||
}
|
||||
}
|
||||
|
||||
// 2. Legacy fallback (worktree_path without new UUID)
|
||||
const legacyPath = conversation.worktree_path ?? conversation.isolation_env_id_legacy;
|
||||
if (legacyPath && (await worktreeExists(legacyPath))) {
|
||||
// Migrate to new model on-the-fly
|
||||
const env = await migrateToIsolationEnvironment(conversation, codebase, legacyPath, platform);
|
||||
if (env) {
|
||||
return { cwd: legacyPath, env, isNew: false };
|
||||
}
|
||||
}
|
||||
|
||||
// 3. No valid isolation - check if we should create
|
||||
if (!codebase) {
|
||||
return { cwd: conversation.cwd ?? '/workspace', env: null, isNew: false };
|
||||
}
|
||||
|
||||
// 4. Create new isolation (auto-isolation for all platforms!)
|
||||
const env = await resolveIsolation(codebase, platform, conversationId, hints);
|
||||
if (env) {
|
||||
await db.updateConversation(conversation.id, {
|
||||
isolation_env_id: env.id,
|
||||
worktree_path: env.working_path,
|
||||
isolation_provider: env.provider,
|
||||
cwd: env.working_path,
|
||||
});
|
||||
return { cwd: env.working_path, env, isNew: true };
|
||||
}
|
||||
|
||||
return { cwd: codebase.default_cwd, env: null, isNew: false };
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve which isolation environment to use
|
||||
* Handles reuse, sharing, adoption, and creation
|
||||
*/
|
||||
async function resolveIsolation(
|
||||
codebase: Codebase,
|
||||
platform: IPlatformAdapter,
|
||||
conversationId: string,
|
||||
hints?: IsolationHints
|
||||
): Promise<IsolationEnvironmentRow | null> {
|
||||
// Determine workflow identity
|
||||
const workflowType = hints?.workflowType ?? 'thread';
|
||||
const workflowId = hints?.workflowId ?? conversationId;
|
||||
|
||||
// 1. Check for existing environment with same workflow
|
||||
const existing = await isolationEnvDb.findByWorkflow(codebase.id, workflowType, workflowId);
|
||||
if (existing && (await worktreeExists(existing.working_path))) {
|
||||
console.log(`[Orchestrator] Reusing environment for ${workflowType}/${workflowId}`);
|
||||
return existing;
|
||||
}
|
||||
|
||||
// 2. Check linked issues for sharing (cross-conversation)
|
||||
if (hints?.linkedIssues?.length) {
|
||||
for (const issueNum of hints.linkedIssues) {
|
||||
const linkedEnv = await isolationEnvDb.findByWorkflow(codebase.id, 'issue', String(issueNum));
|
||||
if (linkedEnv && (await worktreeExists(linkedEnv.working_path))) {
|
||||
console.log(`[Orchestrator] Sharing worktree with linked issue #${String(issueNum)}`);
|
||||
// Send UX message
|
||||
await platform.sendMessage(
|
||||
conversationId,
|
||||
`Reusing worktree from issue #${String(issueNum)}`
|
||||
);
|
||||
return linkedEnv;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// 3. Try PR branch adoption (skill symbiosis)
|
||||
if (hints?.prBranch) {
|
||||
const canonicalPath = await getCanonicalRepoPath(codebase.default_cwd);
|
||||
const adoptedPath = await findWorktreeByBranch(canonicalPath, hints.prBranch);
|
||||
if (adoptedPath && (await worktreeExists(adoptedPath))) {
|
||||
console.log(`[Orchestrator] Adopting existing worktree at ${adoptedPath}`);
|
||||
const env = await isolationEnvDb.create({
|
||||
codebase_id: codebase.id,
|
||||
workflow_type: workflowType,
|
||||
workflow_id: workflowId,
|
||||
working_path: adoptedPath,
|
||||
branch_name: hints.prBranch,
|
||||
created_by_platform: platform.getPlatformType(),
|
||||
metadata: { adopted: true, adopted_from: 'skill' },
|
||||
});
|
||||
return env;
|
||||
}
|
||||
}
|
||||
|
||||
// 4. Create new worktree
|
||||
const provider = getIsolationProvider();
|
||||
const canonicalPath = await getCanonicalRepoPath(codebase.default_cwd);
|
||||
|
||||
try {
|
||||
const isolatedEnv = await provider.create({
|
||||
codebaseId: codebase.id,
|
||||
canonicalRepoPath: canonicalPath,
|
||||
workflowType,
|
||||
identifier: workflowId,
|
||||
prBranch: hints?.prBranch,
|
||||
prSha: hints?.prSha,
|
||||
});
|
||||
|
||||
// Create database record
|
||||
const env = await isolationEnvDb.create({
|
||||
codebase_id: codebase.id,
|
||||
workflow_type: workflowType,
|
||||
workflow_id: workflowId,
|
||||
working_path: isolatedEnv.workingPath,
|
||||
branch_name: isolatedEnv.branchName ?? `${workflowType}-${workflowId}`,
|
||||
created_by_platform: platform.getPlatformType(),
|
||||
metadata: {
|
||||
related_issues: hints?.linkedIssues ?? [],
|
||||
related_prs: hints?.linkedPRs ?? [],
|
||||
},
|
||||
});
|
||||
|
||||
// UX message
|
||||
if (hints?.prSha) {
|
||||
const shortSha = hints.prSha.substring(0, 7);
|
||||
await platform.sendMessage(
|
||||
conversationId,
|
||||
`Reviewing PR at commit \`${shortSha}\` (branch: \`${hints.prBranch}\`)`
|
||||
);
|
||||
} else {
|
||||
await platform.sendMessage(
|
||||
conversationId,
|
||||
`Working in isolated branch \`${env.branch_name}\``
|
||||
);
|
||||
}
|
||||
|
||||
return env;
|
||||
} catch (error) {
|
||||
console.error('[Orchestrator] Failed to create isolation:', error);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Migrate a legacy worktree_path to the new isolation_environments model
|
||||
*/
|
||||
async function migrateToIsolationEnvironment(
|
||||
conversation: Conversation,
|
||||
codebase: Codebase | null,
|
||||
legacyPath: string,
|
||||
platform: IPlatformAdapter
|
||||
): Promise<IsolationEnvironmentRow | null> {
|
||||
if (!codebase) return null;
|
||||
|
||||
try {
|
||||
const { workflowType, workflowId } = inferWorkflowFromConversation(conversation, legacyPath);
|
||||
const branchName = await getBranchNameFromWorktree(legacyPath);
|
||||
|
||||
const env = await isolationEnvDb.create({
|
||||
codebase_id: codebase.id,
|
||||
workflow_type: workflowType,
|
||||
workflow_id: workflowId,
|
||||
working_path: legacyPath,
|
||||
branch_name: branchName,
|
||||
created_by_platform: platform.getPlatformType(),
|
||||
metadata: { migrated: true, migrated_at: new Date().toISOString() },
|
||||
});
|
||||
|
||||
await db.updateConversation(conversation.id, {
|
||||
isolation_env_id: env.id,
|
||||
});
|
||||
|
||||
console.log(`[Orchestrator] Migrated legacy worktree to environment: ${env.id}`);
|
||||
return env;
|
||||
} catch (error) {
|
||||
console.error('[Orchestrator] Failed to migrate legacy worktree:', error);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
function inferWorkflowFromConversation(
|
||||
conversation: Conversation,
|
||||
legacyPath: string
|
||||
): { workflowType: string; workflowId: string } {
|
||||
// Try to infer from platform conversation ID
|
||||
if (conversation.platform_type === 'github') {
|
||||
const match = /#(\d+)$/.exec(conversation.platform_conversation_id);
|
||||
if (match) {
|
||||
const isPR = legacyPath.includes('/pr-') || legacyPath.includes('-pr-');
|
||||
return {
|
||||
workflowType: isPR ? 'pr' : 'issue',
|
||||
workflowId: match[1],
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return {
|
||||
workflowType: 'thread',
|
||||
workflowId: conversation.platform_conversation_id,
|
||||
};
|
||||
}
|
||||
|
||||
async function getBranchNameFromWorktree(path: string): Promise<string> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync('git', [
|
||||
'-C',
|
||||
path,
|
||||
'rev-parse',
|
||||
'--abbrev-ref',
|
||||
'HEAD',
|
||||
]);
|
||||
return stdout.trim();
|
||||
} catch {
|
||||
return 'unknown';
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Wraps command content with execution context to signal the AI should execute immediately
|
||||
|
|
@ -39,7 +295,8 @@ export async function handleMessage(
|
|||
message: string,
|
||||
issueContext?: string, // Optional GitHub issue/PR context to append AFTER command loading
|
||||
threadContext?: string, // Optional thread message history for context
|
||||
parentConversationId?: string // Optional parent channel ID for thread inheritance
|
||||
parentConversationId?: string, // Optional parent channel ID for thread inheritance
|
||||
isolationHints?: IsolationHints // Optional hints from adapter for isolation decisions
|
||||
): Promise<void> {
|
||||
try {
|
||||
console.log(`[Orchestrator] Handling message for conversation ${conversationId}`);
|
||||
|
|
@ -147,9 +404,9 @@ export async function handleMessage(
|
|||
return;
|
||||
}
|
||||
|
||||
// Read command file
|
||||
const cwd = conversation.isolation_env_id ?? conversation.worktree_path ?? conversation.cwd ?? codebase.default_cwd;
|
||||
const commandFilePath = join(cwd, commandDef.path);
|
||||
// Read command file (use worktree_path or cwd for command location)
|
||||
const commandCwd = conversation.worktree_path ?? conversation.cwd ?? codebase.default_cwd;
|
||||
const commandFilePath = join(commandCwd, commandDef.path);
|
||||
|
||||
try {
|
||||
const commandText = await readFile(commandFilePath, 'utf-8');
|
||||
|
|
@ -231,42 +488,33 @@ export async function handleMessage(
|
|||
const aiClient = getAssistantClient(conversation.ai_assistant_type);
|
||||
console.log(`[Orchestrator] Using ${conversation.ai_assistant_type} assistant`);
|
||||
|
||||
// Get or create session (handle plan→execute transition)
|
||||
let session = await sessionDb.getActiveSession(conversation.id);
|
||||
// Get codebase for isolation and session management
|
||||
const codebase = conversation.codebase_id
|
||||
? await codebaseDb.getCodebase(conversation.codebase_id)
|
||||
: null;
|
||||
let cwd =
|
||||
conversation.isolation_env_id ?? conversation.worktree_path ?? conversation.cwd ?? codebase?.default_cwd ?? '/workspace';
|
||||
|
||||
// Validate cwd exists - handle stale worktree paths gracefully
|
||||
try {
|
||||
await access(cwd);
|
||||
} catch {
|
||||
console.warn(`[Orchestrator] Working directory ${cwd} does not exist`);
|
||||
// Validate and resolve isolation - this is the single source of truth
|
||||
const { cwd, isNew: isNewIsolation } = await validateAndResolveIsolation(
|
||||
conversation,
|
||||
codebase,
|
||||
platform,
|
||||
conversationId,
|
||||
isolationHints
|
||||
);
|
||||
|
||||
// Deactivate stale session to force fresh start
|
||||
if (session) {
|
||||
await sessionDb.deactivateSession(session.id);
|
||||
session = null;
|
||||
console.log('[Orchestrator] Deactivated session with stale worktree');
|
||||
}
|
||||
// Get or create session (handle plan→execute transition)
|
||||
let session = await sessionDb.getActiveSession(conversation.id);
|
||||
|
||||
// Clear stale isolation reference from conversation
|
||||
if (conversation.isolation_env_id || conversation.worktree_path) {
|
||||
await db.updateConversation(conversation.id, {
|
||||
worktree_path: null,
|
||||
isolation_env_id: null,
|
||||
isolation_provider: null,
|
||||
cwd: codebase?.default_cwd ?? '/workspace',
|
||||
});
|
||||
console.log('[Orchestrator] Cleared stale isolation environment from conversation');
|
||||
}
|
||||
|
||||
// Use default cwd for this request
|
||||
cwd = codebase?.default_cwd ?? '/workspace';
|
||||
// If cwd changed (new isolation), deactivate stale sessions
|
||||
if (isNewIsolation && session) {
|
||||
console.log('[Orchestrator] New isolation, deactivating existing session');
|
||||
await sessionDb.deactivateSession(session.id);
|
||||
session = null;
|
||||
}
|
||||
|
||||
// Update last_activity_at for staleness tracking
|
||||
await db.touchConversation(conversation.id);
|
||||
|
||||
// Check for plan→execute transition (requires NEW session per PRD)
|
||||
// Supports both regular and GitHub workflows:
|
||||
// - plan-feature → execute (regular workflow)
|
||||
|
|
|
|||
170
src/services/cleanup-service.test.ts
Normal file
170
src/services/cleanup-service.test.ts
Normal file
|
|
@ -0,0 +1,170 @@
|
|||
import { mock, describe, test, expect, beforeEach } from 'bun:test';
|
||||
|
||||
// Mock git utility
|
||||
const mockExecFileAsync = mock(() => Promise.resolve({ stdout: '', stderr: '' }));
|
||||
mock.module('../utils/git', () => ({
|
||||
execFileAsync: mockExecFileAsync,
|
||||
}));
|
||||
|
||||
// Mock isolation provider
|
||||
const mockDestroy = mock(() => Promise.resolve());
|
||||
mock.module('../isolation', () => ({
|
||||
getIsolationProvider: () => ({
|
||||
destroy: mockDestroy,
|
||||
}),
|
||||
}));
|
||||
|
||||
import { hasUncommittedChanges, isBranchMerged, getLastCommitDate } from './cleanup-service';
|
||||
|
||||
describe('cleanup-service', () => {
|
||||
beforeEach(() => {
|
||||
mockExecFileAsync.mockClear();
|
||||
mockDestroy.mockClear();
|
||||
});
|
||||
|
||||
describe('hasUncommittedChanges', () => {
|
||||
test('returns true when git status shows changes', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({
|
||||
stdout: ' M file.ts\n',
|
||||
stderr: '',
|
||||
});
|
||||
|
||||
const result = await hasUncommittedChanges('/workspace/test');
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(mockExecFileAsync).toHaveBeenCalledWith('git', [
|
||||
'-C',
|
||||
'/workspace/test',
|
||||
'status',
|
||||
'--porcelain',
|
||||
]);
|
||||
});
|
||||
|
||||
test('returns false when git status is clean', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({ stdout: '', stderr: '' });
|
||||
|
||||
const result = await hasUncommittedChanges('/workspace/test');
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false when git fails (path not found)', async () => {
|
||||
mockExecFileAsync.mockRejectedValueOnce(new Error('not a git repository'));
|
||||
|
||||
const result = await hasUncommittedChanges('/nonexistent');
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false when git status is only whitespace', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({ stdout: ' \n', stderr: '' });
|
||||
|
||||
const result = await hasUncommittedChanges('/workspace/test');
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('isBranchMerged', () => {
|
||||
test('returns true when branch is in merged list', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({
|
||||
stdout: ' feature-a\n issue-42\n* main\n',
|
||||
stderr: '',
|
||||
});
|
||||
|
||||
const result = await isBranchMerged('/workspace/repo', 'issue-42');
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(mockExecFileAsync).toHaveBeenCalledWith('git', [
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'branch',
|
||||
'--merged',
|
||||
'main',
|
||||
]);
|
||||
});
|
||||
|
||||
test('returns false when branch is not merged', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({
|
||||
stdout: ' feature-a\n* main\n',
|
||||
stderr: '',
|
||||
});
|
||||
|
||||
const result = await isBranchMerged('/workspace/repo', 'issue-42');
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false when git command fails', async () => {
|
||||
mockExecFileAsync.mockRejectedValueOnce(new Error('git error'));
|
||||
|
||||
const result = await isBranchMerged('/workspace/repo', 'issue-42');
|
||||
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
test('handles current branch marker (*)', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({
|
||||
stdout: '* issue-42\n main\n',
|
||||
stderr: '',
|
||||
});
|
||||
|
||||
const result = await isBranchMerged('/workspace/repo', 'issue-42');
|
||||
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
test('uses custom main branch', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({
|
||||
stdout: ' issue-42\n master\n',
|
||||
stderr: '',
|
||||
});
|
||||
|
||||
await isBranchMerged('/workspace/repo', 'issue-42', 'master');
|
||||
|
||||
expect(mockExecFileAsync).toHaveBeenCalledWith('git', [
|
||||
'-C',
|
||||
'/workspace/repo',
|
||||
'branch',
|
||||
'--merged',
|
||||
'master',
|
||||
]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getLastCommitDate', () => {
|
||||
test('returns date from git log', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({
|
||||
stdout: '2025-01-15 10:30:00 +0000\n',
|
||||
stderr: '',
|
||||
});
|
||||
|
||||
const result = await getLastCommitDate('/workspace/test');
|
||||
|
||||
expect(result).toBeInstanceOf(Date);
|
||||
expect(result?.getFullYear()).toBe(2025);
|
||||
expect(result?.getMonth()).toBe(0); // January is 0
|
||||
expect(result?.getDate()).toBe(15);
|
||||
});
|
||||
|
||||
test('returns null when git fails', async () => {
|
||||
mockExecFileAsync.mockRejectedValueOnce(new Error('no commits'));
|
||||
|
||||
const result = await getLastCommitDate('/workspace/test');
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
|
||||
test('handles different date formats', async () => {
|
||||
mockExecFileAsync.mockResolvedValueOnce({
|
||||
stdout: '2024-12-25 23:59:59 -0500\n',
|
||||
stderr: '',
|
||||
});
|
||||
|
||||
const result = await getLastCommitDate('/workspace/test');
|
||||
|
||||
expect(result).toBeInstanceOf(Date);
|
||||
expect(result?.getFullYear()).toBe(2024);
|
||||
});
|
||||
});
|
||||
});
|
||||
192
src/services/cleanup-service.ts
Normal file
192
src/services/cleanup-service.ts
Normal file
|
|
@ -0,0 +1,192 @@
|
|||
/**
|
||||
* Cleanup service for isolation environments
|
||||
* Handles removal triggered by events, schedule, or commands
|
||||
*/
|
||||
import * as isolationEnvDb from '../db/isolation-environments';
|
||||
import * as conversationDb from '../db/conversations';
|
||||
import * as sessionDb from '../db/sessions';
|
||||
import { getIsolationProvider } from '../isolation';
|
||||
import { execFileAsync } from '../utils/git';
|
||||
|
||||
export interface CleanupReport {
|
||||
removed: string[];
|
||||
skipped: { id: string; reason: string }[];
|
||||
errors: { id: string; error: string }[];
|
||||
}
|
||||
|
||||
/**
|
||||
* Called when a platform conversation is closed (e.g., GitHub issue/PR closed)
|
||||
* Cleans up the associated isolation environment if no other conversations use it
|
||||
*/
|
||||
export async function onConversationClosed(
|
||||
platformType: string,
|
||||
platformConversationId: string
|
||||
): Promise<void> {
|
||||
console.log(`[Cleanup] Conversation closed: ${platformType}/${platformConversationId}`);
|
||||
|
||||
// Find the conversation
|
||||
const conversation = await conversationDb.getConversationByPlatformId(
|
||||
platformType,
|
||||
platformConversationId
|
||||
);
|
||||
|
||||
if (!conversation?.isolation_env_id) {
|
||||
console.log('[Cleanup] No isolation environment to clean up');
|
||||
return;
|
||||
}
|
||||
|
||||
const envId = conversation.isolation_env_id;
|
||||
|
||||
// Deactivate any active sessions first
|
||||
const session = await sessionDb.getActiveSession(conversation.id);
|
||||
if (session) {
|
||||
await sessionDb.deactivateSession(session.id);
|
||||
console.log(`[Cleanup] Deactivated session ${session.id}`);
|
||||
}
|
||||
|
||||
// Get the environment
|
||||
const env = await isolationEnvDb.getById(envId);
|
||||
if (!env) {
|
||||
console.log(`[Cleanup] Environment ${envId} not found in database`);
|
||||
return;
|
||||
}
|
||||
|
||||
// Clear this conversation's reference (regardless of whether we remove the worktree)
|
||||
await conversationDb.updateConversation(conversation.id, {
|
||||
isolation_env_id: null,
|
||||
worktree_path: null,
|
||||
isolation_provider: null,
|
||||
// Keep cwd pointing to main repo (will be set by caller or orchestrator)
|
||||
});
|
||||
|
||||
// Check if other conversations still use this environment
|
||||
const otherConversations = await isolationEnvDb.getConversationsUsingEnv(envId);
|
||||
if (otherConversations.length > 0) {
|
||||
console.log(
|
||||
`[Cleanup] Environment still used by ${String(otherConversations.length)} conversation(s), keeping`
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
// No other users - attempt removal
|
||||
await removeEnvironment(envId, { force: false });
|
||||
}
|
||||
|
||||
/**
|
||||
* Remove a specific environment
|
||||
*/
|
||||
export async function removeEnvironment(
|
||||
envId: string,
|
||||
options?: { force?: boolean }
|
||||
): Promise<void> {
|
||||
const env = await isolationEnvDb.getById(envId);
|
||||
if (!env) {
|
||||
console.log(`[Cleanup] Environment ${envId} not found`);
|
||||
return;
|
||||
}
|
||||
|
||||
if (env.status === 'destroyed') {
|
||||
console.log(`[Cleanup] Environment ${envId} already destroyed`);
|
||||
return;
|
||||
}
|
||||
|
||||
const provider = getIsolationProvider();
|
||||
|
||||
try {
|
||||
// Check for uncommitted changes (unless force)
|
||||
if (!options?.force) {
|
||||
const hasChanges = await hasUncommittedChanges(env.working_path);
|
||||
if (hasChanges) {
|
||||
console.warn(`[Cleanup] Environment ${envId} has uncommitted changes, skipping`);
|
||||
return;
|
||||
}
|
||||
}
|
||||
|
||||
// Remove the worktree
|
||||
await provider.destroy(env.working_path, { force: options?.force });
|
||||
|
||||
// Mark as destroyed in database
|
||||
await isolationEnvDb.updateStatus(envId, 'destroyed');
|
||||
|
||||
console.log(`[Cleanup] Removed environment ${envId} at ${env.working_path}`);
|
||||
} catch (error) {
|
||||
const err = error as Error;
|
||||
console.error(`[Cleanup] Failed to remove environment ${envId}:`, err.message);
|
||||
throw err;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a worktree has uncommitted changes
|
||||
*/
|
||||
export async function hasUncommittedChanges(workingPath: string): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync('git', ['-C', workingPath, 'status', '--porcelain']);
|
||||
return stdout.trim().length > 0;
|
||||
} catch {
|
||||
// If git fails, assume it's safe to remove (path might not exist)
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if a branch has been merged into main
|
||||
*/
|
||||
export async function isBranchMerged(
|
||||
repoPath: string,
|
||||
branchName: string,
|
||||
mainBranch = 'main'
|
||||
): Promise<boolean> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync('git', [
|
||||
'-C',
|
||||
repoPath,
|
||||
'branch',
|
||||
'--merged',
|
||||
mainBranch,
|
||||
]);
|
||||
const mergedBranches = stdout.split('\n').map(b => b.trim().replace(/^\* /, ''));
|
||||
return mergedBranches.includes(branchName);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the last commit date for a worktree
|
||||
*/
|
||||
export async function getLastCommitDate(workingPath: string): Promise<Date | null> {
|
||||
try {
|
||||
const { stdout } = await execFileAsync('git', ['-C', workingPath, 'log', '-1', '--format=%ci']);
|
||||
return new Date(stdout.trim());
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Clean up to make room when limit reached (Phase 3D will call this)
|
||||
* Attempts to remove merged branches first
|
||||
*/
|
||||
export async function cleanupToMakeRoom(codebaseId: string, mainRepoPath: string): Promise<number> {
|
||||
const envs = await isolationEnvDb.listByCodebase(codebaseId);
|
||||
let removed = 0;
|
||||
|
||||
for (const env of envs) {
|
||||
// Try merged branches first
|
||||
const merged = await isBranchMerged(mainRepoPath, env.branch_name);
|
||||
if (merged) {
|
||||
const hasChanges = await hasUncommittedChanges(env.working_path);
|
||||
if (!hasChanges) {
|
||||
try {
|
||||
await removeEnvironment(env.id);
|
||||
removed++;
|
||||
} catch {
|
||||
// Continue to next
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return removed;
|
||||
}
|
||||
|
|
@ -8,14 +8,54 @@ export interface Conversation {
|
|||
platform_conversation_id: string;
|
||||
codebase_id: string | null;
|
||||
cwd: string | null;
|
||||
worktree_path: string | null;
|
||||
isolation_env_id: string | null;
|
||||
isolation_provider: string | null;
|
||||
worktree_path: string | null; // Legacy field
|
||||
isolation_env_id_legacy: string | null; // Renamed from isolation_env_id (TEXT)
|
||||
isolation_env_id: string | null; // NEW: UUID FK to isolation_environments
|
||||
isolation_provider: string | null; // Legacy field
|
||||
ai_assistant_type: string;
|
||||
last_activity_at: Date | null; // NEW: for staleness detection
|
||||
created_at: Date;
|
||||
updated_at: Date;
|
||||
}
|
||||
|
||||
/**
|
||||
* Isolation hints provided by adapters to orchestrator
|
||||
* Allows platform-specific context without orchestrator knowing platform internals
|
||||
*/
|
||||
export interface IsolationHints {
|
||||
// Workflow identification (adapter knows this)
|
||||
workflowType?: 'issue' | 'pr' | 'review' | 'thread' | 'task';
|
||||
workflowId?: string;
|
||||
|
||||
// PR-specific (for reproducible reviews)
|
||||
prBranch?: string;
|
||||
prSha?: string;
|
||||
|
||||
// Cross-reference hints (for linking)
|
||||
linkedIssues?: number[];
|
||||
linkedPRs?: number[];
|
||||
|
||||
// Adoption hints
|
||||
suggestedBranch?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Database row for isolation_environments table
|
||||
*/
|
||||
export interface IsolationEnvironmentRow {
|
||||
id: string;
|
||||
codebase_id: string;
|
||||
workflow_type: string;
|
||||
workflow_id: string;
|
||||
provider: string;
|
||||
working_path: string;
|
||||
branch_name: string;
|
||||
status: string;
|
||||
created_at: Date;
|
||||
created_by_platform: string | null;
|
||||
metadata: Record<string, unknown>;
|
||||
}
|
||||
|
||||
export interface Codebase {
|
||||
id: string;
|
||||
name: string;
|
||||
|
|
|
|||
|
|
@ -315,7 +315,14 @@ branch refs/heads/feature/auth
|
|||
// Verify worktree add was called with the local branch created from PR ref
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', repoPath, 'worktree', 'add', expect.any(String), 'pr-42-review']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
repoPath,
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'pr-42-review',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
|
||||
|
|
@ -381,7 +388,9 @@ branch refs/heads/feature/auth
|
|||
callCount++;
|
||||
// First call: worktree add -b fails (branch exists)
|
||||
if (callCount === 1 && args.includes('-b')) {
|
||||
const error = new Error('fatal: A branch named issue-42 already exists.') as Error & { stderr?: string };
|
||||
const error = new Error('fatal: A branch named issue-42 already exists.') as Error & {
|
||||
stderr?: string;
|
||||
};
|
||||
error.stderr = 'fatal: A branch named issue-42 already exists.';
|
||||
throw error;
|
||||
}
|
||||
|
|
@ -394,7 +403,15 @@ branch refs/heads/feature/auth
|
|||
// Verify first call attempted to create new branch
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', repoPath, 'worktree', 'add', expect.any(String), '-b', 'issue-42']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
repoPath,
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'-b',
|
||||
'issue-42',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
|
||||
|
|
@ -418,9 +435,9 @@ branch refs/heads/feature/auth
|
|||
return { stdout: '', stderr: '' };
|
||||
});
|
||||
|
||||
await expect(git.createWorktreeForIssue(repoPath, issueNumber, true, prHeadBranch)).rejects.toThrow(
|
||||
'Failed to create worktree for PR #42'
|
||||
);
|
||||
await expect(
|
||||
git.createWorktreeForIssue(repoPath, issueNumber, true, prHeadBranch)
|
||||
).rejects.toThrow('Failed to create worktree for PR #42');
|
||||
});
|
||||
|
||||
test('provides helpful error message with PR number', async () => {
|
||||
|
|
@ -454,7 +471,15 @@ branch refs/heads/feature/auth
|
|||
// Verify worktree add was called with -b flag for new pr-XX branch
|
||||
expect(execSpy).toHaveBeenCalledWith(
|
||||
'git',
|
||||
expect.arrayContaining(['-C', repoPath, 'worktree', 'add', expect.any(String), '-b', 'pr-42']),
|
||||
expect.arrayContaining([
|
||||
'-C',
|
||||
repoPath,
|
||||
'worktree',
|
||||
'add',
|
||||
expect.any(String),
|
||||
'-b',
|
||||
'pr-42',
|
||||
]),
|
||||
expect.any(Object)
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -21,10 +21,7 @@ export async function execFileAsync(
|
|||
}
|
||||
|
||||
// Mockable mkdir wrapper
|
||||
export async function mkdirAsync(
|
||||
path: string,
|
||||
options?: { recursive?: boolean }
|
||||
): Promise<void> {
|
||||
export async function mkdirAsync(path: string, options?: { recursive?: boolean }): Promise<void> {
|
||||
await fsMkdir(path, options);
|
||||
}
|
||||
|
||||
|
|
@ -199,9 +196,13 @@ export async function createWorktreeForIssue(
|
|||
if (prHeadSha) {
|
||||
// Fetch the specific commit SHA using PR refs (works for both fork and non-fork PRs)
|
||||
// GitHub creates refs/pull/<number>/head for all PRs automatically
|
||||
await execFileAsync('git', ['-C', repoPath, 'fetch', 'origin', `pull/${String(issueNumber)}/head`], {
|
||||
timeout: 30000,
|
||||
});
|
||||
await execFileAsync(
|
||||
'git',
|
||||
['-C', repoPath, 'fetch', 'origin', `pull/${String(issueNumber)}/head`],
|
||||
{
|
||||
timeout: 30000,
|
||||
}
|
||||
);
|
||||
|
||||
// Create worktree at the specific SHA
|
||||
await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, prHeadSha], {
|
||||
|
|
@ -219,14 +220,28 @@ export async function createWorktreeForIssue(
|
|||
} else {
|
||||
// Use GitHub's PR refs which work for both fork and non-fork PRs
|
||||
// GitHub automatically creates refs/pull/<number>/head for all PRs
|
||||
await execFileAsync('git', ['-C', repoPath, 'fetch', 'origin', `pull/${String(issueNumber)}/head:pr-${String(issueNumber)}-review`], {
|
||||
timeout: 30000,
|
||||
});
|
||||
await execFileAsync(
|
||||
'git',
|
||||
[
|
||||
'-C',
|
||||
repoPath,
|
||||
'fetch',
|
||||
'origin',
|
||||
`pull/${String(issueNumber)}/head:pr-${String(issueNumber)}-review`,
|
||||
],
|
||||
{
|
||||
timeout: 30000,
|
||||
}
|
||||
);
|
||||
|
||||
// Create worktree using the fetched PR ref
|
||||
await execFileAsync('git', ['-C', repoPath, 'worktree', 'add', worktreePath, `pr-${String(issueNumber)}-review`], {
|
||||
timeout: 30000,
|
||||
});
|
||||
await execFileAsync(
|
||||
'git',
|
||||
['-C', repoPath, 'worktree', 'add', worktreePath, `pr-${String(issueNumber)}-review`],
|
||||
{
|
||||
timeout: 30000,
|
||||
}
|
||||
);
|
||||
}
|
||||
} catch (error) {
|
||||
const err = error as Error & { stderr?: string };
|
||||
|
|
|
|||
Loading…
Reference in a new issue