fix(member-work-sync): guard proof-missing recovery dispatch
This commit is contained in:
parent
d048113c1d
commit
bc571f5fc7
7 changed files with 350 additions and 2 deletions
|
|
@ -61,6 +61,17 @@ function isReviewPickupOutboxItem(item: MemberWorkSyncOutboxItem): boolean {
|
|||
return item.payload.workSyncIntent === 'review_pickup';
|
||||
}
|
||||
|
||||
function getProofMissingRecoveryOriginalMessageId(item: MemberWorkSyncOutboxItem): string | null {
|
||||
const prefix = 'proof-missing:';
|
||||
const intentKey = item.payload.workSyncIntentKey?.trim();
|
||||
if (!intentKey?.startsWith(prefix)) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const originalMessageId = intentKey.slice(prefix.length).trim();
|
||||
return originalMessageId.length > 0 ? originalMessageId : null;
|
||||
}
|
||||
|
||||
function getPayloadReviewRequestEventIds(item: MemberWorkSyncOutboxItem): string[] {
|
||||
return [...new Set(item.payload.workSyncReviewRequestEventIds ?? [])]
|
||||
.filter((id) => id.length > 0)
|
||||
|
|
@ -463,6 +474,11 @@ export class MemberWorkSyncNudgeDispatcher {
|
|||
}
|
||||
}
|
||||
|
||||
const proofMissingRecovery = await this.revalidateProofMissingRecovery(item, nowIso);
|
||||
if (!proofMissingRecovery.ok) {
|
||||
return proofMissingRecovery;
|
||||
}
|
||||
|
||||
const recentDelivered = await this.deps.outboxStore?.countRecentDelivered({
|
||||
teamName: item.teamName,
|
||||
memberName: item.memberName,
|
||||
|
|
@ -513,6 +529,32 @@ export class MemberWorkSyncNudgeDispatcher {
|
|||
return { ok: true, ...(providerId ? { providerId } : {}) };
|
||||
}
|
||||
|
||||
private async revalidateProofMissingRecovery(
|
||||
item: MemberWorkSyncOutboxItem,
|
||||
nowIso: string
|
||||
): Promise<
|
||||
{ ok: true } | { ok: false; reason: string; retryable: boolean; nextAttemptAt?: string }
|
||||
> {
|
||||
const originalMessageId = getProofMissingRecoveryOriginalMessageId(item);
|
||||
if (!originalMessageId) {
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
const guard = this.deps.proofMissingRecoveryGuard;
|
||||
if (!guard) {
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
return guard.shouldDispatch({
|
||||
teamName: item.teamName,
|
||||
memberName: item.memberName,
|
||||
intentKey: item.payload.workSyncIntentKey ?? '',
|
||||
originalMessageId,
|
||||
taskIds: item.payload.taskRefs.map((taskRef) => taskRef.taskId),
|
||||
nowIso,
|
||||
});
|
||||
}
|
||||
|
||||
private async scheduleDeliveryWake(
|
||||
item: MemberWorkSyncOutboxItem,
|
||||
messageId: string,
|
||||
|
|
|
|||
|
|
@ -210,6 +210,19 @@ export interface MemberWorkSyncBusySignalPort {
|
|||
}): Promise<{ busy: boolean; reason?: string; retryAfterIso?: string }>;
|
||||
}
|
||||
|
||||
export interface MemberWorkSyncProofMissingRecoveryGuardPort {
|
||||
shouldDispatch(input: {
|
||||
teamName: string;
|
||||
memberName: string;
|
||||
intentKey: string;
|
||||
originalMessageId: string;
|
||||
taskIds: string[];
|
||||
nowIso: string;
|
||||
}): Promise<
|
||||
{ ok: true } | { ok: false; reason: string; retryable: boolean; nextAttemptAt?: string }
|
||||
>;
|
||||
}
|
||||
|
||||
export interface MemberWorkSyncNudgeDeliveryWakePort {
|
||||
schedule(input: {
|
||||
teamName: string;
|
||||
|
|
@ -281,6 +294,7 @@ export interface MemberWorkSyncUseCaseDeps {
|
|||
inboxNudge?: MemberWorkSyncInboxNudgePort;
|
||||
watchdogCooldown?: MemberWorkSyncWatchdogCooldownPort;
|
||||
busySignal?: MemberWorkSyncBusySignalPort;
|
||||
proofMissingRecoveryGuard?: MemberWorkSyncProofMissingRecoveryGuardPort;
|
||||
nudgeDeliveryWake?: MemberWorkSyncNudgeDeliveryWakePort;
|
||||
reviewPickupDelivery?: MemberWorkSyncReviewPickupDeliveryPort;
|
||||
reviewPickupEscalation?: MemberWorkSyncReviewPickupEscalationPort;
|
||||
|
|
|
|||
|
|
@ -51,6 +51,7 @@ import type {
|
|||
MemberWorkSyncBusySignalPort,
|
||||
MemberWorkSyncLoggerPort,
|
||||
MemberWorkSyncNudgeDeliveryWakePort,
|
||||
MemberWorkSyncProofMissingRecoveryGuardPort,
|
||||
MemberWorkSyncReviewPickupDeliveryPort,
|
||||
MemberWorkSyncReviewPickupEscalationPort,
|
||||
} from '../../core/application';
|
||||
|
|
@ -173,6 +174,7 @@ export function createMemberWorkSyncFeature(deps: {
|
|||
queueQuietWindowMs?: number;
|
||||
runtimeTurnSettledTargetResolver?: RuntimeTurnSettledTargetResolverPort;
|
||||
extraBusySignals?: MemberWorkSyncBusySignalPort[];
|
||||
proofMissingRecoveryGuard?: MemberWorkSyncProofMissingRecoveryGuardPort;
|
||||
nudgeDeliveryWake?: MemberWorkSyncNudgeDeliveryWakePort;
|
||||
reviewPickupDelivery?: MemberWorkSyncReviewPickupDeliveryPort;
|
||||
reviewPickupEscalation?: MemberWorkSyncReviewPickupEscalationPort;
|
||||
|
|
@ -238,6 +240,9 @@ export function createMemberWorkSyncFeature(deps: {
|
|||
inboxNudge,
|
||||
watchdogCooldown,
|
||||
busySignal,
|
||||
...(deps.proofMissingRecoveryGuard
|
||||
? { proofMissingRecoveryGuard: deps.proofMissingRecoveryGuard }
|
||||
: {}),
|
||||
...(deps.nudgeDeliveryWake ? { nudgeDeliveryWake: deps.nudgeDeliveryWake } : {}),
|
||||
...(deps.reviewPickupDelivery ? { reviewPickupDelivery: deps.reviewPickupDelivery } : {}),
|
||||
...(deps.reviewPickupEscalation ? { reviewPickupEscalation: deps.reviewPickupEscalation } : {}),
|
||||
|
|
@ -382,6 +387,22 @@ export function createMemberWorkSyncFeature(deps: {
|
|||
return { scheduled: false, reason: 'invalid' };
|
||||
}
|
||||
|
||||
const taskRefs = normalizeRecoveryTaskRefs(request.taskRefs);
|
||||
if (taskRefs.length === 0) {
|
||||
await auditJournal.append({
|
||||
timestamp: clock.now().toISOString(),
|
||||
teamName,
|
||||
memberName,
|
||||
event: 'proof_missing_recovery_suppressed',
|
||||
source: 'proof_missing_recovery_scheduler',
|
||||
reason: 'missing_task_refs',
|
||||
metadata: {
|
||||
originalMessageId,
|
||||
},
|
||||
});
|
||||
return { scheduled: false, reason: 'invalid' };
|
||||
}
|
||||
|
||||
const intentKey = buildProofMissingRecoveryIntentKey(originalMessageId);
|
||||
const sinceIso = new Date(
|
||||
clock.now().getTime() - PROOF_MISSING_RECOVERY_RECENT_WINDOW_MS
|
||||
|
|
@ -414,7 +435,6 @@ export function createMemberWorkSyncFeature(deps: {
|
|||
};
|
||||
}
|
||||
|
||||
const taskRefs = normalizeRecoveryTaskRefs(request.taskRefs);
|
||||
await auditJournal.append({
|
||||
timestamp: clock.now().toISOString(),
|
||||
teamName,
|
||||
|
|
|
|||
|
|
@ -1682,6 +1682,48 @@ async function initializeServices(): Promise<void> {
|
|||
isBusy: (input) => teamProvisioningService.getOpenCodeMemberDeliveryBusyStatus(input),
|
||||
},
|
||||
],
|
||||
proofMissingRecoveryGuard: {
|
||||
shouldDispatch: async (input) => {
|
||||
const status = await teamProvisioningService.getOpenCodeRuntimeDeliveryStatus(
|
||||
input.teamName,
|
||||
input.originalMessageId
|
||||
);
|
||||
if (!status) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'proof_missing_recovery_record_missing',
|
||||
retryable: false,
|
||||
};
|
||||
}
|
||||
|
||||
const impact = status.userVisibleImpact;
|
||||
if (impact?.reasonCode === 'protocol_proof_missing') {
|
||||
if (impact.state === 'checking') {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'proof_missing_recovery_still_in_grace',
|
||||
retryable: true,
|
||||
...(impact.nextReviewAt ? { nextAttemptAt: impact.nextReviewAt } : {}),
|
||||
};
|
||||
}
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
if (status.responsePending) {
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'proof_missing_recovery_delivery_still_pending',
|
||||
retryable: true,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
ok: false,
|
||||
reason: 'proof_missing_recovery_suppressed',
|
||||
retryable: false,
|
||||
};
|
||||
},
|
||||
},
|
||||
nudgeDeliveryWake: {
|
||||
schedule: async (input) => {
|
||||
if (input.providerId === 'opencode') {
|
||||
|
|
|
|||
|
|
@ -413,6 +413,7 @@ describe('createMemberWorkSyncFeature composition', () => {
|
|||
teamName,
|
||||
memberName,
|
||||
originalMessageId: 'message-1',
|
||||
taskRefs: [{ taskId: 'task-1', displayId: '11111111', teamName }],
|
||||
})
|
||||
).resolves.toMatchObject({
|
||||
scheduled: false,
|
||||
|
|
@ -425,6 +426,47 @@ describe('createMemberWorkSyncFeature composition', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('does not schedule broad proof-missing recovery without task refs', async () => {
|
||||
const claudeRoot = makeTempRoot();
|
||||
setClaudeBasePathOverride(claudeRoot);
|
||||
const teamsBasePath = getTeamsBasePath();
|
||||
const teamName = 'team-a';
|
||||
const memberName = 'bob';
|
||||
const feature = createMemberWorkSyncFeature({
|
||||
teamsBasePath,
|
||||
configReader: {
|
||||
getConfig: vi.fn(async () => ({
|
||||
name: teamName,
|
||||
members: [{ name: memberName }],
|
||||
})),
|
||||
} as never,
|
||||
taskReader: { getTasks: vi.fn(async () => []) } as never,
|
||||
kanbanManager: {
|
||||
getState: vi.fn(async () => ({ teamName, reviewers: [], tasks: {} })),
|
||||
} as never,
|
||||
membersMetaStore: { getMembers: vi.fn(async () => []) } as never,
|
||||
});
|
||||
|
||||
try {
|
||||
await expect(
|
||||
feature.scheduleProofMissingRecovery({
|
||||
teamName,
|
||||
memberName,
|
||||
originalMessageId: 'message-1',
|
||||
})
|
||||
).resolves.toMatchObject({
|
||||
scheduled: false,
|
||||
reason: 'invalid',
|
||||
});
|
||||
expect(feature.getQueueDiagnostics()).toMatchObject({ queued: 0 });
|
||||
await expect(readInboxMessages({ teamsBasePath, teamName, memberName })).resolves.toEqual(
|
||||
[]
|
||||
);
|
||||
} finally {
|
||||
await feature.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('dispatches a due nudge through the real outbox and inbox by default', async () => {
|
||||
const claudeRoot = makeTempRoot();
|
||||
setClaudeBasePathOverride(claudeRoot);
|
||||
|
|
@ -502,6 +544,104 @@ describe('createMemberWorkSyncFeature composition', () => {
|
|||
}
|
||||
});
|
||||
|
||||
it('suppresses queued proof-missing recovery when the original delivery is no longer proof-missing', async () => {
|
||||
const claudeRoot = makeTempRoot();
|
||||
setClaudeBasePathOverride(claudeRoot);
|
||||
const teamsBasePath = getTeamsBasePath();
|
||||
const teamName = 'team-a';
|
||||
const memberName = 'bob';
|
||||
const proofMissingRecoveryGuard = {
|
||||
shouldDispatch: vi.fn(async () => ({
|
||||
ok: false as const,
|
||||
reason: 'proof_missing_recovery_suppressed',
|
||||
retryable: false,
|
||||
})),
|
||||
};
|
||||
const feature = createMemberWorkSyncFeature({
|
||||
teamsBasePath,
|
||||
configReader: {
|
||||
getConfig: vi.fn(async () => ({
|
||||
name: teamName,
|
||||
members: [{ name: memberName }],
|
||||
})),
|
||||
} as never,
|
||||
taskReader: {
|
||||
getTasks: vi.fn(async () => [
|
||||
{
|
||||
id: 'task-1',
|
||||
displayId: '11111111',
|
||||
subject: 'Ship sync',
|
||||
status: 'pending',
|
||||
owner: memberName,
|
||||
},
|
||||
]),
|
||||
} as never,
|
||||
kanbanManager: {
|
||||
getState: vi.fn(async () => ({
|
||||
teamName,
|
||||
reviewers: [],
|
||||
tasks: {},
|
||||
})),
|
||||
} as never,
|
||||
membersMetaStore: {
|
||||
getMembers: vi.fn(async () => []),
|
||||
} as never,
|
||||
proofMissingRecoveryGuard,
|
||||
});
|
||||
|
||||
try {
|
||||
await seedShadowReadyMetrics({ teamsBasePath, teamName, memberName });
|
||||
const status = await feature.refreshStatus({ teamName, memberName });
|
||||
const store = new JsonMemberWorkSyncStore(new MemberWorkSyncStorePaths(teamsBasePath));
|
||||
await expect(
|
||||
store.ensurePending({
|
||||
id: 'member-work-sync:team-a:bob:proof-missing:message-1',
|
||||
teamName,
|
||||
memberName,
|
||||
agendaFingerprint: status.agenda.fingerprint,
|
||||
payloadHash: 'payload-hash',
|
||||
payload: {
|
||||
from: 'system',
|
||||
to: memberName,
|
||||
messageKind: 'member_work_sync_nudge',
|
||||
source: 'member-work-sync',
|
||||
actionMode: 'do',
|
||||
workSyncIntent: 'agenda_sync',
|
||||
workSyncIntentKey: 'proof-missing:message-1',
|
||||
text: 'Recover proof',
|
||||
taskRefs: [{ taskId: 'task-1', displayId: '11111111', teamName }],
|
||||
},
|
||||
nowIso: status.evaluatedAt,
|
||||
})
|
||||
).resolves.toMatchObject({
|
||||
ok: true,
|
||||
outcome: 'created',
|
||||
});
|
||||
|
||||
await expect(feature.dispatchDueNudges([teamName])).resolves.toEqual({
|
||||
claimed: 1,
|
||||
delivered: 0,
|
||||
superseded: 1,
|
||||
retryable: 0,
|
||||
terminal: 0,
|
||||
});
|
||||
expect(proofMissingRecoveryGuard.shouldDispatch).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
teamName,
|
||||
memberName,
|
||||
intentKey: 'proof-missing:message-1',
|
||||
originalMessageId: 'message-1',
|
||||
taskIds: ['task-1'],
|
||||
})
|
||||
);
|
||||
await expect(readInboxMessages({ teamsBasePath, teamName, memberName })).resolves.toEqual(
|
||||
[]
|
||||
);
|
||||
} finally {
|
||||
await feature.dispose();
|
||||
}
|
||||
});
|
||||
|
||||
it('does not deliver pending nudges until the team is ready for nudge dispatch', async () => {
|
||||
const claudeRoot = makeTempRoot();
|
||||
setClaudeBasePathOverride(claudeRoot);
|
||||
|
|
|
|||
|
|
@ -81,4 +81,53 @@ describe('RuntimeDiagnosticClassifier', () => {
|
|||
actionRequired: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('does not classify message_send Not connected as protocol proof missing', () => {
|
||||
expect(
|
||||
classifyRuntimeDiagnostic(
|
||||
'agent-teams_message_send returned Not connected while sending a visible reply'
|
||||
)
|
||||
).toMatchObject({
|
||||
reasonCode: 'backend_error',
|
||||
actionRequired: false,
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps explicit proof-missing diagnostics narrow', () => {
|
||||
expect(
|
||||
classifyRuntimeDiagnostic(
|
||||
'OpenCode used tools, but did not create a visible reply or task progress proof.'
|
||||
)
|
||||
).toMatchObject({
|
||||
reasonCode: 'protocol_proof_missing',
|
||||
generic: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps quota and auth diagnostics above proof-missing substrings in the same message', () => {
|
||||
expect(
|
||||
classifyRuntimeDiagnostic(
|
||||
'Insufficient credits: OpenCode used tools, but did not create a visible reply or task progress proof.'
|
||||
)
|
||||
).toMatchObject({
|
||||
reasonCode: 'quota_exhausted',
|
||||
actionRequired: true,
|
||||
});
|
||||
|
||||
expect(
|
||||
classifyRuntimeDiagnostic(
|
||||
'authentication_failed: visible_reply_missing_task_refs because API key is invalid'
|
||||
)
|
||||
).toMatchObject({
|
||||
reasonCode: 'auth_error',
|
||||
actionRequired: true,
|
||||
});
|
||||
});
|
||||
|
||||
it('keeps OpenCode bridge command timeout as backend state despite timeout tokens', () => {
|
||||
expect(classifyRuntimeDiagnostic('OpenCode bridge command timed out')).toMatchObject({
|
||||
reasonCode: 'backend_error',
|
||||
generic: true,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -206,6 +206,41 @@ describe('MemberCard OpenCode delivery advisory fixture e2e', () => {
|
|||
'System notice: OpenCode teammate @jack hit a runtime delivery error while handling #task-1.'
|
||||
);
|
||||
});
|
||||
|
||||
it('schedules targeted work-sync recovery for stale protocol proof-missing advisory', async () => {
|
||||
const record = makeDeliveryRecord({
|
||||
failedAt: OLD_FAILURE_ISO,
|
||||
updatedAt: OLD_FAILURE_ISO,
|
||||
lastObservedAt: OLD_FAILURE_ISO,
|
||||
respondedAt: OLD_FAILURE_ISO,
|
||||
responseState: 'responded_non_visible_tool',
|
||||
lastReason: 'non_visible_tool_without_task_progress',
|
||||
diagnostics: [
|
||||
'OpenCode used tools, but did not create a visible reply or task progress proof.',
|
||||
],
|
||||
});
|
||||
await writeDeliveryFixture(record);
|
||||
const scheduleProofMissingRecovery = vi.fn(async () => ({
|
||||
scheduled: true,
|
||||
reason: 'scheduled' as const,
|
||||
intentKey: 'proof-missing:msg-empty-turn',
|
||||
}));
|
||||
|
||||
const sideEffects = await runUserFacingSideEffects(record, scheduleProofMissingRecovery);
|
||||
|
||||
expect(scheduleProofMissingRecovery).toHaveBeenCalledTimes(1);
|
||||
expect(scheduleProofMissingRecovery).toHaveBeenCalledWith({
|
||||
teamName: TEAM_NAME,
|
||||
memberName: MEMBER_NAME,
|
||||
originalMessageId: record.inboxMessageId,
|
||||
taskRefs: record.taskRefs,
|
||||
reason:
|
||||
'OpenCode used tools, but did not create a visible reply or task progress proof.',
|
||||
});
|
||||
expect(sideEffects.addTeamNotification).not.toHaveBeenCalled();
|
||||
expect(sideEffects.sendMessageToRun).not.toHaveBeenCalled();
|
||||
expect(sideEffects.invalidations).toEqual([{ teamName: TEAM_NAME, memberName: MEMBER_NAME }]);
|
||||
});
|
||||
});
|
||||
|
||||
async function readMemberAdvisory(): Promise<MemberRuntimeAdvisory | null> {
|
||||
|
|
@ -252,7 +287,10 @@ async function renderMemberCardText(
|
|||
}
|
||||
|
||||
async function runUserFacingSideEffects(
|
||||
record: OpenCodePromptDeliveryLedgerRecord
|
||||
record: OpenCodePromptDeliveryLedgerRecord,
|
||||
scheduleProofMissingRecovery?: Parameters<
|
||||
TeamProvisioningService['setMemberWorkSyncProofMissingRecoveryScheduler']
|
||||
>[0]
|
||||
): Promise<SideEffectHarness> {
|
||||
const addTeamNotification = vi.fn(() => Promise.resolve(undefined));
|
||||
NotificationManager.setInstance({ addTeamNotification } as never);
|
||||
|
|
@ -269,6 +307,9 @@ async function runUserFacingSideEffects(
|
|||
service.setMemberRuntimeAdvisoryInvalidator((teamName, memberName) => {
|
||||
invalidations.push({ teamName, memberName });
|
||||
});
|
||||
if (scheduleProofMissingRecovery) {
|
||||
service.setMemberWorkSyncProofMissingRecoveryScheduler(scheduleProofMissingRecovery);
|
||||
}
|
||||
access.sendMessageToRun = sendMessageToRun;
|
||||
access.aliveRunByTeam.set(TEAM_NAME, 'lead-run-1');
|
||||
access.runs.set('lead-run-1', {
|
||||
|
|
|
|||
Loading…
Reference in a new issue