ci: add CLI changelog release automation by MLittleprince · Pull Request #25 · MemTensor/MemOS-Cloud-CLI

MLittleprince · 2026-07-28T06:40:13Z

Summary

replace tag-push release automation with a manual dry-run/Draft Release workflow for MemOS CLI changelog sync
add evidence collection, Doc Agent candidate validation/repair, docs preview artifacts, and release safety gates
document the v1.0.6 baseline-tag approval/backfill process and post-merge release flow

Validation

node --test .github/scripts/prepare-cli-release.test.mjs
actionlint .github/workflows/release.yml .github/workflows/release-changelog-ci.yml
node --check .github/scripts/prepare-cli-release.mjs
bash -n .github/scripts/publish-cli-release.sh && bash -n scripts/build-binary.sh
git diff --cached --check
sensitive scan for token/internal URL/local path patterns

Safety

This PR does not create any tag, GitHub Release, Docs PR, npm publish, OSS upload, pre/gray, or production deployment.
Do not backfill v1.0.6 until after this PR is merged, because current main still has the old push-tag release trigger.

Memtensor-AI · 2026-07-28T07:16:04Z

🤖 Open Code Review

Target: PR #25
Task: 004883aa9b81d797
Base: main
Head: docs-sync/memos-cli-changelog-only

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

⚠️ 3 warning(s) occurred during review.

1. `.github/scripts/prepare-cli-release.test.mjs` (L59-L66)

Security (High): Real credentials forwarded to subprocess via trustedDispatchEnv

trustedDispatchEnv spreads the full process.env, so any GITHUB_TOKEN, AWS_*, or other secrets present in the developer's shell or CI environment are inherited by child processes. The 'conflicting existing tag' and 'writes a redacted inspection artifact' tests call execFileSync([SCRIPT_PATH]) with this helper but do not include a GITHUB_TOKEN: '' override (unlike the dry-run end-to-end test that explicitly sets GITHUB_TOKEN: ''). If a real token is present, the script may make live GitHub API calls against the actual repository instead of failing safely.

Suggestion: Explicitly clear all known credential variables in trustedDispatchEnv, or always override them to empty strings at the call sites that don't need them:

function trustedDispatchEnv(overrides = {}) {
  return {
    ...process.env,
    GITHUB_REF: "refs/heads/main",
    GITHUB_REPOSITORY: "MemTensor/MemOS-Cloud-CLI",
    // Prevent real tokens from leaking into test subprocesses
    GITHUB_TOKEN: "",
    GH_TOKEN: "",
    ...overrides,
  };
}

2. `.github/scripts/prepare-cli-release.test.mjs` (L103-L125)

Correctness (High): process.chdir() is process-global — unsafe under concurrent test execution

withFixture mutates process.cwd() for the entire Node.js process. node:test runs top-level tests concurrently by default (up to the number of available CPU cores since Node 21). If two withFixture-based tests are interleaved, their process.chdir() calls will race: git operations, writeVersions, and commit() in test A will target the temp directory of test B, silently corrupting commit history and making assertions fail non-deterministically.

The finally block correctly restores the previous directory, but the window between process.chdir(root) and return fn(root) is still a shared mutable critical section.

Suggestion: Pass the working directory explicitly to all child-process calls, or mark each withFixture test with { concurrency: false } / wrap them in a serial test group:

// Option A: serial group
test("git fixture tests", { concurrency: false }, async (t) => {
  await t.test("collects the entire standalone CLI repository…", () => { … });
  // …
});
// Option B: pass cwd to git() and execFileSync instead of chdir
function git(args, cwd) {
  return execFileSync("git", args, { cwd, encoding: "utf8", stdio: ["ignore","pipe","pipe"] }).trim();
}

3. `.github/scripts/prepare-cli-release.test.mjs` (L234-L288)

Correctness (Medium): Shared mutable evidence and validDraft objects risk cross-test contamination

Both module-level constants are plain object literals. If any imported function under test (e.g. validateDraft, injectDraftFault, buildDocsPreview) mutates an argument, all subsequent tests that reference these shared objects will observe corrupted state — producing misleading failures that look unrelated to the root cause.

