Skip to content

chore: sync MemOS Cloud OpenClaw plugin - #2261

Merged
syzsunshine219 merged 2 commits into
mainfrom
chore/sync-cloud-openclaw-plugin-20260818
Aug 18, 2026
Merged

chore: sync MemOS Cloud OpenClaw plugin#2261
syzsunshine219 merged 2 commits into
mainfrom
chore/sync-cloud-openclaw-plugin-20260818

Conversation

@Hun-ger

@Hun-ger Hun-ger commented Aug 18, 2026

Copy link
Copy Markdown
Collaborator

Description

Sync apps/MemOS-Cloud-OpenClaw-Plugin from the upstream main branch through commit e4b4d8b using git subtree pull --squash.

The target subtree and upstream source tree both resolve to 599741758765f359d839899e953241dc3159138a.

Related Issue: N/A — routine subtree synchronization.

Type of change

  • Chore / subtree synchronization

How Has This Been Tested?

  • Upstream plugin unit tests: npm test — 139 passed, 0 failed
  • Verified the target subtree tree hash matches upstream main
  • Verified the tracked MemOS worktree remains clean

Running the same tests from inside the MemOS subtree produced 137 passes and 2 Git-context-related failures because those release-note tests require upstream tags/history (including v0.1.19). The complete suite passes in the upstream repository context.

make format could not run in the current environment because Poetry is not installed. No Python source files are changed by this PR.

Checklist

  • I have performed a self-review of the synchronized diff
  • Upstream tests pass
  • The subtree metadata records source commit e4b4d8b
  • Documentation updates from upstream are included where applicable

谁在吵着吃糖 added 2 commits August 18, 2026 11:30
…4b4d8b2

e4b4d8b2 Merge pull request #157 from MemTensor/docs-sync/openclaw-cloud-auto-draft-release
28eb5f81 ci: automate cloud plugin draft releases
bf3f9b32 Merge pull request #156 from MemTensor/fix/npm-publish-propagation
df4634d3 ci: validate release fixes before merge
0da6e0a9 ci: harden npm release propagation verification
a3176616 Merge pull request #155 from EF1874/test
2ff7ec75 0.1.20
09714e85 Merge pull request #153 from MemTensor/docs-sync/openclaw-cloud-main-source-release
d33d77fb ci: publish reviewed main versions without action PRs
066f330d Merge pull request #152 from EF1874/test
50ae8cc5 Merge pull request #149 from MemTensor/docs-sync/openclaw-cloud-release-quality
1b027080 ci: surface cloud workflow contract failures
31a17497 ci: isolate read-only release dry runs
8ebc5f02 ci: declare reusable publish secret
e941685b ci: minimize dry-run workflow privileges
6a8de049 ci: align prerelease and stable release channels
41201c0a ci: normalize cloud release evidence refs
8517603d ci: close cloud release failure recovery gaps
58d9efa3 ci: preserve reusable workflow token permissions
7a8cb8fd ci: pin cloud release recovery to npm source
56170ea5 0.1.20-beta.0
557c7269 Merge branch 'main' into test
4d993bcd ci: add auditable cloud release quality gates
066db592 feat: implement deduplication for agent_end snapshots and enhance API call timeout handling
8aee50e2 ci: require recovery mode for missing release metadata
ca0f2357 ci: suppress tag fetch warnings when local tags exist
b06dcf7d ci: avoid tag fetch warnings in originless tests
74d25dd5 ci: validate manual cloud release note categories
98549947 ci: cover publish confirmation guard dry runs
7c9830f4 ci: require confirmation for real cloud releases
34e0f7b2 ci: request candidate self-checks for cloud release notes
0dde13cc ci: harden cloud plugin release previews
fd4bad4c Merge pull request #148 from MemTensor/docs-sync/openclaw-cloud-plugin-semver-previous-tag
c20bf70c fix: compare release tags with semver precedence
becc6f18 Merge pull request #147 from MemTensor/docs-sync/openclaw-cloud-plugin-release
ef831b5d ci: validate telemetry credentials before packaging
2f43ff8c ci: resolve annotated release tags to commits
252e9608 ci: read release tag evidence from commits
7f29b992 ci: add cloud plugin release dry-run automation
d0f2cea7 ci: add OpenClaw cloud plugin release workflow
18cb8c7f Merge pull request #140 from MemTensor/test
3b978092 Merge pull request #135 from EF1874/test
0c2cd2a1 0.1.19
c4ab3104 0.1.19-beta.0
d053b0a2 feat: enhance OpenClaw system prompt detection and handling
e1e7fb0f 0.1.18
ce0a9e16 Merge pull request #132 from Hun-ger/dev_gyh_260617
5152641c feat: add config UI update check
9b9e4ab2 Merge pull request #131 from MemTensor/test
2af0cb2a Merge pull request #129 from EF1874/test
3b0e068e 0.1.17
f7fd95e2 0.1.17-beta.0
70fb1840 feat: update MEMOS_SOURCE to append platform-specific suffixes and add corresponding tests
31d5f6a0 Merge branch 'test' of https://github.com/MemTensor/MemOS-Cloud-OpenClaw-Plugin into test
8b18998d Merge pull request #127 from EF1874/test
e43380d8 Merge pull request #123 from EF1874/test
ec9764d3 0.1.16
0d74ab93 0.1.16-beta.1
ed52b9c1 feat: extend system command detection to include 'clear' command and add internal prompt patterns for session management
00edb855 0.1.16-beta.0
9c773370 feat: enhance memory section handling by adding tool memory support and simplifying filtering logic
f7c1edb9 feat: refactor system event detection logic into dedicated functions for clarity and reuse
eb2bddc8 feat: add function to strip leading system notes from text input
bf25eb78 feat: implement system event detection in recall and agent end hooks to skip processing for heartbeat and command events
869217d7 Merge pull request #122 from MemTensor/test
306841f2 feat: update recall hook registration to use before_prompt_build for newer OpenClaw hosts and add tests for version compatibility
7e8304a7 Merge pull request #120 from EF1874/test
a63116fc 0.1.15
5d2bb28e 0.1.15-beta.0
b4af645e feat: add activation hooks to plugin configurations and improve hook policy handling in index.js
083b6e4b Merge pull request #119 from MemTensor/test
3b5ddaa4 Merge pull request #118 from EF1874/test
e931499a 0.1.14
1a86dc0d 0.1.14-beta.0
d0b88342 feat: add version detection and comparison utilities for host CLI
f7d19029 refactor: enhance configuration UI and improve session handling logic
d982aa0f chore: remove HOOK.md file and clean up package.json by eliminating unused hooks
03fcc33c Merge pull request #115 from MemTensor/test
1e77054b 0.1.13
19e8eb38 0.1.13-beta.1
b32b493a Merge pull request #113 from Hun-ger/fix_cloud_plugin_filter
23a48675 fix:fix filter params
de07588e 0.1.13-beta.0
4451c8a5 Merge pull request #111 from Hun-ger/fix_cloud_plugin_filter
4c84c6c3 fix typos
9f663e50 Merge pull request #110 from Hun-ger/fix_cloud_plugin_filter
92e66d30 feat: Add shared support knowledgebase for multi-agent reuse

git-subtree-dir: apps/MemOS-Cloud-OpenClaw-Plugin
git-subtree-split: e4b4d8b2ad3c50449a1b75f04eee985ebdb324a9
@Memtensor-AI Memtensor-AI added area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 18, 2026
@syzsunshine219
syzsunshine219 merged commit 9b7efc8 into main Aug 18, 2026
17 of 19 checks passed
@syzsunshine219
syzsunshine219 deleted the chore/sync-cloud-openclaw-plugin-20260818 branch August 18, 2026 03:53
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2261
Task: dbd056f2cc29fe6a
Base: main
Head: chore/sync-cloud-openclaw-plugin-20260818

🔍 OpenCodeReview found 76 issue(s) in this PR.

⚠️ 1 warning(s) occurred during review.


1. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.test.mjs (L8-L35)

RELEASE_NOTE_LIMITS is imported but never referenced in any test assertion. This is dead import code and likely represents an untested boundary: the numeric limits (max_items: 12, max_text_cn_characters: 180, max_text_en_characters: 220) are exercised implicitly by other tests but never directly asserted, making it easy for a future change to silently alter these contract values. Consider removing the import if unused, or add a dedicated test that pins the exported limit values.


2. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.test.mjs (L1184)

This resolves a specific commit SHA (d053b0a) from the live repository at test runtime. If the test runs in a shallow clone (e.g. fetch-depth: 1 or fetch-depth: 0 without all commits), or in a fresh fork that has never received this commit, execFileSync will throw a non-descriptive git error rather than a meaningful test failure. The manual-notes test becomes non-portable. Consider making the sourceRef value a stable constant or a fixture, or at minimum wrapping the git call with a try/catch to emit a readable skip message.


3. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.test.mjs (L1193-L1222)

Multiple tests use process.env = previous to restore environment state. While this works correctly within the same Node.js process for subsequent env reads in the test runner, reassigning the process.env object entirely does not propagate to child processes spawned after the reassignment — they inherit the OS-level environment, not the reassigned JS object. More critically, this pattern is inconsistently applied: delete process.env.DOC_AGENT_RELEASE_NOTES_DRAFT_URL in another test is the safer idiom. Consider using a helper that deletes/restores individual keys to avoid subtle cross-test pollution if tests ever run with parallelism, and to be consistent with the delete pattern used in the last test.

💡 Suggested Change

