1. extensions/msteams-channel-archive/src/archive-store.ts, line 671-746 (link)

   TOCTOU race between archiveAttachments and pruneConversation

   archiveAttachments (which physically copies files to the attachment store) is called before the file lock is acquired. The lock is only taken when writing the message record and updating the index. This creates a race:

   1. Thread A calls archiveAttachments and copies attachment → attachments/ab/ab<sha>/file.bin (no lock held).
   2. Thread B calls pruneConversation under the lock, collects all referenced attachment paths from the current index (which doesn't include Thread A's not-yet-written message), and deletes attachments/ab/ab<sha>/file.bin as "unreferenced".
   3. Thread A acquires the lock, writes a message record whose storedPath points to the now-deleted file.

   The result is a message record that permanently references a missing attachment with no way to recover it. The missing: true flag in withAttachmentStatus will surface this, but the data is gone.

   The simplest mitigation is to move the archiveAttachments call inside the lock body so the file's reference is recorded atomically with the copy, or alternatively reverse the pruning logic so it only removes a file after confirming no in-flight writers could reference it (e.g., by re-scanning the index after the message file has been written).

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: extensions/msteams-channel-archive/src/archive-store.ts
   Line: 671-746
   
   Comment:
   **TOCTOU race between `archiveAttachments` and `pruneConversation`**
   
   `archiveAttachments` (which physically copies files to the attachment store) is called **before** the file lock is acquired. The lock is only taken when writing the message record and updating the index. This creates a race:
   
   1. Thread A calls `archiveAttachments` and copies attachment → `attachments/ab/ab<sha>/file.bin` (no lock held).
   2. Thread B calls `pruneConversation` under the lock, collects all referenced attachment paths from the current index (which doesn't include Thread A's not-yet-written message), and deletes `attachments/ab/ab<sha>/file.bin` as "unreferenced".
   3. Thread A acquires the lock, writes a message record whose `storedPath` points to the now-deleted file.
   
   The result is a message record that permanently references a missing attachment with no way to recover it. The `missing: true` flag in `withAttachmentStatus` will surface this, but the data is gone.
   
   The simplest mitigation is to move the `archiveAttachments` call inside the lock body so the file's reference is recorded atomically with the copy, or alternatively reverse the pruning logic so it only removes a file after confirming no in-flight writers could reference it (e.g., by re-scanning the index *after* the message file has been written).
   
   How can I resolve this? If you propose a fix, please make it concise.

Best possible solution:

Land a reviewed version that declares the archive plugin's activation and tool ownership metadata, documents the new hook contract, keeps the Teams archive behavior plugin-owned, and passes the targeted plugin/Teams checks plus the changed gate.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a feature PR rather than a user bug report. The blocking findings are source-reproducible by comparing PR head d019bf0b4df9 with current main's manifest, activation, and tool-registration contracts.

Is this the best way to solve the issue?

No for the PR exactly as submitted. The implementation direction is plausible, but the manifest must declare startup activation and registered tool contracts, and the public hook docs should be aligned before normal maintainer approval.

Full review comments:

• [P2] Declare the archive tools in the manifest — extensions/msteams-channel-archive/openclaw.plugin.json:2-3
  This plugin registers four archive query tools, but the manifest declares no contracts.tools. Current registerTool treats an empty tool contract list as an error and returns, so channel_archive_search, channel_archive_get_message, channel_archive_get_thread, and channel_archive_search_attachments will not register. Add those names to contracts.tools.
  Confidence: 0.95
• [P2] Declare startup activation for archive capture — extensions/msteams-channel-archive/openclaw.plugin.json:2-3
  The archive capture hooks and cleanup service are installed only when register(...) runs, but this manifest has no activation.onStartup. Current docs say omitted startup activation no longer startup-loads a plugin, and the bundled manifest test requires an explicit boolean, so Teams archive capture and deletion cleanup can stay inactive. Add activation.onStartup: true or an equivalent tested activation path that loads this plugin before Teams messages arrive.
  Confidence: 0.92
• [P3] Document the new channel_deleted hook — src/plugins/hook-types.ts:86
  This adds channel_deleted to the exported hook surface, but docs/plugins/hooks.md still documents only the existing message hooks. Since the PR updates the SDK baseline for a new public hook, add a short docs entry describing when it fires and the event metadata shape.
  Confidence: 0.82

Overall correctness: patch is incorrect
Overall confidence: 0.9

Acceptance criteria:

• pnpm test -- src/plugins/contracts/plugin-tool-contracts.test.ts src/plugins/bundled-plugin-metadata.test.ts
• pnpm test -- src/plugins/wired-hooks-message.test.ts src/hooks/message-hook-mappers.test.ts
• pnpm test -- extensions/msteams/src/monitor-handler.file-consent.test.ts extensions/msteams/src/policy.test.ts extensions/msteams/src/messenger.test.ts
• pnpm test -- extensions/msteams-channel-archive/index.test.ts extensions/msteams-channel-archive/src/archive-store.test.ts extensions/msteams-channel-archive/src/channel-cleanup.test.ts
• pnpm exec oxfmt --check --threads=1 CHANGELOG.md docs/plugins/hooks.md extensions/msteams-channel-archive/openclaw.plugin.json

What I checked:

• Current main lacks archive plugin: Current main only has extensions/msteams; there is no extensions/msteams-channel-archive directory or archive tool implementation. (a7c5a0425988)
• Current main lacks channel_deleted hook: PluginHookName and PLUGIN_HOOK_NAMES include message_received, message_sending, and message_sent, but no channel_deleted hook. (src/plugins/hook-types.ts:70, a7c5a0425988)
• Latest PR head confirmed: GitHub API reports the PR head as d019bf0b4df96d7cbc56ad27f717232365d38835, updated 2026-05-03, with mergeable: false. (d019bf0b4df9)
• PR manifest omits activation and tool contracts: The archive plugin manifest contains only id and configSchema; it does not declare activation.onStartup or contracts.tools. (extensions/msteams-channel-archive/openclaw.plugin.json:2, d019bf0b4df9)
• PR runtime work only happens after plugin load: The plugin registers archive tools, cleanup service, message_received, and channel_deleted handlers inside register(...). (extensions/msteams-channel-archive/index.ts:62, d019bf0b4df9)
• PR registers undeclared tools: The PR registers channel_archive_search, channel_archive_get_message, channel_archive_get_thread, and channel_archive_search_attachments. (extensions/msteams-channel-archive/src/tools.ts:93, d019bf0b4df9)

Likely related people:

• shakkernerd: Recent history shows related work enforcing plugin tool manifest contracts and explicit bundled startup metadata, which are the central blockers in this PR. (role: manifest contract owner; confidence: high; commits: 7641783d6b0e, 13987b726aa6, 6f139822127a; files: src/plugins/registry.ts, docs/plugins/building-plugins.md, src/plugins/bundled-plugin-metadata.test.ts)
• steipete: Recent commits touch MS Teams hot paths and plugin activation/tool discovery docs/runtime, making this a likely review route for the Teams plus plugin loader boundary. (role: recent maintainer; confidence: high; commits: 3f002b10d281, 7ac23eeeb581, baadd74b6bb5; files: extensions/msteams/src/monitor-handler.ts, docs/plugins/manifest.md, docs/plugins/building-plugins.md)
• vincentkoc: Recent commits touch plugin registry/hook metadata and message-hook mapper seams, which overlap the PR's hook and metadata propagation changes. (role: adjacent owner; confidence: medium; commits: 1d34564de9ba, 1b25dcf57a9f, e593122465e7; files: src/plugins/registry.ts, src/plugins/hook-types.ts, src/hooks/message-hook-mappers.ts)

Remaining risk / open question:

• GitHub API currently reports mergeable: false, so the source branch may need a rebase or replacement branch in addition to the code repairs.
• This PR adds a persistent bundled archive plugin plus a new public hook surface, so maintainers still need product/ownership approval after the mechanical blockers are fixed.
• No local test run was performed in this read-only sweep; the blockers are source-level checks against current main and the latest PR head.

Codex review notes: model gpt-5.5, reasoning high; reviewed against a7c5a0425988.