mirror of
https://github.com/qwibitai/nanoclaw.git
synced 2026-06-12 18:11:51 +08:00
refactor(pr-factory): remove the supervisor session clear/retrigger workflow
The clear_session + retrigger capability was broken (a clear was meant to always pair with a retrigger, but the flow killed the container, deleted the session, and removed the pr_threads index entry — leaving retrigger unable to re-resolve the thread). Rather than fix it, rip it out entirely. Removed across the pr-factory-core component mirror: - session-ops.ts (clearWorkerSession / retriggerWorker) — file deleted; its only exports were those two functions + their result type. - rebootstrapPrSession() in handler.ts — used solely by retriggerWorker; the synchronize/new-push path has its own inline re-bootstrap (handleSynchronize) and does not share it, so it goes too. - The clear_session / retrigger container MCP tools (defs + registerTools entry) in container/agent-runner/src/mcp-tools/pr-factory.ts. - The pr_clear_session / pr_retrigger host delivery-action registrations and the clearWorkerSession/retriggerWorker re-exports in index.ts. - Test coverage scoped to clear/retrigger (the two tool cases + the four-action registration list); the host default-repo guard now rides pr_submit_test_results. - All clear/retrigger references in SKILL.md, files.txt, and docs/pr-factory.md. The supervisor's propose_skill_edit / skill-feedback capability stays. A skill edit now applies to the next PR the worker triages; the docs say so plainly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
gavrielc
@@ -89,7 +89,7 @@ All suites green. `src/recipe-pr-factory-stack.test.ts` is the composed-stack le
|
||||
|
||||
- **Per-PR worker flow** — webhook → Slack thread (status reactions 🟢⚪🔴🟣) → per-thread agent session seeded with the diff → triage report → review → test plan. The default triage/review/test-plan workflow is seeded into `groups/pr-factory-worker/CLAUDE.local.md` (edit it to tune trusted contributors, merge policy, review depth) or replaced wholesale via `PR_FACTORY_REVIEW_SKILL` — see "Tailoring the bots" below.
|
||||
- **Approval cards for every consequential action** — send-to-testing, retry, skill edits, and every `gh` write (merge, close, comment), executed with the approving human's gh identity.
|
||||
- **Supervisor bot** — its own Slack identity; takes feedback in an admin channel or @-mentioned in PR threads, proposes worker skill/instruction edits behind a diff + approval card, clears and retriggers PR sessions.
|
||||
- **Supervisor bot** — its own Slack identity; takes feedback in an admin channel or @-mentioned in PR threads, proposes worker skill/instruction edits behind a diff + approval card. Approved edits apply to the next PR the worker triages.
|
||||
- **VM test runs** — approved plans clone an ephemeral VM from a template, check out the PR, build, boot, and hand the VM to the tester agent; PASS wakes the worker to propose a merge, FAIL to analyze.
|
||||
- **Canvases** — test plans, results, and review writeups render as Slack Canvases instead of file uploads (paid Slack plan; falls back to `.md` uploads otherwise).
|
||||
|
||||
@@ -100,7 +100,7 @@ The shipped triage/review/test-plan workflow is deliberately generic. The PR Fac
|
||||
- **Container skills** live at `container/skills/<name>/SKILL.md` (read-only at `/app/skills` in every agent container). Each group's container config has a `skills` selection (default `'all'`) that controls which ones are symlinked into that group's `~/.claude/skills` at spawn — so a new skill folder reaches the worker on its next container start, no config change needed.
|
||||
- **Group-private skills**: a directory at `groups/<folder>/.claude/skills/<name>/` is discovered as a project-level skill (the agent's cwd is `/workspace/agent`, the group folder). Use this for a skill only one group should ever see — but note it sits outside the supervisor's edit loop below.
|
||||
- **Precedence**: with `PR_FACTORY_REVIEW_SKILL=<name>` set, every PR trigger opens with `Use the /<name> skill to triage this pull request.` and the generic defaults seeded into `groups/pr-factory-worker/CLAUDE.local.md` are ignored. Without it, the seeded instructions run — editing them in place is the lighter path when the defaults are close.
|
||||
- **Iteration loop**: `container/skills/` is what the supervisor bot edits — `propose_skill_edit` writes `container/skills/<skill>/<file>` behind a diff + approval card, then clears + retriggers affected PRs. Routing repo-specific workflow into a container skill (rather than CLAUDE.local.md) is what makes the feedback loop reviewable.
|
||||
- **Iteration loop**: `container/skills/` is what the supervisor bot edits — `propose_skill_edit` writes `container/skills/<skill>/<file>` behind a diff + approval card; the edit applies to the next PR the worker triages (running sessions keep their old read-only skill view until they next spawn). Routing repo-specific workflow into a container skill (rather than CLAUDE.local.md) is what makes the feedback loop reviewable.
|
||||
|
||||
### Interview the operator, then generate
|
||||
|
||||
|
||||
@@ -174,11 +174,11 @@ CREATE INDEX idx_pr_threads_session ON pr_threads (session_id);
|
||||
|
||||
| Function | Key | Used by |
|
||||
|----------|-----|---------|
|
||||
| `getPrThread(channelId, threadTs)` | PK | session-ops (clear) |
|
||||
| `getPrThreadByRepoPr(repo, prNumber)` | `idx_pr_threads_repo_pr` | handler (dedup, close, synchronize), session-ops, orchestrator |
|
||||
| `getPrThread(channelId, threadTs)` | PK | CRUD primitive (covered by `pr-threads.test.ts`) |
|
||||
| `getPrThreadByRepoPr(repo, prNumber)` | `idx_pr_threads_repo_pr` | handler (dedup, close, synchronize), orchestrator |
|
||||
| `getPrThreadBySession(sessionId)` | `idx_pr_threads_session` | testing-approval, gh-action-approval, reactions |
|
||||
| `updatePrThreadSession(channelId, threadTs, sessionId)` | PK | handler (synchronize repoints the row) |
|
||||
| `deletePrThread(channelId, threadTs)` | PK | session-ops (clear) |
|
||||
| `deletePrThread(channelId, threadTs)` | PK | CRUD primitive (covered by `pr-threads.test.ts`) |
|
||||
|
||||
---
|
||||
|
||||
@@ -262,7 +262,7 @@ A separate agent group (`pr-factory-supervisor`) speaking as its own Slack bot.
|
||||
- **Admin channel** — shared session, engages on every message.
|
||||
- **PR threads** — engages when @-mentioned, sees accumulated thread history.
|
||||
|
||||
Its MCP tools: `clear_session`, `retrigger`, `propose_skill_edit` (see below). The instructions tell it to always clear + retrigger affected PRs after an edit lands.
|
||||
Its MCP tool: `propose_skill_edit` (see below). A proposed edit is posted as a diff for human approval and, on accept, written under `container/skills/`. Skill edits apply to the **next** PR each affected worker session triages — running sessions keep their old read-only skill view until they next spawn; there is no force-rerun of an in-flight session.
|
||||
|
||||
---
|
||||
|
||||
@@ -295,12 +295,10 @@ The template VM is operator-prepared: project at `~/nanoclaw` with an `origin` t
|
||||
|
||||
**File:** `container/agent-runner/src/mcp-tools/pr-factory.ts`
|
||||
|
||||
Six tools, registered in every container via the mcp-tools barrel. Each writes a `kind: 'system'` row to `messages_out`; the host's delivery loop dispatches the `action` string to the matching registered handler. In a non-PR-factory install the actions are unregistered and dropped with "Unknown system action".
|
||||
Four tools, registered in every container via the mcp-tools barrel. Each writes a `kind: 'system'` row to `messages_out`; the host's delivery loop dispatches the `action` string to the matching registered handler. In a non-PR-factory install the actions are unregistered and dropped with "Unknown system action".
|
||||
|
||||
| Tool | Action emitted | Args |
|
||||
|------|----------------|------|
|
||||
| `clear_session` | `pr_clear_session` | `{ pr_number, repo? }` |
|
||||
| `retrigger` | `pr_retrigger` | `{ pr_number, repo? }` |
|
||||
| `propose_skill_edit` | `pr_propose_skill_edit` | `{ skill_name, file_name, content }` |
|
||||
| `send_to_testing` | `pr_send_to_testing` | `{}` (plan located via session → pr_threads → file naming) |
|
||||
| `credentialed_gh` | `pr_gh` | `{ command?, commands?, description }` — `command` is normalized into `commands` |
|
||||
@@ -316,8 +314,6 @@ When the agent omits `repo`, the field is omitted from the payload too — the h
|
||||
|
||||
| Delivery action | Handler | Effect |
|
||||
|--------|-------------|--------|
|
||||
| `pr_clear_session` | `session-ops.clearWorkerSession(repo, pr)` | Kill container, delete session, delete pr_threads row |
|
||||
| `pr_retrigger` | `session-ops.retriggerWorker(repo, pr)` | Kill container, re-fetch diff, re-trigger in the same thread/session |
|
||||
| `pr_send_to_testing` | `testing-approval.handleSendToTesting` | Post plan canvas + approval card |
|
||||
| `pr_propose_skill_edit` | `skill-edit-approval.handleProposeSkillEdit` | Post diff + approval card |
|
||||
| `pr_gh` | gh-action seam → `gh-action-approval.handleGh` | Post command preview + approval card |
|
||||
@@ -431,7 +427,6 @@ tail -f data/pr-activity/<owner>/<repo>/*.log # all PRs
|
||||
| `webhook.ts` | core | GitHub webhook: HMAC, event/action filter, `PREvent` parsing |
|
||||
| `handler.ts` | core | Per-PR lifecycle: opener, session, pr_threads, trigger, wake; synchronize/close/draft handling; OneCLI proxy GitHub fetches |
|
||||
| `supervisor.ts` | core | `SUPERVISOR_FOLDER` + `SUPERVISOR_INSTRUCTIONS` |
|
||||
| `session-ops.ts` | core | `clearWorkerSession` / `retriggerWorker`, keyed by `(repo, pr_number)` |
|
||||
| `testing-approval.ts` | core | Testing gate + retry card |
|
||||
| `skill-edit-approval.ts` | core | Skill-edit gate (traversal-guarded writes into `container/skills/`) |
|
||||
| `orchestrator.ts` | core | Tester wake, 30-min timeout, results → worker |
|
||||
@@ -472,9 +467,9 @@ tail -f data/pr-activity/<owner>/<repo>/*.log # all PRs
|
||||
|
||||
## Manual Operations
|
||||
|
||||
### Clear or retrigger a PR session
|
||||
### Re-run a PR from scratch
|
||||
|
||||
Ask the supervisor bot (its `clear_session` / `retrigger` MCP tools), or from the host REPL the module exports `clearWorkerSession(repo, prNumber)` and `retriggerWorker(repo, prNumber)` from `src/modules/pr-factory/index.ts`.
|
||||
There is no in-place "clear and retrigger" — a skill edit applies to the next PR the worker triages, and a fresh push (a `synchronize` event) re-triages an open PR in the same thread against the updated diff. To force a clean re-run, close and reopen the PR on GitHub, or push a no-op commit; the webhook drives a fresh session either way.
|
||||
|
||||
### Inspect pr_threads
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
---
|
||||
name: pr-factory-core
|
||||
description: PR Factory component — the engine. GitHub pull_request webhook → per-PR Slack thread + worker agent session (triage/review/test-plan via default group instructions), supervisor bootstrap with skill-edit approval flow, send-to-testing approval gate, test-result coordination, session clear/retrigger ops, pr_threads index, and six container MCP tools. Ships seams for the optional gh-action-approval, vm-test-orchestrator, and slack-canvas components.
|
||||
description: PR Factory component — the engine. GitHub pull_request webhook → per-PR Slack thread + worker agent session (triage/review/test-plan via default group instructions), supervisor bootstrap with skill-edit approval flow, send-to-testing approval gate, test-result coordination, pr_threads index, and four container MCP tools. Ships seams for the optional gh-action-approval, vm-test-orchestrator, and slack-canvas components.
|
||||
---
|
||||
|
||||
# pr-factory-core (PR Factory component)
|
||||
@@ -12,8 +12,7 @@ The PR Factory engine, as a host module (`src/modules/pr-factory/`) plus a conta
|
||||
- **Bootstrap** (`bootstrap.ts`) — idempotent, self-correcting setup: worker agent group (default triage/review/test-plan instructions seeded into `groups/pr-factory-worker/CLAUDE.local.md`), default-instance worker messaging group, optional supervisor agent group with `slack-supervisor`-instance messaging groups (admin channel + PR channel), and a `slack-tester`-instance messaging group when the operator-created `pr-tester` agent group exists.
|
||||
- **Approval flows** — send-to-testing (`testing-approval.ts`, plan file → card → human gate), retry-after-failure, and supervisor skill edits (`skill-edit-approval.ts`, diff preview → card → write on approve). One active card per thread (`dismiss-approvals.ts`); a 👀 reaction marks awaiting-approval threads and clears on resolution — the reject path is observed through core's `registerApprovalResolvedHandler` hook.
|
||||
- **Coordination** (`orchestrator.ts`) — wakes the tester agent when a VM is ready, enforces a 30-minute run timeout, posts results, wakes the worker to propose merge (PASS) or analyze (FAIL/PARTIAL).
|
||||
- **Session ops** (`session-ops.ts`) — supervisor-driven `clear_session` / `retrigger` keyed by (repo, pr_number).
|
||||
- **Container MCP tools** (`container/agent-runner/src/mcp-tools/pr-factory.ts`) — `clear_session`, `retrigger`, `propose_skill_edit`, `send_to_testing`, `credentialed_gh`, `submit_test_results`. Each emits a `pr_*` system action via messages_out; the host module registers the six matching delivery actions.
|
||||
- **Container MCP tools** (`container/agent-runner/src/mcp-tools/pr-factory.ts`) — `propose_skill_edit`, `send_to_testing`, `credentialed_gh`, `submit_test_results`. Each emits a `pr_*` system action via messages_out; the host module registers the four matching delivery actions.
|
||||
|
||||
Inert without `GITHUB_WEBHOOK_SECRET`: the module loads (approval handlers bind) but registers no delivery actions and mounts no webhook.
|
||||
|
||||
@@ -99,7 +98,7 @@ SKILL=.claude/skills/recipes/pr-factory/skills/pr-factory-core
|
||||
```bash
|
||||
mkdir -p src/modules/pr-factory
|
||||
for f in index bootstrap handler webhook orchestrator testing-approval skill-edit-approval \
|
||||
dismiss-approvals session-ops reactions activity-log defaults supervisor \
|
||||
dismiss-approvals reactions activity-log defaults supervisor \
|
||||
worker-instructions canvas test-orchestration gh-action; do
|
||||
cp $SKILL/files/src/modules/pr-factory/$f.ts src/modules/pr-factory/$f.ts
|
||||
done
|
||||
@@ -196,7 +195,7 @@ cp $SKILL/files/container/agent-runner/src/mcp-tools/pr-factory-tools.test.ts co
|
||||
|
||||
| Test | Guards |
|
||||
|------|--------|
|
||||
| `src/modules/pr-factory/registration.test.ts` | The modules-barrel line via the REAL barrel, the six `pr_*` delivery actions + three core approval handlers (read-side registries), the `GITHUB_WEBHOOK_SECRET` env gate, the host-side `PR_FACTORY_DEFAULT_REPO` contract, and the gh-action seam's not-installed fallback |
|
||||
| `src/modules/pr-factory/registration.test.ts` | The modules-barrel line via the REAL barrel, the four `pr_*` delivery actions + three core approval handlers (read-side registries), the `GITHUB_WEBHOOK_SECRET` env gate, the host-side `PR_FACTORY_DEFAULT_REPO` contract, and the gh-action seam's not-installed fallback |
|
||||
| `src/modules/pr-factory/bootstrap.test.ts` | Bootstrap's consumption of the entity-model writers on the real migrated schema: instance-keyed messaging groups (worker default, supervisor/tester named), wiring modes, instruction seeding, idempotence, foreign-wiring drop, drift self-correction |
|
||||
| `src/modules/pr-factory/webhook.test.ts` | `registerGitHubWebhook`'s mount on core's raw webhook registry over real HTTP: HMAC accept/reject, event filtering, 405, throwing-handler → 500 |
|
||||
| `src/modules/pr-factory/handler.test.ts` | handler's consumption of resolveSession / writeSessionMessage / pr_threads on real session DBs: the PR_CONTEXT trigger contract, the default triage directive, synchronize kill/re-create, draft deferral, redelivery no-op |
|
||||
@@ -205,7 +204,7 @@ cp $SKILL/files/container/agent-runner/src/mcp-tools/pr-factory-tools.test.ts co
|
||||
| `src/db/pr-threads.test.ts` | Migration barrel presence (real `runMigrations`), v2 schema shape (no bot column), the legacy-upgrade recreate arm, idempotence, CRUD |
|
||||
| `src/db/sessions-approval-helpers.test.ts` | The four appended sessions.ts helpers |
|
||||
| `container/.../pr-factory-registration.test.ts` | The container mcp-tools barrel line (AST: side-effect import before `startMcpServer()`) |
|
||||
| `container/.../pr-factory-tools.test.ts` | The six tools' messages_out contract: exact `pr_*` action strings (paired with the host registrations), odd-seq, repo-omission, command normalization |
|
||||
| `container/.../pr-factory-tools.test.ts` | The four tools' messages_out contract: exact `pr_*` action strings (paired with the host registrations), odd-seq, repo-omission, command normalization |
|
||||
|
||||
## Configuration
|
||||
|
||||
|
||||
@@ -12,7 +12,6 @@ src/modules/pr-factory/orchestrator.ts
|
||||
src/modules/pr-factory/testing-approval.ts
|
||||
src/modules/pr-factory/skill-edit-approval.ts
|
||||
src/modules/pr-factory/dismiss-approvals.ts
|
||||
src/modules/pr-factory/session-ops.ts
|
||||
src/modules/pr-factory/reactions.ts
|
||||
src/modules/pr-factory/activity-log.ts
|
||||
src/modules/pr-factory/defaults.ts
|
||||
|
||||
+2
-41
@@ -16,14 +16,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'bun:test';
|
||||
|
||||
import { initTestSessionDb, closeSessionDb } from '../db/connection.js';
|
||||
import { getUndeliveredMessages } from '../db/messages-out.js';
|
||||
import {
|
||||
clearSession,
|
||||
ghCommand,
|
||||
proposeSkillEdit,
|
||||
retrigger,
|
||||
sendToTesting,
|
||||
submitTestResults,
|
||||
} from './pr-factory.js';
|
||||
import { ghCommand, proposeSkillEdit, sendToTesting, submitTestResults } from './pr-factory.js';
|
||||
|
||||
beforeEach(() => {
|
||||
initTestSessionDb();
|
||||
@@ -41,39 +34,6 @@ function systemRows(): Array<{ seq: number; kind: string; action: string; conten
|
||||
}
|
||||
|
||||
describe('pr-factory MCP tools → messages_out contract', () => {
|
||||
it('clear_session emits pr_clear_session with pr_number, OMITTING repo so the host applies its default', async () => {
|
||||
const res = await clearSession.handler({ pr_number: 42 });
|
||||
expect(res.isError).toBeUndefined();
|
||||
|
||||
const [row] = systemRows();
|
||||
expect(row.kind).toBe('system');
|
||||
expect(row.seq % 2).toBe(1); // container writes odd seq
|
||||
expect(row.action).toBe('pr_clear_session');
|
||||
expect(row.content.pr_number).toBe(42);
|
||||
// Repo absent in the payload → host-side PR_FACTORY_DEFAULT_REPO applies.
|
||||
expect('repo' in row.content).toBe(false);
|
||||
});
|
||||
|
||||
it('clear_session passes an explicit repo through unchanged', async () => {
|
||||
await clearSession.handler({ pr_number: 42, repo: 'acme/widgets' });
|
||||
const [row] = systemRows();
|
||||
expect(row.content.repo).toBe('acme/widgets');
|
||||
});
|
||||
|
||||
it('retrigger emits pr_retrigger and honors an explicit repo', async () => {
|
||||
await retrigger.handler({ pr_number: 7, repo: 'acme/widgets' });
|
||||
const [row] = systemRows();
|
||||
expect(row.action).toBe('pr_retrigger');
|
||||
expect(row.content).toMatchObject({ pr_number: 7, repo: 'acme/widgets' });
|
||||
});
|
||||
|
||||
it('retrigger omits repo from the payload when the agent does not pass one', async () => {
|
||||
await retrigger.handler({ pr_number: 7 });
|
||||
const [row] = systemRows();
|
||||
expect(row.action).toBe('pr_retrigger');
|
||||
expect('repo' in row.content).toBe(false);
|
||||
});
|
||||
|
||||
it('propose_skill_edit emits pr_propose_skill_edit with the full file payload', async () => {
|
||||
await proposeSkillEdit.handler({ skill_name: 'my-review-skill', file_name: 'SKILL.md', content: '# v2' });
|
||||
const [row] = systemRows();
|
||||
@@ -85,6 +45,7 @@ describe('pr-factory MCP tools → messages_out contract', () => {
|
||||
await sendToTesting.handler({});
|
||||
const [row] = systemRows();
|
||||
expect(row.kind).toBe('system');
|
||||
expect(row.seq % 2).toBe(1); // container writes odd seq
|
||||
expect(row.action).toBe('pr_send_to_testing');
|
||||
});
|
||||
|
||||
|
||||
+3
-79
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* PR Factory MCP tools — supervisor session ops, skill-edit proposals,
|
||||
* testing gate, credentialed GitHub commands, and test-result submission.
|
||||
* PR Factory MCP tools — supervisor skill-edit proposals, testing gate,
|
||||
* credentialed GitHub commands, and test-result submission.
|
||||
*
|
||||
* The container can't write to inbound.db (host-owned), so each tool emits
|
||||
* a system action via messages_out and the host's delivery loop dispatches
|
||||
@@ -31,82 +31,6 @@ function genId(prefix: string): string {
|
||||
return `${prefix}-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`;
|
||||
}
|
||||
|
||||
export const clearSession: McpToolDefinition = {
|
||||
tool: {
|
||||
name: 'clear_session',
|
||||
description:
|
||||
"PR Factory supervisor only. Wipe the worker's per-thread session for a PR (kills the container, deletes the session row). Use after editing a skill so the next triage starts from clean state. Always pair with `retrigger` immediately after.",
|
||||
inputSchema: {
|
||||
type: 'object' as const,
|
||||
properties: {
|
||||
pr_number: {
|
||||
type: 'number',
|
||||
description: 'Pull request number, e.g. 2318.',
|
||||
},
|
||||
repo: {
|
||||
type: 'string',
|
||||
description:
|
||||
'Full repository name, e.g. "acme/widgets". Omit to use the host\'s configured default repo (PR_FACTORY_DEFAULT_REPO).',
|
||||
},
|
||||
},
|
||||
required: ['pr_number'],
|
||||
},
|
||||
},
|
||||
async handler(args) {
|
||||
const pr_number = args.pr_number as number;
|
||||
// When the agent omits repo, the field is omitted from the payload too —
|
||||
// the HOST action handler applies PR_FACTORY_DEFAULT_REPO. The container
|
||||
// never sees that env var, so it must not bake in a default of its own.
|
||||
const repo = args.repo as string | undefined;
|
||||
if (!pr_number) return err('pr_number is required');
|
||||
|
||||
writeMessageOut({
|
||||
id: genId('sys'),
|
||||
kind: 'system',
|
||||
content: JSON.stringify({ action: 'pr_clear_session', pr_number, ...(repo ? { repo } : {}) }),
|
||||
});
|
||||
log(`pr_clear_session: ${repo ?? '<default repo>'}#${pr_number}`);
|
||||
return ok(`Clear requested for ${repo ?? 'the default repo'}#${pr_number}`);
|
||||
},
|
||||
};
|
||||
|
||||
export const retrigger: McpToolDefinition = {
|
||||
tool: {
|
||||
name: 'retrigger',
|
||||
description:
|
||||
'PR Factory supervisor only. Re-fetch the PR diff fresh from GitHub and re-bootstrap the worker session for that PR. Use immediately after `clear_session` on every affected PR.',
|
||||
inputSchema: {
|
||||
type: 'object' as const,
|
||||
properties: {
|
||||
pr_number: {
|
||||
type: 'number',
|
||||
description: 'Pull request number, e.g. 2318.',
|
||||
},
|
||||
repo: {
|
||||
type: 'string',
|
||||
description:
|
||||
'Full repository name, e.g. "acme/widgets". Omit to use the host\'s configured default repo (PR_FACTORY_DEFAULT_REPO).',
|
||||
},
|
||||
},
|
||||
required: ['pr_number'],
|
||||
},
|
||||
},
|
||||
async handler(args) {
|
||||
const pr_number = args.pr_number as number;
|
||||
// Omitted repo stays omitted — the host applies PR_FACTORY_DEFAULT_REPO.
|
||||
const repo = args.repo as string | undefined;
|
||||
if (!pr_number) return err('pr_number is required');
|
||||
|
||||
writeMessageOut({
|
||||
id: genId('sys'),
|
||||
kind: 'system',
|
||||
content: JSON.stringify({ action: 'pr_retrigger', pr_number, ...(repo ? { repo } : {}) }),
|
||||
});
|
||||
log(`pr_retrigger: ${repo ?? '<default repo>'}#${pr_number}`);
|
||||
return ok(`Retrigger requested for ${repo ?? 'the default repo'}#${pr_number}`);
|
||||
},
|
||||
};
|
||||
|
||||
export const proposeSkillEdit: McpToolDefinition = {
|
||||
tool: {
|
||||
name: 'propose_skill_edit',
|
||||
@@ -276,4 +200,4 @@ export const submitTestResults: McpToolDefinition = {
|
||||
},
|
||||
};
|
||||
|
||||
registerTools([clearSession, retrigger, proposeSkillEdit, sendToTesting, ghCommand, submitTestResults]);
|
||||
registerTools([proposeSkillEdit, sendToTesting, ghCommand, submitTestResults]);
|
||||
|
||||
@@ -7,9 +7,9 @@
|
||||
* is resolved per messaging group via `messaging_groups.instance`.
|
||||
*
|
||||
* Used by:
|
||||
* - pr-factory handler: insert on PR opened
|
||||
* - pr-factory session-ops (clear / retrigger): lookup by (channel_id, thread_ts)
|
||||
* - pr-factory handler: insert on PR opened; lookup + repoint on synchronize/close
|
||||
* - pr-factory orchestrator: lookup by (repo_full_name, pr_number) when test results land
|
||||
* - pr-factory testing/gh/skill-edit gates + reactions: lookup by (session_id)
|
||||
*/
|
||||
import { getDb } from './connection.js';
|
||||
|
||||
|
||||
-59
@@ -480,62 +480,3 @@ async function handleSynchronize(pr: PREvent, existing: PrThread, cfg: HandlerCo
|
||||
await wakeContainer(fresh);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-bootstrap an existing PR session: re-fetch the diff fresh from GitHub
|
||||
* and write a new trigger message into the same session. Used by retrigger.
|
||||
*
|
||||
* Looks up repo/pr#/session via the pr_threads row keyed by (channel, thread).
|
||||
* Caller (session-ops.retrigger) has already killed the container if needed.
|
||||
*/
|
||||
export async function rebootstrapPrSession(
|
||||
channelId: string,
|
||||
threadTs: string,
|
||||
agentGroupId: string,
|
||||
repoFullName: string,
|
||||
prNumber: number,
|
||||
): Promise<void> {
|
||||
const platformId = channelId; // already in `slack:CXXXX` form on disk
|
||||
const messagingGroup = getMessagingGroupByPlatform('slack', platformId, 'slack');
|
||||
if (!messagingGroup) {
|
||||
throw new Error(`No worker messaging group for ${platformId}`);
|
||||
}
|
||||
const sessionThreadId = `${platformId}:${threadTs}`;
|
||||
|
||||
const { session } = resolveSession(agentGroupId, messagingGroup.id, sessionThreadId, 'per-thread');
|
||||
|
||||
prLog(prNumber, repoFullName, 'retrigger_bootstrapping');
|
||||
await refreshRepoMirror();
|
||||
const diff = await fetchDiff(repoFullName, prNumber);
|
||||
const now = new Date().toISOString();
|
||||
|
||||
const content = [
|
||||
`Re-triage PR #${prNumber} — supervisor requested a rerun after a skill update. ${triageDirective()}`,
|
||||
'',
|
||||
`## Pull Request #${prNumber}`,
|
||||
`**Repository:** ${repoFullName}`,
|
||||
'',
|
||||
'### Diff (re-fetched)',
|
||||
'```diff',
|
||||
diff,
|
||||
'```',
|
||||
'',
|
||||
`[PR_CONTEXT: channel=${platformId} thread=${threadTs} repo=${repoFullName} pr=${prNumber}]`,
|
||||
].join('\n');
|
||||
|
||||
writeSessionMessage(agentGroupId, session.id, {
|
||||
id: generateId('msg-retrigger'),
|
||||
kind: 'chat',
|
||||
timestamp: now,
|
||||
platformId,
|
||||
channelType: 'slack',
|
||||
threadId: sessionThreadId,
|
||||
content: JSON.stringify({ text: content, sender: 'Supervisor', senderId: 'pr-factory-supervisor' }),
|
||||
});
|
||||
|
||||
const fresh = getSession(session.id);
|
||||
if (fresh) {
|
||||
prLog(prNumber, repoFullName, 'retrigger_container_waking');
|
||||
await wakeContainer(fresh);
|
||||
}
|
||||
}
|
||||
|
||||
-10
@@ -37,11 +37,9 @@ import { clearAwaitingApproval } from './reactions.js';
|
||||
import { registerGitHubWebhook } from './webhook.js';
|
||||
import { handlePullRequest } from './handler.js';
|
||||
import { bootstrapPrFactory, TESTER_FOLDER } from './bootstrap.js';
|
||||
import { DEFAULT_REPO } from './defaults.js';
|
||||
import { getTestOrchestrator } from './test-orchestration.js';
|
||||
import { dispatchGhAction } from './gh-action.js';
|
||||
import { initOrchestrator, shutdownOrchestrator, handleTestResults } from './orchestrator.js';
|
||||
import { clearWorkerSession, retriggerWorker } from './session-ops.js';
|
||||
import { handleSendToTesting } from './testing-approval.js';
|
||||
import { handleProposeSkillEdit } from './skill-edit-approval.js';
|
||||
import { getAgentGroupByFolder } from '../../db/agent-groups.js';
|
||||
@@ -85,12 +83,6 @@ if (!GITHUB_WEBHOOK_SECRET) {
|
||||
// same default inside handleTestResults. pr_gh dispatches through the
|
||||
// gh-action seam so the agent gets feedback even when the
|
||||
// gh-action-approval component isn't installed.
|
||||
registerDeliveryAction('pr_clear_session', async (content) => {
|
||||
clearWorkerSession((content.repo as string) || DEFAULT_REPO, content.pr_number as number);
|
||||
});
|
||||
registerDeliveryAction('pr_retrigger', async (content) => {
|
||||
await retriggerWorker((content.repo as string) || DEFAULT_REPO, content.pr_number as number);
|
||||
});
|
||||
registerDeliveryAction('pr_send_to_testing', async (content, session) => {
|
||||
await handleSendToTesting(content, session);
|
||||
});
|
||||
@@ -154,5 +146,3 @@ if (!GITHUB_WEBHOOK_SECRET) {
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
export { clearWorkerSession, retriggerWorker } from './session-ops.js';
|
||||
|
||||
+14
-10
@@ -1,6 +1,6 @@
|
||||
/**
|
||||
* pr-factory-core guard — the modules-barrel line (`import
|
||||
* './pr-factory/index.js'` in src/modules/index.ts), the six pr_* delivery
|
||||
* './pr-factory/index.js'` in src/modules/index.ts), the four pr_* delivery
|
||||
* actions, the three core-owned pr_* approval handlers, the gh-action seam's
|
||||
* not-installed fallback, and the GITHUB_WEBHOOK_SECRET env gate.
|
||||
*
|
||||
@@ -43,8 +43,6 @@ const TEST_DIR = '/tmp/nanoclaw-test-prf-registration';
|
||||
const ORIGINAL_CWD = process.cwd();
|
||||
|
||||
const DELIVERY_ACTIONS = [
|
||||
'pr_clear_session',
|
||||
'pr_retrigger',
|
||||
'pr_send_to_testing',
|
||||
'pr_propose_skill_edit',
|
||||
'pr_gh',
|
||||
@@ -89,7 +87,7 @@ describe('pr-factory module registration via the real modules barrel', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('with the env trio primed before import, all six pr_* delivery actions register', async () => {
|
||||
it('with the env trio primed before import, all four pr_* delivery actions register', async () => {
|
||||
vi.resetModules();
|
||||
process.env.GITHUB_WEBHOOK_SECRET = 'test-secret';
|
||||
process.env.PR_FACTORY_SLACK_CHANNEL_ID = 'C0TEST';
|
||||
@@ -122,7 +120,8 @@ describe('pr-factory module registration via the real modules barrel', () => {
|
||||
const now = new Date().toISOString();
|
||||
const { createAgentGroup } = await import('../../db/agent-groups.js');
|
||||
const { createSession } = await import('../../db/sessions.js');
|
||||
const { createPrThread, getPrThreadByRepoPr } = await import('../../db/pr-threads.js');
|
||||
const { createPrThread } = await import('../../db/pr-threads.js');
|
||||
const { initSessionFolder, outboundDbPath } = await import('../../session-manager.js');
|
||||
createAgentGroup({
|
||||
id: 'ag-prf',
|
||||
name: 'Worker',
|
||||
@@ -141,6 +140,7 @@ describe('pr-factory module registration via the real modules barrel', () => {
|
||||
last_active: null,
|
||||
created_at: now,
|
||||
});
|
||||
initSessionFolder('ag-prf', 'sess-prf');
|
||||
// The pr_threads row is keyed by the DEFAULT repo — only a handler that
|
||||
// fills the omitted `repo` with PR_FACTORY_DEFAULT_REPO can find it.
|
||||
createPrThread({
|
||||
@@ -155,15 +155,19 @@ describe('pr-factory module registration via the real modules barrel', () => {
|
||||
|
||||
await import('../index.js');
|
||||
const { getDeliveryAction } = await import('../../delivery.js');
|
||||
const { killContainer } = await import('../../container-runner.js');
|
||||
const handler = getDeliveryAction('pr_clear_session');
|
||||
const handler = getDeliveryAction('pr_submit_test_results');
|
||||
expect(handler).toBeDefined();
|
||||
|
||||
const session = { id: 'sess-prf', agent_group_id: 'ag-prf' };
|
||||
await handler!({ pr_number: 42 }, session as never, undefined as never);
|
||||
// No `repo` in the payload — the handler must default it to acme/defaulted
|
||||
// to find the pr_threads row keyed by that repo and post the verdict.
|
||||
await handler!({ pr_number: 42, verdict: 'PASS', content: '## results' }, session as never, undefined as never);
|
||||
|
||||
expect(killContainer).toHaveBeenCalledWith('sess-prf', 'cleared by supervisor');
|
||||
expect(getPrThreadByRepoPr('acme/defaulted', 42), 'pr_threads row should be cleared').toBeUndefined();
|
||||
const outDb = new Database(outboundDbPath('ag-prf', 'sess-prf'), { readonly: true });
|
||||
const rows = outDb.prepare('SELECT content FROM messages_out').all() as Array<{ content: string }>;
|
||||
outDb.close();
|
||||
expect(rows.length, 'verdict summary should reach the worker session resolved via the default repo').toBe(1);
|
||||
expect((JSON.parse(rows[0].content) as { text: string }).text).toContain('PASS');
|
||||
} finally {
|
||||
closeDb();
|
||||
}
|
||||
|
||||
-60
@@ -1,60 +0,0 @@
|
||||
/**
|
||||
* Session operations exposed to the supervisor via MCP tools.
|
||||
*
|
||||
* Both functions are keyed by (repo, pr_number) — the natural identifiers
|
||||
* visible in every worker message. Internally they look up the PR via the
|
||||
* pr_threads table and operate on the worker's per-thread session.
|
||||
*/
|
||||
import { getPrThreadByRepoPr, deletePrThread } from '../../db/pr-threads.js';
|
||||
import { getSession, deleteSession } from '../../db/sessions.js';
|
||||
import { killContainer } from '../../container-runner.js';
|
||||
import { log } from '../../log.js';
|
||||
import { prLog } from './activity-log.js';
|
||||
|
||||
import { rebootstrapPrSession } from './handler.js';
|
||||
|
||||
export interface SessionOpResult {
|
||||
ok: boolean;
|
||||
message: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Wipe the worker's per-thread session for a PR. Kills the running
|
||||
* container, deletes the session row, and removes the pr_threads index
|
||||
* entry so a future retrigger re-bootstraps cleanly.
|
||||
*/
|
||||
export function clearWorkerSession(repo: string, prNumber: number): SessionOpResult {
|
||||
const pr = getPrThreadByRepoPr(repo, prNumber);
|
||||
if (!pr) return { ok: false, message: `No PR thread for ${repo}#${prNumber}` };
|
||||
|
||||
const session = getSession(pr.session_id);
|
||||
if (session) {
|
||||
killContainer(session.id, 'cleared by supervisor');
|
||||
deleteSession(session.id);
|
||||
}
|
||||
deletePrThread(pr.channel_id, pr.thread_ts);
|
||||
prLog(prNumber, repo, 'session_cleared', { sessionId: pr.session_id });
|
||||
log.info('Worker session cleared', { repo, prNumber, sessionId: pr.session_id });
|
||||
return { ok: true, message: `Cleared session for PR #${prNumber} (${repo})` };
|
||||
}
|
||||
|
||||
/**
|
||||
* Re-trigger PR triage with a freshly fetched diff. Keeps the same Slack
|
||||
* thread + session id; the worker re-runs its triage workflow against the
|
||||
* latest GitHub state.
|
||||
*/
|
||||
export async function retriggerWorker(repo: string, prNumber: number): Promise<SessionOpResult> {
|
||||
const pr = getPrThreadByRepoPr(repo, prNumber);
|
||||
if (!pr) return { ok: false, message: `No PR thread for ${repo}#${prNumber}` };
|
||||
|
||||
const session = getSession(pr.session_id);
|
||||
if (!session) {
|
||||
return { ok: false, message: `Session ${pr.session_id} for PR #${prNumber} not found` };
|
||||
}
|
||||
|
||||
killContainer(session.id, 'retriggered by supervisor');
|
||||
await rebootstrapPrSession(pr.channel_id, pr.thread_ts, session.agent_group_id, repo, prNumber);
|
||||
prLog(prNumber, repo, 'session_retriggered', { sessionId: pr.session_id });
|
||||
log.info('Worker session retriggered', { repo, prNumber, sessionId: pr.session_id });
|
||||
return { ok: true, message: `Retriggered PR #${prNumber} (${repo})` };
|
||||
}
|
||||
+5
-5
@@ -26,17 +26,17 @@ The PR number is visible in every worker message (e.g. "PR #2318" in the triage
|
||||
|
||||
## MCP tools you own
|
||||
|
||||
- \`mcp__nanoclaw__clear_session({ pr_number, repo? })\` — wipes the worker's session for that PR (kills container, deletes session row). \`repo\` defaults to the host's configured default repo (PR_FACTORY_DEFAULT_REPO).
|
||||
- \`mcp__nanoclaw__retrigger({ pr_number, repo? })\` — re-fetches the PR diff fresh from GitHub, re-bootstraps the session, wakes the worker. After any skill edit, **always** clear + retrigger affected PRs in one step.
|
||||
- \`mcp__nanoclaw__propose_skill_edit({ skill_name, file_name, content })\` — propose a skill file edit. Read the current file from \`/app/skills/\` first, then pass the full new content. The host posts the diff for human approval — the file is only written if approved. **Always use this tool to edit skills — never write to the filesystem directly.**
|
||||
|
||||
Skill edits apply to the **next** PR each affected worker session triages — running sessions keep their old read-only skill view until they next spawn. Tell the human the edit lands going forward; there is no force-rerun of an in-flight session.
|
||||
|
||||
## Two workflows
|
||||
|
||||
### A — Quick fix in a PR thread
|
||||
|
||||
1. Read the thread (already accumulated). Identify what went wrong.
|
||||
2. Propose the change. Use \`propose_skill_edit\` — the host posts the diff and the human approves or rejects.
|
||||
3. On approval, clear + retrigger the PR. Tell the human what changed.
|
||||
3. On approval, tell the human what changed and that it applies to the next PR the worker triages.
|
||||
|
||||
### B — Batch review in admin channel
|
||||
|
||||
@@ -47,7 +47,7 @@ The PR number is visible in every worker message (e.g. "PR #2318" in the triage
|
||||
**Suggested fix:** <your read>
|
||||
\`\`\`
|
||||
2. **Review**: when the human asks you in admin channel, walk them through the collected feedback, propose skill diffs (don't apply yet), iterate.
|
||||
3. **Implement**: on approval — use \`propose_skill_edit\` for each file, then clear + retrigger every affected PR.
|
||||
3. **Implement**: on approval — use \`propose_skill_edit\` for each file. The edits apply to subsequent PRs going forward.
|
||||
|
||||
## Where things are
|
||||
|
||||
@@ -62,5 +62,5 @@ The PR number is visible in every worker message (e.g. "PR #2318" in the triage
|
||||
- **Patterns over one-offs** — fix the skill, not the individual PR.
|
||||
- **Evidence first** — quote the worker's actual output before proposing a fix.
|
||||
- **Human approves** — propose, don't apply.
|
||||
- **Always finish the loop** — after editing a skill, immediately clear + retrigger every affected PR. Never leave it to the human to remember.
|
||||
- **Edits apply going forward** — a skill edit changes how the worker triages the next PR; it does not re-run PRs already in flight.
|
||||
`;
|
||||
|
||||
Reference in New Issue
Block a user