Before:

    Object.assign(process.env, {
      RELEASE_VERSION: "0.1.19",
      RELEASE_TAG: "v0.1.19",
      RELEASE_EVIDENCE_REF: "v0.1.19",
      RELEASE_NOTES_FILE: notesPath,
      MANUAL_RELEASE_NOTES_FILE: reviewedNotesPath,
      GITHUB_OUTPUT: outputPath,
    });

    await main();

    const output = readFileSync(outputPath, "utf8");
    const match = output.match(/docs_preview_markdown_file<<__DOC_AGENT_EOF__\n([\s\S]*?)\n__DOC_AGENT_EOF__/);
    assert.ok(match, "docs preview markdown output should be written");
    const preview = readFileSync(match[1], "utf8");
    assert.match(preview, /MemOS-Docs Plugin Changelog Preview/);
    assert.match(preview, /OpenClaw 云插件/);
    assert.match(preview, /System prompt detection/);
    assert.match(readFileSync(notesPath, "utf8"), /doc-agent: source-id=openclaw-cloud-plugin/);
    const qualityMatch = output.match(
      /quality_report_file<<__DOC_AGENT_EOF__\n([\s\S]*?)\n__DOC_AGENT_EOF__/,
    );
    assert.ok(qualityMatch, "quality report output should be written");
    const quality = JSON.parse(readFileSync(qualityMatch[1], "utf8"));
    assert.equal(quality.ok, true);
    assert.equal(quality.validation_attempt_count, 1);
  } finally {
    process.env = previous;
    rmSync(directory, { recursive: true, force: true });
  }

After:

  } finally {
    // Restore individual keys rather than replacing the object.
    for (const key of Object.keys(process.env)) {
      if (!(key in previous)) delete process.env[key];
    }
    Object.assign(process.env, previous);
    rmSync(directory, { recursive: true, force: true });
  }

4. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.test.mjs (L1410-L1450)

