Skip to content

feat: update 'app link' to not change the app manifest source #396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mwbrooks wants to merge 2 commits into
base: mwbrooks-default-bolt-manifest-source-local
Choose a base branch
from
+40 −224
Open

feat: update 'app link' to not change the app manifest source #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f5ab1ef
feat: update remote manifest warnings for link command and manifest o…
mwbrooks Mar 13, 2026
3632f26
test: update tests for local and remote manifest source
mwbrooks Mar 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 14 additions & 67 deletions cmd/app/link.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ package app

import (
"context"
"fmt"
"path/filepath"
"strings"

Expand All @@ -36,9 +35,6 @@ import (
// LinkAppConfirmPromptText is displayed when prompting to add an existing app
const LinkAppConfirmPromptText = "Do you want to add an existing app?"

// LinkAppManifestSourceConfirmPromptText is displayed before updating the manifest source
const LinkAppManifestSourceConfirmPromptText = "Do you want to update the manifest source to remote?"

// appLinkFlagSet contains flag values to reference
type appLinkFlagSet struct {
environmentFlag string
Expand Down Expand Up @@ -131,11 +127,9 @@ func LinkAppHeaderSection(ctx context.Context, clients *shared.ClientFactory, sh
}

// LinkExistingApp prompts for an existing App ID and saves the details to the project.
// When shouldConfirm is true, a confirmation prompt will ask the user is they want to
// When shouldConfirm is true, a confirmation prompt will ask the user if they want to
// link an existing app and additional information is included in the header.
// The shouldConfirm option is encouraged for third-party callers.
// The link command requires manifest source to be remote. When it is not, a
// confirmation prompt is displayed before updating the manifest source value.
func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *types.App, shouldConfirm bool) (err error) {
// Header section
LinkAppHeaderSection(ctx, clients, shouldConfirm)
Expand All @@ -156,67 +150,20 @@ func LinkExistingApp(ctx context.Context, clients *shared.ClientFactory, app *ty
}
}

// Confirm to update manifest source to remote.
// - Update the manifest source to remote when its a GBP project with a local manifest.
// - Do not update manifest source for ROSI projects, because they can only be local manifests.
manifestSource, err := clients.Config.ProjectConfig.GetManifestSource(ctx)
isManifestSourceRemote := manifestSource.Equals(config.ManifestSourceRemote)
isSlackHostedProject := cmdutil.IsSlackHostedProject(ctx, clients) == nil

if err != nil || (!isManifestSourceRemote && !isSlackHostedProject) {
// When undefined, the default is config.ManifestSourceLocal
if !manifestSource.Exists() {
manifestSource = config.ManifestSourceLocal
}

clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{
Emoji: "warning",
Text: "Warning",
Secondary: []string{
"Linking an existing app requires the app manifest source to be managed by",
fmt.Sprintf("%s.", config.ManifestSourceRemote.Human()),
" ",
fmt.Sprintf(`App manifest source can be %s or %s:`, config.ManifestSourceLocal.Human(), config.ManifestSourceRemote.Human()),
fmt.Sprintf("- %s: uses manifest from your project's source code for all apps", config.ManifestSourceLocal.String()),
fmt.Sprintf("- %s: uses manifest from app settings for each app", config.ManifestSourceRemote.String()),
" ",
fmt.Sprintf(style.Highlight(`Your manifest source is set to %s.`), manifestSource.Human()),
" ",
fmt.Sprintf("Current manifest source in %s:", style.Highlight(filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename))),
fmt.Sprintf(style.Highlight(` %s: "%s"`), "manifest.source", manifestSource.String()),
" ",
fmt.Sprintf("Updating manifest source will be changed in %s:", style.Highlight(filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename))),
fmt.Sprintf(style.Highlight(` %s: "%s"`), "manifest.source", config.ManifestSourceRemote),
},
}))

proceed, err := clients.IO.ConfirmPrompt(ctx, LinkAppManifestSourceConfirmPromptText, false)
if err != nil {
clients.IO.PrintDebug(ctx, "Error prompting to update the manifest source to %s: %s", config.ManifestSourceRemote, err)
return err
}

if !proceed {
// Add newline to match the trailing newline inserted from the footer section
clients.IO.PrintInfo(ctx, false, "")
return nil
}

