Greptile Summary

This PR addresses two independent issues: a singleton guard in monitor.ts to prevent EADDRINUSE crash loops caused by duplicate provider starts, and a security hardening fix in message-handler.ts that closes a sender-allowlist bypass (GHSA-g7cr-9h7q-4qxq).

The singleton guard approach is the right fix for the described race condition and the accompanying regression test is well-structured. However, two issues were found:

• Premature singleton clear in shutdown() (monitor.ts lines 350–361): activeInstancePromise is set to null at the very start of shutdown(), before httpServer.close() completes. Any monitorMSTeamsProvider() call that races into the window between the clear and the port being released will spin up a new _startMSTeamsProvider(), hit expressApp.listen() while the port is still occupied, and reproduce the EADDRINUSE crash loop. The sentinel should be cleared inside the httpServer.close() callback, after the port is confirmed released.

• Second caller's AbortSignal is silently discarded (monitor.ts lines 78–83): When a duplicate call carries a different AbortSignal, the guard returns the existing promise but ignores the new signal entirely. This means aborting the second caller will not stop the server, and aborting the first will stop it regardless of the second caller's intent. The warning message should call this out explicitly, and ideally the guard should handle or at least log the signal mismatch.

Confidence Score: 3/5

• The singleton guard partially solves the EADDRINUSE problem, but a race window in shutdown() can re-introduce it on server restart.
• The core idea is correct and the security fix looks sound, but the premature activeInstancePromise = null in shutdown() leaves a meaningful race that undermines the stated goal of the PR. The AbortSignal discard is a less urgent but still real lifecycle bug for callers with independent abort controllers.
• extensions/msteams/src/monitor.ts — specifically the shutdown() closure and the duplicate-call guard.

This is a comment left during a code review.
Path: extensions/msteams/src/monitor.ts
Line: 350-361

Comment:
**Singleton cleared before server close completes**

`activeInstancePromise = null` is set at the very beginning of `shutdown()`, before `httpServer.close()` finishes. This creates a race window: any caller that invokes `monitorMSTeamsProvider()` after `shutdown()` is entered but before the OS releases the port will start a fresh `_startMSTeamsProvider()`, hit `expressApp.listen()` while the old server is still closing, and get `EADDRINUSE` — the exact crash loop this PR is trying to prevent.

The sentinel should be cleared only after the port is confirmed released:

```suggestion
  const shutdown = async () => {
    log.info("shutting down msteams provider");
    return new Promise<void>((resolve) => {
      httpServer.close((err) => {
        if (err) {
          log.debug?.("msteams server close error", { error: String(err) });
        }
        activeInstancePromise = null;
        resolve();
      });
    });
  };
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: extensions/msteams/src/monitor.ts
Line: 78-83

Comment:
**Second caller's `abortSignal` is silently discarded**

When a duplicate call arrives with a *different* `AbortSignal`, the guard returns the existing promise — but `_startMSTeamsProvider` is already wired to the first call's signal via `keepHttpServerTaskAlive`. The second caller's signal is completely ignored, meaning:

- Aborting the second caller will **not** stop the server.
- Aborting the first caller **will** stop the server even if the second caller never intended it.

In practice, both callers from the gateway race may share the same controller, so this might not manifest today. But this is a hidden contract that could bite future callers. At minimum, a log warning that the incoming signal is being ignored would make debugging easier. A more robust approach is to check whether the already-stored promise was created with a compatible (same or parent) signal and warn loudly if not:

```typescript
if (activeInstancePromise) {
  const core = getMSTeamsRuntime();
  const log = core.logging.getChildLogger({ name: "msteams" });
  log.warn?.("msteams provider already started; returning existing instance — incoming abortSignal ignored");
  return activeInstancePromise;
}
```

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "fix(msteams): preven..."}

Thanks for digging into this and for putting together the repro/test case.

I pushed an update to this branch that fixes the duplicate-start race in the shared gateway channel manager instead of inside the MS Teams monitor.

Why I changed direction:

• The underlying race is in the shared startup path, where two overlapping startChannel calls can both get past the pre-start checks before the task is registered.
• Fixing that in the gateway prevents duplicate boots for the same account across all channels, including MS Teams, instead of adding a Teams-only guard.
• I added a regression test that reproduces the problem with concurrent starts, and a second one that verifies stopping an account mid-boot cancels startup cleanly.

I also removed the extra authz change from extensions/msteams/src/monitor-handler/message-handler.ts from this PR.

Reasoning there:

• It looked unrelated to the duplicate-start bug this PR is fixing.
• In the current flow, that hunk did not appear to change the final group authz decision, because route allowlist enforcement and sender allowlist enforcement already happen later in the handler.
• There is already targeted coverage for that route-allowlist/sender-authz scenario, and it passes on current main without that change.

So the branch is now scoped to the actual root cause: serializing per-account startup in the gateway.

Tests I ran:

• pnpm test -- src/gateway/server-channels.test.ts
• pnpm test -- extensions/msteams/src/monitor.lifecycle.test.ts
• pnpm exec oxfmt --check src/gateway/server-channels.ts src/gateway/server-channels.test.ts
• pnpm exec oxlint src/gateway/server-channels.ts src/gateway/server-channels.test.ts

Thanks again — I think this ends up as a stronger fix with a better regression test surface.

Thanks @sudie-codes!

Landed via temp rebase onto main.

• Focused validation:

  •       -           -         - Source commit: https://github.com/openclaw/openclaw/commit/f80ed12ece232fca44cdee3d3384d207df70416b
    

• Landed commit: 1890089

The broader CI/gate issues were being fixed separately on main during landing.

Thanks again @sudie-codes!

• Source commit: f80ed12
• Landed commit: 1890089

The broader CI/gate issues were being fixed separately on main during landing.