The final assert.match checks for "Skips scheduled tasks, reminders, and background command results" — text that does not appear in either mock draft response. This string is injected by knownReleaseItemsForItem's systemPromptPattern rewrite triggered by 系统事件过滤增强 in text_cn, not by the draft service. The test is therefore silently asserting the behavior of the hardcoded known-topic lookup table rather than the repair loop's core contract (that the validated draft's release_notes_markdown reflects the repaired items). If the topic rewrite table is modified or removed, this assertion will fail with no indication of which layer broke. Replace or supplement the assertion with one that directly tests the repair result, e.g. matching the fixed English text returned by the second mock: /Skips background command results automatically/.

💡 Suggested Change

Before:

test("repairs mixed-language structured drafts by sending validation context back to the drafter", async () => {
  const requests = [];
  const result = await requestValidatedDraft(evidence, {
    requestImpl: async (requestEvidence) => {
      requests.push(requestEvidence);
      if (requests.length === 1) {
        return {
          ok: false,
          needs_review: true,
          release_items: [
            {
              category: "Improved",
              text_cn: "**系统事件过滤增强**:自动跳过后台命令结果。",
              text_en: "**System-event Filtering**:自动跳过后台命令结果。",
              source_refs: ["abc1234"],
            },
          ],
          coverage: { needs_review: true, required_count: 0, covered_required_count: 0, missing_required_count: 0 },
        };
      }
      assert.equal(requestEvidence.release_notes_repair_context.validation_report.language_issue_count, 1);
      return {
        ok: true,
        needs_review: false,
        release_items: [
          {
            category: "Improved",
            text_cn: "**系统事件过滤增强**:自动跳过后台命令结果。",
            text_en: "**System-event Filtering**: Skips background command results automatically.",
            source_refs: ["abc1234"],
          },
        ],
        coverage: { needs_review: false, required_count: 0, covered_required_count: 0, missing_required_count: 0 },
      };
    },
  });
  assert.equal(result.ok, true);
  assert.equal(result.needs_review, false);
  assert.equal(result.validation_attempt_count, 2);
  assert.equal(result.repair_attempt_count, 1);
  assert.match(result.release_notes_markdown, /Skips scheduled tasks, reminders, and background command results/);

After:

  assert.match(result.release_notes_markdown, /Skips background command results automatically/);

5. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.test.mjs (L1538-L1541)

The sanitization assertion pattern includes test-token — a very short, generic token. Looking at cleanError in the implementation, test-token would only be caught if it matches Bearer test-token (the Bearer\s+\S+ pattern). However the error string constructed in this test contains Bearer test-token, so it is correctly redacted. The concern is that arbitrary short bearer tokens not matching the known prefixes (github_pat_, ghp_, npm_, _authToken=) would not be redacted by cleanError. The test only validates test-token via the Bearer prefix pattern, which works here — but does not verify that a bare token (e.g. a service API key without a recognized prefix) would be sanitized. Consider adding an assertion for a bare token format to document and verify the current sanitization boundary.


6. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.test.mjs (L578-L582)

This assertion pins the exact count (7) and source-ref ordering of items produced by postprocessDraftFromEvidence, including the deliberate duplication of 9c77337 (split into two items by the toolMemoryPattern/memoryFilteringPattern rewrite). This is highly coupled to the internal ordering of pattern-match rules in knownReleaseItemsForItem. Any reordering, merging, or renaming of those patterns will silently break the test rather than pointing to a business logic regression. Consider adding inline comments or a // item 5: Tool Memory support, item 6: Memory filtering cleanup from 9c77337 split to document the expected mapping, so future maintainers know what each position represents.


7. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.mjs (L28-L29)

Unknown prerelease identifiers (e.g., rc, canary, dev) silently fall back to the "next" dist-tag with no warning or log message. A release for 1.0.0-rc.1 will be published under next instead of rc, which is likely surprising. Downstream consumers who check for an rc-tagged npm package will not find it, and there will be no log evidence of the substitution.

Consider logging a warning when the fallback is applied so that operators are aware of the mapping:

💡 Suggested Change

Before:

  const identifier = String(parsed.prerelease[0] || "").toLowerCase();
  return ["alpha", "beta", "next"].includes(identifier) ? identifier : "next";

After:

  const identifier = String(parsed.prerelease[0] || "").toLowerCase();
  const knownTags = ["alpha", "beta", "next"];
  if (knownTags.includes(identifier)) return identifier;
  const fallback = "next";
  console.warn(`[resolve-auto-release] Unrecognized prerelease identifier '${identifier}'; falling back to npm dist-tag '${fallback}'.`);
  return fallback;

8. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.mjs (L172-L178)

The SHA validation error message does not distinguish which SHA is invalid (merge vs. base). When both are potentially missing or malformed, the error gives no actionable hint. This is a minor diagnostic improvement but relevant since workflow failures from missing secrets/context variables are common and can be hard to debug.

💡 Suggested Change

Before:

  if (!exactSha(mergeSha) || !exactSha(baseSha)) {
    return {
      ok: false,
      inspect: false,
      reason: "merged PR must provide exact 40-character base and merge commit SHAs",
    };
  }

After:

  if (!exactSha(mergeSha) || !exactSha(baseSha)) {
    const which = !exactSha(mergeSha) ? "merge" : "base";
    return {
      ok: false,
      inspect: false,
      reason: `merged PR must provide exact 40-character ${which} commit SHA`,
    };
  }

9. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.mjs (L193-L202)

validateAutoRelease calls inspectPullRequestEvent a second time (the first call is in main()), including the exactSha regex check. This is harmless but wasteful and adds cognitive overhead. Consider accepting a pre-validated event result as an argument, or documenting that validateAutoRelease is designed to be called independently of main() (e.g., from tests), which justifies the repeated check.


10. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.mjs (L62-L64)

The missing-value check !values[index] uses JavaScript's falsy coercion. Since clean() returns a trimmed string, an empty result "" is correctly caught as falsy. However, the intent of "missing version" is better expressed as an explicit empty-string check (values[index] === ""). The current form would also incorrectly flag a hypothetical version "0" as missing, though in practice SemVer versions always have at least major.minor.patch so this will never occur. Explicit equality improves readability and prevents confusion during future maintenance.

💡 Suggested Change

Before:

function commonVersion(versions, label) {
  const values = VERSION_FILES.map((file) => clean(versions?.[file]));
  const missing = VERSION_FILES.filter((_, index) => !values[index]);

After:

  const missing = VERSION_FILES.filter((_, index) => values[index] === "");

11. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.mjs (L35-L36)

If any of the four VERSION_FILES does not exist at the given path, readFileSync will throw an unhandled ENOENT exception. The error propagates through main() and is caught at the top-level try/catch, but GITHUB_OUTPUT will not have been written at all. Downstream workflow steps that gate on eligible output will receive empty strings and may behave unexpectedly rather than cleanly failing.

Similarly, versionsFromGitRef using execFileSync with stdio: ["ignore", "pipe", "pipe"] means a non-zero exit (e.g., file absent in git ref) throws with git's stderr message — this is at least visible but still leaves outputs unwritten.

Consider writing a safe-failure output (e.g., eligible=false) to GITHUB_OUTPUT in the top-level catch block of main(), or wrapping the file reads in a try/catch that produces a structured { ok: false, reason } result.

💡 Suggested Change

Before:

const payload = JSON.parse(readFileSync(join(root, file), "utf8"));
      return [file, clean(payload.version)];

After:

// In main(), wrap git reads and write a fallback output on failure:
try {
  const currentVersions = versionsFromGitRef(env.PR_MERGE_SHA, root);
  const previousVersions = versionsFromGitRef(env.PR_BASE_SHA, root);
  // ...
} catch (error) {
  writeOutputs({ eligible: false, reason: error.message }, env.GITHUB_OUTPUT);
  throw error;
}

12. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.mjs (L1837-L1838)

The readdirSync error is silently swallowed by an empty catch {} block. When attemptDir is inaccessible or does not exist (e.g., a misconfigured workflow variable), the function silently falls back to ["1.log"] and reports a misleading generic message instead of the actual filesystem error. This makes debugging CI failures much harder. Consider logging a warning with the error reason.

💡 Suggested Change

Before:

  } catch {}
  if (logNames.length === 0) logNames = ["1.log"];

After:

  } catch (err) {
    warn(`Failed to read attempt log directory '${attemptDir}': ${cleanError(err?.message || err)}`);
  }
  if (logNames.length === 0) logNames = ["1.log"];

13. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.mjs (L1897-L1907)

When the server returns needs_review: true with structured release_items, the function returns immediately on the first attempt, bypassing the retry loop entirely. This is problematic: a transient server-side quality issue should trigger a retry rather than early termination, since the outer requestValidatedDraft repair loop will attempt to fix issues anyway. The early return also prevents reportFailureBestEffort from being called if all retries are truly exhausted. Consider allowing the retry loop to continue rather than returning on the first needs_review response that has items.


14. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.mjs (L1933-L1937)

The failure report is only sent when attempts.length === 3. A non-retryable error on attempt 1 or 2 (!entry.retryable is true but attempt < 3) causes the function to fail immediately without ever calling reportFailureBestEffort. Additionally, reportFailure itself has a attempts.length < 3 guard, so if fewer than 3 attempts were made the failure silently goes unreported. Consider sending the report regardless of how many attempts occurred, or removing the inner if (attempts.length === 3) guard and relying solely on reportFailure's own threshold.

💡 Suggested Change

Before:

      if (!entry.retryable || attempt === 3) {
        if (attempts.length === 3) {
          await reportFailureBestEffort({ evidence, attempts, finalError: entry.message, fetchImpl });
        }
        fail(`Release-notes draft request failed on attempt ${attempt}: ${entry.message}`);

After:

      if (!entry.retryable || attempt === 3) {
        await reportFailureBestEffort({ evidence, attempts, finalError: entry.message, fetchImpl });
        fail(`Release-notes draft request failed on attempt ${attempt}: ${entry.message}`);

15. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.mjs (L425-L431)

Errors from writeFileSync are unhandled. A write failure to GITHUB_OUTPUT (e.g., permissions issue, disk full) will crash the entire main() function with an unformatted Node.js stack trace rather than a user-friendly error message. Since all downstream workflow steps depend on these outputs, a silent write failure can be very confusing. Consider wrapping the write in a try/catch with a meaningful warning.

💡 Suggested Change

Before:

function appendOutput(name, value) {
  const outputPath = process.env.GITHUB_OUTPUT;
  if (!outputPath) return;
  writeFileSync(outputPath, `${name}<<__DOC_AGENT_EOF__\n${value}\n__DOC_AGENT_EOF__\n`, {
    flag: "a",
  });
}

After:

function appendOutput(name, value) {
  const outputPath = process.env.GITHUB_OUTPUT;
  if (!outputPath) return;
  try {
    writeFileSync(outputPath, `${name}<<__DOC_AGENT_EOF__\n${value}\n__DOC_AGENT_EOF__\n`, {
      flag: "a",
    });
  } catch (err) {
    warn(`Failed to write GitHub output '${name}': ${cleanError(err?.message || err)}`);
  }
}

16. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/draft-cloud-plugin-release-notes.mjs (L600-L603)

The knownReleaseItemsForItem function contains a very large block of hardcoded regex patterns that rewrite release note text. This creates a tight coupling between commit subject conventions and release note wording, making it brittle: a small commit subject phrasing change will silently produce no match and fall through to return [item], leaving the original LLM-generated text unchanged. Furthermore, the pattern matching happens against lowercased text (subjectBlob) but the regexes themselves are not all lowercase-normalized (e.g., mixed-case literals like before_prompt_build, MemOS). This also makes adding new feature areas require code changes to this function. Consider externalizing these rewrite rules to a configuration object or data structure, which would simplify extension and testing.


17. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.test.mjs (L157-L159)

return inside a for...of loop exits the entire test function, not just the current iteration. If the first SHA (066f330d...) is missing from the local git history, the remaining two pairs are never tested and no assertion failure is raised — all silently passing.

The intent appears to be continue (skip only the current entry). Use continue instead of return, and optionally add a console.warn so that the skip is visible rather than invisible.

💡 Suggested Change

Before:

    } catch {
      return;
    }

After:

    } catch {
      continue;
    }

18. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.test.mjs (L145-L149)

These three SHAs reference specific commits that are almost certainly absent from a standard CI checkout (a shallow clone or a fresh checkout of the current branch). versionsFromGitRef will throw because git show <sha>:<file> will fail, causing the catch block to trigger and silently skip every entry. The test then passes unconditionally in CI, providing zero regression protection.

Consider one of:

  1. Moving this test to the historical-dry-run.yml workflow that has the full git history, removing it from the unit-test suite.
  2. Fetching the needed history in CI before running tests (git fetch --unshallow or git fetch origin <sha>), with explicit documentation.
  3. Adding a comment and a visible skip signal (e.g., test.skip or a console.warn) so that skipped entries are surfaced in test output.

19. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.test.mjs (L75-L76)

The file name "package.json" is hardcoded here (and "moltbot.plugin.json" below) instead of using values from the imported VERSION_FILES array. If VERSION_FILES is ever updated, this test would either silently test the wrong scenario or produce a misleading failure.

Prefer indexing into VERSION_FILES (e.g., VERSION_FILES[0] / VERSION_FILES[2]) or using a named variable derived from the exported constant.

💡 Suggested Change

Before:

  current["package.json"] = "0.1.21";
  const result = inspectVersionTransition({

After:

  current[VERSION_FILES[0]] = "0.1.21"; // package.json
  const result = inspectVersionTransition({

20. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.test.mjs (L86-L87)

Same hardcoded file-name coupling as above — "moltbot.plugin.json" must stay in sync with VERSION_FILES[2] manually. Use VERSION_FILES[2] to keep the test resilient to future renames.

💡 Suggested Change

Before:

  current["moltbot.plugin.json"] = "0.1.22";
  const result = validateAutoRelease(valid({ currentVersions: current }));

After:

  current[VERSION_FILES[2]] = "0.1.22"; // moltbot.plugin.json
  const result = validateAutoRelease(valid({ currentVersions: current }));

21. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/resolve-auto-release.test.mjs (L160-L167)

The outer { ...valid({...}) } spread is redundant — valid() already returns a plain object, so wrapping it in another object literal creates an unnecessary shallow copy with no behavioural difference. Remove the outer braces to reduce noise.

💡 Suggested Change

Before:

    const result = validateAutoRelease({
      ...valid({
        mergeSha,
        baseSha,
        previousVersions: versionsFromGitRef(baseSha),
        currentVersions: versionsFromGitRef(mergeSha),
      }),
    });

After:

    const result = validateAutoRelease(
      valid({
        mergeSha,
        baseSha,
        previousVersions: versionsFromGitRef(baseSha),
        currentVersions: versionsFromGitRef(mergeSha),
      }),
    );

22. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/retry.sh (L68)

Silent output truncation: sed -n '1,160p' discards everything beyond 160 lines without printing any indication that truncation occurred. For verbose commands (build tools, test runners), critical error lines are often near the end of output, so this can make CI failures impossible to diagnose from the workflow log alone.

Consider printing a truncation notice when the log exceeds 160 lines, or making the limit configurable via --log-lines.

💡 Suggested Change

Before:

    sed -n '1,160p' "${attempt_dir}/${attempt}.log"

After:

    total_lines=$(wc -l < "${attempt_dir}/${attempt}.log")
    sed -n '1,160p' "${attempt_dir}/${attempt}.log"
    if [ "${total_lines}" -gt 160 ]; then
      echo "::warning::[retry] Output truncated: showed 160/${total_lines} lines. See artifact ${attempt_dir}/${attempt}.log for the full output." >&2
    fi

23. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/retry.sh (L85)

Integer overflow in exponential backoff: $((delay * (2 ** (attempt - 1)))) uses Bash's signed 64-bit arithmetic. For large values of delay or attempts (e.g., delay=100, attempts≥63), the intermediate 2 ** (attempt - 1) overflows to a negative number, making base_sleep negative. A negative base_sleep passes the [ "${base_sleep}" -gt "${max_delay}" ] guard (since a negative number is not greater than max_delay) and also passes [ "${base_sleep}" -gt 0 ] as false, so jitter stays 0 and sleep_seconds becomes negative — causing sleep to receive a negative argument, which is an error on most systems.

While the default attempts=3 is safe, the script accepts arbitrary values via --attempts and --delay. Add an explicit cap before the exponentiation, or clamp the exponent.

💡 Suggested Change

Before:

  base_sleep=$((delay * (2 ** (attempt - 1))))

After:

  # Cap exponent to avoid signed 64-bit overflow; 62 is a safe ceiling.
  exponent=$(( attempt - 1 < 62 ? attempt - 1 : 62 ))
  base_sleep=$(( delay * (2 ** exponent) ))
  if [ "${base_sleep}" -lt 0 ] || [ "${base_sleep}" -gt "${max_delay}" ]; then
    base_sleep="${max_delay}"
  fi

24. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/retry.sh (L32-L34)

Unrecognized options are silently treated as the start of the command: the * wildcard breaks out of the option-parsing loop without any error. A typo such as --attemps 5 or --max_delay 60 will silently make --attemps the command to execute, leading to a confusing command not found error rather than a clear usage hint.

Consider emitting an error for unknown options to make misuse immediately obvious.

💡 Suggested Change

Before:

    *)
      break
      ;;

After:

    --*)
      echo "::error::retry.sh: unrecognized option '$1'. Did you mean to use '--' to separate options from the command?" >&2
      exit 2
      ;;
    *)
      break
      ;;

25. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/retry.test.mjs (L52-L74)

spawnSync sets result.status to null and populates result.error when the process fails to spawn (e.g., bash not found, wrong script path). The subsequent assert.equal(result.status, 7) will then fail with a confusing null !== 7 message instead of a meaningful diagnostic. Add a guard before the status assertion.

💡 Suggested Change

Before:

    const result = spawnSync(
      "bash",
      [
        script,
        "--attempts",
        "3",
        "--delay",
        "0",
        "--max-delay",
        "0",
        "--label",
        "failed retry",
        "--",
        "bash",
        "-c",
        'printf "retry failed\\n"; exit 7',
      ],
      {
        env: { ...process.env, RETRY_ATTEMPT_DIR: directory },
        encoding: "utf8",
      },
    );
    assert.equal(result.status, 7);

After:

    const result = spawnSync(
      "bash",
      [
        script,
        "--attempts",
        "3",
        "--delay",
        "0",
        "--max-delay",
        "0",
        "--label",
        "failed retry",
        "--",
        "bash",
        "-c",
        'printf "retry failed\\n"; exit 7',
      ],
      {
        env: { ...process.env, RETRY_ATTEMPT_DIR: directory },
        encoding: "utf8",
      },
    );
    assert.equal(result.error, undefined, `Failed to spawn process: ${result.error}`);
    assert.equal(result.status, 7);

26. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/retry.test.mjs (L31-L33)

The deeply escaped inline bash script is hard to read and fragile — a single misplaced backslash silently changes the shell command's semantics (e.g., \' vs '). Consider extracting this into a small fixture shell script in the test directory and passing its path as the command, which would also make the test easier to debug when it fails.


27. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.mjs (L7-L11)

Silent fallback on non-array input masks upstream file format errors. If RELEASE_INVENTORY_FILE contains valid JSON that is not an array (e.g., a plain object {} or null), releasePages silently returns [], causing inspectReleaseInventory to report state: 'absent' with ok: true (unless requireExisting is set). This hides a broken inventory file behind a misleading 'release not found' result rather than a clear format error.

Suggestion: throw on unexpected non-array input so the caller gets an actionable error:

function releasePages(value) {
  if (!Array.isArray(value)) {
    throw new TypeError(`Expected release inventory to be an array or array-of-arrays, got ${typeof value}`);
  }
  if (value.every((item) => Array.isArray(item))) return value;
  return [value];
}

28. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.mjs (L113-L118)

When requiredSourceId is passed as an empty string (or any other falsy value), the entire Doc Agent source ID check is silently skipped due to the if (expectedSourceId && ...) guard. A caller that accidentally passes requiredSourceId: "" would allow a release body with no source ID (or a wrong one) to pass validation without any error. Since this is a security/integrity guard in a release pipeline, silent skip-on-empty is dangerous.

Suggestion: only skip the check when requiredSourceId is explicitly undefined or null (i.e., the caller intentionally opted out), not when it's an empty string:

// Use nullish check instead of truthiness
if (requiredSourceId != null) {
  const expectedSourceId = String(requiredSourceId).trim();
  if (expectedSourceId && (sourceIds.length !== 1 || sourceIds[0] !== expectedSourceId)) {
    errors.push(...);
  }
}

Alternatively, reject an explicitly empty requiredSourceId with an error rather than silently treating it as "no check required".


29. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.mjs (L86-L90)

The allowPublishedForDraftRerun suppression condition is logically correct but non-trivially complex, making it easy to misread or accidentally break in future edits. The triple-nested boolean !(expectedDraft && allowPublishedForDraftRerun && !release.draft) is essentially encoding: "suppress the draft mismatch error only when we expected a draft but got a published release AND the rerun flag is set". Consider extracting this into a named variable or helper for clarity:

const isDraftMismatch = Boolean(release.draft) !== expectedDraft;
const isIdempotentRerun = expectedDraft && allowPublishedForDraftRerun && !release.draft;
if (typeof expectedDraft === "boolean" && isDraftMismatch && !isIdempotentRerun) {
  errors.push(...);
}

30. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-confirmation.mjs (L140)

The import.meta.url === \file://${process.argv[1]}`guard is fragile: on Windows,process.argv[1]uses backslashes, which will not match the forward-slashfile://URL produced byimport.meta.url. It also breaks when the script is invoked via a symlink or when the path contains spaces (which are percent-encoded in the URL but not in process.argv[1]`).

The sibling file resolve-auto-release.mjs in this same directory already uses the correct, robust idiom: pathToFileURL(process.argv[1]).href. Adopt the same pattern for consistency and correctness.

💡 Suggested Change

Before:

if (import.meta.url === `file://${process.argv[1]}`) {

After:

import { pathToFileURL } from "node:url";

// ...

if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) {

31. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.test.mjs (L156-L160)

Security-critical: indexOf ordering assertion silently passes when the first string is absent.

If 'Validate committed release source version' is ever removed from release.yml, workflow.indexOf(...) returns -1, and -1 < positiveIndex evaluates to true, making the assertion silently pass even though the security gate is entirely missing. The same problem applies to the npm publish vs release_flags+=(--draft) ordering check below.

To reliably enforce both presence and ordering, assert that each string exists first, then compare their positions:

assert.ok(
  workflow.indexOf("Validate committed release source version") <
    workflow.indexOf('npm publish --access public --tag "${NPM_DIST_TAG}"'),
  "the committed source version gate must run before npm publish",
);

Should be replaced with:

const validateIdx = workflow.indexOf("Validate committed release source version");
const publishIdx  = workflow.indexOf('npm publish --access public --tag "${NPM_DIST_TAG}"');
assert.ok(validateIdx >= 0, "'Validate committed release source version' must be present in release.yml");
assert.ok(publishIdx  >= 0, "'npm publish --access public' must be present in release.yml");
assert.ok(
  validateIdx < publishIdx,
  "the committed source version gate must run before npm publish",
);

Apply the same fix to the npm publish vs release_flags+=(--draft) ordering assertion.

💡 Suggested Change

Before:

  assert.ok(
    workflow.indexOf("Validate committed release source version") <
      workflow.indexOf('npm publish --access public --tag "${NPM_DIST_TAG}"'),
    "the committed source version gate must run before npm publish",
  );

After:

  const validateIdx = workflow.indexOf("Validate committed release source version");
  const publishIdx  = workflow.indexOf('npm publish --access public --tag "${NPM_DIST_TAG}"');
  assert.ok(validateIdx >= 0, "'Validate committed release source version' must be present in release.yml");
  assert.ok(publishIdx  >= 0, "'npm publish --access public' must be present in release.yml");
  assert.ok(
    validateIdx < publishIdx,
    "the committed source version gate must run before npm publish",
  );

32. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.test.mjs (L226-L230)

Same indexOf silent-pass risk for the npm-before-draft ordering invariant.

If 'npm publish --access public --tag "${NPM_DIST_TAG}"' is absent from release.yml, indexOf returns -1. Then -1 < releaseWorkflow.indexOf("release_flags+=(--draft)") is true (assuming the draft line is present), so the assertion silently passes even though the npm publish step is entirely missing. This is a security-critical invariant (npm must be verified before the human-reviewed Draft is created).

Add explicit presence guards before comparing positions, mirroring the fix described for the other indexOf assertion.

💡 Suggested Change

Before:

  assert.ok(
    releaseWorkflow.indexOf('npm publish --access public --tag "${NPM_DIST_TAG}"') <
      releaseWorkflow.indexOf("release_flags+=(--draft)"),
    "npm must be published and verified before the human-reviewed Draft is created",
  );

After:

  const npmPublishIdx = releaseWorkflow.indexOf('npm publish --access public --tag "${NPM_DIST_TAG}"');
  const draftFlagIdx  = releaseWorkflow.indexOf("release_flags+=(--draft)");
  assert.ok(npmPublishIdx >= 0, "'npm publish --access public' must be present in release.yml");
  assert.ok(draftFlagIdx  >= 0, "'release_flags+=(--draft)' must be present in release.yml");
  assert.ok(
    npmPublishIdx < draftFlagIdx,
    "npm must be published and verified before the human-reviewed Draft is created",
  );

33. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.test.mjs (L93)

Brittle exact error-count assertion.

assert.equal(invalid.errors.length, 4) will break whenever a new validation check is added to inspectReleaseInventory, even if all four expected errors are still present (which the four some() assertions below already verify independently). The exact count adds no additional safety — if a future change produces 5 errors, the some() assertions all still pass — but it creates a maintenance burden.

Consider removing the exact-count assertion and relying solely on the qualitative some() checks, or changing it to assert.ok(invalid.errors.length >= 4) if you want a lower-bound sanity check.

💡 Suggested Change

Before:

  assert.equal(invalid.errors.length, 4);

After:

  assert.ok(invalid.errors.length >= 4, `expected at least 4 errors, got ${invalid.errors.length}: ${invalid.errors.join("; ")}`);}

34. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.test.mjs (L347-L353)

Hardcoded historical version list will silently drift from the workflow.

The version-to-tag pairs ["0.1.15", "v0.1.14"] through ["0.1.19", "v0.1.18"] are currently in sync with historical-dry-run.yml, but there is no mechanism to keep them in sync after new releases are published. When v0.1.20 is released and added to historical-dry-run.yml, this test will not fail — it will silently omit validating the new entry. Over time the test covers a shrinking fraction of the actual historical matrix.

Consider parsing the version matrix directly from historical-dry-run.yml to derive the expected list, or at minimum add a comment reminding maintainers to update this list alongside the workflow.

💡 Suggested Change

Before:

  for (const [version, previousTag] of [
    ["0.1.15", "v0.1.14"],
    ["0.1.16", "v0.1.15"],
    ["0.1.17", "v0.1.16"],
    ["0.1.18", "v0.1.17"],
    ["0.1.19", "v0.1.18"],
  ]) {

After:

  // NOTE: Keep this list in sync with the matrix in historical-dry-run.yml.
  // When a new release is added to that workflow, add a corresponding entry here.
  for (const [version, previousTag] of [
    ["0.1.15", "v0.1.14"],
    ["0.1.16", "v0.1.15"],
    ["0.1.17", "v0.1.16"],
    ["0.1.18", "v0.1.17"],
    ["0.1.19", "v0.1.18"],
  ]) {

35. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-github-release-inventory.test.mjs (L113-L117)

release.yml is read multiple times across separate test() blocks.

release.yml is read with readFileSync in at least two separate top-level test() blocks (lines ~109 and ~209). Other workflow files are also read redundantly. Since these are module-level test() callbacks that all execute in the same process, hoisting the reads to module scope would eliminate the repeated I/O and make the file-reading intent clearer.

For example:

// Module-level
const releaseWorkflow = readFileSync(new URL("../workflows/release.yml", import.meta.url), "utf8");
const dryRunWorkflow  = readFileSync(new URL("../workflows/release-dry-run.yml", import.meta.url), "utf8");
// ... other workflow files

test("...", () => { /* use module-level variables */ });

36. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-source-version.mjs (L78)

The formatReleaseSourceVersionError function hardcodes the literal "four" in its guidance message, but the inspectReleaseSourceVersion function accepts a configurable files array parameter that could have any length. Since both functions are exported and can be called independently, if a caller passes a custom files array (e.g., in tests or extended tooling), the error guidance will be factually wrong ("all four version files" when there might be 2 or 6).

Suggest deriving the count dynamically from the report:

lines.push(
  `Update all ${report.versions.length} version files in a normal reviewed PR, merge it to main, then rerun the dry-run.`,
);
💡 Suggested Change

Before:

    "Update all four version files in a normal reviewed PR, merge it to main, then rerun the dry-run.",

After:

    `Update all ${report.versions.length} version files in a normal reviewed PR, merge it to main, then rerun the dry-run.`,

37. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-source-version.mjs (L5-L10)

The exported RELEASE_VERSION_FILES array is mutable. Since it is used as the default value for the files parameter in inspectReleaseSourceVersion, any external code that imports and mutates it (e.g., RELEASE_VERSION_FILES.push('extra.json')) will silently change the behavior of all subsequent calls to inspectReleaseSourceVersion() that rely on the default. The default parameter is evaluated at call time, so it picks up the mutated state.

Freeze the array to prevent accidental mutation:

export const RELEASE_VERSION_FILES = Object.freeze([
  "package.json",
  ...
]);
💡 Suggested Change

Before:

export const RELEASE_VERSION_FILES = [
  "package.json",
  "openclaw.plugin.json",
  "moltbot.plugin.json",
  "clawdbot.plugin.json",
];

After:

export const RELEASE_VERSION_FILES = Object.freeze([
  "package.json",
  "openclaw.plugin.json",
  "moltbot.plugin.json",
  "clawdbot.plugin.json",
]);

38. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-confirmation.test.mjs (L110-L123)

The main() integration test only covers the beta prerelease path. The stable release path (e.g., RELEASE_VERSION: "0.1.20", NPM_DIST_TAG: "latest") is never exercised, leaving the docs_sync_expected=true and github_release_prerelease=false outputs unverified through main(). A regression in the stable-channel output logic would go undetected.

Consider adding a second main() test for the stable channel, e.g.:

test("exports deterministic stable release metadata for workflows", () => {
  const output = join(mkdtempSync(join(tmpdir(), "openclaw-release-policy-")), "output");
  main({
    RELEASE_VERSION: "0.1.20",
    NPM_DIST_TAG: "latest",
    DRY_RUN: "true",
    PUBLISH_CONFIRMATION: "",
    GITHUB_OUTPUT: output,
  });
  const values = readFileSync(output, "utf8");
  assert.match(values, /^release_channel=stable$/m);
  assert.match(values, /^github_release_prerelease=false$/m);
  assert.match(values, /^docs_sync_expected=true$/m);
});

39. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-confirmation.test.mjs (L110)

The temporary directory created by mkdtempSync is never cleaned up after the test. While ephemeral CI runners make this harmless in production, repeated local test runs will accumulate openclaw-release-policy-* directories under the OS temp folder. Use an after() hook from node:test to remove the directory:

import { mkdtempSync, readFileSync, rmSync } from "node:fs";
// ...
test("exports deterministic beta...", (t) => {
  const tmpDir = mkdtempSync(join(tmpdir(), "openclaw-release-policy-"));
  t.after(() => rmSync(tmpDir, { recursive: true, force: true }));
  const output = join(tmpDir, "output");
  // ...
});

40. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-confirmation.test.mjs (L45-L50)

The rc prerelease case only asserts expected_npm_dist_tag === "next" but does not assert prerelease_identifier. The implementation sets prerelease_identifier to the raw identifier string ("rc"), while expected_npm_dist_tag is "next" (the fallback for non-alpha/beta identifiers). This split is a meaningful contract detail — a future refactor could accidentally unify them or change the mapping. Adding an assertion would lock in the expected behavior:

assert.equal(releaseCandidate.prerelease_identifier, "rc");

41. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/wait-for-npm-release.test.mjs (L22-L27)

The version and latest defaults are independent, so calling packument({ version: '0.2.0' }) produces versions['0.2.0'] but dist-tags.latest still points to '0.1.20', yielding a structurally inconsistent fixture. Any future test that passes only version (without also passing latest) would silently get a mismatched packument, making the test difficult to interpret. Consider tying latest to version so they stay in sync by default:

function packument({ version = '0.1.20', gitHead = sha, latest = version } = {}) {  }
💡 Suggested Change

Before:

function packument({ version = "0.1.20", gitHead = sha, latest = "0.1.20" } = {}) {
  return {
    "dist-tags": { latest },
    versions: { [version]: { gitHead } },
  };
}

After:

function packument({ version = "0.1.20", gitHead = sha, latest = version } = {}) {
  return {
    "dist-tags": { latest },
    versions: { [version]: { gitHead } },
  };
}

42. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/wait-for-npm-release.test.mjs (L158-L188)

There is no test that exercises the requestTimeoutMs / AbortSignal timeout path. The implementation creates an AbortController and sets a timer that aborts the fetch if it exceeds requestTimeoutMs. Without a test where fetchImpl delays beyond this threshold, the abort/retry behaviour (which maps to the AbortError catch branch and the 'registry request timed out' pending reason) is entirely unverified. Consider adding a test where fetchImpl hangs until the controller aborts, then eventually resolves, to confirm that the implementation recovers correctly rather than propagating the error.


43. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-source-version.test.mjs (L14-L24)

Temporary directories created by mkdtempSync are never cleaned up after each test. Over repeated test runs (especially in CI) this can accumulate many short-lived directories under the OS temp folder.

Consider using test.after() (or a try/finally block) to remove the directory with rmSync(root, { recursive: true, force: true }) after each test that creates a fixture.

💡 Suggested Change

Before:

function fixture(versions) {
  const root = mkdtempSync(join(tmpdir(), "openclaw-release-source-"));
  for (const file of RELEASE_VERSION_FILES) {
    writeFileSync(
      join(root, file),
      `${JSON.stringify({ version: versions[file] }, null, 2)}\n`,
      "utf8",
    );
  }
  return root;
}

After:

import { mkdtempSync, writeFileSync, rmSync } from "node:fs";

function fixture(versions) {
  const root = mkdtempSync(join(tmpdir(), "openclaw-release-source-"));
  for (const file of RELEASE_VERSION_FILES) {
    writeFileSync(
      join(root, file),
      `${JSON.stringify({ version: versions[file] }, null, 2)}\n`,
      "utf8",
    );
  }
  return root;
}

// In each test:
// const root = fixture(versions);
// try {
//   ...
// } finally {
//   rmSync(root, { recursive: true, force: true });
// }

44. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-source-version.test.mjs (L56-L62)

The input object hardcodes all four filenames that must exactly match the current contents of RELEASE_VERSION_FILES. If the implementation adds, removes, or renames a file in that array, fixture() will silently create the new file with an undefined version (which clean() treats as ""), causing the new file to appear as a mismatch — potentially inflating or reordering the mismatches array and making the subsequent assert.deepEqual on mismatch filenames fail or pass incorrectly.

Consider building the input object programmatically from RELEASE_VERSION_FILES so any future change is caught explicitly rather than silently.

💡 Suggested Change

Before:

test("reports every mismatched or missing version before publish", () => {
  const root = fixture({
    "package.json": "0.1.20",
    "openclaw.plugin.json": "0.1.20-beta.0",
    "moltbot.plugin.json": "",
    "clawdbot.plugin.json": "0.1.19",
  });

After:

test("reports every mismatched or missing version before publish", () => {
  // Build from RELEASE_VERSION_FILES so new entries are caught explicitly.
  const allMatch = Object.fromEntries(
    RELEASE_VERSION_FILES.map((f) => [f, "0.1.20"]),
  );
  const root = fixture({
    ...allMatch,
    "openclaw.plugin.json": "0.1.20-beta.0",
    "moltbot.plugin.json": "",
    "clawdbot.plugin.json": "0.1.19",
  });

45. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-source-version.test.mjs (L65-L72)

This assertion implicitly requires that the implementation returns mismatches in the same order as RELEASE_VERSION_FILES. While that is currently guaranteed (the implementation iterates an array), this ordering dependency is undocumented. If the implementation ever switches to set/map iteration or sorts the results, this test will fail with a non-obvious error message. A comment or using assert.deepEqual(new Set(...), new Set(...)) for the filenames would make the intent explicit.

💡 Suggested Change

Before:

  assert.deepEqual(
    report.mismatches.map((entry) => entry.file),
    [
      "openclaw.plugin.json",
      "moltbot.plugin.json",
      "clawdbot.plugin.json",
    ],
  );

After:

  // Order must match RELEASE_VERSION_FILES iteration order (implementation guarantee).
  assert.deepEqual(
    report.mismatches.map((entry) => entry.file),
    [
      "openclaw.plugin.json",
      "moltbot.plugin.json",
      "clawdbot.plugin.json",
    ],
  );

46. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-source-version.test.mjs (L85-L95)

The -c user.name and -c user.email flags are only passed to the commit subcommand, not to git add or git init. On CI runners with no global Git identity configured, git add succeeds, but a missing identity is typically only enforced at commit time, so this is fine in practice. However, git init may emit a hint about the default branch name to stderr (piped but not shown), and there is no fallback if git is not on PATH. Consider adding a guard or a skip condition so the test fails fast with a clear message rather than a cryptic execFileSync error when git is unavailable.

💡 Suggested Change

Before:

  git(root, ["init"]);
  git(root, ["add", ...RELEASE_VERSION_FILES]);
  git(root, [
    "-c",
    "user.name=release-test",
    "-c",
    "user.email=release-test@example.com",
    "commit",
    "-m",
    "release fixture",
  ]);

After:

  // Guard: skip if git is not available
  try {
    execFileSync("git", ["--version"], { stdio: "ignore" });
  } catch {
    return; // skip test in environments without git
  }

  git(root, ["init"]);
  git(root, ["-c", "init.defaultBranch=main", "init"]); // suppress branch hint noise
  git(root, ["add", ...RELEASE_VERSION_FILES]);
  git(root, [
    "-c",
    "user.name=release-test",
    "-c",
    "user.email=release-test@example.com",
    "commit",
    "-m",
    "release fixture",
  ]);

47. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/validate-release-source-version.test.mjs (L39-L45)

stdio: ['ignore', 'pipe', 'pipe'] pipes stderr but discards it on failure (the thrown error object from execFileSync includes stderr only if stdio is 'pipe' for stderr, but the output isn't forwarded to the test runner). When a git command fails, the error message shown by node:test will only show the exit code and command, not the git error text, making debugging difficult.

Use stdio: 'pipe' (shorthand for all three) or stdio: ['ignore', 'pipe', 'inherit'] to let stderr flow to the terminal during test failures.

💡 Suggested Change

Before:

function git(root, args) {
  return execFileSync("git", args, {
    cwd: root,
    encoding: "utf8",
    stdio: ["ignore", "pipe", "pipe"],
  });
}

After:

function git(root, args) {
  return execFileSync("git", args, {
    cwd: root,
    encoding: "utf8",
    stdio: ["ignore", "pipe", "inherit"],
  });
}

48. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/wait-for-npm-release.mjs (L128-L134)

The pre-flight validation call passes packument: {}, which causes inspectNpmReleaseVisibility to return early at the packument?.versions?.[targetVersion] check — before it ever reaches the distTag and expectedGitHead validation guards. As a result, if distTag or expectedGitHead is missing or malformed, no error is thrown at startup; instead the bad inputs silently become a pending result on the first poll, and the loop will spin for the full totalTimeoutMs before failing.

Suggest separating input-only validation into a dedicated helper (or inline the three guard checks directly in waitForNpmReleaseVisibility), independent of the packument inspection path.

💡 Suggested Change

Before:

  // Validate immutable inputs before starting the bounded wait.
  inspectNpmReleaseVisibility({
    packument: {},
    version,
    distTag,
    expectedGitHead,
  });

After:

  // Validate immutable inputs before starting the bounded wait.
  // Only validate the scalar inputs here; packument-dependent checks are done per-poll.
  const _version = clean(version);
  const _distTag = clean(distTag);
  const _expected = clean(expectedGitHead).toLowerCase();
  if (!_version) throw new Error("NPM_RELEASE_VERSION is required.");
  if (!_distTag) throw new Error("NPM_DIST_TAG is required.");
  if (!SHA_PATTERN.test(_expected)) {
    throw new Error("NPM_EXPECTED_GIT_HEAD must be an exact 40-character commit SHA.");
  }

49. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/wait-for-npm-release.mjs (L144-L150)

If the fetch implementation does not honour the AbortSignal promptly (e.g., a stalled TCP connection), await fetchImpl(...) may never settle even after the abort fires. Because there is no outer deadline guard around the await fetchImpl call, the polling loop can stall indefinitely — potentially for far longer than totalTimeoutMs. In a CI context this manifests as a step that hangs until the job-level timeout-minutes limit is hit.

Consider wrapping the fetch with a Promise.race against a hard-deadline rejection promise keyed off deadline, so the loop is guaranteed to exit (with an error) even when the underlying network layer ignores the abort signal.

💡 Suggested Change

Before:

    const controller = new AbortController();
    const requestTimer = setTimeout(
      () => controller.abort(),
      Math.min(singleRequestTimeoutMs, remainingBeforeRequest),
    );
    try {
      const response = await fetchImpl(npmPackumentUrl(registryUrl, packageName, now()), {

After:

    const controller = new AbortController();
    const requestTimeoutDuration = Math.min(singleRequestTimeoutMs, remainingBeforeRequest);
    const requestTimer = setTimeout(() => controller.abort(), requestTimeoutDuration);
    // Hard-deadline guard: if fetch ignores the AbortSignal, reject after the same interval.
    const timeoutGuard = new Promise((_, reject) =>
      setTimeout(
        () => reject(Object.assign(new Error("registry request timed out"), { name: "AbortError" })),
        requestTimeoutDuration + 500,
      )
    );
    try {
      const fetchPromise = fetchImpl(npmPackumentUrl(registryUrl, packageName, now()), {

50. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/wait-for-npm-release.mjs (L199-L207)

remaining is computed once before the log call, then the remaining <= 0 break check and Math.min(pollIntervalMs, remaining) both reuse that stale snapshot. Time continues to elapse during the log call and between statements. On the final iteration, if remaining is small but positive, the loop will sleep for Math.min(pollIntervalMs, remaining) using the stale value — which may still be close to pollIntervalMs — even though the actual remaining time by the point sleep is called is already 0 or negative. This causes the loop to overshoot the deadline by up to one full pollIntervalMs.

Recompute remaining immediately before the sleep call, or use a fresh now() reference.

💡 Suggested Change

Before:

    const remaining = deadline - now();
    log(
      `npm visibility attempt ${attempt} pending: ${lastReport.reason}; ` +
        `${Math.max(0, Math.ceil(remaining / 1000))}s remain.`,
    );
    if (remaining <= 0) {
      break;
    }
    await sleep(Math.min(pollIntervalMs, remaining));

After:

    const remaining = deadline - now();
    log(
      `npm visibility attempt ${attempt} pending: ${lastReport.reason}; ` +
        `${Math.max(0, Math.ceil(remaining / 1000))}s remain.`,
    );
    const remainingBeforeSleep = deadline - now();
    if (remainingBeforeSleep <= 0) {
      break;
    }
    await sleep(Math.min(pollIntervalMs, remainingBeforeSleep));

51. apps/MemOS-Cloud-OpenClaw-Plugin/.github/scripts/wait-for-npm-release.mjs (L27)

The cache-bust query parameter is redundant alongside the already-correct cache-control: no-cache, no-store, max-age=0 and pragma: no-cache request headers. The query parameter adds a unique URL per invocation, which may pollute proxy/CDN caches (treating each unique URL as a distinct cache entry) and could trigger 404s or unexpected behaviour on registry mirrors that reject unknown query parameters. The HTTP headers are the appropriate and sufficient mechanism for cache bypass here.

💡 Suggested Change

Before:

  return `${registry}/${encodedName}?cache-bust=${encodeURIComponent(cacheBust)}`;

After:

  return `${registry}/${encodedName}`;

52. apps/MemOS-Cloud-OpenClaw-Plugin/.github/workflows/workflow-contract-lint.yml (L45-L49)

actions/checkout@v7 does not exist. As of 2026, the latest major release of actions/checkout is v4. Using a non-existent tag causes the step to fail immediately at resolution time with an error like "Unable to resolve action", which will prevent the entire job from running. The same issue applies to actions/setup-node@v7 in the release-contracts job below.

Suggestion: pin to actions/checkout@v4 and actions/setup-node@v4.

💡 Suggested Change

Before:

      - uses: actions/checkout@v7
        with:
          persist-credentials: false

      - name: Install verified actionlint

After:

      - uses: actions/checkout@v4
        with:
          persist-credentials: false

      - name: Install verified actionlint

53. apps/MemOS-Cloud-OpenClaw-Plugin/.github/workflows/workflow-contract-lint.yml (L64-L65)

actionlint -color is called without specifying a target path. In a monorepo layout, actions/checkout places files at the repository root, so the CWD when the step runs is the repo root — not the plugin subdirectory apps/MemOS-Cloud-OpenClaw-Plugin/. By default, actionlint searches for **/.github/workflows/*.{yml,yaml} relative to the CWD, which will target the root .github/workflows/ directory rather than the plugin's own workflows under apps/MemOS-Cloud-OpenClaw-Plugin/.github/workflows/. This means the step will silently lint the wrong files (or no files at all if none exist at the root), entirely defeating the purpose of this job.

Suggestion: explicitly pass the plugin's workflow directory as the target.

💡 Suggested Change

Before:

          "${binary}" -version
          "${binary}" -color

After:

          "${binary}" -version
          "${binary}" -color apps/MemOS-Cloud-OpenClaw-Plugin/.github/workflows/

54. apps/MemOS-Cloud-OpenClaw-Plugin/.github/workflows/workflow-contract-lint.yml (L71-L77)

actions/checkout@v7 and actions/setup-node@v7 do not exist; the latest major versions are both v4. This will cause the release-contracts job to fail immediately at action resolution, preventing any contract tests from running.

💡 Suggested Change

Before:

      - uses: actions/checkout@v7
        with:
          persist-credentials: false

      - uses: actions/setup-node@v7
        with:
          node-version: 22

After:

      - uses: actions/checkout@v4
        with:
          persist-credentials: false

      - uses: actions/setup-node@v4
        with:
          node-version: 22

55. apps/MemOS-Cloud-OpenClaw-Plugin/scripts/generate-telemetry-credentials.cjs (L22-L28)

The warning message only mentions MEMOS_ARMS_ENDPOINT, but this branch is reached when both variables are unset. A reader who sets only MEMOS_ARMS_PID (leaving MEMOS_ARMS_ENDPOINT empty) will see this misleading message and assume the issue is solely the endpoint, then exit cleanly — when in fact the intent may have been a partial-credential error. The message should reflect the actual condition.

💡 Suggested Change

Before:

if (!endpoint && !pid) {
  console.warn(
    "[generate-telemetry-credentials] MEMOS_ARMS_ENDPOINT not set; " +
      "skipping. RUM telemetry will be disabled in this build.",
  );
  process.exit(0);
}

After:

if (!endpoint && !pid) {
  console.warn(
    "[generate-telemetry-credentials] Neither MEMOS_ARMS_ENDPOINT nor MEMOS_ARMS_PID is set; " +
      "skipping. RUM telemetry will be disabled in this build.",
  );
  process.exit(0);
}

56. apps/MemOS-Cloud-OpenClaw-Plugin/scripts/generate-telemetry-credentials.cjs (L20)

The double-fallback is confusing: || "prod" on the right of the String(…) call already handles an unset/empty variable, making the outer || "prod" (after .trim()) only relevant when the variable is set to whitespace-only. This intent should be made explicit with a dedicated check rather than chaining two fallbacks.

💡 Suggested Change

Before:

const env = String(process.env.MEMOS_ARMS_ENV || "prod").trim() || "prod";

After:

const envRaw = String(process.env.MEMOS_ARMS_ENV ?? "").trim();
const env = envRaw || "prod";

57. apps/MemOS-Cloud-OpenClaw-Plugin/scripts/generate-telemetry-credentials.cjs (L52)

The credential file is written with the process's default umask (typically 0644), making it world-readable to any other process on the same CI runner. On shared runners this exposes the endpoint and pid values for the duration of the build. Pass a restrictive mode option to limit access to the owner only.

💡 Suggested Change

Before:

fs.writeFileSync(out, JSON.stringify({ endpoint, pid, env }, null, 2) + "\n", "utf-8");

After:

fs.writeFileSync(out, JSON.stringify({ endpoint, pid, env }, null, 2) + "\n", { encoding: "utf-8", mode: 0o600 });

58. apps/MemOS-Cloud-OpenClaw-Plugin/test/check-update-version.test.mjs (L7-L10)

Wrapping comparator results in > 0/< 0 before passing to assert.equal produces unhelpful error messages on failure — the output will only say false !== true instead of revealing the actual numeric value returned. Prefer assert.ok() or, even better, compare the return value directly with assert.equal(..., expectedNumber) (as done on line 11) so that assertion failures are self-describing.

💡 Suggested Change

Before:

  assert.equal(compareVersions("1.0.0-beta.10", "1.0.0-beta.9") > 0, true);
  assert.equal(compareVersions("1.0.0-beta.20", "1.0.0-beta.19") > 0, true);
  assert.equal(compareVersions("1.0.0-beta.1", "1.0.0-beta.alpha") < 0, true);
  assert.equal(compareVersions("1.0.0", "1.0.0-rc.1") > 0, true);

After:

  assert.ok(compareVersions("1.0.0-beta.10", "1.0.0-beta.9") > 0);
  assert.ok(compareVersions("1.0.0-beta.20", "1.0.0-beta.19") > 0);
  assert.ok(compareVersions("1.0.0-beta.1", "1.0.0-beta.alpha") < 0);
  assert.ok(compareVersions("1.0.0", "1.0.0-rc.1") > 0);

59. apps/MemOS-Cloud-OpenClaw-Plugin/test/check-update-version.test.mjs (L8)

This assertion (beta.20 > beta.19) is functionally identical to the previous line (beta.10 > beta.9) — both verify numeric ordering between two single-digit-gap prerelease numbers. It adds no new coverage. Consider replacing it with a distinct boundary case, such as comparing equal versions (compareVersions("1.0.0", "1.0.0") === 0), major/minor/patch ordering (2.0.0 > 1.9.9), or v-prefixed inputs (v1.0.0 vs 1.0.0).

💡 Suggested Change

Before:

  assert.equal(compareVersions("1.0.0-beta.20", "1.0.0-beta.19") > 0, true);

After:

  assert.equal(compareVersions("1.0.0", "1.0.0"), 0);

60. apps/MemOS-Cloud-OpenClaw-Plugin/index.js (L736-L738)

When detectHostVersion() returns null (version undetectable — e.g. custom build, detection failure), needsHookPolicy is set to true, so the plugin will attempt to write openclaw.json on every gateway start. The inline comment explicitly states "older hosts do not understand the field and don't need it patched in", yet an undetectable version — which could be an old host — triggers the patch. This risks config corruption on hosts that predate the hook-policy field. A safer default is false (skip patching when version is unknown), matching the stated intent.

💡 Suggested Change

Before:

  const needsHookPolicy =
        hostVersion === null ||
        compareVersionStrings(hostVersion, HOOK_POLICY_MIN_VERSION) >= 0;

After:

  const needsHookPolicy =
        hostVersion !== null &&
        compareVersionStrings(hostVersion, HOOK_POLICY_MIN_VERSION) >= 0;

61. apps/MemOS-Cloud-OpenClaw-Plugin/index.js (L487-L490)

The eviction check runs AFTER the key is added, so the Set momentarily grows to MAX_CAPTURE_KEYS + 1 (1001) before one entry is removed. Additionally, the duplicate-detection path (recentCaptureKeys.delete(key) then recentCaptureKeys.add(key)) re-adds the key at the tail for LRU ordering but skips the eviction guard entirely — meaning a repeated duplicate hit never triggers eviction at all. Add the eviction check on the duplicate path as well, and check size before adding (or accept the off-by-one and document it).

💡 Suggested Change

Before:

  recentCaptureKeys.add(key);
  if (recentCaptureKeys.size > MAX_CAPTURE_KEYS) {
    recentCaptureKeys.delete(recentCaptureKeys.keys().next().value);
  }

After:

  if (recentCaptureKeys.size >= MAX_CAPTURE_KEYS) {
    recentCaptureKeys.delete(recentCaptureKeys.keys().next().value);
  }
  recentCaptureKeys.add(key);

62. apps/MemOS-Cloud-OpenClaw-Plugin/index.js (L457-L459)

The guard for an invalid eventIdentity uses three strict equality checks (=== undefined, === null, === ""). When stableMessageIdentities is empty and runId is the number 0 (a valid, falsy run identifier), the guard passes and 0 is used as eventIdentity. When stableMessageIdentities is non-empty, eventIdentity is an array, which can never equal any of the three checked values, so the guard is silently bypassed regardless of content. Use a single falsy check or validate the array case explicitly to make the intent clear and handle edge cases consistently.

💡 Suggested Change

Before:

  if (!sessionIdentity || eventIdentity === undefined || eventIdentity === null || eventIdentity === "") {
    return null;
  }

After:

  if (!sessionIdentity || !eventIdentity || (Array.isArray(eventIdentity) && eventIdentity.length === 0)) {
    return null;
  }

63. apps/MemOS-Cloud-OpenClaw-Plugin/index.js (L160-L162)

When agentId is set but userFilter is empty/null, the code now wraps the single condition in an and array: { and: [{ agent_id: agentId }] }. The previous behaviour set filterObj = { agent_id: agentId } (a flat object). A single-element and array is semantically different and may not be handled correctly by backend API versions that expect the flat form. Consider keeping the flat form: userFilter = { agent_id: agentId };

💡 Suggested Change

Before:

    } else {
      userFilter = { and: [{ agent_id: agentId }] };
    }

After:

    } else {
      userFilter = { agent_id: agentId };
    }

64. apps/MemOS-Cloud-OpenClaw-Plugin/index.js (L913-L917)

The throttle check reads lastCaptureTime at the top of the handler, but lastCaptureTime = now is now deferred until after reserveCapture returns non-duplicate. In practice both calls are synchronous, so there is no true concurrency risk in a single Node.js event loop turn. However, the intent of the throttle — to prevent rapid successive captures — is undermined because lastCaptureTime is not updated for throttled-but-not-deduplicated events (those that pass throttle, get reserved, then fail addMessage). Consider setting lastCaptureTime = now immediately after the throttle check passes, before the deduplication logic, to preserve the original throttle semantics.


65. apps/MemOS-Cloud-OpenClaw-Plugin/index.js (L300)

A full data URI string (data:<mediaType>;base64,<payload>) is constructed in memory solely so describeAttachment can extract the mediaType back via regex and report the string length. For large image payloads this creates a multi-megabyte temporary string on every tool-result message. Avoid the round-trip by computing the description directly from the already-available mediaType and claudeData length.

💡 Suggested Change

Before:

      pushText(describeAttachment("image", `data:${mediaType};base64,${String(claudeData)}`));

After:

      pushText(`[image (${mediaType} base64, ~${String(claudeData).length} chars)]`);

66. apps/MemOS-Cloud-OpenClaw-Plugin/index.js (L257-L260)

Several comment blocks are written in Chinese (e.g. lines starting with // 把单个附件值..., // MemOS 是文本记忆服务..., // 把 OpenClaw 的单条原始消息..., // 解析所有协议下的 image 类 block...). The rest of the codebase uses English comments. For an open-source project accepting community PRs, all source-code comments should be in English to maintain accessibility for all contributors.


67. apps/MemOS-Cloud-OpenClaw-Plugin/test/add-dedup.test.mjs (L95-L96)

The parameter is named _url (the conventional "unused" prefix) but is actually used on the very next line to filter by URL path. This is misleading: readers will assume the URL is intentionally ignored, while in reality it gates the adds counter. Rename it to url (or fetchUrl) to match its actual usage and avoid confusing future maintainers.

Suggested fix:

globalThis.fetch = async (url, options) => {
  if (url.endsWith("/add/message")) adds += 1;

68. apps/MemOS-Cloud-OpenClaw-Plugin/test/add-dedup.test.mjs (L281-L285)

This test asserts attempts === 1 after 5 replays with retries: 0, relying on the in-memory dedup set (recentCaptureKeys) preventing any retry after the first failed attempt. However, the dedup key is inserted into the set before addMessage is called (regardless of success or failure — confirmed in reserveCapture). This means even a transient error (e.g. a flaky network blip) will permanently suppress all future retries for that turn until the set is LRU-evicted at 1 000 entries. The test validates this silencing behaviour as correct, but it also masks the fact that a user-configured retries: 3 is rendered ineffective for any event that experienced even one failure. Consider adding a companion test that verifies a transient error (first call fails, second call succeeds) still results in a successful write on the same replay, to ensure the retry budget is honoured within a single delivery attempt.


69. apps/MemOS-Cloud-OpenClaw-Plugin/test/add-dedup.test.mjs (L522-L523)

The two createAgentEndHandler instances share the same sessionKey ("agent:main:test:new-conversation") but use different sessionIds ("session-1" and "session-2"). Because recentCaptureKeys is a module-level singleton (shared across all plugin.register() calls in the same process), and because reserveCapture builds its dedup key using ctx?.sessionId || ctx?.sessionKey, the dedup key for the two handlers will differ (different sessionId values), so both writes correctly proceed. The test currently passes for the right reason, but this relies on an implicit understanding of the key-construction logic. If sessionId were ever dropped from the key (e.g., a future refactor normalises the identity to just sessionKey), this test would start asserting adds === 2 while the actual behaviour silently became adds === 1. Consider adding an explicit assertion about the independence of the two handlers (e.g., checking each handler's individual adds count) or a comment explaining why the shared sessionKey is safe here.


70. apps/MemOS-Cloud-OpenClaw-Plugin/test/add-dedup.test.mjs (L123)

The mock fetch returns a Promise that only rejects when options.signal fires an "abort" event. The abort is triggered by callApi via setTimeout(() => controller.abort(), timeoutMs) with timeoutMs: 10 ms. In a heavily-loaded CI environment the Node.js event loop may delay timers beyond their nominal deadline, causing the test to hang longer than expected. While node:test has a default per-test timeout, there is no explicit { timeout: <ms> } option set on this test, so a stall will block the entire suite until the runner's global timeout (often several minutes) fires. Add an explicit short timeout to bound the worst-case hang:

test("40 redeliveries do not exceed the configured two add attempts", { timeout: 5000 }, async () => {```

<details><summary>💡 Suggested Change</summary>

**Before:**

test("40 redeliveries do not exceed the configured two add attempts", async () => {


**After:**
```suggestion
test("40 redeliveries do not exceed the configured two add attempts", { timeout: 5000 }, async () => {

71. apps/MemOS-Cloud-OpenClaw-Plugin/test/direct-session-user-id.test.mjs (L97-L103)

The expected-value computation is a verbatim copy of the IIFE in index.js that defines MEMOS_SOURCE. Because both sides execute the same runtime logic, this test can never catch a regression where the suffix mapping in production code is changed: if someone renames openclaw_macopenclaw_macos, both the production value and the test's expectedSource would change together and the assertion would still pass.

Consider asserting a fixed, platform-specific string instead, or at minimum verify the suffix matches a known allowlist so the test can actually catch unexpected mutations:

const KNOWN_SUFFIXES = new Map([
  ["win32", "openclaw_win"],
  ["darwin", "openclaw_mac"],
  ["linux", "openclaw_linux"],
]);
const expectedSource = KNOWN_SUFFIXES.get(process.platform) ?? "openclaw";

This way the mapping lives only in the test, not duplicated from production code, and a change to the production IIFE will cause the assertion to fail.


72. apps/MemOS-Cloud-OpenClaw-Plugin/test/direct-session-user-id.test.mjs (L104-L124)

buildAddMessagePayload reads several fields from cfg that are absent here (asyncMode, allowPublic, allowKnowledgebaseIds, appId, tags, conversationIdPrefix, conversationIdSuffix, conversationSuffixMode). They all evaluate to undefined, so the assertion on source still passes, but payload.async_mode and payload.allow_public are set to undefined rather than their expected default values, meaning the test exercises a subtly different code path than production.

To make the fixture accurate and to protect against future guard clauses that might branch on these fields, add the missing properties (matching the defaults used in the other tests in this file):

const cfg = {
  userId: "openclaw-user",
  useDirectSessionUserId: false,
  queryPrefix: "",
  maxQueryChars: 0,
  recallGlobal: true,
  knowledgebaseIds: [],
  memoryLimitNumber: 6,
  includePreference: true,
  preferenceLimitNumber: 6,
  includeToolMemory: false,
  toolMemoryLimitNumber: 0,
  relativity: 0.45,
  multiAgentMode: false,
  // fields required by buildAddMessagePayload
  appId: "",
  tags: [],
  info: {},
  allowPublic: false,
  allowKnowledgebaseIds: [],
  asyncMode: true,
  conversationId: "",
  conversationIdPrefix: "",
  conversationIdSuffix: "",
  conversationSuffixMode: "none",
};

73. apps/MemOS-Cloud-OpenClaw-Plugin/test/query-strip.test.mjs (L263-L267)

The only negative/false-positive boundary test added covers the [cron:...] pattern, but the OPENCLAW_SYSTEM_PROMPT_PATTERNS array in the implementation also includes /^\s*System:\s+\[[^\]]+\]\s+/i — a broad pattern that matches any System: [bracket text] prefix. A legitimate user message such as "System: [INFO] 请帮我检查日志" or "System: [error 500] what does this mean?" would match this pattern and be silently dropped to an empty string by both isOpenClawSystemPrompt and sanitizeSearchPayload.

Consider adding a corresponding negative test to document (and protect) the intended boundary:

test("does not treat generic System: prefix as system prompt", () => {
  const inputs = [
    "System: [INFO] please help me debug this",
    "System: [error 500] 这是什么错误?",
  ];
  for (const input of inputs) {
    assert.equal(isOpenClawSystemPrompt(input), false);
    assert.equal(stripOpenClawInjectedPrefix(input), input);
  }
});

If isOpenClawSystemPrompt intentionally returns true for those inputs (because the pattern is designed to capture anything with that prefix), then sanitizeSearchPayload will silently zero out real user queries, which is a data-loss bug that should be caught by a test.


74. apps/MemOS-Cloud-OpenClaw-Plugin/test/query-strip.test.mjs (L368-L382)

This test verifies that sanitizeSearchPayload sets query to "" for a cron system prompt, but no complementary negative case is added to confirm that a user message that starts with a bracket expression but is not a cron prompt is left untouched.

For example, a user asking "[cron:abc job] 这是什么意思?" (without a UTC reference line) should not be zeroed out. Adding such a case would protect against regressions where the system-prompt detection pattern becomes too greedy:

test("does not empty search payload for cron mentions without time markers", () => {
  const payload = {
    query: "[cron:abc job] 这是什么意思?",
    source: "openclaw",
  };
  assert.deepEqual(sanitizeSearchPayload(payload), payload);
});

75. apps/MemOS-Cloud-OpenClaw-Plugin/test/recall-hook-registration.test.mjs (L46-L56)

Missing test case for the null host-version fallback. When process.argv[1] does not contain an @<version> segment AND no package.json is found by walking up the path (e.g. a CI environment running Node directly with a bare script path), detectHostVersion() returns null. In index.js, the guard hostVersion !== null means this falls back to before_agent_start — but that behaviour is never exercised here. Consider adding a third test that sets process.argv[1] to a plain path (e.g. /usr/local/bin/openclaw) and asserts that before_agent_start is registered and before_prompt_build is not.

💡 Suggested Change

Before:

test("falls back to before_agent_start on older OpenClaw hosts", () => {
  const { api, registeredHooks, restore } = createApi("2026.4.26");
  try {
    plugin.register(api);
  } finally {
    restore();
  }

  assert.ok(registeredHooks.some((hook) => hook.hookName === "before_agent_start"));
  assert.ok(!registeredHooks.some((hook) => hook.hookName === "before_prompt_build"));
});

After:

test("falls back to before_agent_start on older OpenClaw hosts", () => {
  const { api, registeredHooks, restore } = createApi("2026.4.26");
  try {
    plugin.register(api);
  } finally {
    restore();
  }

  assert.ok(registeredHooks.some((hook) => hook.hookName === "before_agent_start"));
  assert.ok(!registeredHooks.some((hook) => hook.hookName === "before_prompt_build"));
});

test("falls back to before_agent_start when host version cannot be detected", () => {
  const registeredHooks = [];
  const originalArgv1 = process.argv[1];
  // Plain path with no '@<version>' segment — detectHostVersion() returns null
  process.argv[1] = "/usr/local/bin/openclaw";
  const api = {
    config: { hooks: { internal: { enabled: false } } },
    logger: {},
    pluginConfig: { apiKey: "mpg-test", recallEnabled: false, addEnabled: false },
    on: (hookName, handler) => { registeredHooks.push({ hookName, handler }); },
    registerHook: () => {},
  };
  try {
    plugin.register(api);
  } finally {
    process.argv[1] = originalArgv1;
  }

  assert.ok(registeredHooks.some((hook) => hook.hookName === "before_agent_start"));
  assert.ok(!registeredHooks.some((hook) => hook.hookName === "before_prompt_build"));
});

76. apps/MemOS-Cloud-OpenClaw-Plugin/test/recall-hook-registration.test.mjs (L9)

The hardcoded process.argv[1] path embeds a developer's personal username (lee). While detectHostVersion() uses a simple /@(\d+\.\d+(?:\.\d+)?)/ regex that is entirely insensitive to the surrounding path, the unnecessarily specific path could mislead future contributors into thinking the username or the exact pnpm store layout matters for the extraction logic. Prefer a minimal path that still contains the @<version> token, e.g. /home/user/.pnpm/openclaw@${hostVersion}/node_modules/openclaw/openclaw.mjs.

💡 Suggested Change

Before:

  process.argv[1] = `C:\\Users\\lee\\AppData\\Local\\pnpm\\global\\5\\.pnpm\\openclaw@${hostVersion}\\node_modules\\openclaw\\openclaw.mjs`;

After:

  process.argv[1] = `/home/user/.pnpm/openclaw@${hostVersion}/node_modules/openclaw/openclaw.mjs`;

🧹 Filtered 2 low-confidence OCR finding(s) before posting/fix-loop (existing_code_mismatch: 1, duplicate: 1).

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

⚠️ Automated Test Results: ENV ISSUE

The test environment encountered an issue that requires manual attention.

Details: Executor error: Command failed: git clone --depth 1 --branch chore/sync-cloud-openclaw-plugin-20260818 git@github.com:MemTensor/MemOS.git /data/test-workspaces/dbd056f2cc29fe6a/repo
Cloning into '/data/test-workspaces/dbd056f2cc29fe6a/repo'...
warning: Could not find remote branch chore/sync-cloud-openclaw-plugin-20260818 to clone.
fatal: Remote branch chore/sync-cloud-openclaw-plugin-20260818 not found in upstream origin
Branch: chore/sync-cloud-openclaw-plugin-20260818

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:plugin OpenClaw & Hermes status:in-progress Someone or AI is working on it | 人工或 AI 正在处理

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants