Skip to content

ci: keep local plugin standalone publishing compatible with MemOS releases - #2198

Merged
syzsunshine219 merged 3 commits into
mainfrom
fix/local-plugin-beta-legacy-publisher
Aug 3, 2026
Merged

ci: keep local plugin standalone publishing compatible with MemOS releases#2198
syzsunshine219 merged 3 commits into
mainfrom
fix/local-plugin-beta-legacy-publisher

Conversation

@MLittleprince

Copy link
Copy Markdown
Collaborator

Summary

  • keep MemOS Release — Publish as the weekly whole-repo release/docs fallback path
  • keep the legacy local-plugin publisher usable for beta/non-latest package releases without Doc Agent/docs payloads
  • keep legacy latest releases docs-capable, while selecting the previous stable local-plugin tag instead of a beta tag for docs evidence

Validation

  • node --test .github/scripts/draft-local-plugin-release-notes.test.mjs
  • node --test .github/scripts/prepare-memos-release.test.mjs
  • ruby workflow YAML parse for .github/workflows/*.yml
  • local package-only beta release-notes simulation

@Memtensor-AI Memtensor-AI added area:core MOS 编排层 / 框架底座 / 跨模块问题 status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 3, 2026
@Memtensor-AI
Memtensor-AI requested a review from WeiminLee August 3, 2026 09:36
@Memtensor-AI

Memtensor-AI commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

🤖 Open Code Review

Target: PR #2198
Task: d906a1a07079ff5f
Base: main
Head: fix/local-plugin-beta-legacy-publisher

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


1. .github/scripts/draft-local-plugin-release-notes.mjs (L1043)

When NPM_DIST_TAG is unset and the version-based check triggers (1.0.0-beta.1 with no dist-tag env var), legacyPackageDraftFromEvidence is called with an empty npmDistTag and defaults to "beta" in the release notes text. This hardcoded fallback may be factually incorrect — the actual publish command may use a different dist-tag (e.g. alpha, rc, next). The two functions use different defaults: isLegacyPackageOnlyRelease treats an empty dist-tag as a non-trigger (only version-based), while legacyPackageDraftFromEvidence masks the absence with "beta".

Consider passing the resolved dist-tag from outside both functions, or at minimum using displayVersion(version) in the bullet to reflect the actual prerelease identifier instead of the hardcoded "beta" fallback.

💡 Suggested Change

Before:

  const distTag = String(npmDistTag || "").trim() || "beta";

After:

  // Avoid hardcoding 'beta': if no dist-tag is known, omit the dist-tag phrase or
  // surface the actual prerelease identifier from the version string.
  const distTag = String(npmDistTag || "").trim();
  const distTagLabel = distTag || parseSemver(cleanVersion(version))?.prerelease || "prerelease";

2. .github/scripts/draft-local-plugin-release-notes.mjs (L452-L454)

The regex in validateLegacyPackageNotes hardcodes the marker strings "doc-agent-release-notes-json" and "doc-agent: source-id=" that are already defined as the constant RELEASE_NOTES_MARKER (line 38) and used verbatim in ensureSourceHint. If either string is ever renamed or updated, this validation regex will silently stop catching them, allowing policy-violating notes to pass through.

Consider building the regex from the constant to stay in sync:

const markerPattern = new RegExp(`${RELEASE_NOTES_MARKER}|doc-agent:\\s*source-id=`);
if (markerPattern.test(text)) { fail(...); }
💡 Suggested Change

Before:

  if (/doc-agent:\s*source-id=|doc-agent-release-notes-json/.test(text)) {
    fail("Legacy package release notes must not include Doc Agent source hints or docs payloads.");
  }

After:

  const legacyForbiddenPattern = new RegExp(`${RELEASE_NOTES_MARKER}|doc-agent:\\s*source-id=`);
  if (legacyForbiddenPattern.test(text)) {
    fail("Legacy package release notes must not include Doc Agent source hints or docs payloads.");
  }

3. .github/scripts/draft-local-plugin-release-notes.mjs (L1038-L1039)

In legacyPackageDraftFromEvidence, version is used raw (from evidence?.target_version) in the release note bullet, while targetPackageVersion applies cleanVersion. This means if target_version carries a v prefix (e.g. "v2.0.13-beta.1"), the display line will be:

Published MemOS Local Plugin v2.0.13-beta.1 as a package prerelease ...
- Package version: 2.0.12 -> 2.0.13-beta.1

The v prefix is inconsistently applied within the same notes block. Consider using displayVersion(version) for consistent formatting in the first bullet, and documenting that the version arrow line deliberately omits it.

💡 Suggested Change

Before:

  const version = evidence?.target_version || "";
  const targetPackageVersion = cleanVersion(version);

After:

  const version = evidence?.target_version || "";
  const displayedVersion = displayVersion(version); // consistent v-prefixed display
  const targetPackageVersion = cleanVersion(version);

4. .github/scripts/prepare-memos-release.mjs (L524-L528)

The pre-release branch unconditionally calls incrementPatchVersion(previousReleasedVersion), which itself calls fail() if previousReleasedVersion happens to be a pre-release version (see incrementPatchVersion lines 163-166). While previousReleasedVersion is unlikely to be a pre-release in normal flow, the function's documented contract is that it won't auto-increment pre-release inputs. If a pre-release tag ever becomes the "previous released version", this path will throw an unexpected error. An explicit guard or comment explaining why previousReleasedVersion is guaranteed stable here would improve safety and maintainability.


5. .github/scripts/prepare-memos-release.mjs (L876)

The pattern terms inject and rank are very generic English words that can easily appear in unrelated commit messages (e.g., "inject dependency", "rank ordering", "inject mock"). This may cause false-positive matches, incorrectly classifying unrelated commits under the "Recall relevance and host input handling" topic. Consider narrowing the pattern with word-boundary anchors (\b) or more specific phrases to reduce misclassification risk.

💡 Suggested Change

Before:

    pattern: /recall|retrieval|host input|inject|rank|dedupe|keyword|relevance/i,

After:

    pattern: /\brecall\b|\bretrieval\b|host[- ]input|\bhost_input\b|\bdedupe\b|\bkeyword\b|\brelevance\b|\brank(?:ing)?\b(?=.*(?:recall|retrieval|relevance))/i,

6. .github/scripts/prepare-memos-release.mjs (L883-L891)

When the input matches the revert capture pattern (first .replace()), the resulting $1 text is then passed through the second .replace() which tries to strip a Conventional Commit prefix again. If the reverted commit subject itself has a prefix like feat(plugin): ..., it will be stripped correctly. However, if the revert pattern fails to match (e.g. revert message uses single quotes or no quotes), the text falls through with the full revert "..." intact, and the second rule won't match it either since it checks for revert at the start but won't capture the inner subject. This means revert subjects that don't exactly match the double-quote convention will silently produce a raw revert string in the output. The category check on line 900 (/^fix|^revert/i.test(source)) still works correctly because it tests source (the raw input), not cleaned — but the displayed text may be ugly.


7. .github/scripts/prepare-memos-release.mjs (L898-L903)

After fallbackSubjectText strips the Conventional Commit prefix (e.g. fix(plugin): ), cleaned may still contain a PR reference like stabilize sandbox widget (#9999) if the \s+\(#\d+\) trailing pattern doesn't match due to extra trailing spaces or punctuation variation. More critically, the category is determined from source (which still has the original prefix like fix(plugin):), while the text body uses cleaned. These two derived from the same source variable diverging is intentional and correct, but a brief comment clarifying this intent would help future maintainers avoid accidentally changing source to cleaned for the category check.


8. .github/scripts/prepare-memos-release.test.mjs (L806-L809)

The allowGeneric: true flag has no effect here because "fix(plugin): improve recall relevance and host input handling (#2196)" matches the static FALLBACK_TOPIC_RULES rule for /recall|retrieval|host input|.../i, so the generic code path (where prefix stripping actually happens) is never reached. The test name claims to verify "without raw commit prefixes", but it only confirms the static rule lookup — not stripping. This means a regression in the generic CC-prefix stripping logic would not be caught by this test.

Consider using a commit message that does not match any static rule to actually exercise the generic path, or add a parallel allowGeneric: false assertion to document that this test deliberately targets the rule-based path.


9. .github/scripts/prepare-memos-release.test.mjs (L821-L824)

The text_en field is not asserted to contain the cleaned text. Since the generic fallback constructs text_en symmetrically with text_cn (line 902 of production code: `**${PRODUCT_TITLE.en} update**: ${cleaned}`), the assertion assert.match(topic.text_en, /fix\(plugin\)/) only verifies the prefix is absent, but never confirms that the cleaned text stabilize sandbox widget is actually present in text_en. A future regression where text_en is set to an empty fallback string would silently pass this test.

Add assert.match(topic.text_en, /stabilize sandbox widget/) to mirror the text_cn assertion.

💡 Suggested Change

Before:

  assert.equal(topic.category, "Fixed");
  assert.match(topic.text_cn, /stabilize sandbox widget/);
  assert.doesNotMatch(topic.text_cn, /fix\(plugin\)/);
  assert.doesNotMatch(topic.text_en, /fix\(plugin\)/);

After:

  assert.equal(topic.category, "Fixed");
  assert.match(topic.text_cn, /stabilize sandbox widget/);
  assert.match(topic.text_en, /stabilize sandbox widget/);
  assert.doesNotMatch(topic.text_cn, /fix\(plugin\)/);
  assert.doesNotMatch(topic.text_en, /fix\(plugin\)/);

Generated by cloud-assistant via Open Code Review.

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (1/1 executed). memos_github_open_source/smoke: 1/1. Duration: 1s

Branch: fix/local-plugin-beta-legacy-publisher

@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (1/1 executed). memos_github_open_source/smoke: 1/1. Duration: 1s

Branch: fix/local-plugin-beta-legacy-publisher

@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 3, 2026
@Memtensor-AI Memtensor-AI added status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 and removed status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 labels Aug 3, 2026
@Memtensor-AI

Copy link
Copy Markdown
Collaborator

✅ Automated Test Results: PASSED

All tests passed (1/1 executed). memos_github_open_source/smoke: 1/1. Duration: 1s

Branch: fix/local-plugin-beta-legacy-publisher

@Memtensor-AI Memtensor-AI added status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发 and removed status:in-progress Someone or AI is working on it | 人工或 AI 正在处理 labels Aug 3, 2026
@MLittleprince MLittleprince changed the title ci: keep local plugin beta releases package-only ci: keep local plugin standalone publishing compatible with MemOS releases Aug 3, 2026
@syzsunshine219
syzsunshine219 merged commit 93e4082 into main Aug 3, 2026
26 checks passed
@syzsunshine219
syzsunshine219 deleted the fix/local-plugin-beta-legacy-publisher branch August 3, 2026 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:core MOS 编排层 / 框架底座 / 跨模块问题 status:ready Ready for implementation; waiting for assignee or AI dispatch | 可进入实现,等待认领或派发

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants