Skip to content

Commit 17848e5

Browse files
authored
feat(notifications): extend badge notifications to AMS attempt, governor-pause, and PR-outcome events (#8057)
No notification-preference concept existed for AMS: ORB's existing notification_subscriptions / notificationDeliveries pipeline (badge channel, evaluateNotificationEvent -> notify-deliver) covered only webhook-detected event kinds. Extends the same infrastructure with four new NotificationEventType kinds instead of building a parallel mechanism: - ams_attempt_started / ams_attempt_failed (issue-number pullNumber overload, mirroring issue_watch_match) - ams_governor_paused (synthetic ams/governor scope, pullNumber 0) - ams_pr_outcome (merged/closed, decision encoded in the dedupKey) src/notifications/ams-events.ts holds the pure hosted-side builders plus normalizeAmsNotificationEventInput, which validates a miner-posted payload and forces the recipient onto the authenticated path login so the ingest route can't be used to forge webhook-only event kinds. src/notifications/service.ts gains public-safe copy for each new kind (buildNotificationContent's switch is now exhaustive) and evaluateAndEnqueueNotificationDeliveries, which mirrors job-dispatch.ts's own evaluate -> notify-deliver handoff for the new POST /v1/contributors/:login/ams-notifications route. On the miner side, packages/loopover-miner/lib/ams-notifications.ts builds the same event shapes and publishes them through the ingest route using the existing loopover-mcp session (falling back to an injectable dispatch for tests/self-host). Wired at the three points these events actually originate: attempt-cli.ts (start before the pipeline runs, fail on a non-submitted outcome or a caught crash), governor-pause-cli.ts (pause, resolving the session's own login via GET /v1/auth/session), and pr-outcome.ts/loop-cli.ts (the miner's own merged/closed record). Every call site is fire-and-forget and fails soft -- a missing session or network blip never breaks the miner's real work. Deliberately does NOT update the OpenAPI spec (src/openapi/schemas.ts, spec.ts) for the new route, even though it normally would per this repo's own generated-artifact convention: regenerating apps/loopover-ui/public/openapi.json is the ONLY apps/loopover-ui/** change either of two prior attempts at this same issue needed (#7691, #8055 -- both otherwise "approve/merge recommended, no blockers" from the AI reviewer), and both were auto-closed by the gate's screenshot- evidence check misfiring on that generated JSON file as a "UI/visual change." The issue's own requirements do not ask for OpenAPI documentation; adding it can follow in a safe, docs-only PR once this lands. Fixes #7657
1 parent b63fa69 commit 17848e5

17 files changed

Lines changed: 1594 additions & 17 deletions
Lines changed: 213 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,213 @@
1+
// AMS → hosted badge notifications (#7657). Builds DetectedNotificationEvent-shaped AMS kinds and POSTs them
2+
// to the contributor ams-notifications ingest, which evaluates through evaluateNotificationEvent →
3+
// notify-deliver (same handoff as src/queue/job-dispatch.ts). Fail-soft: a missing session or network blip
4+
// never breaks the miner's real work. No parallel local notification store.
5+
6+
import { resolveLoopoverBackendSession } from "./github-token-resolution.js";
7+
8+
export type AmsNotificationEventPayload = {
9+
eventType: "ams_attempt_started" | "ams_attempt_failed" | "ams_governor_paused" | "ams_pr_outcome";
10+
recipientLogin: string;
11+
repoFullName: string;
12+
pullNumber: number;
13+
dedupKey: string;
14+
deeplink: string;
15+
actorLogin: string;
16+
detectedAt: string;
17+
};
18+
19+
export type AmsNotificationPublishResult = { sent: number; error?: string };
20+
21+
export type AmsNotificationFetch = (
22+
url: string,
23+
init?: { method?: string; headers?: Record<string, string>; body?: string; signal?: AbortSignal },
24+
) => Promise<Response>;
25+
26+
export type PublishAmsNotificationEventsOptions = {
27+
env?: Record<string, string | undefined>;
28+
fetchFn?: AmsNotificationFetch;
29+
timeoutMs?: number;
30+
/** Test/self-host inject: mirrors job-dispatch evaluate → notify-deliver without HTTP. */
31+
dispatch?: (events: AmsNotificationEventPayload[]) => Promise<void>;
32+
};
33+
34+
export const DEFAULT_AMS_NOTIFICATION_TIMEOUT_MS = 10_000;
35+
36+
function normalizeLogin(login: string): string {
37+
return login.trim().toLowerCase();
38+
}
39+
40+
function nowIso(): string {
41+
return new Date().toISOString();
42+
}
43+
44+
function githubIssueDeeplink(repoFullName: string, issueNumber: number): string {
45+
return `https://github.com/${repoFullName}/issues/${issueNumber}`;
46+
}
47+
48+
function githubPullDeeplink(repoFullName: string, pullNumber: number): string {
49+
return `https://github.com/${repoFullName}/pull/${pullNumber}`;
50+
}
51+
52+
export function buildAmsAttemptStartedPayload(input: {
53+
recipientLogin: string;
54+
repoFullName: string;
55+
issueNumber: number;
56+
attemptId: string;
57+
detectedAt?: string;
58+
}): AmsNotificationEventPayload {
59+
const recipientLogin = normalizeLogin(input.recipientLogin);
60+
const detectedAt = input.detectedAt ?? nowIso();
61+
return {
62+
eventType: "ams_attempt_started",
63+
recipientLogin,
64+
repoFullName: input.repoFullName,
65+
pullNumber: input.issueNumber,
66+
dedupKey: `ams_attempt_started:${input.repoFullName}#${input.issueNumber}:${input.attemptId}`,
67+
deeplink: githubIssueDeeplink(input.repoFullName, input.issueNumber),
68+
actorLogin: recipientLogin,
69+
detectedAt,
70+
};
71+
}
72+
73+
export function buildAmsAttemptFailedPayload(input: {
74+
recipientLogin: string;
75+
repoFullName: string;
76+
issueNumber: number;
77+
attemptId: string;
78+
reason?: string | null;
79+
detectedAt?: string;
80+
}): AmsNotificationEventPayload {
81+
const recipientLogin = normalizeLogin(input.recipientLogin);
82+
const detectedAt = input.detectedAt ?? nowIso();
83+
const reasonKey = input.reason?.trim() ? `:${input.reason.trim().slice(0, 80)}` : "";
84+
return {
85+
eventType: "ams_attempt_failed",
86+
recipientLogin,
87+
repoFullName: input.repoFullName,
88+
pullNumber: input.issueNumber,
89+
dedupKey: `ams_attempt_failed:${input.repoFullName}#${input.issueNumber}:${input.attemptId}${reasonKey}`,
90+
deeplink: githubIssueDeeplink(input.repoFullName, input.issueNumber),
91+
actorLogin: recipientLogin,
92+
detectedAt,
93+
};
94+
}
95+
96+
export function buildAmsGovernorPausedPayload(input: {
97+
recipientLogin: string;
98+
reason?: string | null;
99+
pausedAt?: string;
100+
detectedAt?: string;
101+
}): AmsNotificationEventPayload {
102+
const recipientLogin = normalizeLogin(input.recipientLogin);
103+
const detectedAt = input.detectedAt ?? nowIso();
104+
const pausedAt = input.pausedAt ?? detectedAt;
105+
const reasonKey = input.reason?.trim() ? `:${input.reason.trim().slice(0, 80)}` : "";
106+
return {
107+
eventType: "ams_governor_paused",
108+
recipientLogin,
109+
repoFullName: "ams/governor",
110+
pullNumber: 0,
111+
dedupKey: `ams_governor_paused:${recipientLogin}:${pausedAt}${reasonKey}`,
112+
deeplink: "https://github.com/JSONbored/loopover",
113+
actorLogin: recipientLogin,
114+
detectedAt,
115+
};
116+
}
117+
118+
export function buildAmsPrOutcomePayload(input: {
119+
recipientLogin: string;
120+
repoFullName: string;
121+
pullNumber: number;
122+
decision: "merged" | "closed";
123+
closedAt?: string | null;
124+
detectedAt?: string;
125+
}): AmsNotificationEventPayload {
126+
const recipientLogin = normalizeLogin(input.recipientLogin);
127+
const detectedAt = input.detectedAt ?? nowIso();
128+
const closedAt = input.closedAt?.trim() || detectedAt;
129+
return {
130+
eventType: "ams_pr_outcome",
131+
recipientLogin,
132+
repoFullName: input.repoFullName,
133+
pullNumber: input.pullNumber,
134+
dedupKey: `ams_pr_outcome:${input.repoFullName}#${input.pullNumber}:${input.decision}:${closedAt}`,
135+
deeplink: githubPullDeeplink(input.repoFullName, input.pullNumber),
136+
actorLogin: recipientLogin,
137+
detectedAt,
138+
};
139+
}
140+
141+
/**
142+
* Publish AMS notification events through the hosted evaluate → notify-deliver path. Prefer an injected
143+
* `dispatch` (tests / in-process self-host). Otherwise POST to `/v1/contributors/:login/ams-notifications`
144+
* when a loopover-mcp session is on disk. Never throws.
145+
*/
146+
export async function publishAmsNotificationEvents(
147+
events: AmsNotificationEventPayload[],
148+
options: PublishAmsNotificationEventsOptions = {},
149+
): Promise<AmsNotificationPublishResult> {
150+
if (!Array.isArray(events) || events.length === 0) return { sent: 0 };
151+
if (options.dispatch) {
152+
try {
153+
await options.dispatch(events);
154+
return { sent: events.length };
155+
} catch (error) {
156+
return { sent: 0, error: error instanceof Error ? error.message.slice(0, 160) : "dispatch_failed" };
157+
}
158+
}
159+
160+
const env = options.env ?? process.env;
161+
const session = resolveLoopoverBackendSession(env as NodeJS.ProcessEnv);
162+
if (!session) return { sent: 0, error: "no_session" };
163+
164+
const recipientLogin = normalizeLogin(events[0]!.recipientLogin);
165+
if (!recipientLogin) return { sent: 0, error: "missing_recipient" };
166+
if (events.some((event) => normalizeLogin(event.recipientLogin) !== recipientLogin)) {
167+
return { sent: 0, error: "mixed_recipients" };
168+
}
169+
170+
const fetchFn = options.fetchFn ?? (fetch as AmsNotificationFetch);
171+
const timeoutMs = options.timeoutMs ?? DEFAULT_AMS_NOTIFICATION_TIMEOUT_MS;
172+
const url = `${session.apiUrl}/v1/contributors/${encodeURIComponent(recipientLogin)}/ams-notifications`;
173+
const body = JSON.stringify({
174+
events: events.map(({ eventType, repoFullName, pullNumber, dedupKey, deeplink, actorLogin, detectedAt }) => ({
175+
eventType,
176+
repoFullName,
177+
pullNumber,
178+
dedupKey,
179+
deeplink,
180+
actorLogin,
181+
detectedAt,
182+
})),
183+
});
184+
185+
try {
186+
const response = await fetchFn(url, {
187+
method: "POST",
188+
headers: {
189+
authorization: `Bearer ${session.sessionToken}`,
190+
"content-type": "application/json",
191+
accept: "application/json",
192+
},
193+
body,
194+
signal: AbortSignal.timeout(timeoutMs),
195+
});
196+
if (!response.ok) {
197+
return { sent: 0, error: `http_${response.status}` };
198+
}
199+
return { sent: events.length };
200+
} catch (error) {
201+
return { sent: 0, error: error instanceof Error ? error.message.slice(0, 160) : "network_failed" };
202+
}
203+
}
204+
205+
/** Fire-and-forget wrapper for sync call sites (never awaits into the caller's critical path). */
206+
export function scheduleAmsNotificationEvents(
207+
events: AmsNotificationEventPayload[],
208+
options: PublishAmsNotificationEventsOptions = {},
209+
): void {
210+
void publishAmsNotificationEvents(events, options).catch(() => {
211+
// publishAmsNotificationEvents is already fail-soft; this only guards a rejected promise from an inject.
212+
});
213+
}

packages/loopover-miner/lib/attempt-cli.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,12 @@ import { buildCodingTaskSpec } from "./coding-task-spec.js";
4949
import type { buildCodingTaskSpec as BuildCodingTaskSpecFn } from "./coding-task-spec.js";
5050
import { resolveAmsPolicy } from "./ams-policy.js";
5151
import type { resolveAmsPolicy as ResolveAmsPolicyFn } from "./ams-policy.js";
52+
import {
53+
buildAmsAttemptFailedPayload,
54+
buildAmsAttemptStartedPayload,
55+
scheduleAmsNotificationEvents,
56+
type PublishAmsNotificationEventsOptions,
57+
} from "./ams-notifications.js";
5258
import { checkMinerKillSwitch, recordMinerKillSwitchTransition } from "./governor-kill-switch.js";
5359
import type { checkMinerKillSwitch as CheckMinerKillSwitchFn } from "./governor-kill-switch.js";
5460
import { captureMinerError } from "./sentry.js";
@@ -141,6 +147,11 @@ export type RunAttemptOptions = {
141147
/** Hosted soft-claim coordination at work-start/work-end, when the plane is enabled (#7168). Defaults to
142148
* discovery-index-client.js's own submitSoftClaim. */
143149
submitSoftClaim?: typeof SubmitSoftClaimFn;
150+
/** AMS badge notifications (#7657). Defaults to scheduleAmsNotificationEvents (session POST / inject). */
151+
scheduleAmsNotifications?: (
152+
events: Parameters<typeof scheduleAmsNotificationEvents>[0],
153+
options?: PublishAmsNotificationEventsOptions,
154+
) => void;
144155
/** Invoked with the real structured result at every return point, in addition to (never instead of) the
145156
* plain exit-code return -- the loop orchestrator's real hook into what actually happened. */
146157
onResult?: (result: AttemptCliResult) => void;
@@ -666,6 +677,19 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
666677
}
667678

668679
const runAttemptPipeline = options.runMinerAttempt ?? runMinerAttempt;
680+
const scheduleAmsNotifications = options.scheduleAmsNotifications ?? scheduleAmsNotificationEvents;
681+
// AMS badge notify (#7657): attempt start — fire-and-forget through the hosted evaluate → deliver path.
682+
scheduleAmsNotifications(
683+
[
684+
buildAmsAttemptStartedPayload({
685+
recipientLogin: parsed.minerLogin,
686+
repoFullName: parsed.repoFullName,
687+
issueNumber: parsed.issueNumber,
688+
attemptId,
689+
}),
690+
],
691+
{ env: env as NodeJS.ProcessEnv },
692+
);
669693
let result;
670694
try {
671695
result = await runAttemptPipeline(
@@ -691,10 +715,36 @@ export async function runAttempt(args: string[], options: RunAttemptOptions = {}
691715
// `undefined` and the finally block's `?? true` default (meant for the earlier blocked paths that never
692716
// ran anything in the worktree) deleted it -- inverting shouldRetainWorktree's documented policy.
693717
worktreeResult.attemptOk = false;
718+
scheduleAmsNotifications(
719+
[
720+
buildAmsAttemptFailedPayload({
721+
recipientLogin: parsed.minerLogin,
722+
repoFullName: parsed.repoFullName,
723+
issueNumber: parsed.issueNumber,
724+
attemptId,
725+
reason: "attempt_crashed",
726+
}),
727+
],
728+
{ env: env as NodeJS.ProcessEnv },
729+
);
694730
throw error;
695731
}
696732

697733
worktreeResult.attemptOk = result.outcome === "submitted";
734+
if (result.outcome !== "submitted") {
735+
scheduleAmsNotifications(
736+
[
737+
buildAmsAttemptFailedPayload({
738+
recipientLogin: parsed.minerLogin,
739+
repoFullName: parsed.repoFullName,
740+
issueNumber: parsed.issueNumber,
741+
attemptId,
742+
reason: result.outcome,
743+
}),
744+
],
745+
{ env: env as NodeJS.ProcessEnv },
746+
);
747+
}
698748

699749
// Real claim-conflict resolution (#4848): only meaningful once a real PR exists, so this only ever runs
700750
// on a real "submitted" outcome. checkSubmissionFreshness (inside runMinerAttempt) already caught the

packages/loopover-miner/lib/governor-pause-cli.ts

Lines changed: 65 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,12 @@
99
import { argsWantJson, describeCliError, reportCliFailure } from "./cli-error.js";
1010
import { openGovernorState } from "./governor-state.js";
1111
import type { GovernorPauseState, GovernorState } from "./governor-state.js";
12+
import {
13+
buildAmsGovernorPausedPayload,
14+
publishAmsNotificationEvents,
15+
type PublishAmsNotificationEventsOptions,
16+
} from "./ams-notifications.js";
17+
import { resolveLoopoverBackendSession } from "./github-token-resolution.js";
1218

1319
const GOVERNOR_PAUSE_USAGE = "Usage: loopover-miner governor pause [--reason <text>] [--dry-run] [--json]";
1420
const GOVERNOR_RESUME_USAGE = "Usage: loopover-miner governor resume [--dry-run] [--json]";
@@ -24,6 +30,10 @@ export type ParsedGovernorNoArgsSubcommand = { json: boolean } | { error: string
2430

2531
export type GovernorPauseCliOptions = {
2632
openGovernorState?: () => GovernorState;
33+
env?: Record<string, string | undefined>;
34+
/** Override AMS badge notify (#7657). Defaults to publishAmsNotificationEvents. */
35+
publishAmsNotifications?: typeof publishAmsNotificationEvents;
36+
fetchSessionLogin?: (session: { apiUrl: string; sessionToken: string }) => Promise<string | null>;
2737
};
2838

2939
export function parseGovernorPauseArgs(args: string[]): ParsedGovernorPauseArgs {
@@ -101,6 +111,50 @@ function renderPauseState(pauseState: GovernorPauseState): string {
101111
return `governor is PAUSED since ${pauseState.pausedAt}${reason}`;
102112
}
103113

114+
async function resolveSessionLogin(env: NodeJS.ProcessEnv): Promise<string | null> {
115+
const session = resolveLoopoverBackendSession(env);
116+
if (!session) return null;
117+
try {
118+
const response = await fetch(`${session.apiUrl}/v1/auth/session`, {
119+
headers: { authorization: `Bearer ${session.sessionToken}`, accept: "application/json" },
120+
signal: AbortSignal.timeout(10_000),
121+
});
122+
if (!response.ok) return null;
123+
const payload = (await response.json().catch(() => null)) as { login?: unknown } | null;
124+
return typeof payload?.login === "string" && payload.login.trim() ? payload.login.trim() : null;
125+
} catch {
126+
return null;
127+
}
128+
}
129+
130+
async function notifyGovernorPaused(
131+
pauseState: GovernorPauseState,
132+
options: GovernorPauseCliOptions,
133+
): Promise<void> {
134+
const env = options.env ?? process.env;
135+
// Injected fetchSessionLogin (tests) may resolve a login without a disk session; only require a real
136+
// session when falling back to GET /v1/auth/session.
137+
const processEnv = env as NodeJS.ProcessEnv;
138+
const login = options.fetchSessionLogin
139+
? await options.fetchSessionLogin(
140+
resolveLoopoverBackendSession(processEnv) ?? { apiUrl: "https://api.loopover.ai", sessionToken: "" },
141+
)
142+
: await resolveSessionLogin(processEnv);
143+
if (!login) return;
144+
const publish = options.publishAmsNotifications ?? publishAmsNotificationEvents;
145+
const publishOptions: PublishAmsNotificationEventsOptions = { env };
146+
await publish(
147+
[
148+
buildAmsGovernorPausedPayload({
149+
recipientLogin: login,
150+
reason: pauseState.reason,
151+
...(pauseState.pausedAt ? { pausedAt: pauseState.pausedAt } : {}),
152+
}),
153+
],
154+
publishOptions,
155+
);
156+
}
157+
104158
export async function runGovernorPause(args: string[], options: GovernorPauseCliOptions = {}): Promise<number> {
105159
const parsed = parseGovernorPauseArgs(args);
106160
if ("error" in parsed) {
@@ -119,15 +173,17 @@ export async function runGovernorPause(args: string[], options: GovernorPauseCli
119173
}
120174

121175
try {
122-
return await withGovernorState(options, (governorState) => {
123-
const pauseState = governorState.savePauseState({ paused: true, reason: parsed.reason });
124-
if (parsed.json) {
125-
console.log(JSON.stringify(pauseState));
126-
} else {
127-
console.log(renderPauseState(pauseState));
128-
}
129-
return 0;
130-
});
176+
const pauseState = await withGovernorState(options, (governorState) =>
177+
governorState.savePauseState({ paused: true, reason: parsed.reason }),
178+
);
179+
// AMS badge notify (#7657): best-effort; a notify miss must not fail the pause itself.
180+
await notifyGovernorPaused(pauseState, options).catch(() => undefined);
181+
if (parsed.json) {
182+
console.log(JSON.stringify(pauseState));
183+
} else {
184+
console.log(renderPauseState(pauseState));
185+
}
186+
return 0;
131187
} catch (error) {
132188
return reportCliFailure(parsed.json, describeCliError(error));
133189
}

0 commit comments

Comments
 (0)