diff --git a/README.md b/README.md index 1d80104a..2c414b19 100644 --- a/README.md +++ b/README.md @@ -183,7 +183,7 @@ Use it for the work that spans panes: Add this repo, create three worktree panes for the next features, start Codex in each one, and keep a separate review tab ready for every PR. ``` -Pane Chat keeps the human discussion at the orchestrator level, distills that into concrete briefs, then delegates planning, implementation, review, and PR testing to Claude or Codex panels through RunPane. This keeps the agent doing the work in the right isolated worktree while Pane Chat watches state with `runpane panels wait`, `runpane panels screen`, and `runpane panels output`. +Pane Chat keeps the human discussion at the orchestrator level, distills that into concrete briefs when needed, then delegates authorized lifecycle stages to Claude or Codex panels through RunPane. The active agent's cached `runpane-orchestrator` is the canonical source for the software-work lifecycle, authorization ledger, review-feedback interrupts, current-head evidence rules, and `ready_to_merge` predicate. Pane's generated layer stays focused on the local Pane runtime, skill cache paths, pane/panel/worktree mechanics, and preserving the user's focus. The prompt stays small because Pane writes local project-level skills into the Pane data directory: @@ -192,9 +192,9 @@ The prompt stays small because Pane writes local project-level skills into the P - `skills/pane-chat/runpane-orchestrator.md` - `skills/pane-chat/runtime-context.md` -Pane also caches the important workflow skills from the Pane skills repository, including discussion, plan/simple-plan, implement, implementation-reviewer, PR test automation, prepare-pr, investigate, and commit. The generated orchestrator skill tells Pane Chat to load the workflow map and local skill cache before it coordinates work, so broad or greenfield requests follow the intended discuss -> plan -> implement -> review loop instead of treating a loose prompt as a plan. +Pane also caches the important workflow skills from the Pane skills repository, including discussion, plan/simple-plan, implement, implementation-reviewer, PR test automation, prepare-pr, `gh-address-comments`, investigate, and commit. The generated orchestrator skill tells Pane Chat to load the workflow map and local skill cache before it coordinates work, while deferring lifecycle policy to the cached active-agent `runpane-orchestrator` so Pane does not maintain a second, drifting copy of the workflow. -The top-right toggle switches Pane Chat between Claude and Codex and persists the default orchestrator agent in Pane settings. Both agents share the same local orchestration contract; only the terminal command changes. +The top-right toggle switches Pane Chat between Claude and Codex and persists the default orchestrator agent in Pane settings. Both agents share the same Pane-specific orchestration contract, then follow their own cached downstream skills where the Codex and Claude skill surfaces differ. --- diff --git a/main/src/services/skillCacheManager.test.ts b/main/src/services/skillCacheManager.test.ts index 8c498e75..3ff1cd10 100644 --- a/main/src/services/skillCacheManager.test.ts +++ b/main/src/services/skillCacheManager.test.ts @@ -1,13 +1,50 @@ import fs from 'fs/promises'; +import { EventEmitter } from 'events'; +import https from 'https'; import os from 'os'; import path from 'path'; -import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; import { SkillCacheManager } from './skillCacheManager'; function normalizePathSeparators(value: string): string { return value.replace(/\\/g, '/'); } +function mockRawDownloads(failures = new Set()) { + return vi.spyOn(https, 'get').mockImplementation((url, callback) => { + const request = new EventEmitter() as ReturnType; + const pathname = new URL(String(url)).pathname; + const relativePath = decodeURIComponent(pathname.replace('/dcouple/skills/main/', '')); + const response = new EventEmitter() as IncomingMessageLike; + + response.headers = {}; + response.resume = vi.fn(); + + if (failures.has(relativePath)) { + response.statusCode = 500; + process.nextTick(() => { + callback(response); + response.emit('end'); + }); + return request; + } + + response.statusCode = 200; + process.nextTick(() => { + callback(response); + response.emit('data', Buffer.from(`# ${relativePath}\n`)); + response.emit('end'); + }); + return request; + }); +} + +interface IncomingMessageLike extends EventEmitter { + statusCode?: number; + headers: Record; + resume: () => void; +} + describe('SkillCacheManager Pane Chat guide', () => { const originalPaneDir = process.env.PANE_DIR; let tempDir: string | undefined; @@ -46,6 +83,11 @@ describe('SkillCacheManager Pane Chat guide', () => { expect(guide).toContain('when they ask what to work on next'); expect(guide).toContain('Do not replace orchestration with a normal chat answer for Pane work.'); expect(guide).toContain('verify its state with'); + expect(guide).toContain('## Contract Precedence'); + expect(guide).toContain("active agent's cached RunPane orchestrator skill is authoritative for the"); + expect(guide).toContain('review-feedback interrupts, current-head evidence invalidation, and'); + expect(guide).toContain('`ready_to_merge` predicate'); + expect(guide).toContain('Stop before merge, deploy, release creation, publishing, version changes'); }); it('writes runtime context with same-runtime CLI recovery guidance', async () => { @@ -81,13 +123,19 @@ describe('SkillCacheManager Pane Chat guide', () => { expect(canonicalSkill).toContain('says: "do it yourself in this chat."'); expect(canonicalSkill).toContain('Inspect the workflow map and skill legend'); expect(canonicalSkill).toContain('Do not claim initialization is complete'); + expect(canonicalSkill).toContain('## Contract Precedence'); + expect(canonicalSkill).toContain('The generated runtime context is authoritative'); + expect(canonicalSkill).toContain("active agent's cached RunPane orchestrator skill is authoritative"); + expect(canonicalSkill).toContain('persistent authorization ledger'); + expect(canonicalSkill).toContain('review-feedback interrupts'); + expect(canonicalSkill).toContain('current-head evidence invalidation'); + expect(canonicalSkill).toContain('`ready_to_merge` predicate'); expect(canonicalSkill).toContain('Pane Chat owns discussion and clarification with the user'); - expect(canonicalSkill).toContain('Do not delegate separate'); - expect(canonicalSkill).toContain('distills the discussion into a concise intent brief'); - expect(canonicalSkill).toContain('Delegate plan or simple-plan to the appropriate agent/pane'); - expect(canonicalSkill).toContain('Use best judgment for very small, low-risk tasks'); - expect(canonicalSkill).toContain('Do not skip directly to implement just because the delegated prompt contains an'); - expect(canonicalSkill).toContain('If an implement agent reports that no approved plan file exists'); + expect(canonicalSkill).toContain('Do not maintain a'); + expect(canonicalSkill).toContain('second Pane-generated copy of that lifecycle'); + expect(canonicalSkill).toContain('Treat review feedback as an interrupt owned by the upstream lifecycle'); + expect(canonicalSkill).toContain('routes to `gh-address-comments`'); + expect(canonicalSkill).toContain('rerun stale'); expect(canonicalSkill).toContain('Delegate discussion to another agent only when the user explicitly asks'); expect(canonicalSkill).toContain('create a minimal local git repository and register it with Pane'); expect(canonicalSkill).toContain('Creating a new Pane from a saved repository should normally create an'); @@ -98,6 +146,9 @@ describe('SkillCacheManager Pane Chat guide', () => { expect(canonicalSkill).toContain('pane-work-recap'); expect(canonicalSkill).toContain('pane-work-prioritizer'); expect(canonicalSkill).toContain('Do not start implementation panes for those answers'); + expect(canonicalSkill).toContain('Stop before merge, deploy, release creation, publishing, version changes'); + expect(canonicalSkill).not.toContain('PR test automation or prepare-pr only after implementation review passes'); + expect(canonicalSkill).not.toContain('treating a broad brief as a plan is usually too loose'); }); it('mirrors cached repository skills into project-scoped Codex and Claude skill roots', async () => { @@ -129,4 +180,50 @@ describe('SkillCacheManager Pane Chat guide', () => { 'name: pane-orchestrator', ); }); + + it('downloads required review-feedback fallback skills and mirrors them into project roots', async () => { + const manager = new SkillCacheManager(); + const httpsGet = mockRawDownloads(); + + try { + await (manager as unknown as { downloadFallbackFiles: () => Promise }).downloadFallbackFiles(); + await manager.ensurePaneChatGuide(); + } finally { + httpsGet.mockRestore(); + } + + await expect( + fs.readFile(path.join(manager.codexProjectSkillsRoot, 'gh-address-comments', 'SKILL.md'), 'utf8'), + ).resolves.toContain('parsa/.codex/skills/gh-address-comments/SKILL.md'); + await expect( + fs.readFile(path.join(manager.codexProjectSkillsRoot, 'gh-address-comments', 'agents', 'openai.yaml'), 'utf8'), + ).resolves.toContain('parsa/.codex/skills/gh-address-comments/agents/openai.yaml'); + await expect( + fs.readFile(path.join(manager.claudeProjectSkillsRoot, 'gh-address-comments', 'SKILL.md'), 'utf8'), + ).resolves.toContain('parsa/.claude/skills/gh-address-comments/SKILL.md'); + await expect( + fs.readFile(path.join(manager.claudeProjectSkillsRoot, 'gh-address-comments', 'agents', 'openai.yaml'), 'utf8'), + ).resolves.toContain('parsa/.claude/skills/gh-address-comments/agents/openai.yaml'); + await expect( + fs.readFile(path.join(manager.claudeProjectSkillsRoot, 'review', 'CRITERIA.md'), 'utf8'), + ).resolves.toContain('parsa/.claude/skills/review/CRITERIA.md'); + }); + + it('fails raw fallback when a required lifecycle file download fails even if a stale file exists', async () => { + const manager = new SkillCacheManager(); + const requiredPath = 'parsa/.codex/skills/gh-address-comments/SKILL.md'; + const staleTarget = path.join(manager.cacheRoot, requiredPath); + const httpsGet = mockRawDownloads(new Set([requiredPath])); + + await fs.mkdir(path.dirname(staleTarget), { recursive: true }); + await fs.writeFile(staleTarget, '# stale feedback skill\n', 'utf8'); + + try { + await expect( + (manager as unknown as { downloadFallbackFiles: () => Promise }).downloadFallbackFiles(), + ).rejects.toThrow(`Required download failures: ${requiredPath}`); + } finally { + httpsGet.mockRestore(); + } + }); }); diff --git a/main/src/services/skillCacheManager.ts b/main/src/services/skillCacheManager.ts index 7e2617d4..dcbcd5be 100644 --- a/main/src/services/skillCacheManager.ts +++ b/main/src/services/skillCacheManager.ts @@ -37,6 +37,7 @@ const IMPORTANT_SKILL_PATHS = [ 'parsa/.codex/skills/implementation-reviewer', 'parsa/.codex/skills/pr-test-automation', 'parsa/.codex/skills/prepare-pr', + 'parsa/.codex/skills/gh-address-comments', 'parsa/.codex/skills/teach-back', 'parsa/.codex/skills/investigate', 'parsa/.codex/skills/codebase-explorer', @@ -48,6 +49,8 @@ const IMPORTANT_SKILL_PATHS = [ 'parsa/.claude/skills/implement', 'parsa/.claude/skills/pr-test-automation', 'parsa/.claude/skills/prepare-pr', + 'parsa/.claude/skills/gh-address-comments', + 'parsa/.claude/skills/review', 'parsa/.claude/skills/teach-back', 'parsa/.claude/skills/investigate', 'parsa/.claude/skills/commit', @@ -56,11 +59,15 @@ const IMPORTANT_SKILL_PATHS = [ const REQUIRED_FALLBACK_RAW_FILES = [ ...TOP_LEVEL_FILES, ...IMPORTANT_SKILL_PATHS.map(skillPath => `${skillPath}/SKILL.md`), + 'parsa/.codex/skills/gh-address-comments/agents/openai.yaml', + 'parsa/.codex/skills/pr-test-automation/agents/openai.yaml', + 'parsa/.claude/skills/gh-address-comments/agents/openai.yaml', + 'parsa/.claude/skills/pr-test-automation/agents/openai.yaml', + 'parsa/.claude/skills/review/CRITERIA.md', ] as const; const OPTIONAL_FALLBACK_RAW_FILES = [ 'parsa/.codex/skills/plan/plan_base.md', - 'parsa/.codex/skills/pr-test-automation/agents/openai.yaml', 'parsa/.codex/skills/teach-back/agents/openai.yaml', 'parsa/.codex/skills/pane-work-recap/SKILL.md', 'parsa/.codex/skills/pane-work-recap/agents/openai.yaml', @@ -76,6 +83,8 @@ const FALLBACK_RAW_FILES = [ ...OPTIONAL_FALLBACK_RAW_FILES, ] as const; +const REQUIRED_FALLBACK_RAW_FILE_SET = new Set(REQUIRED_FALLBACK_RAW_FILES); + interface SkillSyncState { lastAttemptAt?: string; lastSuccessAt?: string; @@ -236,6 +245,7 @@ export class SkillCacheManager { private async downloadFallbackFiles(): Promise { await fs.mkdir(this.cacheRoot, { recursive: true }); const failures: string[] = []; + const requiredDownloadFailures: string[] = []; for (const relativePath of FALLBACK_RAW_FILES) { try { @@ -246,6 +256,9 @@ export class SkillCacheManager { } catch (error) { const message = error instanceof Error ? error.message : String(error); failures.push(`${relativePath}: ${message}`); + if (REQUIRED_FALLBACK_RAW_FILE_SET.has(relativePath)) { + requiredDownloadFailures.push(`${relativePath}: ${message}`); + } this.logWarn(`Failed to download skill cache file ${relativePath}`, error); } } @@ -257,12 +270,18 @@ export class SkillCacheManager { } } - if (missingRequiredFiles.length > 0) { + if (requiredDownloadFailures.length > 0 || missingRequiredFiles.length > 0) { const failureSummary = failures.length > 0 ? ` Failed downloads: ${failures.slice(0, 5).join('; ')}${failures.length > 5 ? '; ...' : ''}` : ''; + const failedRequiredSummary = requiredDownloadFailures.length > 0 + ? ` Required download failures: ${requiredDownloadFailures.slice(0, 5).join('; ')}${requiredDownloadFailures.length > 5 ? '; ...' : ''}` + : ''; + const missingRequiredSummary = missingRequiredFiles.length > 0 + ? ` Missing required files: ${missingRequiredFiles.join(', ')}.` + : ''; throw new Error( - `Skill cache fallback missing required files: ${missingRequiredFiles.join(', ')}.${failureSummary}`, + `Skill cache fallback failed for required files.${missingRequiredSummary}${failedRequiredSummary}${failureSummary}`, ); } } @@ -344,9 +363,24 @@ roots so launched agents can discover them by skill name: - \`${this.codexProjectSkillsRoot}\` - \`${this.claudeProjectSkillsRoot}\` -Use the cached RunPane orchestrator skill as the primary workflow reference. The -cached files may be refreshed by Pane in the background; do not fetch GitHub just -to initialize yourself. +## Contract Precedence + +Use one unambiguous hierarchy: + +1. The generated runtime context is authoritative for this exact Pane install, + data directory, shell/runtime, and RunPane command routing. +2. The generated Pane Chat orchestrator skill is authoritative for Pane-specific + role boundaries, focus preservation, pane/panel/worktree mechanics, cache + paths, and delegation through RunPane. +3. The active agent's cached RunPane orchestrator skill is authoritative for the + software-work lifecycle, persistent authorization ledger, stage transitions, + review-feedback interrupts, current-head evidence invalidation, and + \`ready_to_merge\` predicate. +4. Agent-specific downstream skills are authoritative for how each lifecycle + stage is performed. + +The cached files may be refreshed by Pane in the background; do not fetch GitHub +just to initialize yourself. For read-only work questions, use \`pane-work-recap\` when the user asks what they worked on and \`pane-work-prioritizer\` when they ask what to work on next. @@ -380,8 +414,10 @@ through RunPane and observed through Pane state. - Prefer RunPane state and wait commands over guessing from static sleeps. - Create background panes or panels by default when delegating work so the user keeps focus in Pane Chat unless they ask otherwise. -- Stop before merge, deploy, release, version bump, production mutation, or any - irreversible action unless the user explicitly authorizes that exact step. +- Stop before merge, deploy, release creation, publishing, version changes, + production or destructive mutation, deleting user data, scope expansion, or + other irreversible actions unless the user explicitly authorizes that exact + step. ## Generated RunPane Context @@ -450,49 +486,52 @@ create a minimal local git repository and register it with Pane. After that, delegate project implementation to a Pane agent through RunPane and observe the result from Pane state. +## Contract Precedence + +Use one unambiguous hierarchy: + +1. The generated runtime context is authoritative for this exact Pane install, + data directory, shell/runtime, and RunPane command routing. +2. This generated Pane Chat orchestrator skill is authoritative for Pane-specific + role boundaries, focus preservation, pane/panel/worktree mechanics, cache + paths, and delegation through RunPane. +3. The active agent's cached RunPane orchestrator skill is authoritative for the + software-work lifecycle, persistent authorization ledger, stage transitions, + review-feedback interrupts, current-head evidence invalidation, and + \`ready_to_merge\` predicate. +4. Agent-specific downstream skills are authoritative for how each lifecycle + stage is performed. + +If layers appear to conflict, preserve the more specific authority above instead +of merging both instructions into a new lifecycle. + ## Workflow Discipline For substantial work, greenfield projects, multi-agent work, PR preparation, or -anything that will create or change files, the default lifecycle is: - -1. Pane Chat owns discussion and clarification with the user when intent is - ambiguous, broad, creative, or multi-agent. Do not delegate separate - discussion loops to implementation agents by default. -2. Pane Chat distills the discussion into a concise intent brief, constraints, - success criteria, repo/worktree target, and autonomy level. -3. Delegate plan or simple-plan to the appropriate agent/pane using that - distilled brief. The output must be an explicit implementation plan or plan - artifact before implementation starts. -4. Delegate implement only from the approved plan or explicit plan artifact. -5. Delegate implementation review after implementation. -6. PR test automation or prepare-pr only after implementation review passes. - -Use best judgment for very small, low-risk tasks, but greenfield work and -multi-agent work should almost always go through discussion-at-Pane-Chat, -per-lane planning, implementation, and review. - -Do not skip directly to implement just because the delegated prompt contains an -implementation brief. Treat "use implement" as permission to implement only when -an approved plan already exists, or when the user explicitly says to skip -planning and use the brief as the plan. - -When delegating to agents, send the lifecycle stage explicitly. Examples: - -- "Use plan/simple-plan from this Pane Chat discussion brief; do not implement - yet." -- "Use plan/simple-plan first; do not implement yet." -- "Use implement against this approved plan file." -- "Use implementation-reviewer against the completed changes." - -If an implement agent reports that no approved plan file exists, stop and route -the work back through plan/simple-plan unless the user explicitly approved using -the brief as the plan. This is especially important for greenfield repos and -creative tasks, where treating a broad brief as a plan is usually too loose. +anything that will create or change files, load the active agent's cached +\`runpane-orchestrator\` and follow its lifecycle contract. Do not maintain a +second Pane-generated copy of that lifecycle. + +Pane Chat owns discussion and clarification with the user when intent is +ambiguous, broad, creative, or multi-agent. It may distill that conversation +into concise briefs, constraints, success criteria, repo/worktree targets, and +autonomy boundaries before delegating the next lifecycle stage through RunPane. + +When delegating to agents, name the intended lifecycle stage and the relevant +source artifact or brief. The active agent's cached \`runpane-orchestrator\` +decides how already-authorized reversible stages advance through investigation, +planning, implementation, implementation review, PR preparation, review feedback +handling, PR QA, CI/re-review, and \`ready_to_merge\`. + +Treat review feedback as an interrupt owned by the upstream lifecycle. When it +routes to \`gh-address-comments\`, use the implementation authority for source +fixes, separate source-edit grants from external-write grants, and rerun stale +current-head evidence after any head-changing fix. Delegate discussion to another agent only when the user explicitly asks for a separate perspective or when Pane Chat needs parallel research before forming the brief. In that case, Pane Chat still synthesizes the discussion result before -starting planning or implementation. +advancing the upstream lifecycle. ## Pane Workflow Model @@ -549,9 +588,10 @@ Use these local cached files. Do not fetch GitHub just to initialize yourself. ## Hard Stops -Stop before merge, deploy, release, version bump, production mutation, deleting -user data, or irreversible actions unless the user explicitly authorizes that -exact step. +Stop before merge, deploy, release creation, publishing, version changes, +production or destructive mutation, deleting user data, scope expansion, or +other irreversible actions unless the user explicitly authorizes that exact +step. `; }