[MM-69594] Write resolved manifest (with version) into bundled plugin.json

[cursor]

Summary

The released mattermost-plugin-boards bundle shipped a plugin.json with no version field, unlike Playbooks. The bundle Makefile target copied the source plugin.json verbatim (cp $(MANIFEST_FILE) ...), and the checked-in plugin.json has never had a version key. The boards build/manifest tool already resolved a build-time version in findManifest(), but no step wrote that resolved manifest into the bundle.

This matches the Playbooks / mattermost-plugin-starter-template pattern:

• Add a dist subcommand to build/manifest/main.go that writes the resolved manifest (including the build-time version and generated release_notes_url) as plugin.json into the bundle directory.
• Update the bundle target to call ./build/bin/manifest dist $(BUNDLE_DIR)/$(PLUGIN_NAME) instead of the raw cp.

The source plugin.json is intentionally left without a version (resolved at build time), and the bundle now always carries one.

Ticket

MM-69594

Testing

• Reproduced by running the bundle recipe: the bundled dist/boards/plugin.json had no version field.
• After the fix, the bundled plugin.json carries version (e.g. 9.3.0-rc3, resolved from the git tag) and a populated release_notes_url; the tarball name was already correct.
• Added Go tests in build/manifest/main_test.go covering distManifest, the version-resolution branches (tag at current commit, latest-tag fallback, dev fallback, version-already-present), release-notes URL generation, and an end-to-end test asserting the repo's real (version-less) plugin.json yields a versioned bundled manifest.
• gofmt, go vet, and go test ./... for the build module all pass.

Before

After

  

Change Impact: 🟡 Medium

Regression Risk: Limited to build/bundling behavior, but it changes how the bundle’s plugin.json is produced (via new dist output) and relies on manifest resolution; bundling failures or incorrect metadata are possible if path/CLI wiring differs from expectations.

QA Recommendation: Light manual QA: run the bundle build once and confirm the bundled plugin.json includes a resolved version and release_notes_url in addition to the automated tests.
Generated by CodeRabbitAI

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 84209406-257a-4834-bb5f-0fd241d14d83

📥 Commits

Reviewing files that changed from the base of the PR and between afb9eab and 77dec1b.

📒 Files selected for processing (1)

• build/manifest/main_test.go

🚧 Files skipped from review as they are similar to previous changes (1)

• build/manifest/main_test.go

---

📝 Walkthrough

Walkthrough

This PR adds a dist subcommand to the manifest build tool that writes a resolved plugin.json into a target directory, updates the Makefile bundle target to invoke it instead of copying the manifest file directly, and adds tests for manifest writing and resolution behavior.

Changes

Manifest dist generation

Layer / File(s)	Summary
Manifest CLI dist command and writer build/manifest/main.go	Adds a dist command requiring a destination directory, and a distManifest function that JSON-marshals the manifest and writes it as plugin.json with 0600 permissions, returning wrapped errors on failure.
Bundle target wiring Makefile	Replaces the direct copy of $(MANIFEST_FILE) with ./build/bin/manifest dist $(BUNDLE_DIR)/$(PLUGIN_NAME) in the bundle target.
Tests for distManifest and findManifest build/manifest/main_test.go	Adds tests verifying distManifest writes correct plugin.json content and errors on missing destination, plus tests for findManifest version resolution (tag precedence, dev versions, explicit version preservation) and release_notes_url generation, including a regression test for source manifests lacking a version.

🎯 2 (Simple) | ⏱️ ~12 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main change: writing the resolved manifest with version into the bundled plugin.json.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch cursor/jira-bug-fix-agent-13d8

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

🧹 Nitpick comments (1)

build/manifest/main_test.go (1)

50-64: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value

Global build-var/cwd mutation across subtests relies on test execution order.

writeTempManifest mutates process-wide state (os.Chdir, and indirectly BuildTagCurrent/BuildTagLatest/BuildHashShort via cleanup) shared across all tests in the package. This works today because no test calls t.Parallel() and tests execute sequentially in source order, but it's fragile: a future contributor adding t.Parallel() to any of these tests (or a new test using these globals) would silently produce racy/incorrect results since there's no synchronization on the shared globals or cwd.