The fault-injection test line assert.equal(injectDraftFault(validDraft, evidence, "mixed_language", { validationRound: 2 }), validDraft) implicitly assumes injectDraftFault does not mutate validDraft — but that assumption is not tested; it's only checked via reference equality.

Suggestion: Freeze the shared constants, or regenerate them per-test:

const evidence = Object.freeze({
  commits: Object.freeze([Object.freeze({ … })]),
  // …
});
const validDraft = Object.freeze({
  release_items: Object.freeze([Object.freeze({ … })]),
  // …
});

Or use a factory function and call it inside each test.

4. `.github/scripts/prepare-cli-release.test.mjs` (L141)

Security (Medium): Mock bin directory uses world-readable permissions (TOCTOU risk)

mkdirSync(mockBin, { recursive: true }) creates the directory with default permissions (0o755 on most systems). On a shared CI machine, another user process could list /tmp, discover the new mock-bin directory, and replace the gh or git mock scripts between the write()/chmodSync() call and execFileSync("bash", [PUBLISH_SCRIPT_PATH]). The replacement script could exfiltrate the GH_TOKEN env var.

Suggestion: Create the directory with mode 0o700:

mkdirSync(mockBin, { recursive: true, mode: 0o700 });

5. `.github/scripts/prepare-cli-release.test.mjs` (L321-L347)

Test Coverage (Medium): No test for pre-release version strings in validatePublishConfirmation

The test only exercises plain '1.0.7'. Pre-release versions like '1.0.7-rc.1' are valid semver and are exercised by the 'marks prereleases' publish-state-machine test, but validatePublishConfirmation is never tested with them.

The risk: if the implementation builds a confirmation regex as new RegExp('PUBLISH v' + version) without escaping, the dots in 1.0.7-rc.1 become wildcard metacharacters, so 'PUBLISH v1007-rc01' would falsely satisfy the check for version '1.0.7-rc.1'.

Suggestion: Add pre-release coverage:

assert.doesNotThrow(() =>
  validatePublishConfirmation({
    dryRun: "false",
    version: "1.0.7-rc.1",
    confirmation: "PUBLISH v1.0.7-rc.1",
  }),
);
assert.throws(
  () =>
    validatePublishConfirmation({
      dryRun: "false",
      version: "1.0.7-rc.1",
      confirmation: "PUBLISH v1007-rc01",  // dots treated as wildcards
    }),
  /PUBLISH v1\.0\.7-rc\.1/,
);

6. `.github/scripts/prepare-cli-release.test.mjs` (L901-L913)

Maintainability (Medium): Repeated manual process.env save/restore in three async tests

The pattern of saving env vars, mutating them, running the test, and restoring them in a finally block is copy-pasted verbatim across three async tests. A missed delete or a new env variable added to one test but not the others will cause cross-test contamination that's very hard to debug.

Suggestion: Extract a small helper:

async function withEnv(vars, fn) {
  const previous = Object.fromEntries(
    Object.keys(vars).map((k) => [k, process.env[k]]),
  );
  try {
    for (const [k, v] of Object.entries(vars)) {
      if (v === undefined) delete process.env[k];
      else process.env[k] = v;
    }
    return await fn();
  } finally {
    for (const [k, v] of Object.entries(previous)) {
      if (v === undefined) delete process.env[k];
      else process.env[k] = v;
    }
  }
}

Then each test becomes:

await withEnv({
  DOC_AGENT_RELEASE_NOTES_DRAFT_URL: "https://example.invalid/draft",
  DOC_AGENT_RELEASE_NOTES_DRAFT_TOKEN: "test-token",
}, async () => { … });

7. `.github/scripts/prepare-cli-release.test.mjs` (L1529-L1532)

Test Coverage (Low): readFileSync on docs runbook throws ENOENT instead of a clear assertion failure

If the file docs/release-changelog-automation.md is absent (shallow checkout, renamed file, or stripped CI workspace), readFileSync throws Error: ENOENT and the test fails with a cryptic filesystem error rather than a clear assertion message indicating which contract is broken.

Suggestion: Check existence first and surface a descriptive error:

const runbookPath = "docs/release-changelog-automation.md";
assert.ok(
  existsSync(runbookPath),
  `docs runbook must exist at ${runbookPath} — run the baseline audit first`,
);
const runbook = readFileSync(runbookPath, "utf8");

