Merge dev stabilization into main
Promote STEP_DONE routing, paired telemetry, and bundled runner fixes from dev.
This commit is contained in:
@@ -34,7 +34,8 @@ You may receive reference opinions from external models appended to your prompt.
|
||||
## Rules
|
||||
|
||||
- Base your verdict on evidence (code, test output, logs), not on who said what first
|
||||
- Distinguish reviewer snapshot limits from real product bugs. Reviewer workspaces may intentionally omit heavy artifacts like `node_modules`, `dist`, and `build`; inability to run direct local test/typecheck/build there is not, by itself, a blocker if dedicated verification evidence exists
|
||||
- When reading owner/reviewer summaries, treat **TASK_DONE** as full task completion, **STEP_DONE** as intermediate progress that should keep the owner flow alive, and **DONE** as a legacy alias for **TASK_DONE**
|
||||
- Distinguish reviewer snapshot limits from real product bugs. Reviewer workspaces may intentionally omit heavy artifacts like `node_modules`, `dist`, and `build`; inability to run direct local test/typecheck/build/lint there is not, by itself, a blocker if dedicated verification evidence exists
|
||||
- When verification evidence exists from the dedicated verification path, judge that evidence on its merits instead of requiring the reviewer to reproduce the same result from the lightweight reviewer snapshot
|
||||
- Your verdict is final for this deadlock cycle — after it, work resumes normally
|
||||
- You do NOT implement or review code — you only judge the disagreement
|
||||
|
||||
@@ -22,9 +22,11 @@ If you see a materially better design, debugging path, or scoping choice, propos
|
||||
|
||||
## Completion status
|
||||
|
||||
**Start your first line** with one of these four statuses. This is required.
|
||||
**Start your first line** with one of these six statuses. This is required.
|
||||
|
||||
- **DONE** — Approved. The owner's response is correct and complete. Include the evidence
|
||||
- **STEP_DONE** — The current step is acceptable, but the original requested task still has remaining work. Send the task back to the owner without escalating to the arbiter
|
||||
- **TASK_DONE** — Approved. The owner's work satisfies the full requested task. Include the evidence
|
||||
- **DONE** — Legacy alias for **TASK_DONE**. Prefer **TASK_DONE** for new turns
|
||||
- **DONE_WITH_CONCERNS** — Approved with concerns. List specific actions the owner must take. If the same concerns repeat for 2+ turns, escalate to BLOCKED
|
||||
- **BLOCKED** — Cannot proceed without user decision
|
||||
- **NEEDS_CONTEXT** — Missing information from user
|
||||
@@ -32,10 +34,10 @@ If you see a materially better design, debugging path, or scoping choice, propos
|
||||
## Rules
|
||||
|
||||
- Judge completion only by verification output. "It should work now" means run it. "I'm confident" means nothing — confidence is not evidence. "I tested earlier" means test again if code changed since. "It's a trivial change" means verify anyway
|
||||
- Reviewer runs against the owner's current workspace in read-only mode. Do not treat the inability to run direct local test/typecheck/build from the reviewer workspace as a product bug by itself
|
||||
- Reviewer runs against the owner's current workspace in read-only mode. Do not treat the inability to run direct local test/typecheck/build/lint from the reviewer workspace as a product bug by itself
|
||||
- Treat `EJCLAW_WORK_DIR` as the canonical verification root for this turn. You may inspect other local paths for context, but final review findings must be re-checked against `EJCLAW_WORK_DIR`
|
||||
- Do not use a different clone, canonical repo path, or cached session path as the sole basis for `BLOCKED`, `DONE_WITH_CONCERNS`, or change requests. If another path disagrees with `EJCLAW_WORK_DIR`, prefer `EJCLAW_WORK_DIR` and explicitly call out the mismatch
|
||||
- When test/typecheck/build evidence is needed, prefer the dedicated verification path (`run_verification`) over assuming the reviewer workspace should execute the full project locally
|
||||
- When test/typecheck/build/lint evidence is needed, prefer the dedicated verification path (`run_verification`) over assuming the reviewer workspace should execute the full project locally
|
||||
- Separate correctness issues from improvement ideas. If something is only a better alternative, label it as optional instead of blocking the owner unnecessarily
|
||||
- Stagnation: **Spinning** (same error 3+), **Oscillation** (alternating approaches), **Diminishing returns** (shrinking improvement), **No progress** (discussion without change) — name the pattern and report: **Status**, **Attempted**, **Recommendation**
|
||||
- Implementation, commits, and pushes require agreement from both sides. Either can veto
|
||||
|
||||
@@ -18,16 +18,19 @@ Challenge the reviewer's reasoning. Point out logical gaps, over-engineering, sc
|
||||
|
||||
## Completion status
|
||||
|
||||
**Start your first line** with one of these four statuses. This is required.
|
||||
**Start your first line** with one of these six statuses. This is required.
|
||||
|
||||
- **DONE** — All steps completed. Include the evidence (test output, build log, diff)
|
||||
- **STEP_DONE** — A meaningful intermediate step is complete, but the original task still has remaining work. This keeps the task active and continues the owner flow without reviewer or arbiter intervention
|
||||
- **TASK_DONE** — The original requested task is complete. Include the evidence (test output, build log, diff)
|
||||
- **DONE** — Legacy alias for **TASK_DONE**. Prefer **TASK_DONE** for new turns
|
||||
- **DONE_WITH_CONCERNS** — Completed, but there are issues worth flagging. If the reviewer raises the same concerns again, fix them or escalate to BLOCKED
|
||||
- **BLOCKED** — Cannot proceed. State what is stopping you
|
||||
- **NEEDS_CONTEXT** — Missing information needed to continue
|
||||
|
||||
### Finalize semantics
|
||||
|
||||
- When the reviewer already approved and you are finalizing, **DONE** closes the paired turn
|
||||
- When the reviewer already approved and you are finalizing, **TASK_DONE** closes the paired turn
|
||||
- In that same finalize step, **STEP_DONE** keeps the task active and resumes the owner flow because the original request still has remaining work
|
||||
- In that same finalize step, **DONE_WITH_CONCERNS** does not close the turn — it intentionally reopens review
|
||||
- Use **DONE_WITH_CONCERNS** on finalize only when you are explicitly asking the reviewer loop to resume
|
||||
|
||||
|
||||
@@ -45,7 +45,7 @@ function platformCandidates(
|
||||
return [
|
||||
{
|
||||
pkg: `@anthropic-ai/claude-agent-sdk-darwin-${arch}`,
|
||||
file: arch === 'arm64' ? 'cli' : 'cli',
|
||||
file: 'claude',
|
||||
},
|
||||
];
|
||||
}
|
||||
@@ -53,7 +53,7 @@ function platformCandidates(
|
||||
return [
|
||||
{
|
||||
pkg: `@anthropic-ai/claude-agent-sdk-win32-${arch}`,
|
||||
file: 'cli.exe',
|
||||
file: 'claude.exe',
|
||||
},
|
||||
];
|
||||
}
|
||||
@@ -127,12 +127,22 @@ export function resolveBundledClaudeCodeExecutable(options?: {
|
||||
* Default package directory resolver. Uses `require.resolve` against this
|
||||
* module's location so it works regardless of whether agent-runner is invoked
|
||||
* from its own node_modules layout or via a parent workspace.
|
||||
*
|
||||
* Important: the SDK's optional native-binary packages may not expose a bare
|
||||
* package entrypoint, so `require.resolve(pkg)` can fail even when the package
|
||||
* and binary are installed. Resolve `package.json` first, then fall back to the
|
||||
* bare package only for package layouts that do expose an entrypoint.
|
||||
*/
|
||||
function defaultResolvePackageDir(pkg: string): string | null {
|
||||
const req = createRequire(import.meta.url);
|
||||
try {
|
||||
const pkgJson = req.resolve(`${pkg}/package.json`);
|
||||
return path.dirname(pkgJson);
|
||||
} catch {
|
||||
// Fall through to legacy/bare-entrypoint resolution below.
|
||||
}
|
||||
|
||||
try {
|
||||
// Resolve the package entrypoint instead of package.json because the SDK's
|
||||
// exports map does not expose `./package.json`.
|
||||
const req = createRequire(import.meta.url);
|
||||
const entrypoint = req.resolve(pkg);
|
||||
return path.dirname(entrypoint);
|
||||
} catch {
|
||||
@@ -142,5 +152,6 @@ function defaultResolvePackageDir(pkg: string): string | null {
|
||||
|
||||
export const __test__ = {
|
||||
platformCandidates,
|
||||
defaultResolvePackageDir,
|
||||
ENV_OVERRIDE,
|
||||
};
|
||||
|
||||
@@ -479,12 +479,12 @@ server.tool(
|
||||
|
||||
server.tool(
|
||||
'run_verification',
|
||||
'Run a fixed verification profile directly on the host against the current repo snapshot using a restricted environment and snapshot checks. Use this instead of broad shell write access for test/typecheck/build verification.',
|
||||
'Run a fixed verification profile directly on the host against the current repo snapshot using a restricted environment and snapshot checks. Use this instead of broad shell write access for test/typecheck/build/lint verification.',
|
||||
{
|
||||
profile: z
|
||||
.enum(VERIFICATION_PROFILES)
|
||||
.describe(
|
||||
'Fixed verification profile. Runs the workspace test/typecheck/build scripts with the repo-configured package manager.',
|
||||
'Fixed verification profile. Runs the workspace test/typecheck/build/lint scripts with the repo-configured package manager.',
|
||||
),
|
||||
},
|
||||
async (args) => {
|
||||
|
||||
@@ -3,7 +3,12 @@ import path from 'path';
|
||||
import { pathToFileURL } from 'url';
|
||||
export { computeVerificationSnapshotId } from '../../../shared/verification-snapshot.js';
|
||||
|
||||
export const VERIFICATION_PROFILES = ['test', 'typecheck', 'build'] as const;
|
||||
export const VERIFICATION_PROFILES = [
|
||||
'test',
|
||||
'typecheck',
|
||||
'build',
|
||||
'lint',
|
||||
] as const;
|
||||
|
||||
export type VerificationProfile = (typeof VERIFICATION_PROFILES)[number];
|
||||
|
||||
|
||||
@@ -1,8 +1,12 @@
|
||||
import fs from 'fs';
|
||||
import { createRequire } from 'module';
|
||||
import path from 'path';
|
||||
import { afterEach, describe, expect, it } from 'vitest';
|
||||
|
||||
import { resolveBundledClaudeCodeExecutable } from '../src/bundled-cli-path.js';
|
||||
import {
|
||||
__test__,
|
||||
resolveBundledClaudeCodeExecutable,
|
||||
} from '../src/bundled-cli-path.js';
|
||||
|
||||
describe('resolveBundledClaudeCodeExecutable', () => {
|
||||
const origEnv = process.env.EJCLAW_CLAUDE_CLI_PATH;
|
||||
@@ -83,10 +87,10 @@ describe('resolveBundledClaudeCodeExecutable', () => {
|
||||
expect(result).toBe(path.join(muslDir, 'claude'));
|
||||
});
|
||||
|
||||
it('resolves Darwin arm64 binary under `cli`', () => {
|
||||
it('resolves Darwin arm64 binary under `claude`', () => {
|
||||
const dir =
|
||||
'/fake/node_modules/@anthropic-ai/claude-agent-sdk-darwin-arm64';
|
||||
const existsSync = (p: string): boolean => p === path.join(dir, 'cli');
|
||||
const existsSync = (p: string): boolean => p === path.join(dir, 'claude');
|
||||
const result = resolveBundledClaudeCodeExecutable({
|
||||
env: {},
|
||||
existsSync,
|
||||
@@ -95,13 +99,13 @@ describe('resolveBundledClaudeCodeExecutable', () => {
|
||||
resolvePackageDir: (pkg) =>
|
||||
pkg === '@anthropic-ai/claude-agent-sdk-darwin-arm64' ? dir : null,
|
||||
});
|
||||
expect(result).toBe(path.join(dir, 'cli'));
|
||||
expect(result).toBe(path.join(dir, 'claude'));
|
||||
});
|
||||
|
||||
it('resolves Windows binary under `cli.exe`', () => {
|
||||
it('resolves Windows binary under `claude.exe`', () => {
|
||||
const dir =
|
||||
'C:\\\\fake\\\\node_modules\\\\@anthropic-ai\\\\claude-agent-sdk-win32-x64';
|
||||
const binary = path.join(dir, 'cli.exe');
|
||||
const binary = path.join(dir, 'claude.exe');
|
||||
const existsSync = (p: string): boolean => p === binary;
|
||||
const result = resolveBundledClaudeCodeExecutable({
|
||||
env: {},
|
||||
@@ -114,6 +118,17 @@ describe('resolveBundledClaudeCodeExecutable', () => {
|
||||
expect(result).toBe(binary);
|
||||
});
|
||||
|
||||
it('default resolver handles SDK optional packages without a bare entrypoint', () => {
|
||||
if (process.platform !== 'linux' || process.arch !== 'x64') return;
|
||||
|
||||
const dir = __test__.defaultResolvePackageDir(
|
||||
'@anthropic-ai/claude-agent-sdk-linux-x64',
|
||||
);
|
||||
if (!dir) return;
|
||||
|
||||
expect(fs.existsSync(path.join(dir, 'claude'))).toBe(true);
|
||||
});
|
||||
|
||||
it('throws a descriptive error when no binary is found', () => {
|
||||
const existsSync = () => false;
|
||||
expect(() =>
|
||||
@@ -127,6 +142,34 @@ describe('resolveBundledClaudeCodeExecutable', () => {
|
||||
).toThrow(/Unable to locate a bundled Claude Code CLI binary/);
|
||||
});
|
||||
|
||||
it('resolves the linux x64 optional package even when it has no JS entrypoint', () => {
|
||||
if (process.platform !== 'linux' || process.arch !== 'x64') {
|
||||
return;
|
||||
}
|
||||
|
||||
const req = createRequire(import.meta.url);
|
||||
let packageJsonPath: string;
|
||||
try {
|
||||
packageJsonPath = req.resolve(
|
||||
'@anthropic-ai/claude-agent-sdk-linux-x64/package.json',
|
||||
);
|
||||
} catch {
|
||||
return;
|
||||
}
|
||||
|
||||
try {
|
||||
req.resolve('@anthropic-ai/claude-agent-sdk-linux-x64');
|
||||
return;
|
||||
} catch {
|
||||
// This is the regression case: package.json resolves, bare package does not.
|
||||
}
|
||||
|
||||
const result = resolveBundledClaudeCodeExecutable({ env: {} });
|
||||
|
||||
expect(result).toBe(path.join(path.dirname(packageJsonPath), 'claude'));
|
||||
expect(fs.existsSync(result)).toBe(true);
|
||||
});
|
||||
|
||||
it('end-to-end: resolves the actual bundled binary on this host when installed', () => {
|
||||
// Real-world smoke test. Some CI/host sandboxes do not install the optional
|
||||
// per-platform SDK package, so treat "package not resolvable" as a host
|
||||
|
||||
@@ -136,7 +136,7 @@ describe('claude reviewer runtime guard', () => {
|
||||
['/repo/work'],
|
||||
'linux',
|
||||
'best-effort',
|
||||
);
|
||||
) as unknown as { failIfUnavailable?: boolean };
|
||||
|
||||
expect(sandbox.failIfUnavailable).toBe(false);
|
||||
});
|
||||
@@ -150,7 +150,7 @@ describe('claude reviewer runtime guard', () => {
|
||||
['/repo/work'],
|
||||
'darwin',
|
||||
'best-effort',
|
||||
);
|
||||
) as unknown as { failIfUnavailable?: boolean };
|
||||
|
||||
expect(sandbox.failIfUnavailable).toBe(false);
|
||||
});
|
||||
@@ -169,6 +169,9 @@ describe('claude reviewer runtime guard', () => {
|
||||
const env = buildReviewerGitGuardEnv({ PATH: process.env.PATH }, true);
|
||||
expect(env.PATH).toContain('ejclaw-reviewer-git-');
|
||||
expect(env.EJCLAW_REAL_GIT).toBeTruthy();
|
||||
expect(env.GIT_CONFIG_GLOBAL).toContain('global.gitconfig');
|
||||
expect(env.GIT_CONFIG_NOSYSTEM).toBe('1');
|
||||
expect(fs.existsSync(env.GIT_CONFIG_GLOBAL!)).toBe(true);
|
||||
});
|
||||
|
||||
it('prefers an executable HOME-scoped wrapper dir before tmp', () => {
|
||||
@@ -245,6 +248,30 @@ describe('claude reviewer runtime guard', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('overrides problematic git config paths so read-only git queries still work', () => {
|
||||
const cwd = createTempRepo('ejclaw-reviewer-readonly-git-');
|
||||
const env = buildReviewerGitGuardEnv(
|
||||
{
|
||||
PATH: process.env.PATH,
|
||||
HOME: cwd,
|
||||
EJCLAW_WORK_DIR: cwd,
|
||||
GIT_CONFIG_GLOBAL: path.join(cwd, '.gitconfig'),
|
||||
},
|
||||
true,
|
||||
);
|
||||
|
||||
expect(env.GIT_CONFIG_GLOBAL).not.toBe(path.join(cwd, '.gitconfig'));
|
||||
expect(() =>
|
||||
execFileSync('git', ['log', '--oneline', '-1'], {
|
||||
cwd,
|
||||
env,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
}),
|
||||
).not.toThrow();
|
||||
expect(fs.existsSync(path.join(cwd, '.gitconfig'))).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts a mounted local origin path that resolves as a git repo', () => {
|
||||
const originDir = createTempRepo('ejclaw-reviewer-origin-');
|
||||
const cwd = createTempRepo('ejclaw-reviewer-workspace-');
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
"": {
|
||||
"name": "ejclaw-codex-runner",
|
||||
"dependencies": {
|
||||
"@openai/codex": "^0.120.0",
|
||||
"@openai/codex": "^0.124.0",
|
||||
"ejclaw-runners-shared": "file:../shared",
|
||||
},
|
||||
"devDependencies": {
|
||||
@@ -15,19 +15,19 @@
|
||||
},
|
||||
},
|
||||
"packages": {
|
||||
"@openai/codex": ["@openai/codex@0.120.0", "", { "optionalDependencies": { "@openai/codex-darwin-arm64": "npm:@openai/codex@0.120.0-darwin-arm64", "@openai/codex-darwin-x64": "npm:@openai/codex@0.120.0-darwin-x64", "@openai/codex-linux-arm64": "npm:@openai/codex@0.120.0-linux-arm64", "@openai/codex-linux-x64": "npm:@openai/codex@0.120.0-linux-x64", "@openai/codex-win32-arm64": "npm:@openai/codex@0.120.0-win32-arm64", "@openai/codex-win32-x64": "npm:@openai/codex@0.120.0-win32-x64" }, "bin": { "codex": "bin/codex.js" } }, "sha512-e2P1Gya3dwsRe9IPOiswVz5JfR700u+/sWCqDc3jkqv2QViPkNiBmZoGhFnZL5jBpKakSjehC4/Fpspg70nHTw=="],
|
||||
"@openai/codex": ["@openai/codex@0.124.0", "", { "optionalDependencies": { "@openai/codex-darwin-arm64": "npm:@openai/codex@0.124.0-darwin-arm64", "@openai/codex-darwin-x64": "npm:@openai/codex@0.124.0-darwin-x64", "@openai/codex-linux-arm64": "npm:@openai/codex@0.124.0-linux-arm64", "@openai/codex-linux-x64": "npm:@openai/codex@0.124.0-linux-x64", "@openai/codex-win32-arm64": "npm:@openai/codex@0.124.0-win32-arm64", "@openai/codex-win32-x64": "npm:@openai/codex@0.124.0-win32-x64" }, "bin": { "codex": "bin/codex.js" } }, "sha512-1EVAuPyAQZ8zIVMw3bPJ6a4R8ifLAZ7LGsOyknj5c2he9AFXVRCmWx12WrdZJ25wcBvOEKt1n1Zx+QAj0EVGbQ=="],
|
||||
|
||||
"@openai/codex-darwin-arm64": ["@openai/codex@0.120.0-darwin-arm64", "", { "os": "darwin", "cpu": "arm64" }, "sha512-7CU+I5kBaMuoqfG3xisq0mUWzxoEHvfu34cB8a0KpBiIhAgu12fKpmYgZ4/DvRP6Wm9Fu6LJYKVF5apUHFp8nQ=="],
|
||||
"@openai/codex-darwin-arm64": ["@openai/codex@0.124.0-darwin-arm64", "", { "os": "darwin", "cpu": "arm64" }, "sha512-lnuqeAdl+RjPWsqVZ8rrPskRIOZmzlyr8e5q/wFVEnMPsm9dWgjRW1PKa84UmDSQVOuz9GObF28DEHFyC4A8NA=="],
|
||||
|
||||
"@openai/codex-darwin-x64": ["@openai/codex@0.120.0-darwin-x64", "", { "os": "darwin", "cpu": "x64" }, "sha512-d7joNYuwrmd5iIdp/xAE5f8bZT1r82MnmU6Hzgxq3G+xClwEyhxU737ZWnstFSpnZNfxJ5zXCuFUJh4CAkHNtQ=="],
|
||||
"@openai/codex-darwin-x64": ["@openai/codex@0.124.0-darwin-x64", "", { "os": "darwin", "cpu": "x64" }, "sha512-Br+VlL83IOu96Urw+mkZ1PQXLsQXGAYEH6kXNt4ULuVt7qAdQY2FKYwIgLLVLl0vpMk8qWxdfdVMqhiu2uCHlg=="],
|
||||
|
||||
"@openai/codex-linux-arm64": ["@openai/codex@0.120.0-linux-arm64", "", { "os": "linux", "cpu": "arm64" }, "sha512-sVYY25/URlpZPtb0Q0ryLh+lcq9UTEtHAkdZKa0a/R7mAdyPuhpU9V6jWmxwiUh7s53XZOEVFoKmLfH8YIDWCQ=="],
|
||||
"@openai/codex-linux-arm64": ["@openai/codex@0.124.0-linux-arm64", "", { "os": "linux", "cpu": "arm64" }, "sha512-QXafFVQJxPfU1LSCI5afuHsF5evKAcbQpFuKou0Kl241/HG/zYHdwoWj546zH844gJgegIPAxPf36+RSkUsbbA=="],
|
||||
|
||||
"@openai/codex-linux-x64": ["@openai/codex@0.120.0-linux-x64", "", { "os": "linux", "cpu": "x64" }, "sha512-VcP9B/c/O+EFEgqoetCzvHrLfAdo8vrt09Gx1lJ8ikewctqAuJ/ozj/6wuvlz7XaaK64ib5cge01pOAeCyt2Sg=="],
|
||||
"@openai/codex-linux-x64": ["@openai/codex@0.124.0-linux-x64", "", { "os": "linux", "cpu": "x64" }, "sha512-cJ4QfQIrz2gkXVWephiGo9JDyD3qLKhvkTTLk7gI8rKd7MZpVXn9/1e7pZCQnJVA7Dk6ocA5G+sxnR78SoIejw=="],
|
||||
|
||||
"@openai/codex-win32-arm64": ["@openai/codex@0.120.0-win32-arm64", "", { "os": "win32", "cpu": "arm64" }, "sha512-SAaTQU1XHa1qDnmQldmbyROIY5SiaspF+Cw3ziWeeTgyAET3rWusm4ELOElx6QiY1ugYW5ZD+7AFufS2z1xtpQ=="],
|
||||
"@openai/codex-win32-arm64": ["@openai/codex@0.124.0-win32-arm64", "", { "os": "win32", "cpu": "arm64" }, "sha512-oDSI/CQh0kagBwWVPG9EiUBfHiY27ju3LPrjTVZg4VMpVJVNA31JTK8rHMZ6G/2gfNmOl6DMBA6+0ICxSkkshg=="],
|
||||
|
||||
"@openai/codex-win32-x64": ["@openai/codex@0.120.0-win32-x64", "", { "os": "win32", "cpu": "x64" }, "sha512-zja1GNrbHyOUTvOy5FVMa+rAYIs3m+FOS8rAXftxMEhodMmkMw2O8zcvso657SHhZR0hIEiZ6T70lcyH2YX0mQ=="],
|
||||
"@openai/codex-win32-x64": ["@openai/codex@0.124.0-win32-x64", "", { "os": "win32", "cpu": "x64" }, "sha512-t+lAwmCWwIWQKk6dKaYetRhQsmA+uDmQy1NLka1xAAv3PlNOH8iNz9Lsisr3qYkBR8Tf7tbQlt+pl9tw/A5mfA=="],
|
||||
|
||||
"@types/node": ["@types/node@22.19.15", "", { "dependencies": { "undici-types": "~6.21.0" } }, "sha512-F0R/h2+dsy5wJAUe3tAU6oqa2qbWY5TpNfL/RGmo1y38hiyO1w3x2jPtt76wmuaJI4DQnOBu21cNXQ2STIUUWg=="],
|
||||
|
||||
|
||||
@@ -11,7 +11,7 @@
|
||||
},
|
||||
"dependencies": {
|
||||
"ejclaw-runners-shared": "file:../shared",
|
||||
"@openai/codex": "^0.120.0"
|
||||
"@openai/codex": "^0.124.0"
|
||||
},
|
||||
"devDependencies": {
|
||||
"@types/node": "^22.10.7",
|
||||
|
||||
@@ -75,6 +75,9 @@ describe('codex reviewer runtime guard', () => {
|
||||
const env = buildReviewerGitGuardEnv({ PATH: process.env.PATH }, true);
|
||||
expect(env.PATH).toContain('ejclaw-reviewer-git-');
|
||||
expect(env.EJCLAW_REAL_GIT).toBeTruthy();
|
||||
expect(env.GIT_CONFIG_GLOBAL).toContain('global.gitconfig');
|
||||
expect(env.GIT_CONFIG_NOSYSTEM).toBe('1');
|
||||
expect(fs.existsSync(env.GIT_CONFIG_GLOBAL!)).toBe(true);
|
||||
});
|
||||
|
||||
it('prefers an executable HOME-scoped wrapper dir before tmp', () => {
|
||||
@@ -151,6 +154,30 @@ describe('codex reviewer runtime guard', () => {
|
||||
}
|
||||
});
|
||||
|
||||
it('overrides problematic git config paths so read-only git queries still work', () => {
|
||||
const cwd = createTempRepo('ejclaw-reviewer-readonly-git-');
|
||||
const env = buildReviewerGitGuardEnv(
|
||||
{
|
||||
PATH: process.env.PATH,
|
||||
HOME: cwd,
|
||||
EJCLAW_WORK_DIR: cwd,
|
||||
GIT_CONFIG_GLOBAL: path.join(cwd, '.gitconfig'),
|
||||
},
|
||||
true,
|
||||
);
|
||||
|
||||
expect(env.GIT_CONFIG_GLOBAL).not.toBe(path.join(cwd, '.gitconfig'));
|
||||
expect(() =>
|
||||
execFileSync('git', ['log', '--oneline', '-1'], {
|
||||
cwd,
|
||||
env,
|
||||
encoding: 'utf-8',
|
||||
stdio: ['ignore', 'pipe', 'pipe'],
|
||||
}),
|
||||
).not.toThrow();
|
||||
expect(fs.existsSync(path.join(cwd, '.gitconfig'))).toBe(false);
|
||||
});
|
||||
|
||||
it('accepts a mounted local origin path that resolves as a git repo', () => {
|
||||
const originDir = createTempRepo('ejclaw-reviewer-origin-');
|
||||
const cwd = createTempRepo('ejclaw-reviewer-workspace-');
|
||||
|
||||
@@ -78,6 +78,18 @@ function createExecutableWrapperDir(baseEnv: NodeJS.ProcessEnv): string {
|
||||
);
|
||||
}
|
||||
|
||||
function prepareGitConfigSandbox(wrapperDir: string): {
|
||||
gitConfigGlobalPath: string;
|
||||
} {
|
||||
const configDir = path.join(wrapperDir, 'git-config');
|
||||
fs.mkdirSync(configDir, { recursive: true });
|
||||
const gitConfigGlobalPath = path.join(configDir, 'global.gitconfig');
|
||||
if (!fs.existsSync(gitConfigGlobalPath)) {
|
||||
fs.writeFileSync(gitConfigGlobalPath, '');
|
||||
}
|
||||
return { gitConfigGlobalPath };
|
||||
}
|
||||
|
||||
export function buildReviewerGitGuardEnv(
|
||||
baseEnv: NodeJS.ProcessEnv,
|
||||
reviewerRuntime: boolean,
|
||||
@@ -90,6 +102,7 @@ export function buildReviewerGitGuardEnv(
|
||||
const protectedWorkDir = baseEnv.EJCLAW_WORK_DIR || '';
|
||||
const wrapperDir = createExecutableWrapperDir(baseEnv);
|
||||
const wrapperPath = path.join(wrapperDir, 'git');
|
||||
const { gitConfigGlobalPath } = prepareGitConfigSandbox(wrapperDir);
|
||||
const blocked = [...BLOCKED_GIT_SUBCOMMANDS]
|
||||
.map((value) => `'${value}'`)
|
||||
.join(' ');
|
||||
@@ -179,6 +192,8 @@ exec "$real_git" "$@"
|
||||
...baseEnv,
|
||||
EJCLAW_REAL_GIT: realGitPath,
|
||||
EJCLAW_PROTECTED_WORK_DIR: protectedWorkDir,
|
||||
GIT_CONFIG_GLOBAL: gitConfigGlobalPath,
|
||||
GIT_CONFIG_NOSYSTEM: '1',
|
||||
PATH: `${wrapperDir}:${baseEnv.PATH || ''}`,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1242,10 +1242,53 @@ describe('paired task state', () => {
|
||||
expect(outputs.map((output) => output.turn_number)).toEqual([1, 2]);
|
||||
expect(outputs[0].role).toBe('owner');
|
||||
expect(outputs[0].output_text).toHaveLength(50_000);
|
||||
expect(outputs[0].verdict).toBe('continue');
|
||||
expect(outputs[1].output_text).toBe('review turn');
|
||||
expect(outputs[1].verdict).toBe('continue');
|
||||
expect(getLatestTurnNumber('paired-task-turn-output')).toBe(2);
|
||||
});
|
||||
|
||||
it('stores the parsed visible verdict with paired turn outputs', () => {
|
||||
createPairedTask({
|
||||
id: 'paired-task-turn-output-verdict',
|
||||
chat_jid: 'dc:paired',
|
||||
group_folder: 'paired-room',
|
||||
owner_service_id: 'codex-main',
|
||||
reviewer_service_id: 'codex-review',
|
||||
title: null,
|
||||
source_ref: null,
|
||||
plan_notes: null,
|
||||
round_trip_count: 0,
|
||||
review_requested_at: null,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
created_at: '2026-03-28T00:00:00.000Z',
|
||||
updated_at: '2026-03-28T00:00:00.000Z',
|
||||
});
|
||||
|
||||
insertPairedTurnOutput(
|
||||
'paired-task-turn-output-verdict',
|
||||
1,
|
||||
'owner',
|
||||
'STEP_DONE\n1단계 완료',
|
||||
);
|
||||
insertPairedTurnOutput(
|
||||
'paired-task-turn-output-verdict',
|
||||
2,
|
||||
'owner',
|
||||
'TASK_DONE\n요청 범위 전체 완료',
|
||||
);
|
||||
|
||||
const outputs = getPairedTurnOutputs('paired-task-turn-output-verdict');
|
||||
|
||||
expect(outputs.map((output) => output.verdict)).toEqual([
|
||||
'step_done',
|
||||
'task_done',
|
||||
]);
|
||||
});
|
||||
|
||||
it('preserves explicit created_at when inserting a paired turn output', () => {
|
||||
createPairedTask({
|
||||
id: 'paired-task-turn-output-created-at',
|
||||
|
||||
@@ -119,6 +119,10 @@ export function applyBaseSchema(database: Database): void {
|
||||
review_requested_at TEXT,
|
||||
round_trip_count INTEGER NOT NULL DEFAULT 0,
|
||||
owner_failure_count INTEGER NOT NULL DEFAULT 0,
|
||||
owner_step_done_streak INTEGER NOT NULL DEFAULT 0,
|
||||
finalize_step_done_count INTEGER NOT NULL DEFAULT 0,
|
||||
task_done_then_user_reopen_count INTEGER NOT NULL DEFAULT 0,
|
||||
empty_step_done_streak INTEGER NOT NULL DEFAULT 0,
|
||||
status TEXT NOT NULL DEFAULT 'active',
|
||||
arbiter_verdict TEXT,
|
||||
arbiter_requested_at TEXT,
|
||||
@@ -154,6 +158,7 @@ export function applyBaseSchema(database: Database): void {
|
||||
turn_number INTEGER NOT NULL,
|
||||
role TEXT NOT NULL,
|
||||
output_text TEXT NOT NULL,
|
||||
verdict TEXT,
|
||||
created_at TEXT NOT NULL,
|
||||
UNIQUE(task_id, turn_number, role)
|
||||
);
|
||||
|
||||
@@ -36,6 +36,7 @@ function getExpectedSchemaMigrations(): Array<{
|
||||
{ version: 9, name: 'paired_workspace_project_schema_cleanup' },
|
||||
{ version: 10, name: 'paired_turn_provenance_upgrade' },
|
||||
{ version: 11, name: 'owner_failure_count' },
|
||||
{ version: 12, name: 'paired_verdict_and_step_telemetry' },
|
||||
];
|
||||
}
|
||||
|
||||
|
||||
75
src/db/migrations/012_paired-verdict-and-step-telemetry.ts
Normal file
75
src/db/migrations/012_paired-verdict-and-step-telemetry.ts
Normal file
@@ -0,0 +1,75 @@
|
||||
import type { Database } from 'bun:sqlite';
|
||||
|
||||
import { tableHasColumn } from './helpers.js';
|
||||
import type { SchemaMigrationDefinition } from './types.js';
|
||||
|
||||
export const PAIRED_VERDICT_AND_STEP_TELEMETRY_MIGRATION: SchemaMigrationDefinition =
|
||||
{
|
||||
version: 12,
|
||||
name: 'paired_verdict_and_step_telemetry',
|
||||
apply(database: Database) {
|
||||
if (!tableHasColumn(database, 'paired_turn_outputs', 'verdict')) {
|
||||
database.exec(`
|
||||
ALTER TABLE paired_turn_outputs
|
||||
ADD COLUMN verdict TEXT
|
||||
`);
|
||||
}
|
||||
|
||||
if (!tableHasColumn(database, 'paired_tasks', 'owner_step_done_streak')) {
|
||||
database.exec(`
|
||||
ALTER TABLE paired_tasks
|
||||
ADD COLUMN owner_step_done_streak INTEGER NOT NULL DEFAULT 0
|
||||
`);
|
||||
}
|
||||
|
||||
if (
|
||||
!tableHasColumn(database, 'paired_tasks', 'finalize_step_done_count')
|
||||
) {
|
||||
database.exec(`
|
||||
ALTER TABLE paired_tasks
|
||||
ADD COLUMN finalize_step_done_count INTEGER NOT NULL DEFAULT 0
|
||||
`);
|
||||
}
|
||||
|
||||
if (
|
||||
!tableHasColumn(
|
||||
database,
|
||||
'paired_tasks',
|
||||
'task_done_then_user_reopen_count',
|
||||
)
|
||||
) {
|
||||
database.exec(`
|
||||
ALTER TABLE paired_tasks
|
||||
ADD COLUMN task_done_then_user_reopen_count INTEGER NOT NULL DEFAULT 0
|
||||
`);
|
||||
}
|
||||
|
||||
if (!tableHasColumn(database, 'paired_tasks', 'empty_step_done_streak')) {
|
||||
database.exec(`
|
||||
ALTER TABLE paired_tasks
|
||||
ADD COLUMN empty_step_done_streak INTEGER NOT NULL DEFAULT 0
|
||||
`);
|
||||
}
|
||||
|
||||
database.exec(`
|
||||
UPDATE paired_tasks
|
||||
SET owner_step_done_streak = 0
|
||||
WHERE owner_step_done_streak IS NULL
|
||||
`);
|
||||
database.exec(`
|
||||
UPDATE paired_tasks
|
||||
SET finalize_step_done_count = 0
|
||||
WHERE finalize_step_done_count IS NULL
|
||||
`);
|
||||
database.exec(`
|
||||
UPDATE paired_tasks
|
||||
SET task_done_then_user_reopen_count = 0
|
||||
WHERE task_done_then_user_reopen_count IS NULL
|
||||
`);
|
||||
database.exec(`
|
||||
UPDATE paired_tasks
|
||||
SET empty_step_done_streak = 0
|
||||
WHERE empty_step_done_streak IS NULL
|
||||
`);
|
||||
},
|
||||
};
|
||||
@@ -11,6 +11,7 @@ import { PAIRED_TASK_SCHEMA_CLEANUP_MIGRATION } from './008_paired-task-schema-c
|
||||
import { PAIRED_WORKSPACE_PROJECT_SCHEMA_CLEANUP_MIGRATION } from './009_paired-workspace-project-schema-cleanup.js';
|
||||
import { PAIRED_TURN_PROVENANCE_UPGRADE_MIGRATION } from './010_paired-turn-provenance-upgrade.js';
|
||||
import { OWNER_FAILURE_COUNT_MIGRATION } from './011_owner-failure-count.js';
|
||||
import { PAIRED_VERDICT_AND_STEP_TELEMETRY_MIGRATION } from './012_paired-verdict-and-step-telemetry.js';
|
||||
import type {
|
||||
SchemaMigrationArgs,
|
||||
SchemaMigrationDefinition,
|
||||
@@ -30,6 +31,7 @@ const ORDERED_SCHEMA_MIGRATIONS: readonly SchemaMigrationDefinition[] = [
|
||||
PAIRED_WORKSPACE_PROJECT_SCHEMA_CLEANUP_MIGRATION,
|
||||
PAIRED_TURN_PROVENANCE_UPGRADE_MIGRATION,
|
||||
OWNER_FAILURE_COUNT_MIGRATION,
|
||||
PAIRED_VERDICT_AND_STEP_TELEMETRY_MIGRATION,
|
||||
];
|
||||
|
||||
function ensureSchemaMigrationsTable(database: Database): void {
|
||||
|
||||
@@ -51,6 +51,10 @@ export type PairedTaskUpdates = Partial<
|
||||
| 'review_requested_at'
|
||||
| 'round_trip_count'
|
||||
| 'owner_failure_count'
|
||||
| 'owner_step_done_streak'
|
||||
| 'finalize_step_done_count'
|
||||
| 'task_done_then_user_reopen_count'
|
||||
| 'empty_step_done_streak'
|
||||
| 'status'
|
||||
| 'arbiter_verdict'
|
||||
| 'arbiter_requested_at'
|
||||
@@ -173,6 +177,10 @@ export function createPairedTaskInDatabase(
|
||||
review_requested_at,
|
||||
round_trip_count,
|
||||
owner_failure_count,
|
||||
owner_step_done_streak,
|
||||
finalize_step_done_count,
|
||||
task_done_then_user_reopen_count,
|
||||
empty_step_done_streak,
|
||||
status,
|
||||
arbiter_verdict,
|
||||
arbiter_requested_at,
|
||||
@@ -180,7 +188,7 @@ export function createPairedTaskInDatabase(
|
||||
created_at,
|
||||
updated_at
|
||||
)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
`,
|
||||
)
|
||||
.run(
|
||||
@@ -198,6 +206,10 @@ export function createPairedTaskInDatabase(
|
||||
task.review_requested_at,
|
||||
task.round_trip_count,
|
||||
task.owner_failure_count ?? 0,
|
||||
task.owner_step_done_streak ?? 0,
|
||||
task.finalize_step_done_count ?? 0,
|
||||
task.task_done_then_user_reopen_count ?? 0,
|
||||
task.empty_step_done_streak ?? 0,
|
||||
task.status,
|
||||
task.arbiter_verdict,
|
||||
task.arbiter_requested_at,
|
||||
@@ -306,6 +318,22 @@ export function updatePairedTaskInDatabase(
|
||||
fields.push('owner_failure_count = ?');
|
||||
values.push(updates.owner_failure_count);
|
||||
}
|
||||
if (updates.owner_step_done_streak !== undefined) {
|
||||
fields.push('owner_step_done_streak = ?');
|
||||
values.push(updates.owner_step_done_streak);
|
||||
}
|
||||
if (updates.finalize_step_done_count !== undefined) {
|
||||
fields.push('finalize_step_done_count = ?');
|
||||
values.push(updates.finalize_step_done_count);
|
||||
}
|
||||
if (updates.task_done_then_user_reopen_count !== undefined) {
|
||||
fields.push('task_done_then_user_reopen_count = ?');
|
||||
values.push(updates.task_done_then_user_reopen_count);
|
||||
}
|
||||
if (updates.empty_step_done_streak !== undefined) {
|
||||
fields.push('empty_step_done_streak = ?');
|
||||
values.push(updates.empty_step_done_streak);
|
||||
}
|
||||
if (updates.status !== undefined) {
|
||||
fields.push('status = ?');
|
||||
values.push(updates.status);
|
||||
@@ -369,6 +397,22 @@ export function updatePairedTaskIfUnchangedInDatabase(
|
||||
fields.push('owner_failure_count = ?');
|
||||
values.push(updates.owner_failure_count);
|
||||
}
|
||||
if (updates.owner_step_done_streak !== undefined) {
|
||||
fields.push('owner_step_done_streak = ?');
|
||||
values.push(updates.owner_step_done_streak);
|
||||
}
|
||||
if (updates.finalize_step_done_count !== undefined) {
|
||||
fields.push('finalize_step_done_count = ?');
|
||||
values.push(updates.finalize_step_done_count);
|
||||
}
|
||||
if (updates.task_done_then_user_reopen_count !== undefined) {
|
||||
fields.push('task_done_then_user_reopen_count = ?');
|
||||
values.push(updates.task_done_then_user_reopen_count);
|
||||
}
|
||||
if (updates.empty_step_done_streak !== undefined) {
|
||||
fields.push('empty_step_done_streak = ?');
|
||||
values.push(updates.empty_step_done_streak);
|
||||
}
|
||||
if (updates.status !== undefined) {
|
||||
fields.push('status = ?');
|
||||
values.push(updates.status);
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { Database } from 'bun:sqlite';
|
||||
|
||||
import { logger } from '../logger.js';
|
||||
import { parseVisibleVerdict } from '../paired-verdict.js';
|
||||
import { PairedRoomRole, PairedTurnOutput } from '../types.js';
|
||||
|
||||
const MAX_TURN_OUTPUT_CHARS = 50_000;
|
||||
@@ -29,14 +30,15 @@ export function insertPairedTurnOutputInDatabase(
|
||||
database
|
||||
.prepare(
|
||||
`INSERT OR REPLACE INTO paired_turn_outputs
|
||||
(task_id, turn_number, role, output_text, created_at)
|
||||
VALUES (?, ?, ?, ?, ?)`,
|
||||
(task_id, turn_number, role, output_text, verdict, created_at)
|
||||
VALUES (?, ?, ?, ?, ?, ?)`,
|
||||
)
|
||||
.run(
|
||||
taskId,
|
||||
turnNumber,
|
||||
role,
|
||||
outputText.slice(0, MAX_TURN_OUTPUT_CHARS),
|
||||
parseVisibleVerdict(outputText),
|
||||
createdAt ?? new Date().toISOString(),
|
||||
);
|
||||
}
|
||||
|
||||
@@ -14,6 +14,7 @@ import {
|
||||
completePairedExecutionContext,
|
||||
type PreparedPairedExecutionContext,
|
||||
} from './paired-execution-context.js';
|
||||
import { parseVisibleVerdict } from './paired-execution-context-shared.js';
|
||||
import { resolvePairedFollowUpQueueAction } from './message-agent-executor-rules.js';
|
||||
import { enqueuePairedFollowUpAfterEvent } from './message-runtime-follow-up.js';
|
||||
import type { PairedTurnIdentity } from './paired-turn-identity.js';
|
||||
@@ -438,6 +439,7 @@ export function createPairedExecutionLifecycle(args: {
|
||||
executionStatus: effectiveStatus,
|
||||
sawOutput: sawOutputForFollowUp,
|
||||
taskStatus: finishedTask?.status ?? null,
|
||||
outputSummary: pairedExecutionSummary,
|
||||
});
|
||||
if (queueAction !== 'pending' || !finishedTask) {
|
||||
return;
|
||||
@@ -452,6 +454,10 @@ export function createPairedExecutionLifecycle(args: {
|
||||
executionStatus: effectiveStatus,
|
||||
sawOutput: sawOutputForFollowUp,
|
||||
fallbackLastTurnOutputRole: sawOutputForFollowUp ? completedRole : null,
|
||||
fallbackLastTurnOutputVerdict:
|
||||
sawOutputForFollowUp && pairedExecutionSummary
|
||||
? parseVisibleVerdict(pairedExecutionSummary)
|
||||
: null,
|
||||
enqueueMessageCheck,
|
||||
});
|
||||
if (followUpResult.kind !== 'paired-follow-up') {
|
||||
|
||||
@@ -216,6 +216,18 @@ describe('message-agent-executor-rules', () => {
|
||||
).toBe('none');
|
||||
});
|
||||
|
||||
it('does not request a duplicate executor-side follow-up after successful owner STEP_DONE output', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
completedRole: 'owner',
|
||||
executionStatus: 'succeeded',
|
||||
sawOutput: true,
|
||||
taskStatus: 'active',
|
||||
outputSummary: 'STEP_DONE\nkeep going',
|
||||
}),
|
||||
).toBe('none');
|
||||
});
|
||||
|
||||
it('returns none after successful output when no next-turn action is needed', () => {
|
||||
expect(
|
||||
resolvePairedFollowUpQueueAction({
|
||||
|
||||
@@ -2,6 +2,7 @@ import {
|
||||
resolveFollowUpDispatch,
|
||||
resolveNextTurnAction,
|
||||
} from './message-runtime-rules.js';
|
||||
import { parseVisibleVerdict } from './paired-execution-context-shared.js';
|
||||
import type { PairedRoomRole, PairedTaskStatus } from './types.js';
|
||||
export {
|
||||
isRetryableClaudeSessionFailureAttempt,
|
||||
@@ -20,10 +21,15 @@ export function resolvePairedFollowUpQueueAction(args: {
|
||||
executionStatus: 'succeeded' | 'failed';
|
||||
sawOutput: boolean;
|
||||
taskStatus: PairedTaskStatus | null;
|
||||
outputSummary?: string | null;
|
||||
}): PairedFollowUpQueueAction {
|
||||
const nextTurnAction = resolveNextTurnAction({
|
||||
taskStatus: args.taskStatus,
|
||||
lastTurnOutputRole: args.sawOutput ? args.completedRole : null,
|
||||
lastTurnOutputVerdict:
|
||||
args.sawOutput && args.outputSummary
|
||||
? parseVisibleVerdict(args.outputSummary)
|
||||
: null,
|
||||
});
|
||||
const dispatch = resolveFollowUpDispatch({
|
||||
source: 'executor-recovery',
|
||||
|
||||
@@ -24,6 +24,7 @@ import {
|
||||
} from './message-runtime-rules.js';
|
||||
import type { ExecuteTurnFn } from './message-runtime-types.js';
|
||||
import { buildPairedTurnIdentity } from './paired-turn-identity.js';
|
||||
import { resolveStoredVisibleVerdict } from './paired-verdict.js';
|
||||
import {
|
||||
claimPairedTurnExecution,
|
||||
type ScheduledPairedFollowUpIntentKind,
|
||||
@@ -127,6 +128,10 @@ export function buildPendingPairedTurn(args: {
|
||||
const nextTurnAction = resolveNextTurnAction({
|
||||
taskStatus,
|
||||
lastTurnOutputRole: lastTurnOutput?.role ?? null,
|
||||
lastTurnOutputVerdict: resolveStoredVisibleVerdict({
|
||||
verdict: lastTurnOutput?.verdict ?? null,
|
||||
outputText: lastTurnOutput?.output_text ?? null,
|
||||
}),
|
||||
});
|
||||
const recentMessages = getRecentChatMessages(chatJid, 20);
|
||||
const lastHumanMessage = getLastHumanMessageContent(chatJid);
|
||||
|
||||
@@ -83,6 +83,33 @@ describe('message-runtime-follow-up', () => {
|
||||
expect(enqueue).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('does not use fallback owner STEP_DONE verdict to schedule owner follow-ups while active', () => {
|
||||
const enqueue = vi.fn();
|
||||
const result = dispatchPairedFollowUpForEvent({
|
||||
chatJid: 'group@test',
|
||||
runId: 'run-owner-step-done',
|
||||
task: {
|
||||
id: 'task-owner-step-done',
|
||||
status: 'active',
|
||||
round_trip_count: 1,
|
||||
updated_at: '2026-03-30T00:00:00.000Z',
|
||||
},
|
||||
source: 'owner-delivery-success',
|
||||
completedRole: 'owner',
|
||||
fallbackLastTurnOutputRole: 'owner',
|
||||
fallbackLastTurnOutputVerdict: 'step_done',
|
||||
enqueue,
|
||||
});
|
||||
|
||||
expect(result).toMatchObject({
|
||||
kind: 'none',
|
||||
taskStatus: 'active',
|
||||
lastTurnOutputRole: 'owner',
|
||||
lastTurnOutputVerdict: 'step_done',
|
||||
});
|
||||
expect(enqueue).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('prefers the latest persisted turn output over the fallback delivery role when both are present', () => {
|
||||
const enqueue = vi.fn();
|
||||
vi.mocked(getPairedTurnOutputs).mockReturnValue([
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { getPairedTurnOutputs } from './db.js';
|
||||
import { type VisibleVerdict } from './paired-execution-context-shared.js';
|
||||
import { resolveStoredVisibleVerdict } from './paired-verdict.js';
|
||||
import {
|
||||
matchesExpectedPairedFollowUpIntent,
|
||||
resolveFollowUpDispatch,
|
||||
@@ -20,6 +22,7 @@ export interface PairedFollowUpDecision {
|
||||
taskId: string | null;
|
||||
taskStatus: PairedTaskStatus | null;
|
||||
lastTurnOutputRole: PairedRoomRole | null;
|
||||
lastTurnOutputVerdict: VisibleVerdict | null;
|
||||
nextTurnAction: NextTurnAction;
|
||||
dispatch: FollowUpDispatch;
|
||||
}
|
||||
@@ -37,15 +40,27 @@ export type PairedFollowUpDispatchResult =
|
||||
scheduled: boolean;
|
||||
});
|
||||
|
||||
export function resolveLatestPairedTurnOutputRole(args: {
|
||||
export function resolveLatestPairedTurnOutputContext(args: {
|
||||
task: Pick<PairedTask, 'id'> | null | undefined;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
}): PairedRoomRole | null {
|
||||
return (
|
||||
(args.task ? getPairedTurnOutputs(args.task.id).at(-1)?.role : null) ??
|
||||
args.fallbackLastTurnOutputRole ??
|
||||
null
|
||||
);
|
||||
fallbackLastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
}): {
|
||||
role: PairedRoomRole | null;
|
||||
verdict: VisibleVerdict | null;
|
||||
} {
|
||||
const latestOutput = args.task
|
||||
? getPairedTurnOutputs(args.task.id).at(-1)
|
||||
: null;
|
||||
return {
|
||||
role: latestOutput?.role ?? args.fallbackLastTurnOutputRole ?? null,
|
||||
verdict:
|
||||
resolveStoredVisibleVerdict({
|
||||
verdict: latestOutput?.verdict ?? null,
|
||||
outputText: latestOutput?.output_text ?? null,
|
||||
}) ??
|
||||
args.fallbackLastTurnOutputVerdict ??
|
||||
null,
|
||||
};
|
||||
}
|
||||
|
||||
export function resolvePairedFollowUpDecision(args: {
|
||||
@@ -55,14 +70,18 @@ export function resolvePairedFollowUpDecision(args: {
|
||||
executionStatus?: 'succeeded' | 'failed';
|
||||
sawOutput?: boolean;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
fallbackLastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
}): PairedFollowUpDecision {
|
||||
const lastTurnOutputRole = resolveLatestPairedTurnOutputRole({
|
||||
task: args.task,
|
||||
fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole,
|
||||
});
|
||||
const { role: lastTurnOutputRole, verdict: lastTurnOutputVerdict } =
|
||||
resolveLatestPairedTurnOutputContext({
|
||||
task: args.task,
|
||||
fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole,
|
||||
fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict,
|
||||
});
|
||||
const nextTurnAction = resolveNextTurnAction({
|
||||
taskStatus: args.task?.status ?? null,
|
||||
lastTurnOutputRole,
|
||||
lastTurnOutputVerdict,
|
||||
});
|
||||
const dispatch = resolveFollowUpDispatch({
|
||||
source: args.source,
|
||||
@@ -76,6 +95,7 @@ export function resolvePairedFollowUpDecision(args: {
|
||||
taskId: args.task?.id ?? null,
|
||||
taskStatus: args.task?.status ?? null,
|
||||
lastTurnOutputRole,
|
||||
lastTurnOutputVerdict,
|
||||
nextTurnAction,
|
||||
dispatch,
|
||||
};
|
||||
@@ -91,23 +111,31 @@ export function schedulePairedFollowUpIntent(args: {
|
||||
intentKind: ScheduledPairedFollowUpIntentKind;
|
||||
enqueue: () => void;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
fallbackLastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
lastTurnOutputRole?: PairedRoomRole | null;
|
||||
lastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
}): boolean {
|
||||
if (!args.task) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const lastTurnOutputRole =
|
||||
args.lastTurnOutputRole ??
|
||||
resolveLatestPairedTurnOutputRole({
|
||||
task: args.task,
|
||||
fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole,
|
||||
});
|
||||
const latestOutputContext =
|
||||
args.lastTurnOutputRole != null || args.lastTurnOutputVerdict != null
|
||||
? {
|
||||
role: args.lastTurnOutputRole ?? null,
|
||||
verdict: args.lastTurnOutputVerdict ?? null,
|
||||
}
|
||||
: resolveLatestPairedTurnOutputContext({
|
||||
task: args.task,
|
||||
fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole,
|
||||
fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict,
|
||||
});
|
||||
|
||||
if (
|
||||
!matchesExpectedPairedFollowUpIntent({
|
||||
taskStatus: args.task.status,
|
||||
lastTurnOutputRole,
|
||||
lastTurnOutputRole: latestOutputContext.role,
|
||||
lastTurnOutputVerdict: latestOutputContext.verdict,
|
||||
intentKind: args.intentKind,
|
||||
})
|
||||
) {
|
||||
@@ -133,7 +161,9 @@ export function schedulePairedFollowUpWithMessageCheck(args: {
|
||||
intentKind: ScheduledPairedFollowUpIntentKind;
|
||||
enqueueMessageCheck: () => void;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
fallbackLastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
lastTurnOutputRole?: PairedRoomRole | null;
|
||||
lastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
}): boolean {
|
||||
return schedulePairedFollowUpIntent({
|
||||
chatJid: args.chatJid,
|
||||
@@ -142,7 +172,9 @@ export function schedulePairedFollowUpWithMessageCheck(args: {
|
||||
intentKind: args.intentKind,
|
||||
enqueue: args.enqueueMessageCheck,
|
||||
fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole,
|
||||
fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict,
|
||||
lastTurnOutputRole: args.lastTurnOutputRole,
|
||||
lastTurnOutputVerdict: args.lastTurnOutputVerdict,
|
||||
});
|
||||
}
|
||||
|
||||
@@ -158,6 +190,7 @@ export function dispatchPairedFollowUpForEvent(args: {
|
||||
executionStatus?: 'succeeded' | 'failed';
|
||||
sawOutput?: boolean;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
fallbackLastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
enqueue: () => void;
|
||||
enqueueMessageCheck?: () => void;
|
||||
}): PairedFollowUpDispatchResult {
|
||||
@@ -168,6 +201,7 @@ export function dispatchPairedFollowUpForEvent(args: {
|
||||
executionStatus: args.executionStatus,
|
||||
sawOutput: args.sawOutput,
|
||||
fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole,
|
||||
fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict,
|
||||
});
|
||||
|
||||
if (
|
||||
@@ -193,6 +227,7 @@ export function dispatchPairedFollowUpForEvent(args: {
|
||||
intentKind: decision.nextTurnAction.kind,
|
||||
enqueue: args.enqueue,
|
||||
lastTurnOutputRole: decision.lastTurnOutputRole,
|
||||
lastTurnOutputVerdict: decision.lastTurnOutputVerdict,
|
||||
});
|
||||
|
||||
return {
|
||||
@@ -221,6 +256,7 @@ export function enqueuePairedFollowUpAfterEvent(args: {
|
||||
executionStatus?: 'succeeded' | 'failed';
|
||||
sawOutput?: boolean;
|
||||
fallbackLastTurnOutputRole?: PairedRoomRole | null;
|
||||
fallbackLastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
enqueueMessageCheck: () => void;
|
||||
}): PairedFollowUpDispatchResult {
|
||||
return dispatchPairedFollowUpForEvent({
|
||||
@@ -232,6 +268,7 @@ export function enqueuePairedFollowUpAfterEvent(args: {
|
||||
executionStatus: args.executionStatus,
|
||||
sawOutput: args.sawOutput,
|
||||
fallbackLastTurnOutputRole: args.fallbackLastTurnOutputRole,
|
||||
fallbackLastTurnOutputVerdict: args.fallbackLastTurnOutputVerdict,
|
||||
enqueue: args.enqueueMessageCheck,
|
||||
enqueueMessageCheck: args.enqueueMessageCheck,
|
||||
});
|
||||
|
||||
@@ -212,7 +212,8 @@ export function buildFinalizePendingPrompt(args: {
|
||||
? `\n\nReviewer's final assessment:\n${lastReviewerOutput.output_text.slice(0, 2000)}`
|
||||
: '';
|
||||
|
||||
return `The reviewer approved your work (DONE). Finalize and report the result.
|
||||
If you intend to close this paired turn now, your first line must be DONE.
|
||||
return `The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.
|
||||
If you intend to close this paired turn now, your first line must be TASK_DONE.
|
||||
If the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE.
|
||||
If your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.${reviewerSummary}`;
|
||||
}
|
||||
|
||||
@@ -11,6 +11,7 @@ import {
|
||||
isBotOnlyPairedRoomTurn,
|
||||
} from './message-runtime-flow.js';
|
||||
import { buildPairedTurnIdentity } from './paired-turn-identity.js';
|
||||
import { resolveStoredVisibleVerdict } from './paired-verdict.js';
|
||||
import {
|
||||
advanceLastAgentCursor,
|
||||
resolveActiveRole,
|
||||
@@ -183,6 +184,12 @@ export async function runQueuedGroupTurn(args: {
|
||||
const lastTurnOutputRole = currentTask
|
||||
? (turnOutputs.at(-1)?.role ?? null)
|
||||
: null;
|
||||
const lastTurnOutputVerdict = currentTask
|
||||
? resolveStoredVisibleVerdict({
|
||||
verdict: turnOutputs.at(-1)?.verdict ?? null,
|
||||
outputText: turnOutputs.at(-1)?.output_text ?? null,
|
||||
})
|
||||
: null;
|
||||
const turnRole = currentTask
|
||||
? hasHumanMsg
|
||||
? resolveQueuedTurnRole({
|
||||
@@ -193,6 +200,7 @@ export async function runQueuedGroupTurn(args: {
|
||||
taskStatus,
|
||||
hasHumanMessage: false,
|
||||
lastTurnOutputRole,
|
||||
lastTurnOutputVerdict,
|
||||
})
|
||||
: 'owner';
|
||||
if (!turnRole) {
|
||||
|
||||
@@ -113,6 +113,14 @@ describe('message-runtime-rules', () => {
|
||||
intentKind: 'owner-follow-up',
|
||||
}),
|
||||
).toBe(true);
|
||||
expect(
|
||||
matchesExpectedPairedFollowUpIntent({
|
||||
taskStatus: 'active',
|
||||
lastTurnOutputRole: 'owner',
|
||||
lastTurnOutputVerdict: 'step_done',
|
||||
intentKind: 'owner-follow-up',
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('maps active tasks with reviewer output to an owner follow-up', () => {
|
||||
@@ -124,6 +132,16 @@ describe('message-runtime-rules', () => {
|
||||
).toEqual({ kind: 'owner-follow-up' });
|
||||
});
|
||||
|
||||
it('does not map active owner STEP_DONE output to an owner follow-up', () => {
|
||||
expect(
|
||||
resolveNextTurnAction({
|
||||
taskStatus: 'active',
|
||||
lastTurnOutputRole: 'owner',
|
||||
lastTurnOutputVerdict: 'step_done',
|
||||
}),
|
||||
).toEqual({ kind: 'none' });
|
||||
});
|
||||
|
||||
it('returns none when an active task has no reviewer or arbiter handoff output', () => {
|
||||
expect(
|
||||
resolveNextTurnAction({
|
||||
@@ -152,6 +170,15 @@ describe('message-runtime-rules', () => {
|
||||
kind: 'enqueue',
|
||||
queueKind: 'paired-follow-up',
|
||||
});
|
||||
expect(
|
||||
resolveFollowUpDispatch({
|
||||
source: 'owner-delivery-success',
|
||||
nextTurnAction: { kind: 'owner-follow-up' },
|
||||
}),
|
||||
).toEqual({
|
||||
kind: 'enqueue',
|
||||
queueKind: 'paired-follow-up',
|
||||
});
|
||||
expect(
|
||||
resolveFollowUpDispatch({
|
||||
source: 'owner-delivery-success',
|
||||
@@ -295,6 +322,14 @@ describe('message-runtime-rules', () => {
|
||||
lastTurnOutputRole: 'owner',
|
||||
}),
|
||||
).toBe('reviewer');
|
||||
expect(
|
||||
resolveQueuedPairedTurnRole({
|
||||
taskStatus: 'active',
|
||||
hasHumanMessage: false,
|
||||
lastTurnOutputRole: 'owner',
|
||||
lastTurnOutputVerdict: 'step_done',
|
||||
}),
|
||||
).toBeNull();
|
||||
});
|
||||
|
||||
it('resolves reviewer execution target from review_ready task status', () => {
|
||||
|
||||
@@ -4,6 +4,7 @@ import { normalizeStoredSeqCursor } from './message-cursor.js';
|
||||
import { isTriggerAllowed, loadSenderAllowlist } from './sender-allowlist.js';
|
||||
import { isTaskStatusControlMessage } from './task-watch-status.js';
|
||||
import { ARBITER_AGENT_TYPE, REVIEWER_AGENT_TYPE } from './config.js';
|
||||
import type { VisibleVerdict } from './paired-execution-context-shared.js';
|
||||
import {
|
||||
hasReviewerLease,
|
||||
resolveLeaseServiceId,
|
||||
@@ -79,6 +80,7 @@ export function resolveQueuedPairedTurnRole(args: {
|
||||
taskStatus?: PairedTaskStatus | null;
|
||||
hasHumanMessage: boolean;
|
||||
lastTurnOutputRole?: PairedRoomRole | null;
|
||||
lastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
}): 'owner' | 'reviewer' | 'arbiter' | null {
|
||||
if (args.hasHumanMessage) {
|
||||
return resolveQueuedTurnRole({
|
||||
@@ -90,6 +92,7 @@ export function resolveQueuedPairedTurnRole(args: {
|
||||
const nextTurnAction = resolveNextTurnAction({
|
||||
taskStatus: args.taskStatus,
|
||||
lastTurnOutputRole: args.lastTurnOutputRole,
|
||||
lastTurnOutputVerdict: args.lastTurnOutputVerdict,
|
||||
});
|
||||
|
||||
switch (nextTurnAction.kind) {
|
||||
@@ -125,6 +128,7 @@ export type FollowUpDispatch =
|
||||
export function resolveNextTurnAction(args: {
|
||||
taskStatus?: PairedTaskStatus | null;
|
||||
lastTurnOutputRole?: PairedRoomRole | null;
|
||||
lastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
}): NextTurnAction {
|
||||
switch (args.taskStatus) {
|
||||
case 'review_ready':
|
||||
@@ -154,12 +158,14 @@ export function resolveNextTurnAction(args: {
|
||||
export function matchesExpectedPairedFollowUpIntent(args: {
|
||||
taskStatus?: PairedTaskStatus | null;
|
||||
lastTurnOutputRole?: PairedRoomRole | null;
|
||||
lastTurnOutputVerdict?: VisibleVerdict | null;
|
||||
intentKind: ScheduledNextTurnActionKind;
|
||||
}): boolean {
|
||||
return (
|
||||
resolveNextTurnAction({
|
||||
taskStatus: args.taskStatus,
|
||||
lastTurnOutputRole: args.lastTurnOutputRole,
|
||||
lastTurnOutputVerdict: args.lastTurnOutputVerdict,
|
||||
}).kind === args.intentKind
|
||||
);
|
||||
}
|
||||
@@ -184,7 +190,8 @@ export function resolveFollowUpDispatch(args: {
|
||||
args.completedRole === 'owner'
|
||||
) {
|
||||
return args.nextTurnAction.kind === 'reviewer-turn' ||
|
||||
args.nextTurnAction.kind === 'arbiter-turn'
|
||||
args.nextTurnAction.kind === 'arbiter-turn' ||
|
||||
args.nextTurnAction.kind === 'owner-follow-up'
|
||||
? { kind: 'enqueue', queueKind: 'paired-follow-up' }
|
||||
: { kind: 'none' };
|
||||
}
|
||||
@@ -219,6 +226,7 @@ export function resolveFollowUpDispatch(args: {
|
||||
return { kind: 'none' };
|
||||
}
|
||||
return args.nextTurnAction.kind === 'reviewer-turn' ||
|
||||
args.nextTurnAction.kind === 'owner-follow-up' ||
|
||||
args.nextTurnAction.kind === 'arbiter-turn' ||
|
||||
args.nextTurnAction.kind === 'finalize-owner-turn'
|
||||
? { kind: 'enqueue', queueKind: 'paired-follow-up' }
|
||||
|
||||
@@ -842,6 +842,127 @@ describe('createMessageRuntime', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('suppresses a stale owner work item when a new human message arrives while the paired task is still active', async () => {
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('codex');
|
||||
const channel = makeChannel(chatJid);
|
||||
const enqueueMessageCheck = vi.fn();
|
||||
const activeTask = {
|
||||
id: 'task-active-owner-follow-up',
|
||||
chat_jid: chatJid,
|
||||
group_folder: group.folder,
|
||||
owner_service_id: 'claude',
|
||||
reviewer_service_id: 'codex-main',
|
||||
title: null,
|
||||
source_ref: 'HEAD',
|
||||
plan_notes: null,
|
||||
review_requested_at: null,
|
||||
round_trip_count: 0,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
created_at: '2026-03-30T00:00:00.000Z',
|
||||
updated_at: '2026-03-30T00:00:05.000Z',
|
||||
} as any;
|
||||
|
||||
vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
|
||||
vi.mocked(db.getOpenWorkItem).mockReturnValue({
|
||||
id: 109,
|
||||
group_folder: group.folder,
|
||||
chat_jid: chatJid,
|
||||
agent_type: 'codex',
|
||||
service_id: 'claude',
|
||||
delivery_role: 'owner',
|
||||
status: 'delivery_retry',
|
||||
start_seq: 1,
|
||||
end_seq: 1,
|
||||
result_payload: '이전 step 결과입니다.',
|
||||
delivery_attempts: 1,
|
||||
created_at: '2026-03-30T00:00:05.000Z',
|
||||
updated_at: '2026-03-30T00:00:05.000Z',
|
||||
delivered_at: null,
|
||||
delivery_message_id: null,
|
||||
last_error: 'discord send failed',
|
||||
});
|
||||
vi.mocked(db.getMessagesSince).mockReturnValue([
|
||||
{
|
||||
id: 'msg-2',
|
||||
chat_jid: chatJid,
|
||||
sender: 'user@test',
|
||||
sender_name: 'User',
|
||||
content: '이전 답 말고 이 방향으로 진행해줘',
|
||||
timestamp: '2026-03-30T00:00:10.000Z',
|
||||
seq: 2,
|
||||
},
|
||||
]);
|
||||
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(activeTask);
|
||||
vi.mocked(agentRunner.runAgentProcess).mockImplementation(
|
||||
async (_group, input, _onProcess, onOutput) => {
|
||||
expect(input.prompt).toContain('이 방향으로 진행해줘');
|
||||
expect(input.prompt).not.toContain('이전 step 결과입니다.');
|
||||
await onOutput?.({
|
||||
status: 'success',
|
||||
phase: 'final',
|
||||
result: 'STEP_DONE\n새 입력 기준으로 계속 진행',
|
||||
output: {
|
||||
visibility: 'public',
|
||||
text: 'STEP_DONE\n새 입력 기준으로 계속 진행',
|
||||
},
|
||||
} as any);
|
||||
return {
|
||||
status: 'success',
|
||||
result: 'STEP_DONE\n새 입력 기준으로 계속 진행',
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
const runtime = createMessageRuntime({
|
||||
assistantName: 'Andy',
|
||||
idleTimeout: 1_000,
|
||||
pollInterval: 1_000,
|
||||
timezone: 'UTC',
|
||||
triggerPattern: /^@Andy\b/i,
|
||||
channels: [channel],
|
||||
queue: {
|
||||
registerProcess: vi.fn(),
|
||||
closeStdin: vi.fn(),
|
||||
notifyIdle: vi.fn(),
|
||||
enqueueMessageCheck,
|
||||
} as any,
|
||||
getRoomBindings: () => ({ [chatJid]: group }),
|
||||
getSessions: () => ({}),
|
||||
getLastTimestamp: () => '',
|
||||
setLastTimestamp: vi.fn(),
|
||||
getLastAgentTimestamps: () => ({}),
|
||||
saveState: vi.fn(),
|
||||
persistSession: vi.fn(),
|
||||
clearSession: vi.fn(),
|
||||
});
|
||||
|
||||
const result = await runtime.processGroupMessages(chatJid, {
|
||||
runId: 'run-suppress-stale-active-owner-work-item',
|
||||
reason: 'messages',
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(db.markWorkItemDelivered).toHaveBeenCalledWith(109);
|
||||
expect(channel.sendMessage).not.toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'이전 step 결과입니다.',
|
||||
);
|
||||
expect(agentRunner.runAgentProcess).toHaveBeenCalledTimes(1);
|
||||
expect(logger.info).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
chatJid,
|
||||
workItemId: 109,
|
||||
taskId: 'task-active-owner-follow-up',
|
||||
taskStatus: 'active',
|
||||
}),
|
||||
'Suppressed stale owner delivery retry because a new human message arrived while the paired task was still active',
|
||||
);
|
||||
});
|
||||
|
||||
it('suppresses duplicate stale work item delivery and logs the suppression reason', async () => {
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('codex');
|
||||
@@ -1632,8 +1753,9 @@ describe('createMessageRuntime', () => {
|
||||
vi.mocked(agentRunner.runAgentProcess).mockImplementation(
|
||||
async (_group, input, _onProcess, onOutput) => {
|
||||
expect(input.prompt).toBe(
|
||||
`The reviewer approved your work (DONE). Finalize and report the result.
|
||||
If you intend to close this paired turn now, your first line must be DONE.
|
||||
`The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.
|
||||
If you intend to close this paired turn now, your first line must be TASK_DONE.
|
||||
If the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE.
|
||||
If your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.\n\nReviewer's final assessment:\n${truncatedReviewerOutput}`,
|
||||
);
|
||||
expect(input.prompt).not.toContain(longReviewerOutput);
|
||||
@@ -1741,7 +1863,7 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
vi.mocked(agentRunner.runAgentProcess).mockImplementation(
|
||||
async (_group, input, _onProcess, onOutput) => {
|
||||
expect(input.prompt).toBe(
|
||||
"The reviewer approved your work (DONE). Finalize and report the result.\nIf you intend to close this paired turn now, your first line must be DONE.\nIf your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.\n\nReviewer's final assessment:\n리뷰 승인 요약",
|
||||
"The reviewer approved the current task scope (TASK_DONE / legacy DONE). Finalize and report the result.\nIf you intend to close this paired turn now, your first line must be TASK_DONE.\nIf the original request still has remaining work and the owner flow should continue, your first line may be STEP_DONE.\nIf your first line is DONE_WITH_CONCERNS, the system will reopen review instead of finishing.\n\nReviewer's final assessment:\n리뷰 승인 요약",
|
||||
);
|
||||
expect(input.prompt).not.toContain('DONE\n승인합니다.');
|
||||
await onOutput?.({
|
||||
@@ -2004,6 +2126,105 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
expect(pending?.prompt).toContain('이전 reviewer 피드백');
|
||||
});
|
||||
|
||||
it('does not build an owner follow-up pending turn from owner STEP_DONE on an active task', () => {
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('claude-code');
|
||||
const task = {
|
||||
id: 'task-owner-step-done-pending',
|
||||
chat_jid: chatJid,
|
||||
group_folder: group.folder,
|
||||
owner_service_id: 'claude',
|
||||
reviewer_service_id: 'codex-main',
|
||||
title: null,
|
||||
source_ref: 'HEAD',
|
||||
plan_notes: null,
|
||||
review_requested_at: null,
|
||||
round_trip_count: 1,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
created_at: '2026-03-30T00:00:10.000Z',
|
||||
updated_at: '2026-03-30T00:00:10.000Z',
|
||||
} as any;
|
||||
|
||||
vi.mocked(db.getLatestPreviousPairedTaskForChat).mockReturnValue(undefined);
|
||||
vi.mocked(db.getPairedTurnOutputs).mockReturnValue([
|
||||
{
|
||||
id: 1,
|
||||
task_id: task.id,
|
||||
turn_number: 1,
|
||||
role: 'owner',
|
||||
output_text: 'STEP_DONE\n1단계는 끝났고 2단계로 이어가야 함',
|
||||
created_at: '2026-03-30T00:00:11.000Z',
|
||||
},
|
||||
] as any);
|
||||
|
||||
const pending = buildPendingPairedTurn({
|
||||
chatJid,
|
||||
timezone: 'UTC',
|
||||
task,
|
||||
rawMissedMessages: [{ seq: 42, timestamp: '2026-03-30T00:00:12.000Z' }],
|
||||
recentHumanMessages: [],
|
||||
labeledRecentMessages: [],
|
||||
resolveChannel: () => makeChannel(chatJid),
|
||||
});
|
||||
|
||||
expect(pending).toBeNull();
|
||||
});
|
||||
|
||||
it('builds a reviewer pending turn when a review_ready task follows owner TASK_DONE output', () => {
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('claude-code');
|
||||
const task = {
|
||||
id: 'task-owner-task-done-pending',
|
||||
chat_jid: chatJid,
|
||||
group_folder: group.folder,
|
||||
owner_service_id: 'claude',
|
||||
reviewer_service_id: 'codex-main',
|
||||
title: null,
|
||||
source_ref: 'HEAD',
|
||||
plan_notes: null,
|
||||
review_requested_at: '2026-03-30T00:00:10.000Z',
|
||||
round_trip_count: 1,
|
||||
status: 'review_ready',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
created_at: '2026-03-30T00:00:10.000Z',
|
||||
updated_at: '2026-03-30T00:00:10.000Z',
|
||||
} as any;
|
||||
|
||||
vi.mocked(db.getLatestPreviousPairedTaskForChat).mockReturnValue(undefined);
|
||||
vi.mocked(db.getPairedTurnOutputs).mockReturnValue([
|
||||
{
|
||||
id: 1,
|
||||
task_id: task.id,
|
||||
turn_number: 1,
|
||||
role: 'owner',
|
||||
output_text: 'TASK_DONE\n요청 범위 전체 완료',
|
||||
created_at: '2026-03-30T00:00:11.000Z',
|
||||
},
|
||||
] as any);
|
||||
|
||||
const pending = buildPendingPairedTurn({
|
||||
chatJid,
|
||||
timezone: 'UTC',
|
||||
task,
|
||||
rawMissedMessages: [{ seq: 42, timestamp: '2026-03-30T00:00:12.000Z' }],
|
||||
recentHumanMessages: [],
|
||||
labeledRecentMessages: [],
|
||||
resolveChannel: () => makeChannel(chatJid, 'discord-review', false),
|
||||
});
|
||||
|
||||
expect(pending).not.toBeNull();
|
||||
expect(pending).toMatchObject({
|
||||
role: 'reviewer',
|
||||
intentKind: 'reviewer-turn',
|
||||
taskId: task.id,
|
||||
});
|
||||
});
|
||||
|
||||
it('re-enqueues reviewer after a successful owner delivery moves the task to review_ready', async () => {
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('codex');
|
||||
@@ -2109,6 +2330,128 @@ If your first line is DONE_WITH_CONCERNS, the system will reopen review instead
|
||||
);
|
||||
});
|
||||
|
||||
it('does not re-enqueue owner follow-up after a successful owner STEP_DONE delivery keeps the task active', async () => {
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('codex');
|
||||
const ownerChannel: Channel = {
|
||||
...makeChannel(chatJid),
|
||||
isOwnMessage: vi.fn((msg) => msg.sender === 'owner-bot@test'),
|
||||
};
|
||||
const reviewerChannel = makeChannel(chatJid, 'discord-review', false);
|
||||
const enqueueMessageCheck = vi.fn();
|
||||
const pairedTask = {
|
||||
id: 'task-owner-step-done-follow-up',
|
||||
chat_jid: chatJid,
|
||||
group_folder: group.folder,
|
||||
owner_service_id: 'claude',
|
||||
reviewer_service_id: 'codex-main',
|
||||
title: null,
|
||||
source_ref: 'HEAD',
|
||||
plan_notes: null,
|
||||
review_requested_at: null,
|
||||
round_trip_count: 1,
|
||||
owner_failure_count: 2,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
completion_reason: null,
|
||||
created_at: '2026-03-30T00:00:00.000Z',
|
||||
updated_at: '2026-03-30T00:00:00.000Z',
|
||||
} as any;
|
||||
let turnOutputs: any[] = [];
|
||||
|
||||
vi.mocked(serviceRouting.hasReviewerLease).mockReturnValue(true);
|
||||
vi.mocked(db.getLatestOpenPairedTaskForChat).mockImplementation(
|
||||
() => pairedTask,
|
||||
);
|
||||
vi.mocked(db.getPairedTurnOutputs).mockImplementation((taskId: string) =>
|
||||
taskId === pairedTask.id ? turnOutputs : [],
|
||||
);
|
||||
vi.mocked(db.getMessagesSince).mockReturnValue([
|
||||
{
|
||||
id: 'human-step-done-1',
|
||||
chat_jid: chatJid,
|
||||
sender: 'user@test',
|
||||
sender_name: 'User',
|
||||
content: '이 리팩토링 이어서 해줘',
|
||||
timestamp: '2026-03-30T00:00:00.000Z',
|
||||
seq: 1,
|
||||
is_bot_message: false,
|
||||
} as any,
|
||||
]);
|
||||
vi.mocked(agentRunner.runAgentProcess).mockImplementation(
|
||||
async (_group, _input, _onProcess, onOutput) => {
|
||||
pairedTask.status = 'active';
|
||||
pairedTask.owner_failure_count = 0;
|
||||
turnOutputs = [
|
||||
{
|
||||
id: 1,
|
||||
task_id: pairedTask.id,
|
||||
turn_number: 1,
|
||||
role: 'owner',
|
||||
output_text: 'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행',
|
||||
created_at: '2026-03-30T00:00:01.000Z',
|
||||
},
|
||||
];
|
||||
await onOutput?.({
|
||||
status: 'success',
|
||||
phase: 'final',
|
||||
result: 'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행',
|
||||
newSessionId: 'session-owner-step-done-follow-up',
|
||||
});
|
||||
return {
|
||||
status: 'success',
|
||||
result: 'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행',
|
||||
newSessionId: 'session-owner-step-done-follow-up',
|
||||
};
|
||||
},
|
||||
);
|
||||
|
||||
const runtime = createMessageRuntime({
|
||||
assistantName: 'Andy',
|
||||
idleTimeout: 1_000,
|
||||
pollInterval: 1_000,
|
||||
timezone: 'UTC',
|
||||
triggerPattern: /^@Andy\b/i,
|
||||
channels: [ownerChannel, reviewerChannel],
|
||||
queue: {
|
||||
registerProcess: vi.fn(),
|
||||
closeStdin: vi.fn(),
|
||||
notifyIdle: vi.fn(),
|
||||
enqueueMessageCheck,
|
||||
} as any,
|
||||
getRoomBindings: () => ({ [chatJid]: group }),
|
||||
getSessions: () => ({}),
|
||||
getLastTimestamp: () => '',
|
||||
setLastTimestamp: vi.fn(),
|
||||
getLastAgentTimestamps: () => ({}),
|
||||
saveState: vi.fn(),
|
||||
persistSession: vi.fn(),
|
||||
clearSession: vi.fn(),
|
||||
});
|
||||
|
||||
const result = await runtime.processGroupMessages(chatJid, {
|
||||
runId: 'run-owner-step-done-follow-up',
|
||||
reason: 'messages',
|
||||
});
|
||||
|
||||
expect(result).toBe(true);
|
||||
expect(ownerChannel.sendMessage).toHaveBeenCalledWith(
|
||||
chatJid,
|
||||
'STEP_DONE\n리팩토링 1단계 완료, 다음 단계 진행',
|
||||
);
|
||||
expect(enqueueMessageCheck).not.toHaveBeenCalled();
|
||||
expect(pairedTask.status).toBe('active');
|
||||
expect(pairedTask.round_trip_count).toBe(1);
|
||||
expect(pairedTask.owner_failure_count).toBe(0);
|
||||
expect(logger.info).not.toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
intentKind: 'owner-follow-up',
|
||||
}),
|
||||
'Queued paired follow-up after successful owner delivery',
|
||||
);
|
||||
});
|
||||
|
||||
it('defers reviewer enqueue after owner delivery when a CI watcher is still active', async () => {
|
||||
const chatJid = 'group@test';
|
||||
const group = makeGroup('codex');
|
||||
|
||||
@@ -56,6 +56,7 @@ import {
|
||||
} from './types.js';
|
||||
import { createScopedLogger, logger } from './logger.js';
|
||||
import { hasReviewerLease } from './service-routing.js';
|
||||
import type { WorkItem } from './db/work-items.js';
|
||||
export {
|
||||
resolveHandoffCursorKey,
|
||||
resolveHandoffRoleOverride,
|
||||
@@ -113,6 +114,54 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
deps.queue,
|
||||
);
|
||||
|
||||
const getFreshHumanPreflightMessages = (
|
||||
chatJid: string,
|
||||
channel: Channel,
|
||||
): NewMessage[] => {
|
||||
const sinceSeqCursor = deps.getLastAgentTimestamps()[chatJid] || '0';
|
||||
const preflightRawMessages = getMessagesSinceSeq(
|
||||
chatJid,
|
||||
sinceSeqCursor,
|
||||
deps.assistantName,
|
||||
);
|
||||
const preflightMessages = filterLoopingPairedBotMessages(
|
||||
chatJid,
|
||||
getProcessableMessages(chatJid, preflightRawMessages, channel),
|
||||
FAILURE_FINAL_TEXT,
|
||||
);
|
||||
return preflightMessages.filter(
|
||||
(message) => message.is_from_me !== true && !message.is_bot_message,
|
||||
);
|
||||
};
|
||||
|
||||
const hasHumanMessageAfterWorkItem = (
|
||||
openWorkItem: WorkItem,
|
||||
freshHumanMessages: NewMessage[],
|
||||
): boolean => {
|
||||
if (freshHumanMessages.length === 0) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const workItemSeq = openWorkItem.end_seq ?? openWorkItem.start_seq ?? null;
|
||||
const workItemUpdatedAt = Date.parse(openWorkItem.updated_at);
|
||||
|
||||
return freshHumanMessages.some((message) => {
|
||||
if (message.seq != null && workItemSeq != null) {
|
||||
return message.seq > workItemSeq;
|
||||
}
|
||||
|
||||
const messageTimestamp = Date.parse(message.timestamp);
|
||||
if (
|
||||
Number.isFinite(messageTimestamp) &&
|
||||
Number.isFinite(workItemUpdatedAt)
|
||||
) {
|
||||
return messageTimestamp > workItemUpdatedAt;
|
||||
}
|
||||
|
||||
return true;
|
||||
});
|
||||
};
|
||||
|
||||
const scheduleQueuedPairedFollowUp = (args: {
|
||||
chatJid: string;
|
||||
runId: string;
|
||||
@@ -351,25 +400,15 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
? getLatestOpenPairedTaskForChat(chatJid)
|
||||
: null;
|
||||
let openWorkItem = getOpenWorkItemForChat(chatJid, SERVICE_SESSION_SCOPE);
|
||||
if (
|
||||
pendingTask?.status === 'merge_ready' &&
|
||||
openWorkItem?.delivery_role === 'owner'
|
||||
) {
|
||||
const sinceSeqCursor = deps.getLastAgentTimestamps()[chatJid] || '0';
|
||||
const preflightRawMessages = getMessagesSinceSeq(
|
||||
if (openWorkItem?.delivery_role === 'owner' && pendingTask) {
|
||||
const freshHumanMessages = getFreshHumanPreflightMessages(
|
||||
chatJid,
|
||||
sinceSeqCursor,
|
||||
deps.assistantName,
|
||||
channel,
|
||||
);
|
||||
const preflightMessages = filterLoopingPairedBotMessages(
|
||||
chatJid,
|
||||
getProcessableMessages(chatJid, preflightRawMessages, channel),
|
||||
FAILURE_FINAL_TEXT,
|
||||
);
|
||||
const hasFreshHumanMessage = preflightMessages.some(
|
||||
(message) => message.is_from_me !== true && !message.is_bot_message,
|
||||
);
|
||||
if (hasFreshHumanMessage) {
|
||||
if (
|
||||
pendingTask.status === 'merge_ready' &&
|
||||
freshHumanMessages.length > 0
|
||||
) {
|
||||
const resolvedTask = resolveOwnerTaskForHumanMessage({
|
||||
group,
|
||||
chatJid,
|
||||
@@ -389,6 +428,23 @@ export function createMessageRuntime(deps: MessageRuntimeDeps): {
|
||||
);
|
||||
openWorkItem = undefined;
|
||||
}
|
||||
} else if (
|
||||
pendingTask.status === 'active' &&
|
||||
hasHumanMessageAfterWorkItem(openWorkItem, freshHumanMessages)
|
||||
) {
|
||||
markWorkItemDelivered(openWorkItem.id);
|
||||
log.info(
|
||||
{
|
||||
chatJid,
|
||||
workItemId: openWorkItem.id,
|
||||
taskId: pendingTask.id,
|
||||
taskStatus: pendingTask.status,
|
||||
workItemEndSeq: openWorkItem.end_seq ?? null,
|
||||
freshHumanMessageCount: freshHumanMessages.length,
|
||||
},
|
||||
'Suppressed stale owner delivery retry because a new human message arrived while the paired task was still active',
|
||||
);
|
||||
openWorkItem = undefined;
|
||||
}
|
||||
}
|
||||
const openWorkItemOutcome = await processOpenWorkItemDelivery({
|
||||
|
||||
@@ -22,6 +22,7 @@ import type { PairedTask } from './types.js';
|
||||
|
||||
type OwnerFinalizeOutcome = 'stop' | 're_review';
|
||||
const OWNER_FAILURE_ESCALATION_THRESHOLD = 2;
|
||||
const EMPTY_STEP_DONE_THRESHOLD = 2;
|
||||
|
||||
export function handleFailedOwnerExecution(args: {
|
||||
task: PairedTask;
|
||||
@@ -101,6 +102,14 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
const hasNewChanges = workspace?.workspace_dir
|
||||
? hasCodeChangesSinceRef(workspace.workspace_dir, task.source_ref)
|
||||
: null;
|
||||
const nextFinalizeStepDoneCount =
|
||||
ownerVerdict === 'step_done'
|
||||
? (task.finalize_step_done_count ?? 0) + 1
|
||||
: (task.finalize_step_done_count ?? 0);
|
||||
const nextEmptyStepDoneStreak =
|
||||
ownerVerdict === 'step_done' && hasNewChanges === false
|
||||
? (task.empty_step_done_streak ?? 0) + 1
|
||||
: 0;
|
||||
const signal = resolveOwnerCompletionSignal({
|
||||
phase: 'finalize',
|
||||
visibleVerdict: ownerVerdict,
|
||||
@@ -138,6 +147,41 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
},
|
||||
patch: {
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: nextFinalizeStepDoneCount,
|
||||
empty_step_done_streak: nextEmptyStepDoneStreak,
|
||||
},
|
||||
});
|
||||
return 'stop';
|
||||
}
|
||||
|
||||
if (
|
||||
ownerVerdict === 'step_done' &&
|
||||
hasNewChanges === false &&
|
||||
nextEmptyStepDoneStreak >= EMPTY_STEP_DONE_THRESHOLD
|
||||
) {
|
||||
requestArbiterOrEscalate({
|
||||
taskId,
|
||||
currentStatus: task.status,
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
now,
|
||||
arbiterLogMessage:
|
||||
'Owner repeated STEP_DONE during finalize without code changes — requesting arbiter',
|
||||
escalateLogMessage:
|
||||
'Owner repeated STEP_DONE during finalize without code changes — escalating to user',
|
||||
logContext: {
|
||||
taskId,
|
||||
ownerVerdict,
|
||||
hasNewChanges,
|
||||
emptyStepDoneStreak: nextEmptyStepDoneStreak,
|
||||
finalizeStepDoneCount: nextFinalizeStepDoneCount,
|
||||
summary: summary?.slice(0, 100),
|
||||
},
|
||||
patch: {
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: nextFinalizeStepDoneCount,
|
||||
empty_step_done_streak: nextEmptyStepDoneStreak,
|
||||
},
|
||||
});
|
||||
return 'stop';
|
||||
@@ -153,6 +197,10 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: nextFinalizeStepDoneCount,
|
||||
empty_step_done_streak:
|
||||
ownerVerdict === 'step_done' ? nextEmptyStepDoneStreak : 0,
|
||||
},
|
||||
});
|
||||
}
|
||||
@@ -165,9 +213,27 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
},
|
||||
ownerVerdict === 'done_with_concerns'
|
||||
? 'Owner raised concerns during finalize — task set back to active'
|
||||
: 'Owner made changes after reviewer approval — task set back to active before re-review',
|
||||
: ownerVerdict === 'step_done'
|
||||
? 'Owner reported STEP_DONE during finalize — task set back to active before review'
|
||||
: 'Owner made changes after reviewer approval — task set back to active before re-review',
|
||||
);
|
||||
return 're_review';
|
||||
maybeAutoTriggerReviewerAfterOwnerCompletion({
|
||||
task,
|
||||
taskId,
|
||||
now,
|
||||
logMessage:
|
||||
ownerVerdict === 'step_done'
|
||||
? 'Auto-triggered reviewer after owner finalize STEP_DONE'
|
||||
: 'Auto-triggered reviewer after owner finalize required re-review',
|
||||
patch:
|
||||
ownerVerdict === 'step_done'
|
||||
? {
|
||||
finalize_step_done_count: nextFinalizeStepDoneCount,
|
||||
empty_step_done_streak: nextEmptyStepDoneStreak,
|
||||
}
|
||||
: undefined,
|
||||
});
|
||||
return 'stop';
|
||||
}
|
||||
|
||||
transitionPairedTaskStatus({
|
||||
@@ -179,6 +245,9 @@ function handleOwnerFinalizeCompletion(args: {
|
||||
patch: {
|
||||
completion_reason: 'done',
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: nextFinalizeStepDoneCount,
|
||||
empty_step_done_streak: 0,
|
||||
},
|
||||
});
|
||||
logger.info(
|
||||
@@ -193,6 +262,11 @@ function maybeAutoTriggerReviewerAfterOwnerCompletion(args: {
|
||||
taskId: string;
|
||||
now: string;
|
||||
logMessage: string;
|
||||
patch?: {
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
};
|
||||
}): void {
|
||||
const { task, taskId, now, logMessage } = args;
|
||||
if (task.round_trip_count >= PAIRED_MAX_ROUND_TRIPS) {
|
||||
@@ -220,6 +294,9 @@ function maybeAutoTriggerReviewerAfterOwnerCompletion(args: {
|
||||
patch: {
|
||||
round_trip_count: task.round_trip_count + 1,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
empty_step_done_streak: 0,
|
||||
...args.patch,
|
||||
},
|
||||
});
|
||||
if (hasActiveCiWatcherForChat(task.chat_jid)) {
|
||||
@@ -265,6 +342,8 @@ export function handleOwnerCompletion(args: {
|
||||
}
|
||||
|
||||
const ownerVerdict = parseVisibleVerdict(summary);
|
||||
const nextOwnerStepDoneStreak =
|
||||
ownerVerdict === 'step_done' ? (task.owner_step_done_streak ?? 0) + 1 : 0;
|
||||
const signal = resolveOwnerCompletionSignal({
|
||||
phase: 'normal',
|
||||
visibleVerdict: ownerVerdict,
|
||||
@@ -281,15 +360,27 @@ export function handleOwnerCompletion(args: {
|
||||
logContext: {
|
||||
taskId,
|
||||
ownerVerdict,
|
||||
ownerStepDoneStreak: nextOwnerStepDoneStreak,
|
||||
summary: summary?.slice(0, 100),
|
||||
},
|
||||
patch: {
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: nextOwnerStepDoneStreak,
|
||||
},
|
||||
});
|
||||
return;
|
||||
}
|
||||
|
||||
if (nextOwnerStepDoneStreak !== (task.owner_step_done_streak ?? 0)) {
|
||||
applyPairedTaskPatch({
|
||||
taskId,
|
||||
expectedUpdatedAt: task.updated_at,
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
owner_step_done_streak: nextOwnerStepDoneStreak,
|
||||
},
|
||||
});
|
||||
}
|
||||
maybeAutoTriggerReviewerAfterOwnerCompletion({
|
||||
task,
|
||||
taskId,
|
||||
|
||||
@@ -9,6 +9,8 @@ import {
|
||||
|
||||
describe('paired execution context shared verdict helpers', () => {
|
||||
it('parses visible verdicts from the first summary line only', () => {
|
||||
expect(parseVisibleVerdict('STEP_DONE\nmore to do')).toBe('step_done');
|
||||
expect(parseVisibleVerdict('TASK_DONE\nall done')).toBe('task_done');
|
||||
expect(
|
||||
parseVisibleVerdict(
|
||||
'DONE_WITH_CONCERNS\n\nfollow-up detail that should not affect parsing',
|
||||
@@ -19,6 +21,15 @@ describe('paired execution context shared verdict helpers', () => {
|
||||
});
|
||||
|
||||
it('maps normal owner completion verdicts to reviewer or arbiter signals', () => {
|
||||
expect(
|
||||
resolveOwnerCompletionSignal({
|
||||
phase: 'normal',
|
||||
visibleVerdict: 'step_done',
|
||||
}),
|
||||
).toEqual({
|
||||
kind: 'request_reviewer',
|
||||
resetStatusToActive: false,
|
||||
});
|
||||
expect(
|
||||
resolveOwnerCompletionSignal({
|
||||
phase: 'normal',
|
||||
@@ -37,6 +48,18 @@ describe('paired execution context shared verdict helpers', () => {
|
||||
});
|
||||
|
||||
it('maps finalize owner outcomes to complete, re-review, or arbiter', () => {
|
||||
expect(
|
||||
resolveOwnerCompletionSignal({
|
||||
phase: 'finalize',
|
||||
visibleVerdict: 'step_done',
|
||||
hasChangesSinceApproval: false,
|
||||
roundTripCount: 0,
|
||||
deadlockThreshold: 2,
|
||||
}),
|
||||
).toEqual({
|
||||
kind: 'request_reviewer',
|
||||
resetStatusToActive: true,
|
||||
});
|
||||
expect(
|
||||
resolveOwnerCompletionSignal({
|
||||
phase: 'finalize',
|
||||
@@ -70,6 +93,20 @@ describe('paired execution context shared verdict helpers', () => {
|
||||
});
|
||||
|
||||
it('maps reviewer completion verdicts to finalize, owner changes, or arbiter', () => {
|
||||
expect(
|
||||
resolveReviewerCompletionSignal({
|
||||
visibleVerdict: 'task_done',
|
||||
roundTripCount: 0,
|
||||
deadlockThreshold: 3,
|
||||
}),
|
||||
).toEqual({ kind: 'request_owner_finalize' });
|
||||
expect(
|
||||
resolveReviewerCompletionSignal({
|
||||
visibleVerdict: 'step_done',
|
||||
roundTripCount: 0,
|
||||
deadlockThreshold: 3,
|
||||
}),
|
||||
).toEqual({ kind: 'request_owner_changes' });
|
||||
expect(
|
||||
resolveReviewerCompletionSignal({
|
||||
visibleVerdict: 'done',
|
||||
@@ -94,6 +131,11 @@ describe('paired execution context shared verdict helpers', () => {
|
||||
});
|
||||
|
||||
it('maps reviewer failure verdicts to explicit failure signals', () => {
|
||||
expect(
|
||||
resolveReviewerFailureSignal({
|
||||
visibleVerdict: 'task_done',
|
||||
}),
|
||||
).toEqual({ kind: 'request_owner_finalize' });
|
||||
expect(
|
||||
resolveReviewerFailureSignal({
|
||||
visibleVerdict: 'done',
|
||||
|
||||
@@ -3,15 +3,9 @@ import { execFileSync } from 'child_process';
|
||||
import { isArbiterEnabled } from './config.js';
|
||||
import { updatePairedTaskIfUnchanged } from './db.js';
|
||||
import { logger } from './logger.js';
|
||||
import { parseVisibleVerdict, type VisibleVerdict } from './paired-verdict.js';
|
||||
import type { PairedTaskStatus } from './types.js';
|
||||
|
||||
export type VisibleVerdict =
|
||||
| 'done'
|
||||
| 'done_with_concerns'
|
||||
| 'blocked'
|
||||
| 'needs_context'
|
||||
| 'continue';
|
||||
|
||||
export type CompletionSignal =
|
||||
| { kind: 'request_reviewer'; resetStatusToActive: boolean }
|
||||
| { kind: 'request_owner_finalize' }
|
||||
@@ -20,22 +14,8 @@ export type CompletionSignal =
|
||||
| { kind: 'complete'; completionReason: 'done' | 'escalated' }
|
||||
| { kind: 'preserve_review_ready' };
|
||||
|
||||
export function parseVisibleVerdict(
|
||||
summary: string | null | undefined,
|
||||
): VisibleVerdict {
|
||||
if (!summary) return 'continue';
|
||||
const cleaned = summary.replace(/<internal>[\s\S]*?<\/internal>/g, '').trim();
|
||||
if (!cleaned) return 'continue';
|
||||
const firstLine = cleaned.split('\n')[0].trim();
|
||||
if (/^\*{0,2}BLOCKED\*{0,2}\b/i.test(firstLine)) return 'blocked';
|
||||
if (/^\*{0,2}NEEDS_CONTEXT\*{0,2}\b/i.test(firstLine)) return 'needs_context';
|
||||
if (/^\*{0,2}DONE_WITH_CONCERNS\*{0,2}\b/i.test(firstLine))
|
||||
return 'done_with_concerns';
|
||||
if (/^\*{0,2}DONE\*{0,2}\b/i.test(firstLine)) return 'done';
|
||||
if (/^\*{0,2}Approved\.?\*{0,2}/i.test(firstLine)) return 'done';
|
||||
if (/^\*{0,2}LGTM\*{0,2}/i.test(firstLine)) return 'done';
|
||||
return 'continue';
|
||||
}
|
||||
export { parseVisibleVerdict };
|
||||
export type { VisibleVerdict };
|
||||
|
||||
export function resolveOwnerCompletionSignal(args: {
|
||||
phase: 'normal' | 'finalize';
|
||||
@@ -63,6 +43,13 @@ export function resolveOwnerCompletionSignal(args: {
|
||||
};
|
||||
}
|
||||
|
||||
if (visibleVerdict === 'step_done') {
|
||||
return {
|
||||
kind: 'request_reviewer',
|
||||
resetStatusToActive: true,
|
||||
};
|
||||
}
|
||||
|
||||
const needsReReview =
|
||||
visibleVerdict === 'done_with_concerns' || hasChangesSinceApproval === true;
|
||||
|
||||
@@ -90,8 +77,11 @@ export function resolveReviewerCompletionSignal(args: {
|
||||
const { visibleVerdict, roundTripCount, deadlockThreshold } = args;
|
||||
|
||||
switch (visibleVerdict) {
|
||||
case 'task_done':
|
||||
case 'done':
|
||||
return { kind: 'request_owner_finalize' };
|
||||
case 'step_done':
|
||||
return { kind: 'request_owner_changes' };
|
||||
case 'blocked':
|
||||
case 'needs_context':
|
||||
return { kind: 'request_arbiter' };
|
||||
@@ -111,6 +101,7 @@ export function resolveReviewerFailureSignal(args: {
|
||||
const { visibleVerdict } = args;
|
||||
|
||||
switch (visibleVerdict) {
|
||||
case 'task_done':
|
||||
case 'done':
|
||||
return { kind: 'request_owner_finalize' };
|
||||
case 'blocked':
|
||||
@@ -248,6 +239,10 @@ export function transitionPairedTaskStatus(args: {
|
||||
review_requested_at?: string | null;
|
||||
round_trip_count?: number;
|
||||
owner_failure_count?: number;
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
task_done_then_user_reopen_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
@@ -292,6 +287,10 @@ export function applyPairedTaskPatch(args: {
|
||||
review_requested_at?: string | null;
|
||||
round_trip_count?: number;
|
||||
owner_failure_count?: number;
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
task_done_then_user_reopen_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
status?: PairedTaskStatus;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
@@ -333,6 +332,10 @@ export function requestArbiterOrEscalate(args: {
|
||||
review_requested_at?: string | null;
|
||||
round_trip_count?: number;
|
||||
owner_failure_count?: number;
|
||||
owner_step_done_streak?: number;
|
||||
finalize_step_done_count?: number;
|
||||
task_done_then_user_reopen_count?: number;
|
||||
empty_step_done_streak?: number;
|
||||
arbiter_verdict?: string | null;
|
||||
arbiter_requested_at?: string | null;
|
||||
completion_reason?: string | null;
|
||||
|
||||
@@ -143,6 +143,10 @@ function buildPairedTask(overrides: Partial<PairedTask> = {}): PairedTask {
|
||||
review_requested_at: null,
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: 0,
|
||||
task_done_then_user_reopen_count: 0,
|
||||
empty_step_done_streak: 0,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
@@ -293,6 +297,62 @@ describe('paired execution context', () => {
|
||||
expect(db.insertPairedTurnOutput).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('records a quick reopen when a new owner task starts shortly after TASK_DONE completion', () => {
|
||||
const previousTask = buildPairedTask({
|
||||
id: 'task-completed',
|
||||
status: 'completed',
|
||||
completion_reason: 'done',
|
||||
updated_at: new Date(Date.now() - 60_000).toISOString(),
|
||||
task_done_then_user_reopen_count: 0,
|
||||
});
|
||||
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(undefined);
|
||||
vi.mocked(db.getLatestPairedTaskForChat).mockReturnValue(previousTask);
|
||||
|
||||
const result = resolveOwnerTaskForHumanMessage({
|
||||
group,
|
||||
chatJid: 'dc:test',
|
||||
roomRoleContext: ownerContext,
|
||||
existingTask: null,
|
||||
});
|
||||
|
||||
expect(result.task).not.toBeNull();
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-completed',
|
||||
expect.objectContaining({
|
||||
task_done_then_user_reopen_count: 1,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('resets active STEP_DONE loop counters when a new human message continues an active task', () => {
|
||||
vi.mocked(db.getLatestOpenPairedTaskForChat).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
owner_failure_count: 1,
|
||||
owner_step_done_streak: 2,
|
||||
empty_step_done_streak: 2,
|
||||
}),
|
||||
);
|
||||
|
||||
preparePairedExecutionContext({
|
||||
group,
|
||||
chatJid: 'dc:test',
|
||||
runId: 'run-human-reset-step-done-loop',
|
||||
roomRoleContext: ownerContext,
|
||||
hasHumanMessage: true,
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
empty_step_done_streak: 0,
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('uses room role context agent overrides when creating a paired task', () => {
|
||||
preparePairedExecutionContext({
|
||||
group,
|
||||
@@ -742,6 +802,184 @@ describe('paired execution context', () => {
|
||||
).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('requests review when the owner reports STEP_DONE in active mode', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
round_trip_count: 1,
|
||||
}),
|
||||
);
|
||||
vi.mocked(pairedWorkspaceManager.markPairedTaskReviewReady).mockReturnValue(
|
||||
{
|
||||
ownerWorkspace: buildWorkspace('owner', '/tmp/paired/task-1/owner'),
|
||||
reviewerWorkspace: buildWorkspace(
|
||||
'reviewer',
|
||||
'/tmp/paired/task-1/reviewer',
|
||||
),
|
||||
},
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'succeeded',
|
||||
summary: 'STEP_DONE\n1단계 완료, 후속 작업 계속',
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
round_trip_count: 2,
|
||||
owner_step_done_streak: 0,
|
||||
empty_step_done_streak: 0,
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
pairedWorkspaceManager.markPairedTaskReviewReady,
|
||||
).toHaveBeenCalledWith('task-1');
|
||||
});
|
||||
|
||||
it('requests review instead of arbiter when active STEP_DONE repeats without code changes', () => {
|
||||
vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true);
|
||||
|
||||
const repoDir = createCanonicalRepoWithCommit('active step done loop');
|
||||
const sourceRef = resolveTreeRef(repoDir);
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'active',
|
||||
source_ref: sourceRef,
|
||||
owner_step_done_streak: 2,
|
||||
empty_step_done_streak: 2,
|
||||
}),
|
||||
);
|
||||
vi.mocked(db.getPairedWorkspace).mockImplementation((_taskId, role) =>
|
||||
role === 'owner' ? buildWorkspace('owner', repoDir) : undefined,
|
||||
);
|
||||
vi.mocked(pairedWorkspaceManager.markPairedTaskReviewReady).mockReturnValue(
|
||||
{
|
||||
ownerWorkspace: buildWorkspace('owner', repoDir),
|
||||
reviewerWorkspace: buildWorkspace(
|
||||
'reviewer',
|
||||
'/tmp/paired/task-1/reviewer',
|
||||
),
|
||||
},
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'succeeded',
|
||||
summary: 'STEP_DONE\n요약만 반복되고 코드 변경은 없음',
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
round_trip_count: 1,
|
||||
owner_step_done_streak: 0,
|
||||
empty_step_done_streak: 0,
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
pairedWorkspaceManager.markPairedTaskReviewReady,
|
||||
).toHaveBeenCalledWith('task-1');
|
||||
expect(db.updatePairedTask).not.toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'arbiter_requested',
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it('re-triggers review when the owner reports STEP_DONE during finalize', () => {
|
||||
const repoDir = createCanonicalRepoWithCommit('reviewed');
|
||||
const approvedSourceRef = resolveTreeRef(repoDir);
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'merge_ready',
|
||||
source_ref: approvedSourceRef,
|
||||
round_trip_count: 1,
|
||||
}),
|
||||
);
|
||||
vi.mocked(db.getPairedWorkspace).mockImplementation((_taskId, role) =>
|
||||
role === 'owner' ? buildWorkspace('owner', repoDir) : undefined,
|
||||
);
|
||||
vi.mocked(pairedWorkspaceManager.markPairedTaskReviewReady).mockReturnValue(
|
||||
{
|
||||
ownerWorkspace: buildWorkspace('owner', repoDir),
|
||||
reviewerWorkspace: buildWorkspace(
|
||||
'reviewer',
|
||||
'/tmp/paired/task-1/reviewer',
|
||||
),
|
||||
},
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'succeeded',
|
||||
summary: 'STEP_DONE\n남은 범위가 있어서 계속 진행',
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'active',
|
||||
owner_failure_count: 0,
|
||||
finalize_step_done_count: 1,
|
||||
empty_step_done_streak: 1,
|
||||
}),
|
||||
);
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
round_trip_count: 2,
|
||||
finalize_step_done_count: 1,
|
||||
empty_step_done_streak: 1,
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
pairedWorkspaceManager.markPairedTaskReviewReady,
|
||||
).toHaveBeenCalledWith('task-1');
|
||||
});
|
||||
|
||||
it('requests arbiter when finalize STEP_DONE repeats without code changes', () => {
|
||||
vi.spyOn(config, 'isArbiterEnabled').mockReturnValue(true);
|
||||
|
||||
const repoDir = createCanonicalRepoWithCommit('reviewed');
|
||||
const approvedSourceRef = resolveTreeRef(repoDir);
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
status: 'merge_ready',
|
||||
source_ref: approvedSourceRef,
|
||||
empty_step_done_streak: 1,
|
||||
finalize_step_done_count: 1,
|
||||
}),
|
||||
);
|
||||
vi.mocked(db.getPairedWorkspace).mockImplementation((_taskId, role) =>
|
||||
role === 'owner' ? buildWorkspace('owner', repoDir) : undefined,
|
||||
);
|
||||
|
||||
completePairedExecutionContext({
|
||||
taskId: 'task-1',
|
||||
role: 'owner',
|
||||
status: 'succeeded',
|
||||
summary: 'STEP_DONE\n아직 남았지만 코드 변경은 없음',
|
||||
});
|
||||
|
||||
expect(db.updatePairedTask).toHaveBeenCalledWith(
|
||||
'task-1',
|
||||
expect.objectContaining({
|
||||
status: 'arbiter_requested',
|
||||
empty_step_done_streak: 2,
|
||||
finalize_step_done_count: 2,
|
||||
}),
|
||||
);
|
||||
expect(
|
||||
pairedWorkspaceManager.markPairedTaskReviewReady,
|
||||
).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('re-triggers review once when owner reports DONE_WITH_CONCERNS during finalize', () => {
|
||||
vi.mocked(db.getPairedTaskById).mockReturnValue(
|
||||
buildPairedTask({
|
||||
|
||||
@@ -30,6 +30,7 @@ import {
|
||||
hasActiveCiWatcherForChat,
|
||||
insertPairedTurnOutput,
|
||||
releasePairedTaskExecutionLease,
|
||||
updatePairedTask,
|
||||
upsertPairedProject,
|
||||
} from './db.js';
|
||||
import { logger } from './logger.js';
|
||||
@@ -72,6 +73,8 @@ import type {
|
||||
} from './types.js';
|
||||
import { resolveRoleAgentPlan } from './role-agent-plan.js';
|
||||
|
||||
const TASK_DONE_REOPEN_WINDOW_MS = 10 * 60_000;
|
||||
|
||||
function ensurePairedProject(
|
||||
group: RegisteredGroup,
|
||||
chatJid: string,
|
||||
@@ -144,6 +147,10 @@ function createActiveTaskForRoom(args: {
|
||||
review_requested_at: null,
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
finalize_step_done_count: 0,
|
||||
task_done_then_user_reopen_count: 0,
|
||||
empty_step_done_streak: 0,
|
||||
status: 'active',
|
||||
arbiter_verdict: null,
|
||||
arbiter_requested_at: null,
|
||||
@@ -164,6 +171,39 @@ function createActiveTaskForRoom(args: {
|
||||
return task;
|
||||
}
|
||||
|
||||
function maybeRecordTaskDoneReopen(previousTask: PairedTask | null): void {
|
||||
if (
|
||||
!previousTask ||
|
||||
previousTask.status !== 'completed' ||
|
||||
previousTask.completion_reason !== 'done'
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
const completedAt = Date.parse(previousTask.updated_at);
|
||||
if (!Number.isFinite(completedAt)) {
|
||||
return;
|
||||
}
|
||||
if (Date.now() - completedAt > TASK_DONE_REOPEN_WINDOW_MS) {
|
||||
return;
|
||||
}
|
||||
|
||||
updatePairedTask(previousTask.id, {
|
||||
task_done_then_user_reopen_count:
|
||||
(previousTask.task_done_then_user_reopen_count ?? 0) + 1,
|
||||
updated_at: new Date().toISOString(),
|
||||
});
|
||||
logger.info(
|
||||
{
|
||||
taskId: previousTask.id,
|
||||
chatJid: previousTask.chat_jid,
|
||||
reopenCount: (previousTask.task_done_then_user_reopen_count ?? 0) + 1,
|
||||
completionReason: previousTask.completion_reason,
|
||||
},
|
||||
'Recorded paired task reopen shortly after TASK_DONE completion',
|
||||
);
|
||||
}
|
||||
|
||||
function cancelOutstandingFinalizeOwnerTurn(task: PairedTask): void {
|
||||
const turnIdentity = buildPairedTurnIdentity({
|
||||
taskId: task.id,
|
||||
@@ -253,6 +293,7 @@ export function resolveOwnerTaskForHumanMessage(args: {
|
||||
args.existingTask ?? getLatestOpenPairedTaskForChat(args.chatJid) ?? null;
|
||||
|
||||
if (!existing) {
|
||||
maybeRecordTaskDoneReopen(getLatestPairedTaskForChat(args.chatJid) ?? null);
|
||||
return {
|
||||
task: canonicalWorkDir
|
||||
? createActiveTaskForRoom({
|
||||
@@ -432,7 +473,12 @@ export function preparePairedExecutionContext(args: {
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
...(hasHuman
|
||||
? { round_trip_count: 0, owner_failure_count: 0 }
|
||||
? {
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
empty_step_done_streak: 0,
|
||||
}
|
||||
: {}),
|
||||
},
|
||||
});
|
||||
@@ -443,7 +489,12 @@ export function preparePairedExecutionContext(args: {
|
||||
updatedAt: now,
|
||||
patch: {
|
||||
...(hasHuman
|
||||
? { round_trip_count: 0, owner_failure_count: 0 }
|
||||
? {
|
||||
round_trip_count: 0,
|
||||
owner_failure_count: 0,
|
||||
owner_step_done_streak: 0,
|
||||
empty_step_done_streak: 0,
|
||||
}
|
||||
: {}),
|
||||
},
|
||||
});
|
||||
|
||||
40
src/paired-verdict.ts
Normal file
40
src/paired-verdict.ts
Normal file
@@ -0,0 +1,40 @@
|
||||
export type VisibleVerdict =
|
||||
| 'step_done'
|
||||
| 'task_done'
|
||||
| 'done'
|
||||
| 'done_with_concerns'
|
||||
| 'blocked'
|
||||
| 'needs_context'
|
||||
| 'continue';
|
||||
|
||||
export function parseVisibleVerdict(
|
||||
summary: string | null | undefined,
|
||||
): VisibleVerdict {
|
||||
if (!summary) return 'continue';
|
||||
const cleaned = summary.replace(/<internal>[\s\S]*?<\/internal>/g, '').trim();
|
||||
if (!cleaned) return 'continue';
|
||||
const firstLine = cleaned.split('\n')[0].trim();
|
||||
if (/^\*{0,2}BLOCKED\*{0,2}\b/i.test(firstLine)) return 'blocked';
|
||||
if (/^\*{0,2}NEEDS_CONTEXT\*{0,2}\b/i.test(firstLine)) return 'needs_context';
|
||||
if (/^\*{0,2}STEP_DONE\*{0,2}\b/i.test(firstLine)) return 'step_done';
|
||||
if (/^\*{0,2}TASK_DONE\*{0,2}\b/i.test(firstLine)) return 'task_done';
|
||||
if (/^\*{0,2}DONE_WITH_CONCERNS\*{0,2}\b/i.test(firstLine))
|
||||
return 'done_with_concerns';
|
||||
if (/^\*{0,2}DONE\*{0,2}\b/i.test(firstLine)) return 'done';
|
||||
if (/^\*{0,2}Approved\.?\*{0,2}/i.test(firstLine)) return 'done';
|
||||
if (/^\*{0,2}LGTM\*{0,2}/i.test(firstLine)) return 'done';
|
||||
return 'continue';
|
||||
}
|
||||
|
||||
export function resolveStoredVisibleVerdict(args: {
|
||||
verdict?: VisibleVerdict | null;
|
||||
outputText?: string | null;
|
||||
}): VisibleVerdict | null {
|
||||
if (args.verdict) {
|
||||
return args.verdict;
|
||||
}
|
||||
if (!args.outputText) {
|
||||
return null;
|
||||
}
|
||||
return parseVisibleVerdict(args.outputText);
|
||||
}
|
||||
@@ -1,3 +1,5 @@
|
||||
import type { VisibleVerdict } from './paired-verdict.js';
|
||||
|
||||
export interface AgentConfig {
|
||||
timeout?: number; // Default: 300000 (5 minutes)
|
||||
// Per-group model/effort overrides (take precedence over global env vars)
|
||||
@@ -83,6 +85,10 @@ export interface PairedTask {
|
||||
review_requested_at: string | null;
|
||||
round_trip_count: number;
|
||||
owner_failure_count?: number | null;
|
||||
owner_step_done_streak?: number | null;
|
||||
finalize_step_done_count?: number | null;
|
||||
task_done_then_user_reopen_count?: number | null;
|
||||
empty_step_done_streak?: number | null;
|
||||
status: PairedTaskStatus;
|
||||
arbiter_verdict: string | null;
|
||||
arbiter_requested_at: string | null;
|
||||
@@ -97,6 +103,7 @@ export interface PairedTurnOutput {
|
||||
turn_number: number;
|
||||
role: PairedRoomRole;
|
||||
output_text: string;
|
||||
verdict?: VisibleVerdict | null;
|
||||
created_at: string;
|
||||
}
|
||||
|
||||
|
||||
@@ -20,7 +20,7 @@ describe('verification helpers', () => {
|
||||
expect(isVerificationProfile('test')).toBe(true);
|
||||
expect(isVerificationProfile('typecheck')).toBe(true);
|
||||
expect(isVerificationProfile('build')).toBe(true);
|
||||
expect(isVerificationProfile('lint')).toBe(false);
|
||||
expect(isVerificationProfile('lint')).toBe(true);
|
||||
});
|
||||
|
||||
it('builds deterministic commands for each profile', () => {
|
||||
@@ -44,6 +44,49 @@ describe('verification helpers', () => {
|
||||
args: ['run', 'build'],
|
||||
requiredScript: 'build',
|
||||
});
|
||||
expect(buildVerificationCommand('lint', repoDir)).toMatchObject({
|
||||
file: 'npm',
|
||||
args: ['run', 'lint'],
|
||||
requiredScript: 'lint',
|
||||
});
|
||||
});
|
||||
|
||||
it('prefers lint:ci and lint:check before plain lint', () => {
|
||||
const repoDir = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), 'ejclaw-verification-lint-'),
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(repoDir, 'package.json'),
|
||||
JSON.stringify({
|
||||
scripts: {
|
||||
lint: 'eslint .',
|
||||
'lint:check': 'eslint --max-warnings 0 .',
|
||||
'lint:ci': 'eslint --format compact .',
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(buildVerificationCommand('lint', repoDir)).toMatchObject({
|
||||
file: 'npm',
|
||||
args: ['run', 'lint:ci'],
|
||||
requiredScript: 'lint:ci',
|
||||
});
|
||||
|
||||
fs.writeFileSync(
|
||||
path.join(repoDir, 'package.json'),
|
||||
JSON.stringify({
|
||||
scripts: {
|
||||
lint: 'eslint .',
|
||||
'lint:check': 'eslint --max-warnings 0 .',
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
expect(buildVerificationCommand('lint', repoDir)).toMatchObject({
|
||||
file: 'npm',
|
||||
args: ['run', 'lint:check'],
|
||||
requiredScript: 'lint:check',
|
||||
});
|
||||
});
|
||||
|
||||
it('selects the workspace package manager for verification commands', () => {
|
||||
@@ -250,6 +293,53 @@ describe('verification helpers', () => {
|
||||
expect(result.snapshotId).toBe(expectedSnapshotId);
|
||||
});
|
||||
|
||||
it('runs lint verification using the preferred lint script', async () => {
|
||||
const repoDir = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), 'ejclaw-verification-direct-lint-'),
|
||||
);
|
||||
fs.mkdirSync(path.join(repoDir, 'node_modules', '.bin'), {
|
||||
recursive: true,
|
||||
});
|
||||
fs.mkdirSync(path.join(repoDir, 'src'), { recursive: true });
|
||||
fs.writeFileSync(
|
||||
path.join(repoDir, 'package.json'),
|
||||
JSON.stringify({
|
||||
name: 'verification-direct-lint',
|
||||
packageManager: 'bun@1.3.11',
|
||||
scripts: {
|
||||
'lint:check':
|
||||
"node -e \"process.stdout.write('lint-check:' + (process.env.CI || 'missing'))\"",
|
||||
},
|
||||
}),
|
||||
);
|
||||
fs.writeFileSync(path.join(repoDir, 'bun.lock'), '');
|
||||
fs.writeFileSync(
|
||||
path.join(repoDir, 'node_modules', '.bin', 'placeholder'),
|
||||
'',
|
||||
);
|
||||
fs.writeFileSync(
|
||||
path.join(repoDir, 'src', 'index.ts'),
|
||||
'export const value = 1;\n',
|
||||
);
|
||||
ensureWorkspaceDependenciesInstalled(repoDir);
|
||||
|
||||
const expectedSnapshotId = computeVerificationSnapshot(repoDir).snapshotId;
|
||||
const result = await runVerificationRequest(
|
||||
{
|
||||
requestId: 'req-direct-lint',
|
||||
profile: 'lint',
|
||||
expectedSnapshotId,
|
||||
},
|
||||
{ repoDir },
|
||||
);
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.command).toContain('lint:check');
|
||||
expect(result.stdout).toContain('lint-check:1');
|
||||
expect(result.snapshotId).toBe(expectedSnapshotId);
|
||||
});
|
||||
|
||||
it('does not leak host secrets into direct verification commands', async () => {
|
||||
const repoDir = fs.mkdtempSync(
|
||||
path.join(os.tmpdir(), 'ejclaw-verification-direct-secret-'),
|
||||
|
||||
@@ -11,7 +11,12 @@ import {
|
||||
} from './workspace-package-manager.js';
|
||||
import { computeVerificationSnapshotId } from '../shared/verification-snapshot.js';
|
||||
|
||||
export const VERIFICATION_PROFILES = ['test', 'typecheck', 'build'] as const;
|
||||
export const VERIFICATION_PROFILES = [
|
||||
'test',
|
||||
'typecheck',
|
||||
'build',
|
||||
'lint',
|
||||
] as const;
|
||||
|
||||
export type VerificationProfile = (typeof VERIFICATION_PROFILES)[number];
|
||||
|
||||
@@ -91,12 +96,19 @@ export function buildVerificationCommand(
|
||||
profile: VerificationProfile,
|
||||
repoDir: string = process.cwd(),
|
||||
): VerificationCommandSpec {
|
||||
const scripts = readPackageScripts(repoDir);
|
||||
const scriptName =
|
||||
profile === 'test'
|
||||
? 'test'
|
||||
: profile === 'typecheck'
|
||||
? 'typecheck'
|
||||
: 'build';
|
||||
: profile === 'build'
|
||||
? 'build'
|
||||
: scripts['lint:ci']
|
||||
? 'lint:ci'
|
||||
: scripts['lint:check']
|
||||
? 'lint:check'
|
||||
: 'lint';
|
||||
const command = buildWorkspaceScriptCommand(repoDir, scriptName);
|
||||
return {
|
||||
file: command.file,
|
||||
|
||||
Reference in New Issue
Block a user