feat: add slack manifest diff command#591
Conversation
Compares the project manifest against app settings and prints any differences. Read-only — no API mutations, no file writes, no prompts. Useful for CI guardrails, pre-flight visibility before deploy, and debugging manifest drift. Carved out from the larger two-way manifest sync work (#543) so the diff capability can land independently with a smaller diff.
mwbrooks
added
enhancement
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #591 +/- ##
==========================================
+ Coverage 71.67% 71.86% +0.18%
==========================================
Files 226 230 +4
Lines 19176 19394 +218
==========================================
+ Hits 13744 13937 +193
- Misses 4221 4233 +12
- Partials 1211 1224 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Shows the non-interactive form using --app and --token, matching the intended CI guardrail use case.
Replaces "Detect manifest drift in CI" with "Show manifest differences without prompts" so the language is plain and accessible to developers who haven't encountered drift terminology before.
Documents flattenRecursive, diffFlat, valuesEqual, and formatValue to match the project's existing convention (e.g. cmd/manifest/info.go documents both exported and unexported helpers).
DisplayDiffs printed a leading blank inside the loop, which combined with the trailing newline from style.Sectionf to produce a double blank between the header and the first difference. Skip the blank on the first iteration so spacing only appears between entries.
apps.manifest.export does not echo _metadata back, so projects that declared it (e.g. SDK schema annotations) saw a noisy "(only in project)" entry on every diff run. Apply a small ignored-paths filter inside Diff so these never reach the user, and leave the list extensible for any future one-sided fields.
formatValue used a byte-based slice (s[:77]) for the truncation, which could cut a multi-byte UTF-8 character in half and produce an invalid string in the middle of a localized field value. Switch to a rune-based truncate helper and cover it with a test that includes multi-byte input.
The Test_Diff and Test_Flatten loops previously only verified that expected entries were present, so a regression that produced extra unrelated entries would not have failed. Add a Len assertion before the per-entry checks. Also update the function-related cases to account for ManifestFunction.InputParameters and OutputParameters (which lack the omitempty tag and therefore always serialize as null).
Slack's apps.manifest.export appends " (local)" to display_information.name and features.bot_user.display_name when returning the manifest of a dev-installed app. Fresh installs would otherwise show two phantom diffs immediately. Suppress these specifically when removing the suffix would make the values equal, so genuine renames still surface.
apps.manifest.export emits settings.is_mcp_enabled: false for every app even when the project never declared the field, so users would otherwise see a phantom "(only in app settings)" entry on every run. Suppress it specifically when the value is false; if a user sets it to true locally, the resulting Modified diff still surfaces.
"In sync" reads as version-control jargon and is awkward when paired with the diff command's read-only contract. Use plainer language that any developer can parse without translation.
Local-only and remote-only fields previously printed in a different shape than modified fields, forcing the reader to mentally translate "only in app settings" into "Project doesn't have it." Render every diff with the same Project/App settings layout and use "(not set)" on the absent side. The header still announces the count and the direction is conveyed by which side reads "(not set)".
Replaces "Found N difference(s)" with the proper singular/plural form via the existing style.Pluralize helper.
Adds focused test cases for the remaining uncovered branches: DisplayDiffs's empty-result early return and multi-diff sort path, formatValue's nil and long-value paths, and truncateRunes's short-max early return. The json.Marshal error path inside formatValue stays uncovered because reproducing it requires a value that the marshaller cannot encode (channels, funcs); the contortion is not worth the line of coverage.
The bare "display" name was ambiguous now that the package sits alongside cmd/manifest/info.go, which also "displays" a manifest. Prefix the file with the command it belongs to so a reader navigating the package can place the file at a glance.
mwbrooks
left a comment
mwbrooks
left a comment
There was a problem hiding this comment.
Comments for the kind reviewers 💬
| func NewDiffCommand(clients *shared.ClientFactory) *cobra.Command { | ||
| return &cobra.Command{ | ||
| Use: "diff", | ||
| Short: "Show differences between the project manifest and app settings", |
There was a problem hiding this comment.
note: I feel we should say App Settings Slack API, but the CLI currently references App Settings, so I thought I'd stay consistent.
| Text: "App Manifest", | ||
| Secondary: []string{"Project manifest and app settings are in sync"}, | ||
| })) | ||
| return nil |
There was a problem hiding this comment.
note: We don't return a non-zero exit code when there is a manifest diff. Ideally, we should. However, manifest validate does not return a non-zero exist code on when the manifest is invalid and the CLI plumbing would surface a visual error to interactive users. So, we should have a follow-up task to standardize non-zero exit code handling.
| // ignoredDiffPaths are top-level manifest fields that the project may declare | ||
| // but Slack's apps.manifest.export does not echo back. Diffs at or under these | ||
| // paths are dropped to avoid spurious "only in project" entries on every run. | ||
| var ignoredDiffPaths = []string{ | ||
| "_metadata", // SDK-side schema annotations; not stored in app settings | ||
| } |
There was a problem hiding this comment.
note: The manifest.export API method doesn't return the _metadata version properties. This creates an infinite conflict between the project and app settings, so I've chosen to ignore the _metadata properties during the diff.
| // devLocalSuffixPaths are flattened paths where Slack's apps.manifest.export | ||
| // appends " (local)" for dev-installed apps. Diffs at these paths are | ||
| // dropped only when removing the suffix would make the values equal; real | ||
| // renames still surface. | ||
| var devLocalSuffixPaths = []string{ | ||
| "display_information.name", | ||
| "features.bot_user.display_name", | ||
| } | ||
|
|
||
| const devLocalSuffix = " (local)" |
There was a problem hiding this comment.
note: For local dev apps, the App Settings manifest has (local) in the app name fields, but the manifest.json does not. This creates an infinite conflict. So, we include a list of affected fields and expected suffix (local).
Follow-up PR should update the GetManifestLocal function to include (local), but this is larger change and could have unexpected side-effects. It's out-of-scope of this PR.
On the next semver major, we would like to drop (local) entirely. We can discuss whether can do this sooner as well. However, the change would need to be coordinated with docs.
This implementation will continue to work after (local) is removed.
| // remoteFalseDefaultPaths are flattened paths where Slack's | ||
| // apps.manifest.export emits a default `false` for every app, even when the | ||
| // project has not declared the field. Remote-only diffs at these paths are | ||
| // dropped when the value is false so users do not see a phantom entry on | ||
| // every run; a real disagreement (e.g. local sets the field to true) still | ||
| // surfaces as a Modified diff. | ||
| var remoteFalseDefaultPaths = []string{ | ||
| "settings.is_mcp_enabled", | ||
| } |
There was a problem hiding this comment.
note: I don't love this, but it seems to be the safest approach that is forward-change compatible.
Current problem: The manifest schema defines settings.is_mcp_enabled as optional. However, all manifest.exports return settings.is_mcp_enabled: false when it is unset. Essentially, making it not optional. 😅 The reason is that there is a database write that transforms unset to false.
We could change the CLI manifest schema to default to false. However, this means all apps - including ROSI apps - will suddenly set this property to false on the next run or deploy. This could lead to new, unexpected issues.
So, I believe the safer approach is to have the manifest diff command aware of this BE quirk. After all, it may be fixed later.
1 participant
Changelog
Summary
This pull request adds a read-only
slack manifest diffcommand that compares the project manifest against the app settings on Slack and prints any differences.internal/manifest.slack manifest synccommand will build on these primitives.slack manifest validate(which exits 0 even when the manifest is invalid). Standardizing exit-code semantics for scripting use-cases is a separate, project-wide conversation that should changevalidateanddifftogether.Preview
2026-06-15-manifest-diff.mov
Testing
Testing outside of a project will error:
Setup a new a project:
$ ./bin/slack create my-app --template slack-samples/bolt-js-starter-template $ cd my-app/Testing inside of a project with no App IDs will error:
Setup a new App ID for testing:
$ ../bin/slack install -E localTesting a new install should return no differences:
Testing different values for project and app settings:
Testing project missing a value:
Testing app settings missing a value:
Clean up:
$ lack delete -f $ cd ../ $ rm -rf my-app/Requirements