Fix plugin version not showing in System Console#204
Merged
Conversation
The bundle target was copying the raw plugin.json (which has no version field) into the dist archive instead of using the manifest tool to write a version-injected plugin.json. This caused the System Console to show no version number after installing the plugin. - Add dist command to build/manifest/main.go to write the enriched manifest (with version and release_notes_url) to the dist directory - Update Makefile bundle target to use ./build/bin/manifest dist - Add homepage_url and support_url to plugin.json for release notes URL generation Made-with: Cursor
📝 WalkthroughWalkthroughMakefile's bundle target now calls the manifest tool ( Changes
Sequence Diagram(s)sequenceDiagram
participant Makefile as Makefile (invoker)
participant Manifest as manifest binary
participant FS as Filesystem (dist/)
Makefile->>Manifest: run "./build/bin/manifest dist"
Manifest->>Manifest: load/resolve manifest, compute ReleaseNotesURL if needed
Manifest->>FS: mkdir -p dist/<manifest.Id> && write plugin.json
FS-->>Makefile: dist/<id>/plugin.json available
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@build/manifest/main.go`:
- Around line 196-203: The distManifest function currently writes to
"dist/<id>/plugin.json" without ensuring the parent directory exists; update
distManifest to compute the output directory for manifest.Id (e.g.,
"dist/<id>"), call os.MkdirAll on that directory with a safe mode (e.g., 0755)
before writing, and use filepath.Join to build the final file path
("<dir>/plugin.json") so os.WriteFile won't fail when the directory is missing;
ensure errors from MkdirAll are returned/wrapped similarly to the existing error
handling.
- Around line 142-147: The release-notes URL is being built with BuildTagLatest
regardless of the manifest's derived version, which can point to the wrong tag;
change the logic that sets manifest.ReleaseNotesURL (the block using
url.JoinPath and BuildTagLatest) to prefer manifest.Version (which is derived
from BuildTagCurrent) as the tag to append, falling back to BuildTagLatest only
if manifest.Version is empty, then call url.JoinPath(manifest.HomepageURL,
"releases", "tag", tag) and handle the error as before.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b12935b6-080c-4ab9-b620-37a998507616
📒 Files selected for processing (3)
Makefilebuild/manifest/main.goplugin.json
Made-with: Cursor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
build/manifest/main.go (1)
142-147:⚠️ Potential issue | 🟠 MajorRelease notes URL can point to a different version than the resolved manifest.
manifest.Versionis resolved from current tags first (Line 118-140), but Line 143 always usesBuildTagLatest. Building a non-latest tag can generate a mismatched release URL.💡 Proposed fix
- if manifest.ReleaseNotesURL == "" && BuildTagLatest != "" && manifest.HomepageURL != "" { - manifest.ReleaseNotesURL, err = url.JoinPath(manifest.HomepageURL, "releases", "tag", BuildTagLatest) + releaseTag := BuildTagLatest + if manifest.Version != "" { + releaseTag = "v" + strings.SplitN(manifest.Version, "+", 2)[0] + } + + if manifest.ReleaseNotesURL == "" && releaseTag != "" && manifest.HomepageURL != "" { + manifest.ReleaseNotesURL, err = url.JoinPath(manifest.HomepageURL, "releases", "tag", releaseTag) if err != nil { return nil, errors.Wrap(err, "failed to generate release notes URL") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@build/manifest/main.go` around lines 142 - 147, The release notes URL construction currently uses BuildTagLatest which can mismatch the resolved manifest.Version; change the logic in the block that sets manifest.ReleaseNotesURL (the url.JoinPath call) to use manifest.Version (or fall back to BuildTagLatest only if manifest.Version is empty) so the generated URL matches the resolved manifest version, and ensure the condition still checks manifest.HomepageURL and that url.JoinPath error handling remains the same.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@build/manifest/main.go`:
- Around line 142-147: The release notes URL construction currently uses
BuildTagLatest which can mismatch the resolved manifest.Version; change the
logic in the block that sets manifest.ReleaseNotesURL (the url.JoinPath call) to
use manifest.Version (or fall back to BuildTagLatest only if manifest.Version is
empty) so the generated URL matches the resolved manifest version, and ensure
the condition still checks manifest.HomepageURL and that url.JoinPath error
handling remains the same.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 84a19de3-ac10-46b9-b582-7969ecdb1eb9
📒 Files selected for processing (1)
build/manifest/main.go
|
@coderabbitai review |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai approve |
✅ Actions performedComments resolved and changes approved. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The plugin version was not displaying in the System Console after installation. The root cause was that the
bundleMakefile target copied the rawplugin.json(which intentionally omits theversionfield) into the dist archive, instead of using the manifest build tool to write a version-injectedplugin.json.distcommand tobuild/manifest/main.goto write the enriched manifest (with version derived from git tags and auto-generated release notes URL) to the dist directorybundletarget to use./build/bin/manifest distinstead ofcp $(MANIFEST_FILE)homepage_urlandsupport_urltoplugin.jsonfor release notes URL generationTicket Link
https://mattermost.atlassian.net/browse/MM-68234
Change Impact: 🟡 Medium
Reasoning: The change replaces a simple manifest file copy in the build pipeline with a new manifest CLI
distcommand that writes an enriched manifest (including version derived from git tags and generated release_notes_url) into the dist directory. This is isolated to the build/bundle process and runtime code is unaffected, but it introduces a new build-time dependency and a failure mode if the manifest tool fails.Regression Risk: Moderate. The Makefile now depends on
./build/bin/manifest distsucceeding; while the new command ensures the dist directory is created before writing (reducing a prior failure mode), there are no evident unit tests for the newdistpath and the Makefile has no explicit fallback if the tool fails. The change does not touch authentication, data persistence, or public runtime APIs.QA Recommendation: Perform manual QA: (1) run
make bundleto ensure the build completes, (2) verifydist/<plugin-id>/plugin.jsoncontains the injectedversion,homepage_url,support_url, andrelease_notes_url, and (3) install the resulting plugin and confirm the version appears in the System Console. Skipping manual QA is not recommended given the build pipeline dependency.Generated by CodeRabbitAI