Align Pane Chat skill contract

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.
 
 ---
 

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<string>()) {
+  return vi.spyOn(https, 'get').mockImplementation((url, callback) => {
+    const request = new EventEmitter() as ReturnType<typeof https.get>;
+    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<string, string | string[] | undefined>;
+  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<void> }).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<void> }).downloadFallbackFiles(),
+      ).rejects.toThrow(`Required download failures: ${requiredPath}`);
+    } finally {
+      httpsGet.mockRestore();
+    }
+  });
 });