8. `.github/scripts/prepare-cli-release.test.mjs` (L141-L142)

mkdirSync(mockBin, { recursive: true }) creates the directory with the process umask (typically 0o755 on CI). On a shared host, any local user can list /tmp, discover the directory name, and replace the mock gh or git scripts between chmodSync and execFileSync. A malicious replacement could read the GH_TOKEN env var passed to the subprocess.

Suggestion: create the mock-bin directory with mode 0o700:

mkdirSync(mockBin, { recursive: true, mode: 0o700 });

9. `.github/scripts/prepare-cli-release.test.mjs` (L234-L246)

The module-level evidence and validDraft objects are plain mutable objects shared across many tests. If any function under test mutates an argument in place (e.g. injectDraftFault splicing release_items), all subsequent tests that reference these constants will silently observe corrupted state, producing misleading failures far from the root cause.

The fault-injection test line assert.equal(injectDraftFault(validDraft, evidence, "mixed_language", { validationRound: 2 }), validDraft) checks only reference equality — it does not catch in-place mutations to array/object properties.

Suggestion: Object.freeze() the shared constants (recursively), or use factory functions that rebuild them per test.

10. `.github/scripts/prepare-cli-release.test.mjs` (L121-L125)

Temp directories created by mkdtempSync are never deleted. withFixture and runPublishFixture each leave behind a full git repository tree in /tmp without any cleanup in their finally blocks. On long-lived CI agents that run the suite on every PR, this accumulates hundreds of MB and can eventually cause disk-full failures in unrelated jobs.

Suggestion: add an rmSync in the finally block, gated on an opt-out env var so fixtures remain inspectable on failure:

} finally {
  process.chdir(previous);
  if (!process.env.KEEP_TEST_FIXTURES) {
    rmSync(root, { recursive: true, force: true });
  }
}

11. `.github/scripts/prepare-cli-release.test.mjs` (L875-L886)

The save-mutate-restore process.env pattern is copy-pasted verbatim across three async Doc Agent tests. Any future addition of a new env variable to one test but not the others will cause cross-test contamination that is very hard to debug.

Suggestion: extract a small withEnv helper to eliminate the repetition and guarantee symmetric cleanup:

async function withEnv(vars, fn) {
  const saved = Object.fromEntries(
    Object.keys(vars).map((k) => [k, process.env[k]])
  );
  try {
    for (const [k, v] of Object.entries(vars)) {
      if (v === undefined) delete process.env[k];
      else process.env[k] = v;
    }
    return await fn();
  } finally {
    for (const [k, v] of Object.entries(saved)) {
      if (v === undefined) delete process.env[k];
      else process.env[k] = v;
    }
  }
}

12. `.github/scripts/prepare-cli-release.test.mjs` (L340-L346)

validatePublishConfirmation is only tested with the plain version '1.0.7'. Pre-release versions like '1.0.7-rc.1' are valid semver and are used elsewhere (the 'marks prereleases' publish-state-machine test). If the implementation builds the confirmation regex without escaping the version string — e.g. new RegExp('PUBLISH v' + version) — then dots become regex wildcards, and 'PUBLISH v1007-rc01' would falsely satisfy the check for '1.0.7-rc.1'.

Suggestion: add a pre-release case:

assert.doesNotThrow(() =>
  validatePublishConfirmation({
    dryRun: "false",
    version: "1.0.7-rc.1",
    confirmation: "PUBLISH v1.0.7-rc.1",
  }),
);
assert.throws(
  () =>
    validatePublishConfirmation({
      dryRun: "false",
      version: "1.0.7-rc.1",
      confirmation: "PUBLISH v1007-rc01",
    }),
  /PUBLISH v1\.0\.7-rc\.1/,
);

13. `.github/workflows/release-changelog-ci.yml` (L41)

The docker:// image reference uses a digest pinned to rhysd/actionlint, but the image name itself is not prefixed with a registry hostname (e.g., docker.io/). More importantly, Docker Hub image names with only a single-component path segment (rhysd/actionlint) resolve to Docker Hub, which is an unauthenticated, mutable registry for the tag layer even when a digest is provided — a forced-push or registry compromise could swap the manifest before the runner resolves it. Consider using ghcr.io/rhysd/actionlint (which the project already publishes) with the same SHA, or the official rhysd/actionlint-action GitHub Action pinned by commit SHA, so the provenance chain stays within GHCR/GitHub infrastructure.