This is acceptable for now given the sequential execution, but worth a comment noting the implicit ordering dependency, or guarding with t.Parallel() exclusion notes.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@build/manifest/main_test.go` around lines 50 - 64, `writeTempManifest` in
`main_test.go` mutates process-wide state (`os.Chdir` and the shared build vars
`BuildTagCurrent`, `BuildTagLatest`, `BuildHashShort`) in a way that depends on
sequential, ordered test execution. Add an explicit note near
`writeTempManifest` and the affected tests documenting that these tests must not
use `t.Parallel()` and rely on shared cwd/global state, or otherwise make the
helper/test setup self-contained so future changes don’t accidentally introduce
racy behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@build/manifest/main_test.go`:
- Around line 50-64: `writeTempManifest` in `main_test.go` mutates process-wide
state (`os.Chdir` and the shared build vars `BuildTagCurrent`, `BuildTagLatest`,
`BuildHashShort`) in a way that depends on sequential, ordered test execution.
Add an explicit note near `writeTempManifest` and the affected tests documenting
that these tests must not use `t.Parallel()` and rely on shared cwd/global
state, or otherwise make the helper/test setup self-contained so future changes
don’t accidentally introduce racy behavior.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 7639b4c9-5f58-492e-bc61-0fafbeddfb5f

📥 Commits

Reviewing files that changed from the base of the PR and between d454327 and afb9eab.

📒 Files selected for processing (3)

• Makefile
• build/manifest/main.go
• build/manifest/main_test.go

[mm-cloud-bot]

Test server destroyed

[avasconcelos114]

LGTM!

[avasconcelos114]

/cherry-pick release-9.2

[mattermost-build]

Cherry pick is scheduled.

[mattermost-build]

Error trying doing the automated Cherry picking. Please do this manually

+++ Updating remotes...
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
From github.com:mattermost/mattermost-plugin-boards
   9c9c2211..d061aab6  MM-69622    -> upstream/MM-69622
   2df9d9db..deb9b456  automated-cherry-pick-of-MM-69032-fix-channelid-authorization-boards-and-blocks-release-9.2 -> upstream/automated-cherry-pick-of-MM-69032-fix-channelid-authorization-boards-and-blocks-release-9.2
   ab5935a1..a807c9e6  main        -> upstream/main
   bde292ad..f82b6d6b  mm-69606    -> upstream/mm-69606
   2a190038..4eb6cf51  release-9.2 -> upstream/release-9.2
 * [new tag]           v9.2.6-rc1  -> v9.2.6-rc1
Fetching upstream
hostfile_replace_entries: mkstemp: Read-only file system
update_known_hosts: hostfile_replace_entries failed for /app/.ssh/known_hosts: Read-only file system
+++ Updating remotes done...
+++ Creating local branch automated-cherry-pick-of-cursor-jira-bug-fix-agent-13d8-release-9.2-1784186247
Switched to a new branch 'automated-cherry-pick-of-cursor-jira-bug-fix-agent-13d8-release-9.2-1784186247'
Branch 'automated-cherry-pick-of-cursor-jira-bug-fix-agent-13d8-release-9.2-1784186247' set up to track remote branch 'release-9.2' from 'upstream'.

+++ About to attempt cherry pick of PR #217 with merge commit 162e5a59d50048b7a03a824e68c54175c4fc551b.

Auto-merging Makefile
CONFLICT (content): Merge conflict in Makefile
error: could not apply 162e5a59... [MM-69594] Write resolved manifest (with version) into bundled plugin.json (#217)
hint: After resolving the conflicts, mark them with
hint: "git add/rm <pathspec>", then run
hint: "git cherry-pick --continue".
hint: You can instead skip this commit with "git cherry-pick --skip".
hint: To abort and get back to the state before "git cherry-pick",
hint: run "git cherry-pick --abort".

+++ Conflicts detected:

UU Makefile
Aborting.

+++ Aborting in-progress git cherry-pick.

+++ Returning you to the main branch and cleaning up.

[avasconcelos114]

/cherry-pick release-9.3

[mattermost-build]

Cherry pick is scheduled.