if err := config.SetManifestSource(ctx, clients.Fs, clients.Os, config.ManifestSourceRemote); err != nil {
// Log the error to the verbose output
clients.IO.PrintDebug(ctx, "Error setting manifest source in project-level config: %s", err)
// Display a user-friendly error with a workaround
slackErr := slackerror.New(slackerror.ErrProjectConfigManifestSource).
WithMessage("Failed to update the manifest source to %s", config.ManifestSourceRemote).
WithRemediation(
"You can manually update the manifest source by setting the following\nproperty in %s:\n %s",
filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename),
fmt.Sprintf(`manifest.source: "%s"`, config.ManifestSourceRemote),
).
WithRootCause(err)
clients.IO.PrintError(ctx, "%s", slackErr.Error())
}
// App Manifest section
manifestSource, _ := clients.Config.ProjectConfig.GetManifestSource(ctx)
if !manifestSource.Exists() {
manifestSource = config.ManifestSourceLocal
Copy link
Member Author

@mwbrooks mwbrooks Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note(out-of-scope): We should have the default manifest source return by a common method instead of manually setting it in each spot.

}
configPath := filepath.Join(config.ProjectConfigDirName, config.ProjectConfigJSONFilename)
clients.IO.PrintInfo(ctx, false, "%s", style.Sectionf(style.TextSection{
Emoji: "books",
Text: "App Manifest",
Secondary: []string{
"Manifest source is " + style.Highlight(manifestSource.Human()),
"Manifest source is configured in " + style.Highlight(configPath),
},
}))

// Prompt to get app details
var auth *types.SlackAuth
Expand Down
168 changes: 21 additions & 147 deletions cmd/app/link_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,135 +431,18 @@ func Test_Apps_Link(t *testing.T) {
CmdArgs: []string{},
ExpectedError: slackerror.New(slackerror.ErrAppNotFound),
},
"accepting manifest source prompt should save information about the provided deployed app": {
"links app when manifest source is local": {
Copy link
Member Author

@mwbrooks mwbrooks Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Test linking an app when the manifest source is local (manifest file).

Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
cm.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{
mockLinkSlackAuth2,
mockLinkSlackAuth1,
}, nil)
cm.AddDefaultMocks()
setupAppLinkCommandMocks(t, ctx, cm, cf)
// Set manifest source to project to trigger confirmation prompt
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
}
// Accept manifest source confirmation prompt
cm.IO.On("ConfirmPrompt",
mock.Anything,
LinkAppManifestSourceConfirmPromptText,
mock.Anything,
).Return(true, nil)
cm.IO.On("SelectPrompt",
mock.Anything,
"Select the existing app team",
mock.Anything,
mock.Anything,
mock.Anything,
).Return(iostreams.SelectPromptResponse{
Flag: true,
Option: mockLinkSlackAuth1.TeamDomain,
}, nil)
cm.IO.On("InputPrompt",
mock.Anything,
"Enter the existing app ID",
mock.Anything,
).Return(mockLinkAppID1, nil)
cm.IO.On("SelectPrompt",
mock.Anything,
"Choose the app environment",
mock.Anything,
mock.Anything,
mock.Anything,
).Return(iostreams.SelectPromptResponse{
Flag: true,
Option: "deployed",
}, nil)
cm.API.On(
"GetAppStatus",
mock.Anything,
mockLinkSlackAuth1.Token,
[]string{mockLinkAppID1},
mockLinkSlackAuth1.TeamID,
).Return(api.GetAppStatusResult{}, nil)
},
CmdArgs: []string{},
ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
expectedApp := types.App{
AppID: mockLinkAppID1,
TeamDomain: mockLinkSlackAuth1.TeamDomain,
TeamID: mockLinkSlackAuth1.TeamID,
EnterpriseID: mockLinkSlackAuth1.EnterpriseID,
}
actualApp, err := cm.AppClient.GetDeployed(
ctx,
mockLinkSlackAuth1.TeamID,
)
require.NoError(t, err)
assert.Equal(t, expectedApp, actualApp)
// Assert manifest confirmation prompt accepted
cm.IO.AssertCalled(t, "ConfirmPrompt",
mock.Anything,
LinkAppManifestSourceConfirmPromptText,
mock.Anything,
)
},
},
"declining manifest source prompt should not link app": {
Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
cm.AddDefaultMocks()
setupAppLinkCommandMocks(t, ctx, cm, cf)
// Set manifest source to project to trigger confirmation prompt
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
}
// Decline manifest source confirmation prompt
cm.IO.On("ConfirmPrompt",
mock.Anything,
LinkAppManifestSourceConfirmPromptText,
mock.Anything,
).Return(false, nil)
},
CmdArgs: []string{},
ExpectedAsserts: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock) {
// Assert manifest confirmation prompt accepted
cm.IO.AssertCalled(t, "ConfirmPrompt",
mock.Anything,
LinkAppManifestSourceConfirmPromptText,
mock.Anything,
)

// Assert no apps saved
apps, _, err := cm.AppClient.GetDeployedAll(ctx)
require.NoError(t, err)
require.Len(t, apps, 0)

apps, err = cm.AppClient.GetLocalAll(ctx)
require.NoError(t, err)
require.Len(t, apps, 0)
},
},
"manifest source prompt should not display for Run-on-Slack apps with local manifest source": {
Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
cm.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{
mockLinkSlackAuth1,
mockLinkSlackAuth2,
}, nil)
cm.AddDefaultMocks()
setupAppLinkCommandMocks(t, ctx, cm, cf)
// Set manifest source to local
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
}
// Mock manifest for Run-on-Slack app
manifestMock := &app.ManifestMockObject{}
manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{
AppManifest: types.AppManifest{
Settings: &types.AppSettings{
FunctionRuntime: types.SlackHosted,
},
},
}, nil)
cf.AppClient().Manifest = manifestMock
cm.IO.On("SelectPrompt",
mock.Anything,
"Select the existing app team",
Expand All @@ -583,7 +466,7 @@ func Test_Apps_Link(t *testing.T) {
mock.Anything,
).Return(iostreams.SelectPromptResponse{
Flag: true,
Option: "deployed",
Option: "local",
}, nil)
cm.API.On(
"GetAppStatus",
Expand All @@ -600,42 +483,33 @@ func Test_Apps_Link(t *testing.T) {
TeamDomain: mockLinkSlackAuth1.TeamDomain,
TeamID: mockLinkSlackAuth1.TeamID,
EnterpriseID: mockLinkSlackAuth1.EnterpriseID,
UserID: mockLinkSlackAuth1.UserID,
IsDev: true,
}
actualApp, err := cm.AppClient.GetDeployed(
actualApp, err := cm.AppClient.GetLocal(
ctx,
mockLinkSlackAuth1.TeamID,
)
require.NoError(t, err)
assert.Equal(t, expectedApp, actualApp)
// Assert manifest confirmation prompt was not displayed
cm.IO.AssertNotCalled(t, "ConfirmPrompt",
mock.Anything,
LinkAppManifestSourceConfirmPromptText,
mock.Anything,
)
// Assert manifest source info is displayed
output := cm.GetCombinedOutput()
assert.Contains(t, output, "App Manifest")
assert.Contains(t, output, `"project" (local)`)
},
},
"manifest source prompt should display for GBP apps with local manifest source": {
"links app when manifest source is remote": {
Copy link
Member Author

@mwbrooks mwbrooks Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: Test linking an app when the manifest source is remote (app settings).

Setup: func(t *testing.T, ctx context.Context, cm *shared.ClientsMock, cf *shared.ClientFactory) {
cm.Auth.On("Auths", mock.Anything).Return([]types.SlackAuth{
mockLinkSlackAuth1,
mockLinkSlackAuth2,
mockLinkSlackAuth1,
}, nil)
cm.AddDefaultMocks()
setupAppLinkCommandMocks(t, ctx, cm, cf)
// Set manifest source to local
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceLocal); err != nil {
// Set manifest source to remote
if err := config.SetManifestSource(ctx, cm.Fs, cm.Os, config.ManifestSourceRemote); err != nil {
require.FailNow(t, fmt.Sprintf("Failed to set the manifest source in the memory-based file system: %s", err))
}
// Mock manifest for Run-on-Slack app
manifestMock := &app.ManifestMockObject{}
manifestMock.On("GetManifestLocal", mock.Anything, mock.Anything, mock.Anything).Return(types.SlackYaml{}, nil)
cf.AppClient().Manifest = manifestMock
cm.IO.On("ConfirmPrompt",
mock.Anything,
LinkAppManifestSourceConfirmPromptText,
mock.Anything,
).Return(true, nil)
cm.IO.On("SelectPrompt",
mock.Anything,
"Select the existing app team",
Expand All @@ -659,7 +533,7 @@ func Test_Apps_Link(t *testing.T) {
mock.Anything,
).Return(iostreams.SelectPromptResponse{
Flag: true,
Option: "deployed",
Option: "local",
}, nil)
cm.API.On(
"GetAppStatus",
Expand All @@ -676,19 +550,19 @@ func Test_Apps_Link(t *testing.T) {
TeamDomain: mockLinkSlackAuth1.TeamDomain,
TeamID: mockLinkSlackAuth1.TeamID,
EnterpriseID: mockLinkSlackAuth1.EnterpriseID,
UserID: mockLinkSlackAuth1.UserID,
IsDev: true,
}
actualApp, err := cm.AppClient.GetDeployed(
actualApp, err := cm.AppClient.GetLocal(
ctx,
mockLinkSlackAuth1.TeamID,
)
require.NoError(t, err)
assert.Equal(t, expectedApp, actualApp)
// Assert manifest confirmation prompt was displayed
cm.IO.AssertCalled(t, "ConfirmPrompt",
mock.Anything,
LinkAppManifestSourceConfirmPromptText,
mock.Anything,
)
// Assert manifest source info is displayed
output := cm.GetCombinedOutput()
assert.Contains(t, output, "App Manifest")
assert.Contains(t, output, `"app settings" (remote)`)
},
},
}, func(clients *shared.ClientFactory) *cobra.Command {
Expand Down
4 changes: 1 addition & 3 deletions cmd/project/init_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,10 +108,8 @@ func Test_Project_InitCommand(t *testing.T) {
}, nil)
// Default setup
setupProjectInitCommandMocks(t, ctx, cm, cf)
// Do not link an existing app
// Link an existing app
cm.IO.On("ConfirmPrompt", mock.Anything, app.LinkAppConfirmPromptText, mock.Anything).Return(true, nil)
// Mock prompt to link an existing app
cm.IO.On("ConfirmPrompt", mock.Anything, app.LinkAppManifestSourceConfirmPromptText, mock.Anything).Return(true, nil)
// Mock prompt for team
cm.IO.On("SelectPrompt",
mock.Anything,
Expand Down
7 changes: 2 additions & 5 deletions internal/pkg/apps/install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -752,7 +752,7 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap
case saved.Equals(""):
notice = "Manifest values for this app are overwritten on reinstall"
default:
notice = "The manifest on app settings has been changed since last update!"
notice = style.Yellow("The manifest on app settings has been changed since last update")
Copy link
Member Author

@mwbrooks mwbrooks Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note: I'm open to other formatting (Red?) but I think we should make this pop and stand out in some way.

}
clients.IO.PrintInfo(ctx, false, "\n%s", style.Sectionf(style.TextSection{
Emoji: "books",
Expand All @@ -764,10 +764,7 @@ func shouldUpdateManifest(ctx context.Context, clients *shared.ClientFactory, ap
}
continues, err := clients.IO.ConfirmPrompt(
ctx,
fmt.Sprintf(
"Update app settings with changes to the %s manifest?",
config.ManifestSourceLocal.String(),
),
"Overwrite manifest on app settings with the project's manifest file?",
false,
)
if err != nil {
Expand Down
4 changes: 2 additions & 2 deletions internal/pkg/apps/install_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,7 +574,7 @@ func TestInstall(t *testing.T) {
clientsMock.IO.On(
"ConfirmPrompt",
mock.Anything,
"Update app settings with changes to the local manifest?",
"Overwrite manifest on app settings with the project's manifest file?",
false,
).Return(
tc.mockConfirmPrompt,
Expand Down Expand Up @@ -1396,7 +1396,7 @@ func TestInstallLocalApp(t *testing.T) {
clientsMock.IO.On(
"ConfirmPrompt",
mock.Anything,
"Update app settings with changes to the local manifest?",
"Overwrite manifest on app settings with the project's manifest file?",
false,
).Return(
tc.mockConfirmPrompt,
Expand Down
Loading