refactor: centralize paired room role normalization (#193)
This commit is contained in:
@@ -10,7 +10,7 @@ import { z } from 'zod';
|
|||||||
import fs from 'fs';
|
import fs from 'fs';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
import { CronExpressionParser } from 'cron-parser';
|
import { CronExpressionParser } from 'cron-parser';
|
||||||
import { EJCLAW_ENV } from 'ejclaw-runners-shared';
|
import { EJCLAW_ENV, normalizePairedRoomRole } from 'ejclaw-runners-shared';
|
||||||
import {
|
import {
|
||||||
buildCiWatchPrompt,
|
buildCiWatchPrompt,
|
||||||
DEFAULT_WATCH_CI_CONTEXT_MODE,
|
DEFAULT_WATCH_CI_CONTEXT_MODE,
|
||||||
@@ -51,11 +51,7 @@ function currentAgentType(): 'claude-code' | 'codex' {
|
|||||||
}
|
}
|
||||||
|
|
||||||
function currentRoomRole(): 'owner' | 'reviewer' | 'arbiter' | undefined {
|
function currentRoomRole(): 'owner' | 'reviewer' | 'arbiter' | undefined {
|
||||||
return roomRole === 'owner' ||
|
return normalizePairedRoomRole(roomRole);
|
||||||
roomRole === 'reviewer' ||
|
|
||||||
roomRole === 'arbiter'
|
|
||||||
? roomRole
|
|
||||||
: undefined;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
function writeIpcFile(dir: string, data: object): string {
|
function writeIpcFile(dir: string, data: object): string {
|
||||||
|
|||||||
@@ -3,6 +3,13 @@ export {
|
|||||||
type RoomRoleContext,
|
type RoomRoleContext,
|
||||||
} from './room-role-context.js';
|
} from './room-role-context.js';
|
||||||
export { EJCLAW_ENV, type EjclawEnvName } from './ejclaw-env.js';
|
export { EJCLAW_ENV, type EjclawEnvName } from './ejclaw-env.js';
|
||||||
|
export {
|
||||||
|
isPairedRoomRole,
|
||||||
|
normalizePairedRoomRole,
|
||||||
|
normalizePairedRoomRoleOrNull,
|
||||||
|
PAIRED_ROOM_ROLES,
|
||||||
|
type PairedRoomRole,
|
||||||
|
} from './paired-room-role.js';
|
||||||
export {
|
export {
|
||||||
extractMarkdownImageAttachments,
|
extractMarkdownImageAttachments,
|
||||||
extractMediaAttachments,
|
extractMediaAttachments,
|
||||||
|
|||||||
19
runners/shared/src/paired-room-role.ts
Normal file
19
runners/shared/src/paired-room-role.ts
Normal file
@@ -0,0 +1,19 @@
|
|||||||
|
export const PAIRED_ROOM_ROLES = ['owner', 'reviewer', 'arbiter'] as const;
|
||||||
|
|
||||||
|
export type PairedRoomRole = (typeof PAIRED_ROOM_ROLES)[number];
|
||||||
|
|
||||||
|
export function isPairedRoomRole(value: unknown): value is PairedRoomRole {
|
||||||
|
return value === 'owner' || value === 'reviewer' || value === 'arbiter';
|
||||||
|
}
|
||||||
|
|
||||||
|
export function normalizePairedRoomRole(
|
||||||
|
value: unknown,
|
||||||
|
): PairedRoomRole | undefined {
|
||||||
|
return isPairedRoomRole(value) ? value : undefined;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function normalizePairedRoomRoleOrNull(
|
||||||
|
value: unknown,
|
||||||
|
): PairedRoomRole | null {
|
||||||
|
return normalizePairedRoomRole(value) ?? null;
|
||||||
|
}
|
||||||
@@ -1,8 +1,9 @@
|
|||||||
|
import type { PairedRoomRole } from './paired-room-role.js';
|
||||||
import type { RunnerAgentType } from './reviewer-runtime-policy.js';
|
import type { RunnerAgentType } from './reviewer-runtime-policy.js';
|
||||||
|
|
||||||
export interface RoomRoleContext {
|
export interface RoomRoleContext {
|
||||||
serviceId: string;
|
serviceId: string;
|
||||||
role: 'owner' | 'reviewer' | 'arbiter';
|
role: PairedRoomRole;
|
||||||
ownerServiceId: string;
|
ownerServiceId: string;
|
||||||
reviewerServiceId: string;
|
reviewerServiceId: string;
|
||||||
ownerAgentType?: RunnerAgentType;
|
ownerAgentType?: RunnerAgentType;
|
||||||
|
|||||||
26
runners/shared/test/paired-room-role.test.ts
Normal file
26
runners/shared/test/paired-room-role.test.ts
Normal file
@@ -0,0 +1,26 @@
|
|||||||
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
|
import {
|
||||||
|
PAIRED_ROOM_ROLES,
|
||||||
|
isPairedRoomRole,
|
||||||
|
normalizePairedRoomRole,
|
||||||
|
normalizePairedRoomRoleOrNull,
|
||||||
|
} from '../src/paired-room-role.js';
|
||||||
|
|
||||||
|
describe('paired room role helpers', () => {
|
||||||
|
it('defines the supported paired roles', () => {
|
||||||
|
expect(PAIRED_ROOM_ROLES).toEqual(['owner', 'reviewer', 'arbiter']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('recognizes valid paired room roles', () => {
|
||||||
|
expect(isPairedRoomRole('owner')).toBe(true);
|
||||||
|
expect(isPairedRoomRole('reviewer')).toBe(true);
|
||||||
|
expect(isPairedRoomRole('arbiter')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('normalizes unknown values consistently', () => {
|
||||||
|
expect(normalizePairedRoomRole('main')).toBeUndefined();
|
||||||
|
expect(normalizePairedRoomRole(null)).toBeUndefined();
|
||||||
|
expect(normalizePairedRoomRoleOrNull('main')).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -9,7 +9,11 @@ import {
|
|||||||
inferRoleFromServiceShadow,
|
inferRoleFromServiceShadow,
|
||||||
resolveRoleServiceShadow,
|
resolveRoleServiceShadow,
|
||||||
} from '../role-service-shadow.js';
|
} from '../role-service-shadow.js';
|
||||||
import type { AgentType, PairedRoomRole } from '../types.js';
|
import {
|
||||||
|
normalizePairedRoomRoleOrNull,
|
||||||
|
type AgentType,
|
||||||
|
type PairedRoomRole,
|
||||||
|
} from '../types.js';
|
||||||
import { normalizeStoredAgentType } from './room-registration.js';
|
import { normalizeStoredAgentType } from './room-registration.js';
|
||||||
|
|
||||||
interface RequiredRoleMetadataInput {
|
interface RequiredRoleMetadataInput {
|
||||||
@@ -336,14 +340,6 @@ export function readCanonicalChannelOwnerLeaseMetadata(input: {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
function normalizeHandoffRole(
|
|
||||||
role: string | null | undefined,
|
|
||||||
): PairedRoomRole | null {
|
|
||||||
return role === 'owner' || role === 'reviewer' || role === 'arbiter'
|
|
||||||
? role
|
|
||||||
: null;
|
|
||||||
}
|
|
||||||
|
|
||||||
function assertStoredRoleMatchesServiceShadow(args: {
|
function assertStoredRoleMatchesServiceShadow(args: {
|
||||||
context: string;
|
context: string;
|
||||||
storedRole: PairedRoomRole | null;
|
storedRole: PairedRoomRole | null;
|
||||||
@@ -389,11 +385,11 @@ export function fillCanonicalServiceHandoffMetadata(input: {
|
|||||||
}): CanonicalServiceHandoffMetadata {
|
}): CanonicalServiceHandoffMetadata {
|
||||||
const context = `service_handoffs(${input.id})`;
|
const context = `service_handoffs(${input.id})`;
|
||||||
const sourceRole =
|
const sourceRole =
|
||||||
normalizeHandoffRole(input.source_role) ??
|
normalizePairedRoomRoleOrNull(input.source_role) ??
|
||||||
normalizeHandoffRole(input.intended_role);
|
normalizePairedRoomRoleOrNull(input.intended_role);
|
||||||
const targetRole =
|
const targetRole =
|
||||||
normalizeHandoffRole(input.target_role) ??
|
normalizePairedRoomRoleOrNull(input.target_role) ??
|
||||||
normalizeHandoffRole(input.intended_role);
|
normalizePairedRoomRoleOrNull(input.intended_role);
|
||||||
const storedSourceAgentType = normalizeStoredAgentType(
|
const storedSourceAgentType = normalizeStoredAgentType(
|
||||||
input.source_agent_type,
|
input.source_agent_type,
|
||||||
);
|
);
|
||||||
@@ -541,11 +537,11 @@ export function readCanonicalServiceHandoffMetadata(input: {
|
|||||||
}): CanonicalServiceHandoffMetadata {
|
}): CanonicalServiceHandoffMetadata {
|
||||||
const context = `service_handoffs(${input.id})`;
|
const context = `service_handoffs(${input.id})`;
|
||||||
const sourceRole =
|
const sourceRole =
|
||||||
normalizeHandoffRole(input.source_role) ??
|
normalizePairedRoomRoleOrNull(input.source_role) ??
|
||||||
normalizeHandoffRole(input.intended_role);
|
normalizePairedRoomRoleOrNull(input.intended_role);
|
||||||
const targetRole =
|
const targetRole =
|
||||||
normalizeHandoffRole(input.target_role) ??
|
normalizePairedRoomRoleOrNull(input.target_role) ??
|
||||||
normalizeHandoffRole(input.intended_role);
|
normalizePairedRoomRoleOrNull(input.intended_role);
|
||||||
const sourceServiceId = readStoredHandoffServiceId({
|
const sourceServiceId = readStoredHandoffServiceId({
|
||||||
context,
|
context,
|
||||||
field: 'source_service_id',
|
field: 'source_service_id',
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import { resolveRuntimeAttachmentBaseDirs } from './attachment-base-dirs.js';
|
|||||||
import { deliverOpenWorkItem } from './message-runtime-delivery.js';
|
import { deliverOpenWorkItem } from './message-runtime-delivery.js';
|
||||||
import { parseVisibleVerdict } from './paired-verdict.js';
|
import { parseVisibleVerdict } from './paired-verdict.js';
|
||||||
import { resolveChannelForDeliveryRole } from './router.js';
|
import { resolveChannelForDeliveryRole } from './router.js';
|
||||||
|
import { normalizePairedRoomRole } from './types.js';
|
||||||
import type {
|
import type {
|
||||||
Channel,
|
Channel,
|
||||||
OutboundAttachment,
|
OutboundAttachment,
|
||||||
@@ -66,19 +67,6 @@ export type IpcOutboundDeliveryResult =
|
|||||||
| 'queued_retry'
|
| 'queued_retry'
|
||||||
| 'skipped_recorded_terminal';
|
| 'skipped_recorded_terminal';
|
||||||
|
|
||||||
function normalizeDeliveryRole(
|
|
||||||
senderRole: string | undefined,
|
|
||||||
): PairedRoomRole | undefined {
|
|
||||||
if (
|
|
||||||
senderRole === 'owner' ||
|
|
||||||
senderRole === 'reviewer' ||
|
|
||||||
senderRole === 'arbiter'
|
|
||||||
) {
|
|
||||||
return senderRole;
|
|
||||||
}
|
|
||||||
return undefined;
|
|
||||||
}
|
|
||||||
|
|
||||||
function isTerminalStatusMessage(text: string): boolean {
|
function isTerminalStatusMessage(text: string): boolean {
|
||||||
return parseVisibleVerdict(text) !== 'continue';
|
return parseVisibleVerdict(text) !== 'continue';
|
||||||
}
|
}
|
||||||
@@ -143,7 +131,7 @@ export async function deliverIpcOutboundMessage(
|
|||||||
deps: IpcOutboundDeliveryDeps,
|
deps: IpcOutboundDeliveryDeps,
|
||||||
): Promise<IpcOutboundDeliveryResult> {
|
): Promise<IpcOutboundDeliveryResult> {
|
||||||
const log = deps.log ?? logger;
|
const log = deps.log ?? logger;
|
||||||
const deliveryRole = normalizeDeliveryRole(args.senderRole);
|
const deliveryRole = normalizePairedRoomRole(args.senderRole);
|
||||||
if (
|
if (
|
||||||
args.runId &&
|
args.runId &&
|
||||||
(deliveryRole === 'reviewer' || deliveryRole === 'arbiter') &&
|
(deliveryRole === 'reviewer' || deliveryRole === 'arbiter') &&
|
||||||
|
|||||||
@@ -9,7 +9,7 @@ import {
|
|||||||
isWatchCiTask,
|
isWatchCiTask,
|
||||||
} from '../task-watch-status.js';
|
} from '../task-watch-status.js';
|
||||||
import type { IpcDeps, TaskIpcPayload } from '../ipc-types.js';
|
import type { IpcDeps, TaskIpcPayload } from '../ipc-types.js';
|
||||||
import type { PairedRoomRole, RegisteredGroup } from '../types.js';
|
import { normalizePairedRoomRole, type RegisteredGroup } from '../types.js';
|
||||||
|
|
||||||
type RoomBindings = ReturnType<IpcDeps['roomBindings']>;
|
type RoomBindings = ReturnType<IpcDeps['roomBindings']>;
|
||||||
type ScheduleType = 'cron' | 'interval' | 'once';
|
type ScheduleType = 'cron' | 'interval' | 'once';
|
||||||
@@ -20,14 +20,6 @@ interface ResolvedScheduleTarget {
|
|||||||
room: RegisteredGroup;
|
room: RegisteredGroup;
|
||||||
}
|
}
|
||||||
|
|
||||||
function normalizePairedRoomRole(
|
|
||||||
role: string | null | undefined,
|
|
||||||
): PairedRoomRole | undefined {
|
|
||||||
return role === 'owner' || role === 'reviewer' || role === 'arbiter'
|
|
||||||
? role
|
|
||||||
: undefined;
|
|
||||||
}
|
|
||||||
|
|
||||||
export function handleScheduleTask(
|
export function handleScheduleTask(
|
||||||
data: TaskIpcPayload,
|
data: TaskIpcPayload,
|
||||||
sourceGroup: string,
|
sourceGroup: string,
|
||||||
|
|||||||
@@ -1,6 +1,11 @@
|
|||||||
import { describe, expect, it } from 'vitest';
|
import { describe, expect, it } from 'vitest';
|
||||||
|
|
||||||
import { normalizeAgentOutputPhase, toVisiblePhase } from './types.js';
|
import {
|
||||||
|
normalizeAgentOutputPhase,
|
||||||
|
normalizePairedRoomRole,
|
||||||
|
normalizePairedRoomRoleOrNull,
|
||||||
|
toVisiblePhase,
|
||||||
|
} from './types.js';
|
||||||
|
|
||||||
describe('phase helpers', () => {
|
describe('phase helpers', () => {
|
||||||
it('maps agent output phases to visible phases', () => {
|
it('maps agent output phases to visible phases', () => {
|
||||||
@@ -15,3 +20,17 @@ describe('phase helpers', () => {
|
|||||||
expect(normalizeAgentOutputPhase('progress')).toBe('progress');
|
expect(normalizeAgentOutputPhase('progress')).toBe('progress');
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('paired room role helpers', () => {
|
||||||
|
it('normalizes valid paired room roles', () => {
|
||||||
|
expect(normalizePairedRoomRole('owner')).toBe('owner');
|
||||||
|
expect(normalizePairedRoomRole('reviewer')).toBe('reviewer');
|
||||||
|
expect(normalizePairedRoomRole('arbiter')).toBe('arbiter');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('rejects invalid paired room roles', () => {
|
||||||
|
expect(normalizePairedRoomRole('single')).toBeUndefined();
|
||||||
|
expect(normalizePairedRoomRole(undefined)).toBeUndefined();
|
||||||
|
expect(normalizePairedRoomRoleOrNull('codex')).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
17
src/types.ts
17
src/types.ts
@@ -1,5 +1,20 @@
|
|||||||
|
import {
|
||||||
|
PAIRED_ROOM_ROLES,
|
||||||
|
isPairedRoomRole,
|
||||||
|
normalizePairedRoomRole,
|
||||||
|
normalizePairedRoomRoleOrNull,
|
||||||
|
type PairedRoomRole,
|
||||||
|
} from 'ejclaw-runners-shared';
|
||||||
import type { VisibleVerdict } from './paired-verdict.js';
|
import type { VisibleVerdict } from './paired-verdict.js';
|
||||||
|
|
||||||
|
export {
|
||||||
|
PAIRED_ROOM_ROLES,
|
||||||
|
isPairedRoomRole,
|
||||||
|
normalizePairedRoomRole,
|
||||||
|
normalizePairedRoomRoleOrNull,
|
||||||
|
type PairedRoomRole,
|
||||||
|
};
|
||||||
|
|
||||||
export interface AgentConfig {
|
export interface AgentConfig {
|
||||||
timeout?: number; // Default: 300000 (5 minutes)
|
timeout?: number; // Default: 300000 (5 minutes)
|
||||||
// Per-group model/effort overrides (take precedence over global env vars)
|
// Per-group model/effort overrides (take precedence over global env vars)
|
||||||
@@ -55,8 +70,6 @@ export interface DeleteRecentMessagesByContentOptions {
|
|||||||
limit?: number;
|
limit?: number;
|
||||||
}
|
}
|
||||||
|
|
||||||
export type PairedRoomRole = 'owner' | 'reviewer' | 'arbiter';
|
|
||||||
|
|
||||||
export type PairedTaskStatus =
|
export type PairedTaskStatus =
|
||||||
| 'active'
|
| 'active'
|
||||||
| 'review_ready'
|
| 'review_ready'
|
||||||
|
|||||||
Reference in New Issue
Block a user