Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
32 changes: 28 additions & 4 deletions internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,16 +48,26 @@ func NewClient(

// UpdateDefaultProjectFiles should update any project specific files if any
func UpdateDefaultProjectFiles(fs afero.Fs, dirPath string, appDirName string) error {
var filenames = []string{"manifest.json", "manifest.js", "manifest.ts"}
// Files and their corresponding app name replacement functions
projectFiles := []struct {
filename string
replacer func([]byte, string) []byte
}{
{"manifest.json", regexReplaceAppNameInManifest},
{"manifest.js", regexReplaceAppNameInManifest},
{"manifest.ts", regexReplaceAppNameInManifest},
{"package.json", regexReplaceAppNameInPackageJSON},
{"pyproject.toml", regexReplaceAppNameInPyprojectToml},
}

for _, filename := range filenames {
filePath := filepath.Join(dirPath, filename)
for _, pf := range projectFiles {
filePath := filepath.Join(dirPath, pf.filename)
fileData, err := afero.ReadFile(fs, filePath)
if err != nil {
continue
}

fileData = regexReplaceAppNameInManifest(fileData, appDirName)
fileData = pf.replacer(fileData, appDirName)
if err := afero.WriteFile(fs, filePath, fileData, 0644); err != nil {
return err
}
Expand Down Expand Up @@ -149,3 +159,17 @@ func regexReplaceAppNameInManifest(src []byte, appName string) []byte {

return srcUpdated
}

// regexReplaceAppNameInPackageJSON replaces the top-level "name" field in a package.json file
func regexReplaceAppNameInPackageJSON(src []byte, appName string) []byte {
re := regexp.MustCompile(`(?m)^(\s*"name"\s*:\s*")([^"]*)(")`)
repl := fmt.Sprintf("${1}%s${3}", appName)
return re.ReplaceAll(src, []byte(repl))
}
Comment on lines +163 to +168
Copy link
Member

Choose a reason for hiding this comment

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

suggestion(blocking): I think this regex will match all "name" keys in package.json instead of just the top-level "name" field. While other "name" keys are rare, I think that's a little too risky.

A few ideas:

  1. Update the regex to only match the top-level key
  2. Just match the name key with 2 spaces
  3. Do nothing when we more than one "name" key exists (better to error on doing nothing than corrupting the package.json file)

Here is an example of multiple matches and why we'd need a fix:

Image


// regexReplaceAppNameInPyprojectToml replaces the "name" field under the [project] section in a pyproject.toml file
func regexReplaceAppNameInPyprojectToml(src []byte, appName string) []byte {
re := regexp.MustCompile(`(\[project\][^\[]*?name\s*=\s*")([^"]*)(")`)
repl := fmt.Sprintf("${1}%s${3}", appName)
return re.ReplaceAll(src, []byte(repl))
}
Comment on lines +170 to +175
Copy link
Member

Choose a reason for hiding this comment

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

note: I've confirmed that this will only match name= under the [project] section. Great work @srtaalej!

While we're using teh re.ReplaceAll it should only match "all" names under the project and seems okay.

74 changes: 74 additions & 0 deletions internal/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,40 @@ func Test_App_UpdateDefaultProjectFiles(t *testing.T) {
},
expectedErrorType: nil,
},
"package.json file exists": {
appDirName: "vibrant-butterfly-1234",
existingFiles: map[string]string{
"package.json": string(testdata.PackageJSON),
},
expectedFiles: map[string]string{
"package.json": string(testdata.PackageJSONAppName),
},
expectedErrorType: nil,
},
"pyproject.toml file exists": {
appDirName: "vibrant-butterfly-1234",
existingFiles: map[string]string{
"pyproject.toml": string(testdata.PyprojectTOML),
},
expectedFiles: map[string]string{
"pyproject.toml": string(testdata.PyprojectTOMLAppName),
},
expectedErrorType: nil,
},
"Multiple project files exist": {
appDirName: "vibrant-butterfly-1234",
existingFiles: map[string]string{
"manifest.json": string(testdata.ManifestJSON),
"package.json": string(testdata.PackageJSON),
"pyproject.toml": string(testdata.PyprojectTOML),
},
expectedFiles: map[string]string{
"manifest.json": string(testdata.ManifestJSONAppName),
"package.json": string(testdata.PackageJSONAppName),
"pyproject.toml": string(testdata.PyprojectTOMLAppName),
},
expectedErrorType: nil,
},
"No manifest files exist": {
appDirName: "vibrant-butterfly-1234",
existingFiles: map[string]string{},
Expand Down Expand Up @@ -161,3 +195,43 @@ func Test_RegexReplaceAppNameInManifest(t *testing.T) {
})
}
}

func Test_RegexReplaceAppNameInPackageJSON(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Can we add a test where package.json contains 2 name keys. I think this would build confidence in our regex and help avoid regretful regressions later in life by developers unfamiliar with the regex.

We could put the "name" in the config section:

{
  "name": "vibrant-butterfly-1234",
  "version": "1.0.0",
  "description": "A Slack app built with Bolt",
  "main": "app.js",
  "scripts": {
    "start": "node app.js"
  },
  "dependencies": {
    "@slack/bolt": "^4.0.0"
  },
  "config": {
    "name": "local-server-name",
    "host": "localhost",
    "port": "8080"
  }
}

tests := map[string]struct {
src []byte
appName string
expectedSrc []byte
}{
"package.json name is replaced": {
src: testdata.PackageJSON,
appName: "vibrant-butterfly-1234",
expectedSrc: testdata.PackageJSONAppName,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
actualSrc := regexReplaceAppNameInPackageJSON(tc.src, tc.appName)
require.Equal(t, tc.expectedSrc, actualSrc)
})
}
}

func Test_RegexReplaceAppNameInPyprojectToml(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

suggestion: Likewise, I think we should add an extra name = under another section to make sure we never replace other name keys.

An example could be:

[project]
name = "vibrant-butterfly-1234"
version = "0.1.0"
requires-python = ">=3.9"
dependencies = [
    "slack-sdk==3.40.0",
    "slack-bolt==1.27.0",
    "slack-cli-hooks<1.0.0",
]

[tool.ruff]
[tool.ruff.lint]
[tool.ruff.format]

[tool.pytest.ini_options]
testpaths = ["tests"]

[project.scripts]
name = "my_package.name:main_function"

tests := map[string]struct {
src []byte
appName string
expectedSrc []byte
}{
"pyproject.toml name is replaced": {
src: testdata.PyprojectTOML,
appName: "vibrant-butterfly-1234",
expectedSrc: testdata.PyprojectTOMLAppName,
},
}
for name, tc := range tests {
t.Run(name, func(t *testing.T) {
actualSrc := regexReplaceAppNameInPyprojectToml(tc.src, tc.appName)
require.Equal(t, tc.expectedSrc, actualSrc)
})
}
}
12 changes: 12 additions & 0 deletions test/testdata/package-app-name.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "vibrant-butterfly-1234",
"version": "1.0.0",
"description": "A Slack app built with Bolt",
"main": "app.js",
"scripts": {
"start": "node app.js"
},
"dependencies": {
"@slack/bolt": "^4.0.0"
}
}
12 changes: 12 additions & 0 deletions test/testdata/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
{
"name": "bolt-app-template",
"version": "1.0.0",
"description": "A Slack app built with Bolt",
"main": "app.js",
"scripts": {
"start": "node app.js"
},
"dependencies": {
"@slack/bolt": "^4.0.0"
}
}
16 changes: 16 additions & 0 deletions test/testdata/pyproject-app-name.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[project]
name = "vibrant-butterfly-1234"
version = "0.1.0"
requires-python = ">=3.9"
dependencies = [
"slack-sdk==3.40.0",
"slack-bolt==1.27.0",
"slack-cli-hooks<1.0.0",
]

[tool.ruff]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[tool.ruff]
# This comment appears before more details
[tool.ruff]

👾 suggestion(non-blocking): I understand the advantage of using regex to be comments and indentation kept during replacements. Using a package like go-toml would make me more confident in updating structured file, but IIRC templates might include comments that we want to avoid removing on accident!

Copy link
Member

@mwbrooks mwbrooks Feb 20, 2026

Choose a reason for hiding this comment

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

Unfortunately, I don't think we should do that. I've experimented with go-toml for our pyproject.toml work. When you read-in and write-out, it will re-order the sections and keys based on a random order of the Golang map. So, I've taken a regex approach with the venv as well.

[tool.ruff.lint]
[tool.ruff.format]

[tool.pytest.ini_options]
testpaths = ["tests"]
16 changes: 16 additions & 0 deletions test/testdata/pyproject.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[project]
name = "bolt-python-ai-agent-template"
version = "0.1.0"
requires-python = ">=3.9"
dependencies = [
"slack-sdk==3.40.0",
"slack-bolt==1.27.0",
"slack-cli-hooks<1.0.0",
]

[tool.ruff]
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
[tool.ruff]
# This comment appears before more details
[tool.ruff]

🔭 suggestion(non-blocking): This comment follows the one before!

[tool.ruff.lint]
[tool.ruff.format]

[tool.pytest.ini_options]
testpaths = ["tests"]
12 changes: 12 additions & 0 deletions test/testdata/testdata.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,3 +27,15 @@ var ManifestSDKTS []byte

//go:embed manifest-sdk-app-name.ts
var ManifestSDKTSAppName []byte

//go:embed package.json
var PackageJSON []byte

//go:embed package-app-name.json
var PackageJSONAppName []byte

//go:embed pyproject.toml
var PyprojectTOML []byte

//go:embed pyproject-app-name.toml
var PyprojectTOMLAppName []byte
Comment on lines +31 to +41
Copy link
Member

Choose a reason for hiding this comment

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

praise: Great use of the embeds to keep test data clean and easy to maintain!

Loading