Skip to content

Commit 447ea15

Browse files
alphayedalphayed
committed
fix(msgraph-mail-wake): honor Graph's real expiration ceiling, stop duplicate subscriptions, surface Graph errors safely, self-heal orphans
Found by live Microsoft Graph tenant validation: - expiration: real mail ceiling is 10070 min, not 10080 (7d). MAX->10070, default 10000 (clock-skew margin); adaptive one-shot retry clamps to a tenant-reported ceiling and applies the learned ceiling to renew too. - duplicate subscriptions: serialize plugin (re)registrations so concurrent startups can't each create a subscription. - diagnostics: sanitized GraphRequestError (op/status + pattern-gated code) replaces the opaque "error=Error". - orphans: on start, delete Graph subscriptions on our notificationUrl that we no longer track (best-effort, ours-only).
1 parent fe7f8be commit 447ea15

11 files changed

Lines changed: 712 additions & 40 deletions

File tree

‎docs/plugins/msgraph-mail-wake.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,11 +82,11 @@ Per-mailbox options:
8282

8383
Subscription lifecycle options (`subscription`):
8484

85-
| Option | Default | Description |
86-
| ----------------------- | ------- | ---------------------------------------------------------------------------------------------------------- |
87-
| `expirationMinutes` | `10080` | Requested lifetime; Graph can return an earlier expiry, and mail subscriptions expire in under seven days. |
88-
| `renewEveryMinutes` | `1440` | Renewal interval; renewals PATCH the expiration before it lapses. |
89-
| `handleLifecycleEvents` | `true` | Handle Graph lifecycle notifications (`reauthorizationRequired`, `missed`, `subscriptionRemoved`). |
85+
| Option | Default | Description |
86+
| ----------------------- | ------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
87+
| `expirationMinutes` | `10000` | Requested lifetime, in minutes. Graph caps mail subscriptions at 10070 minutes in the future; the default sits below that so clock skew never trips the limit, and the plugin adaptively clamps to a tenant-reported ceiling. Graph can also return an earlier expiry. |
88+
| `renewEveryMinutes` | `1440` | Renewal interval; renewals PATCH the expiration before it lapses. |
89+
| `handleLifecycleEvents` | `true` | Handle Graph lifecycle notifications (`reauthorizationRequired`, `missed`, `subscriptionRemoved`). |
9090

9191
## How it works
9292

‎extensions/msgraph-mail-wake/index.test.ts‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@ const renewSubscriptionMock = vi.fn(async () => ({
1313
expirationDateTime: new Date().toISOString(),
1414
}));
1515
const deleteSubscriptionMock = vi.fn(async () => {});
16+
const listSubscriptionsMock = vi.fn(async () => []);
1617
const createGraphClientMock = vi.fn(
1718
(): GraphClient => ({
1819
createSubscription: createSubscriptionMock as unknown as GraphClient["createSubscription"],
1920
renewSubscription: renewSubscriptionMock as unknown as GraphClient["renewSubscription"],
2021
deleteSubscription: deleteSubscriptionMock as unknown as GraphClient["deleteSubscription"],
22+
listSubscriptions: listSubscriptionsMock as unknown as GraphClient["listSubscriptions"],
2123
fetchMessage: vi.fn(async () => null),
2224
}),
2325
);
@@ -76,6 +78,7 @@ beforeEach(() => {
7678
createSubscriptionMock.mockClear();
7779
renewSubscriptionMock.mockClear();
7880
deleteSubscriptionMock.mockClear();
81+
listSubscriptionsMock.mockClear();
7982
});
8083

8184
describe("msgraph-mail-wake plugin registration", () => {
@@ -155,4 +158,36 @@ describe("msgraph-mail-wake plugin registration", () => {
155158
overflowPolicy: "reject-new",
156159
});
157160
});
161+
162+
it("serializes repeated registration so it never double-creates a subscription", async () => {
163+
// The gateway can call register() more than once per startup. Both calls
164+
// share the same durable store; without serialization their start() calls
165+
// race an empty store and each create a subscription. One create is correct.
166+
const state = new Map<string, unknown>();
167+
const sharedStore = () => ({
168+
register: (key: string, value: unknown) => void state.set(key, value),
169+
lookup: (key: string) => state.get(key),
170+
delete: (key: string) => state.delete(key),
171+
entries: () => [...state.entries()].map(([key, value]) => ({ key, value, createdAt: 0 })),
172+
});
173+
174+
plugin.register(
175+
createApi({ pluginConfig: VALID_CONFIG, openSyncKeyedStore: sharedStore as never }),
176+
);
177+
plugin.register(
178+
createApi({ pluginConfig: VALID_CONFIG, openSyncKeyedStore: sharedStore as never }),
179+
);
180+
181+
// Let both serialized registration chains settle.
182+
await vi.waitFor(() => {
183+
expect(state.get("main")).toBeDefined();
184+
});
185+
await new Promise((resolve) => {
186+
setTimeout(resolve, 0);
187+
});
188+
189+
// First registration creates; the second is superseded (or renews the
190+
// already-persisted record) — never a second create.
191+
expect(createSubscriptionMock).toHaveBeenCalledTimes(1);
192+
});
158193
});

‎extensions/msgraph-mail-wake/index.ts‎

Lines changed: 24 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,11 @@ const AUTH_CONFIG_PATH = "plugins.entries.msgraph-mail-wake.config.auth";
2323
// Re-registration (config reload, gateway restart in the same process) must
2424
// stop the previous manager's timers before a new one starts.
2525
let activeManager: GraphSubscriptionManager | null = null;
26+
// The gateway may call register() more than once per startup (e.g. http-server
27+
// init + agent-runtime pre-warm). Serialize each registration's stop-previous →
28+
// start-new so two starts can never read an empty store concurrently and each
29+
// create a duplicate Graph subscription.
30+
let startChain: Promise<void> = Promise.resolve();
2631

2732
function openSubscriptionStore(api: OpenClawPluginApi): GraphWakeSubscriptionStore {
2833
// Graph subscription ids and clientState values are required after restart
@@ -76,16 +81,25 @@ function registerGraphWake(api: OpenClawPluginApi, config: GraphWakePluginConfig
7681

7782
const previousManager = activeManager;
7883
activeManager = manager;
79-
void (async () => {
80-
if (previousManager) {
81-
await previousManager.stop({ deleteRemote: false });
82-
}
83-
await manager.start();
84-
})().catch((err: unknown) => {
85-
api.logger.error?.(
86-
`[msgraph-mail-wake] subscription_manager_start_failed; error=${describeErrorRedacted(err)}`,
87-
);
88-
});
84+
startChain = startChain
85+
.then(async () => {
86+
if (previousManager) {
87+
await previousManager.stop({ deleteRemote: false });
88+
}
89+
// A later registration may have superseded this one while we awaited the
90+
// previous stop; if so, retire this manager without starting it so we
91+
// never start a manager that is no longer the active one.
92+
if (activeManager !== manager) {
93+
await manager.stop({ deleteRemote: false });
94+
return;
95+
}
96+
await manager.start();
97+
})
98+
.catch((err: unknown) => {
99+
api.logger.error?.(
100+
`[msgraph-mail-wake] subscription_manager_start_failed; error=${describeErrorRedacted(err)}`,
101+
);
102+
});
89103

90104
api.lifecycle.registerRuntimeLifecycle({
91105
id: "msgraph-mail-wake",

‎extensions/msgraph-mail-wake/src/config.test.ts‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import {
66
buildGraphMailboxResource,
77
DEFAULT_GRAPH_WAKE_PATH,
88
DEFAULT_RENEW_EVERY_MINUTES,
9+
DEFAULT_SUBSCRIPTION_EXPIRATION_MINUTES,
910
MAX_DURABLE_GRAPH_MAILBOXES,
1011
MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES,
1112
resolveGraphWakePluginConfig,
@@ -40,7 +41,7 @@ describe("resolveGraphWakePluginConfig", () => {
4041
expect(resolved).not.toBeNull();
4142
expect(resolved?.path).toBe(DEFAULT_GRAPH_WAKE_PATH);
4243
expect(resolved?.subscription).toEqual({
43-
expirationMinutes: MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES,
44+
expirationMinutes: DEFAULT_SUBSCRIPTION_EXPIRATION_MINUTES,
4445
renewEveryMinutes: DEFAULT_RENEW_EVERY_MINUTES,
4546
handleLifecycleEvents: true,
4647
});
@@ -180,7 +181,25 @@ describe("resolveGraphWakePluginConfig", () => {
180181
expect(manifestAcceptsAuth(auth)).toBe(true);
181182
});
182183

184+
it("uses the real Graph mail ceiling (10070) and a clock-skew-safe default (10000)", () => {
185+
// Live Graph rejects 10080 with "can only be 10070 minutes in the future";
186+
// the default sits below the ceiling so clock skew never trips the limit.
187+
expect(MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES).toBe(10_070);
188+
expect(DEFAULT_SUBSCRIPTION_EXPIRATION_MINUTES).toBe(10_000);
189+
expect(DEFAULT_SUBSCRIPTION_EXPIRATION_MINUTES).toBeLessThan(
190+
MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES,
191+
);
192+
});
193+
183194
it("caps subscription expiration at the Graph mail limit", () => {
195+
expect(
196+
resolveGraphWakePluginConfig({
197+
pluginConfig: {
198+
...BASE_CONFIG,
199+
subscription: { expirationMinutes: MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES },
200+
},
201+
})?.subscription.expirationMinutes,
202+
).toBe(MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES);
184203
expect(() =>
185204
resolveGraphWakePluginConfig({
186205
pluginConfig: {

‎extensions/msgraph-mail-wake/src/config.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,14 @@ import { normalizeWebhookPath } from "../runtime-api.js";
55
export const DEFAULT_GRAPH_WAKE_PATH = "/plugins/msgraph-mail-wake";
66
/** One durable SQLite row is required for every enabled mailbox. */
77
export const MAX_DURABLE_GRAPH_MAILBOXES = 256;
8-
// Graph change notifications on Outlook mail resources allow subscriptions of
9-
// up to 10080 minutes (7 days), so renewals must land inside that window.
10-
// Renew daily by default to keep a wide margin.
11-
export const MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES = 10_080;
12-
export const DEFAULT_SUBSCRIPTION_EXPIRATION_MINUTES = MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES;
8+
// Graph change notifications on Outlook mail resources cap subscription
9+
// expiration at 10070 minutes in the future — Graph's real ceiling, not the
10+
// "7 days"/10080 value the docs imply (live Graph rejects 10080 with
11+
// "Subscription expiration can only be 10070 minutes in the future."). Default
12+
// below the ceiling so clock skew between us and Graph never trips the limit;
13+
// daily renewal keeps expiration fresh.
14+
export const MAX_GRAPH_SUBSCRIPTION_EXPIRATION_MINUTES = 10_070;
15+
export const DEFAULT_SUBSCRIPTION_EXPIRATION_MINUTES = 10_000;
1316
export const DEFAULT_RENEW_EVERY_MINUTES = 24 * 60;
1417

1518
const secretRefSchema = z

0 commit comments

Comments
 (0)