14. `.github/workflows/release-changelog-ci.yml` (L49-L52)

The Test changelog evidence and safety guards step uses run: without an explicit shell: declaration. Although the top-level runner is Linux (where the default is bash), the review checklist specifically calls out that shell should be explicit for run: steps, especially when the surrounding steps already set shell: bash. Add shell: bash (or shell: sh) to make the intent unambiguous and portable.

15. `.github/workflows/release-changelog-ci.yml` (L73)

scripts/build-binary.sh (note: different root directory from .github/scripts/) is referenced in the bash -n syntax-check but is not listed in the changed-files set and cannot be found in the repository. If the file does not exist on the runner, bash -n scripts/build-binary.sh will exit non-zero with "No such file or directory" and fail the entire CI step. This should either be guarded with an existence check or the path should be corrected. Additionally, the paths: filter in the on: triggers does not include scripts/**, so a change to that script would never re-run this check — an inconsistency worth noting.

16. `.github/workflows/release-changelog-ci.yml`

The BEFORE_SHA regex allows a 64-character hex string ([0-9a-f]{40}([0-9a-f]{24})?$). A standard Git commit SHA is exactly 40 hex characters; 64 characters is not a valid SHA-1 or SHA-256 commit OID format used by GitHub. The secondary group ([0-9a-f]{24})? appears to be a copy/paste artifact and will silently accept a malformed 64-char value, potentially causing git cat-file or git diff to fail with confusing errors rather than falling through to the safe else branch.

17. `.github/workflows/release-changelog-ci.yml` (L45-L47)

Node.js 22 is specified without a patch version or lockfile caching. While this is not a blocking issue for a CI/lint-only job, using 22.x (explicit minor/patch tracking) together with cache: 'npm' (if a package-lock.json exists) would improve reproducibility and speed. Without caching, Node modules are re-downloaded on every run.

18. `.github/workflows/release.yml` (L165-L172)

The build job checks out with the default fetch-depth: 1 and never fetches tags. If either ./scripts/build-binary.sh or ./scripts/build-binary.ps1 uses git describe (a very common pattern for embedding the version string into a binary), the call will fail or produce a meaningless fallback like UNKNOWN because no tags or history are available in the shallow clone.

The prepare job resolves the version and stores it in steps.prepare.outputs.current_tag, but there is no mechanism to pass that value (e.g. as a RELEASE_VERSION env var) to the build steps, so the build scripts cannot fall back to it.

Recommend either:

Adding fetch-depth: 0 to the checkout and a git fetch --tags --force origin step (mirrors what the prepare and release jobs already do), or
Passing RELEASE_VERSION: ${{ needs.prepare.outputs.current_tag }} as an env var to the build steps and updating the build scripts to prefer that value over git describe.

💡 Suggested Change

Before:

      - uses: actions/checkout@v4
        with:
          ref: ${{ needs.prepare.outputs.target_sha }}
          persist-credentials: false

      - uses: actions/setup-python@v5
        with:
          python-version: "3.10"

After:

      - uses: actions/checkout@v4
        with:
          ref: ${{ needs.prepare.outputs.target_sha }}
          fetch-depth: 0
          persist-credentials: false

      - name: Fetch release tags
        shell: bash
        run: git fetch --tags --force origin

      - uses: actions/setup-python@v5
        with:
          python-version: "3.10"
          cache: pip

19. `.github/workflows/release.yml` (L170-L172)

No pip dependency caching is configured. Every matrix leg (Linux + Windows) performs a full pip install on every run. Adding cache: pip to actions/setup-python is a one-line change that reuses the pip HTTP cache across runs and meaningfully reduces build time.

💡 Suggested Change

Before:

      - uses: actions/setup-python@v5
        with:
          python-version: "3.10"

After:

      - uses: actions/setup-python@v5
        with:
          python-version: "3.10"
          cache: pip

🧹 Filtered 4 low-confidence OCR finding(s) before posting/fix-loop (duplicate: 4).

Generated by cloud-assistant via Open Code Review.

Memtensor-AI · 2026-07-28T07:17:17Z