Skip to content

Conversation

@ainsleyclark
Copy link
Contributor

Users can now specify a custom working_directory for each command in their app.json configuration. This allows commands to run in different directories than the app's default path.

Changes:

  • Added WorkingDirectory field to CommandSpec struct
  • Updated pr.yaml template to use command-specific working_directory
  • Regenerated JSON schema to include new field
  • Added tests for working_directory field unmarshalling
  • Updated playground workflows with regenerated templates

Example usage:
{
"commands": { "test": { "command": "pnpm test", "working_directory": "packages/core" } } }

Backwards compatible: commands without working_directory continue to use the app's path as before.

Users can now specify a custom working_directory for each command
in their app.json configuration. This allows commands to run in
different directories than the app's default path.

Changes:
- Added WorkingDirectory field to CommandSpec struct
- Updated pr.yaml template to use command-specific working_directory
- Regenerated JSON schema to include new field
- Added tests for working_directory field unmarshalling
- Updated playground workflows with regenerated templates

Example usage:
{
  "commands": {
    "test": {
      "command": "pnpm test",
      "working_directory": "packages/core"
    }
  }
}

Backwards compatible: commands without working_directory continue
to use the app's path as before.
@claude

This comment has been minimized.

@codecov
Copy link

codecov bot commented Dec 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 69.47%. Comparing base (7f6b060) to head (3742752).
⚠️ Report is 427 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #353      +/-   ##
==========================================
+ Coverage   64.59%   69.47%   +4.87%     
==========================================
  Files         154      184      +30     
  Lines        6064     7279    +1215     
==========================================
+ Hits         3917     5057    +1140     
+ Misses       2064     2025      -39     
- Partials       83      197     +114     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The setup function in cmd_test.go has parameters that appear unused
because the test is skipped with t.Skip(). Added nolint:unparam to
suppress the linter warning until the test is properly implemented.
@claude

This comment has been minimized.

Updated the Commands section in the apps documentation to reflect:
- The three command formats (boolean, string, object)
- Correct field names (command instead of run)
- New working_directory field with examples
- Other fields like skip_ci and timeout
- Removed outdated 'enabled' field from examples

This aligns the documentation with the actual implementation.
@claude
Copy link

claude bot commented Dec 15, 2025

Review summary

  • Overall score: 8/10
  • Critical issues: 0
  • Warnings: 1
  • Suggestions: 2
  • Recommendation: ⚠️ Approve with comments

This PR successfully adds working directory support to commands in a clean, backwards-compatible manner. The implementation is solid with good test coverage for the struct unmarshalling. The feature is well-documented with clear examples. However, there's a missing integration test to verify the template generation works correctly with the new field.

Critical issues 🔴

None

Warnings 🟡

Missing integration test for template generation
The PR adds tests for CommandSpec.UnmarshalJSON (internal/appdef/commands_test.go:33-40) which verify the field parses correctly, but there's no test in internal/cmd/cicd/pr_test.go to verify that the working-directory field is actually rendered in the generated pr.yaml workflow. Consider adding a test case similar to the existing ones that verifies:

  1. When a command has working_directory set, the workflow contains working-directory: <value>
  2. When a command doesn't have working_directory, it falls back to the app's path
  3. When both are absent, no working-directory is set
Suggested test case
t.Run("WorkingDirectory Field", func(t *testing.T) {
    t.Parallel()

    appDef := &appdef.Definition{
        Apps: []appdef.App{
            {
                Name:  "web",
                Title: "Web",
                Path:  "./web",
                Type:  appdef.AppTypeGoLang,
                Commands: types.NewOrderedMap[Command, CommandSpec](),
            },
        },
    }
    
    // Add a command with working_directory
    appDef.Apps[0].Commands.Set(CommandTest, CommandSpec{
        Cmd: "go test ./...",
        WorkingDirectory: "./packages/core",
    })

    input := setup(t, afero.NewMemMapFs(), appDef)
    err := PR(t.Context(), input)
    require.NoError(t, err)

    file, err := afero.ReadFile(input.FS, filepath.Join(workflowsPath, "pr.yaml"))
    require.NoError(t, err)

    content := string(file)
    assert.Contains(t, content, "working-directory: ./packages/core")
}

Suggestions 🟢

1. Consider path validation
The WorkingDirectory field (internal/appdef/commands.go:18) accepts any string without validation. Consider whether you want to validate that:

  • Paths don't escape the project root (e.g., ../../etc/passwd)
  • Paths use forward slashes consistently for cross-platform compatibility
  • Paths are relative (don't start with /) if that's the intended design

This could be added in a follow-up PR if desired.

2. Documentation clarity on path resolution
The documentation (docs/manifest/apps.md:275-289) shows an example where working_directory is apps/web/src but the app path is apps/web. It's not immediately clear whether working_directory is:

  • Relative to the project root
  • Relative to the app's path

Consider adding a sentence to clarify: "The working_directory is always relative to the project root, not relative to the app's path."

@ainsleyclark ainsleyclark merged commit c74c9ec into main Dec 15, 2025
6 checks passed
@ainsleyclark ainsleyclark deleted the claude/add-working-directory-support-4wKBW branch December 15, 2025 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants