diff --git a/docs/adr/29483-automate-removal-of-deprecated-sandbox-keys-via-codemods.md b/docs/adr/29483-automate-removal-of-deprecated-sandbox-keys-via-codemods.md new file mode 100644 index 00000000000..587196073e3 --- /dev/null +++ b/docs/adr/29483-automate-removal-of-deprecated-sandbox-keys-via-codemods.md @@ -0,0 +1,70 @@ +# ADR-29483: Automate Removal of Deprecated Sandbox Keys via Codemods + +**Date**: 2026-05-01 +**Status**: Draft +**Deciders**: Unknown + +--- + +## Part 1 — Narrative (Human-Friendly) + +### Context + +The `gh-aw` platform introduced strict mode validation that rejects three sandbox configuration keys that were previously supported: `sandbox.mcp.container`, `sandbox.mcp.version`, and `sandbox.agent: false`. The MCP gateway container and version are now managed internally by the platform, making user-supplied values obsolete. Setting `sandbox.agent: false` was an escape hatch to disable agent sandbox firewall protections, which is no longer permitted in strict mode for security reasons. These keys were already flagged as deprecated, but the `gh aw fix` command could not remove them automatically, forcing maintainers to perform manual edits across 28+ files in official repositories. + +### Decision + +We will add three new codemods (`sandbox-mcp-container-removal`, `sandbox-mcp-version-removal`, `sandbox-agent-false-removal`) to the `GetAllCodemods()` registry so that `gh aw fix --write` can automatically remove these deprecated keys from workflow frontmatter. Each codemod applies a targeted line-level transform that removes only the specific key, leaving all sibling keys (e.g., `sandbox.mcp.port`, non-`false` `agent` values) intact. The `sandbox.agent: false` codemod is deliberately scoped to the boolean `false` value to avoid accidentally removing valid `agent` configurations such as object or string values. + +### Alternatives Considered + +#### Alternative 1: Documentation-Only Migration Guide + +Provide a migration guide asking users to manually remove the deprecated keys. This was already the de-facto approach and produced the 28+ manual edits problem that motivated this PR. Manual migration is error-prone, time-consuming, and blocks users from enabling strict mode until edits are complete. It does not scale as more repos adopt the platform. + +#### Alternative 2: Soft Deprecation with Warnings Only + +Continue to accept the deprecated keys but emit deprecation warnings rather than errors. This was rejected because it would contradict the intent of strict mode — the keys were already classified as rejected in strict mode, and softening the enforcement would delay cleanup indefinitely. It would also leave security-sensitive behavior (`sandbox.agent: false` disabling the firewall) silently in place. + +### Consequences + +#### Positive +- Users can run `gh aw fix --write` to migrate all three deprecated keys in a single automated pass, eliminating manual edits across potentially dozens of files. +- Enforces the security posture of strict mode: `sandbox.agent: false` previously disabled the agent sandbox firewall; automated removal restores the secure default. +- The codemod pattern is surgical — sibling keys (e.g., `sandbox.mcp.port`) are preserved, reducing the risk of unintended data loss. + +#### Negative +- Codemods make permanent file modifications; if applied without review, authors may lose context about why a key was present (e.g., a temporary workaround still in use). +- The `sandbox.agent: false` codemod only handles the boolean `false` case; any other representation of disabling the agent (e.g., a string `"false"`) would require a separate codemod or manual action. + +#### Neutral +- The three codemods are added to the end of the `GetAllCodemods()` slice; the order is stable but appended, which shifts the total codemod count from 38 to 41 (reflected in the registry count test). +- Each codemod carries an `IntroducedIn: "0.26.0"` field that records the version in which the corresponding keys became deprecated. + +--- + +## Part 2 — Normative Specification (RFC 2119) + +> The key words **MUST**, **MUST NOT**, **REQUIRED**, **SHALL**, **SHALL NOT**, **SHOULD**, **SHOULD NOT**, **RECOMMENDED**, **MAY**, and **OPTIONAL** in this section are to be interpreted as described in [RFC 2119](https://www.rfc-editor.org/rfc/rfc2119). + +### Codemod Registration + +1. Each codemod for removing a deprecated sandbox key **MUST** be registered in `GetAllCodemods()` so that it runs as part of `gh aw fix --write`. +2. Codemods **MUST** be implemented as `Codemod` struct values with non-empty `ID`, `Name`, `Description`, and `IntroducedIn` fields. +3. Codemod `ID` values **MUST** be unique within the registry and **MUST** use kebab-case (e.g., `sandbox-mcp-container-removal`). + +### Codemod Behavior + +1. A codemod **MUST** return `(content, false, nil)` unchanged when the deprecated key is not present in the frontmatter — it **MUST NOT** modify files that do not need migration. +2. The `sandbox-mcp-container-removal` codemod **MUST** remove only the `container` key within `sandbox.mcp` and **MUST NOT** remove any sibling keys (e.g., `port`, `api-key`). +3. The `sandbox-mcp-version-removal` codemod **MUST** remove only the `version` key within `sandbox.mcp` and **MUST NOT** remove any sibling keys. +4. The `sandbox-agent-false-removal` codemod **MUST** remove the `agent` key from the `sandbox` block only when its value is the boolean `false`. It **MUST NOT** remove `agent` keys whose value is `true`, a string, or an object. +5. Codemods **MUST** preserve all content outside the removed key line, including markdown body content below the frontmatter delimiter. + +### Conformance + +An implementation is considered conformant with this ADR if it satisfies all **MUST** and **MUST NOT** requirements above. Failure to meet any **MUST** or **MUST NOT** requirement constitutes non-conformance. + +--- + +*This is a DRAFT ADR generated by the [Design Decision Gate](https://github.com/github/gh-aw/actions/runs/25210508659) workflow. The PR author must review, complete, and finalize this document before the PR can merge.* diff --git a/pkg/cli/codemod_sandbox_agent_false_removal.go b/pkg/cli/codemod_sandbox_agent_false_removal.go new file mode 100644 index 00000000000..b731eba0b61 --- /dev/null +++ b/pkg/cli/codemod_sandbox_agent_false_removal.go @@ -0,0 +1,52 @@ +package cli + +import "github.com/github/gh-aw/pkg/logger" + +var sandboxAgentFalseRemovalCodemodLog = logger.New("cli:codemod_sandbox_agent_false_removal") + +// getSandboxAgentFalseRemovalCodemod creates a codemod that removes the deprecated +// sandbox.agent: false key. Setting sandbox.agent to false was previously supported as +// an escape hatch for testing but is now rejected in strict mode because it disables +// important agent sandbox security protections. Remove the key to restore default +// (sandboxed) behavior, or set 'strict: false' to opt out of strict mode. +func getSandboxAgentFalseRemovalCodemod() Codemod { + return Codemod{ + ID: "sandbox-agent-false-removal", + Name: "Remove deprecated sandbox.agent: false field", + Description: "Removes 'sandbox.agent: false' which is no longer allowed in strict mode. The agent sandbox firewall is now always enabled by default. Remove this key to restore sandboxed behavior, or set 'strict: false' to opt out of strict mode.", + IntroducedIn: "0.26.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + if isFrontmatterStrictFalse(frontmatter) { + return content, false, nil + } + if !isSandboxAgentFalse(frontmatter) { + return content, false, nil + } + newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { + return removeFieldFromBlock(lines, "agent", "sandbox") + }) + if applied { + sandboxAgentFalseRemovalCodemodLog.Print("Removed deprecated sandbox.agent: false") + } + return newContent, applied, err + }, + } +} + +// isSandboxAgentFalse returns true when frontmatter["sandbox"]["agent"] is boolean false. +func isSandboxAgentFalse(frontmatter map[string]any) bool { + sandboxVal, ok := frontmatter["sandbox"] + if !ok { + return false + } + sandboxMap, ok := sandboxVal.(map[string]any) + if !ok { + return false + } + agentVal, ok := sandboxMap["agent"] + if !ok { + return false + } + agentBool, ok := agentVal.(bool) + return ok && !agentBool +} diff --git a/pkg/cli/codemod_sandbox_agent_false_removal_test.go b/pkg/cli/codemod_sandbox_agent_false_removal_test.go new file mode 100644 index 00000000000..9e8d505a116 --- /dev/null +++ b/pkg/cli/codemod_sandbox_agent_false_removal_test.go @@ -0,0 +1,269 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGetSandboxAgentFalseRemovalCodemod(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + assert.Equal(t, "sandbox-agent-false-removal", codemod.ID) + assert.Equal(t, "Remove deprecated sandbox.agent: false field", codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.Equal(t, "0.26.0", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) +} + +func TestSandboxAgentFalseRemoval_RemovesAgentFalse(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + agent: false +permissions: + contents: read +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "agent": false, + }, + "permissions": map[string]any{ + "contents": "read", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "agent: false") + assert.Contains(t, result, "sandbox:") +} + +func TestSandboxAgentFalseRemoval_PreservesOtherSandboxKeys(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + agent: false + mcp: + port: 8080 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "agent": false, + "mcp": map[string]any{ + "port": 8080, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "agent: false") + assert.Contains(t, result, "mcp:") + assert.Contains(t, result, "port: 8080") +} + +func TestSandboxAgentFalseRemoval_NoSandboxKey(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +permissions: + contents: read +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "permissions": map[string]any{ + "contents": "read", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxAgentFalseRemoval_AgentNotFalse(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + agent: awf +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "agent": "awf", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxAgentFalseRemoval_AgentObject(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + agent: + id: awf +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "agent": map[string]any{ + "id": "awf", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxAgentFalseRemoval_AgentTrue(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + agent: true +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "agent": true, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxAgentFalseRemoval_SkipsWhenStrictFalse(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +strict: false +sandbox: + agent: false +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "strict": false, + "sandbox": map[string]any{ + "agent": false, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied, "should not apply when strict: false is set") + assert.Equal(t, content, result) +} + +func TestSandboxAgentFalseRemoval_PreservesMarkdown(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + agent: false +--- + +# Workflow Title + +This workflow was using the nosandbox escape hatch.` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "agent": false, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "# Workflow Title") + assert.NotContains(t, result, "agent: false") +} + +func TestSandboxAgentFalseRemoval_NoAgentKey(t *testing.T) { + codemod := getSandboxAgentFalseRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + mcp: + port: 8080 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "mcp": map[string]any{ + "port": 8080, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} diff --git a/pkg/cli/codemod_sandbox_mcp_internal.go b/pkg/cli/codemod_sandbox_mcp_internal.go new file mode 100644 index 00000000000..03cb92c4353 --- /dev/null +++ b/pkg/cli/codemod_sandbox_mcp_internal.go @@ -0,0 +1,81 @@ +package cli + +import "github.com/github/gh-aw/pkg/logger" + +var sandboxMCPInternalCodemodLog = logger.New("cli:codemod_sandbox_mcp_internal") + +// getSandboxMCPContainerRemovalCodemod creates a codemod that removes the deprecated +// sandbox.mcp.container field. The MCP gateway container is now managed internally by +// gh-aw based on the declared MCP toolsets. +func getSandboxMCPContainerRemovalCodemod() Codemod { + return Codemod{ + ID: "sandbox-mcp-container-removal", + Name: "Remove deprecated sandbox.mcp.container field", + Description: "Removes 'sandbox.mcp.container' as the MCP gateway container is now managed internally. Remove this key or set 'strict: false' to disable strict mode.", + IntroducedIn: "0.26.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + if isFrontmatterStrictFalse(frontmatter) { + return content, false, nil + } + if !hasSandboxMCPField(frontmatter, "container") { + return content, false, nil + } + newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { + return removeFieldFromBlock(lines, "container", "mcp") + }) + if applied { + sandboxMCPInternalCodemodLog.Print("Removed deprecated sandbox.mcp.container") + } + return newContent, applied, err + }, + } +} + +// getSandboxMCPVersionRemovalCodemod creates a codemod that removes the deprecated +// sandbox.mcp.version field. The MCP gateway version is now managed internally by +// gh-aw based on the declared MCP toolsets. +func getSandboxMCPVersionRemovalCodemod() Codemod { + return Codemod{ + ID: "sandbox-mcp-version-removal", + Name: "Remove deprecated sandbox.mcp.version field", + Description: "Removes 'sandbox.mcp.version' as the MCP gateway version is now managed internally. Remove this key or set 'strict: false' to disable strict mode.", + IntroducedIn: "0.26.0", + Apply: func(content string, frontmatter map[string]any) (string, bool, error) { + if isFrontmatterStrictFalse(frontmatter) { + return content, false, nil + } + if !hasSandboxMCPField(frontmatter, "version") { + return content, false, nil + } + newContent, applied, err := applyFrontmatterLineTransform(content, func(lines []string) ([]string, bool) { + return removeFieldFromBlock(lines, "version", "mcp") + }) + if applied { + sandboxMCPInternalCodemodLog.Print("Removed deprecated sandbox.mcp.version") + } + return newContent, applied, err + }, + } +} + +// hasSandboxMCPField checks whether frontmatter["sandbox"]["mcp"][fieldName] exists. +func hasSandboxMCPField(frontmatter map[string]any, fieldName string) bool { + sandboxVal, ok := frontmatter["sandbox"] + if !ok { + return false + } + sandboxMap, ok := sandboxVal.(map[string]any) + if !ok { + return false + } + mcpVal, ok := sandboxMap["mcp"] + if !ok { + return false + } + mcpMap, ok := mcpVal.(map[string]any) + if !ok { + return false + } + _, hasField := mcpMap[fieldName] + return hasField +} diff --git a/pkg/cli/codemod_sandbox_mcp_internal_test.go b/pkg/cli/codemod_sandbox_mcp_internal_test.go new file mode 100644 index 00000000000..7c6ceef2f52 --- /dev/null +++ b/pkg/cli/codemod_sandbox_mcp_internal_test.go @@ -0,0 +1,365 @@ +//go:build !integration + +package cli + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// ----- getSandboxMCPContainerRemovalCodemod tests ----- + +func TestGetSandboxMCPContainerRemovalCodemod(t *testing.T) { + codemod := getSandboxMCPContainerRemovalCodemod() + + assert.Equal(t, "sandbox-mcp-container-removal", codemod.ID) + assert.Equal(t, "Remove deprecated sandbox.mcp.container field", codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.Equal(t, "0.26.0", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) +} + +func TestSandboxMCPContainerRemoval_RemovesContainer(t *testing.T) { + codemod := getSandboxMCPContainerRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + mcp: + container: ghcr.io/example/gateway + port: 8080 +permissions: + contents: read +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "mcp": map[string]any{ + "container": "ghcr.io/example/gateway", + "port": 8080, + }, + }, + "permissions": map[string]any{ + "contents": "read", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "container:") + assert.Contains(t, result, "mcp:") + assert.Contains(t, result, "port: 8080") +} + +func TestSandboxMCPContainerRemoval_NoSandboxKey(t *testing.T) { + codemod := getSandboxMCPContainerRemovalCodemod() + + content := `--- +on: workflow_dispatch +permissions: + contents: read +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "permissions": map[string]any{ + "contents": "read", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxMCPContainerRemoval_NoMCPKey(t *testing.T) { + codemod := getSandboxMCPContainerRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + agent: awf +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "agent": "awf", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxMCPContainerRemoval_NoContainerField(t *testing.T) { + codemod := getSandboxMCPContainerRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + mcp: + port: 8080 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "mcp": map[string]any{ + "port": 8080, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxMCPContainerRemoval_PreservesMarkdown(t *testing.T) { + codemod := getSandboxMCPContainerRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + mcp: + container: github/gh-aw-mcpg +--- + +# Workflow Title + +This is a test workflow.` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "mcp": map[string]any{ + "container": "github/gh-aw-mcpg", + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.Contains(t, result, "# Workflow Title") + assert.Contains(t, result, "This is a test workflow.") + assert.NotContains(t, result, "container:") +} + +// ----- getSandboxMCPVersionRemovalCodemod tests ----- + +func TestGetSandboxMCPVersionRemovalCodemod(t *testing.T) { + codemod := getSandboxMCPVersionRemovalCodemod() + + assert.Equal(t, "sandbox-mcp-version-removal", codemod.ID) + assert.Equal(t, "Remove deprecated sandbox.mcp.version field", codemod.Name) + assert.NotEmpty(t, codemod.Description) + assert.Equal(t, "0.26.0", codemod.IntroducedIn) + require.NotNil(t, codemod.Apply) +} + +func TestSandboxMCPVersionRemoval_RemovesVersion(t *testing.T) { + codemod := getSandboxMCPVersionRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + mcp: + version: v1.0.0 + port: 8080 +permissions: + contents: read +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "mcp": map[string]any{ + "version": "v1.0.0", + "port": 8080, + }, + }, + "permissions": map[string]any{ + "contents": "read", + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "version: v1.0.0") + assert.Contains(t, result, "mcp:") + assert.Contains(t, result, "port: 8080") +} + +func TestSandboxMCPVersionRemoval_NoVersionField(t *testing.T) { + codemod := getSandboxMCPVersionRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + mcp: + port: 8080 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "mcp": map[string]any{ + "port": 8080, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxMCPVersionRemoval_NoSandboxKey(t *testing.T) { + codemod := getSandboxMCPVersionRemovalCodemod() + + content := `--- +on: workflow_dispatch +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied) + assert.Equal(t, content, result) +} + +func TestSandboxMCPContainerRemoval_SkipsWhenStrictFalse(t *testing.T) { + codemod := getSandboxMCPContainerRemovalCodemod() + + content := `--- +on: workflow_dispatch +strict: false +sandbox: + mcp: + container: github/gh-aw-mcpg + port: 8080 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "strict": false, + "sandbox": map[string]any{ + "mcp": map[string]any{ + "container": "github/gh-aw-mcpg", + "port": 8080, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied, "should not apply when strict: false is set") + assert.Equal(t, content, result) +} + +func TestSandboxMCPVersionRemoval_SkipsWhenStrictFalse(t *testing.T) { + codemod := getSandboxMCPVersionRemovalCodemod() + + content := `--- +on: workflow_dispatch +strict: false +sandbox: + mcp: + version: v1.0.0 + port: 8080 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "strict": false, + "sandbox": map[string]any{ + "mcp": map[string]any{ + "version": "v1.0.0", + "port": 8080, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.False(t, applied, "should not apply when strict: false is set") + assert.Equal(t, content, result) +} + +func TestSandboxMCPVersionRemoval_BothContainerAndVersion(t *testing.T) { + // Verify that version removal does not affect the container key. + codemod := getSandboxMCPVersionRemovalCodemod() + + content := `--- +on: workflow_dispatch +sandbox: + mcp: + container: github/gh-aw-mcpg + version: v0.0.12 + port: 8080 +--- + +# Test` + + frontmatter := map[string]any{ + "on": "workflow_dispatch", + "sandbox": map[string]any{ + "mcp": map[string]any{ + "container": "github/gh-aw-mcpg", + "version": "v0.0.12", + "port": 8080, + }, + }, + } + + result, applied, err := codemod.Apply(content, frontmatter) + + require.NoError(t, err) + assert.True(t, applied) + assert.NotContains(t, result, "version: v0.0.12") + assert.Contains(t, result, "container: github/gh-aw-mcpg") + assert.Contains(t, result, "port: 8080") +} diff --git a/pkg/cli/fix_codemods.go b/pkg/cli/fix_codemods.go index addc0d604e2..4427d9c3e3d 100644 --- a/pkg/cli/fix_codemods.go +++ b/pkg/cli/fix_codemods.go @@ -60,6 +60,9 @@ func GetAllCodemods() []Codemod { getCliProxyFeatureToGitHubModeCodemod(), // Migrate features.cli-proxy: true to tools.github.mode: gh-proxy getDIFCProxyToIntegrityProxyCodemod(), // Migrate deprecated features.difc-proxy to tools.github.integrity-proxy getMountAsCLIsToCLIProxyCodemod(), // Rename tools.mount-as-clis to tools.cli-proxy and remove features.mcp-cli + getSandboxMCPContainerRemovalCodemod(), // Remove deprecated sandbox.mcp.container (now managed internally) + getSandboxMCPVersionRemovalCodemod(), // Remove deprecated sandbox.mcp.version (now managed internally) + getSandboxAgentFalseRemovalCodemod(), // Remove deprecated sandbox.agent: false (rejected in strict mode) } fixCodemodsLog.Printf("Loaded codemod registry: %d codemods available", len(codemods)) return codemods diff --git a/pkg/cli/fix_codemods_test.go b/pkg/cli/fix_codemods_test.go index 32b4c18457b..0e577a3c7c6 100644 --- a/pkg/cli/fix_codemods_test.go +++ b/pkg/cli/fix_codemods_test.go @@ -43,7 +43,7 @@ func TestGetAllCodemods_ReturnsAllCodemods(t *testing.T) { codemods := GetAllCodemods() // Verify we have the expected number of codemods - expectedCount := 38 + expectedCount := 41 assert.Len(t, codemods, expectedCount, "Should return all %d codemods", expectedCount) // Verify all codemods have required fields @@ -150,6 +150,9 @@ func TestGetAllCodemods_InExpectedOrder(t *testing.T) { "features-cli-proxy-to-tools-github-mode", "features-difc-proxy-to-tools-github", "mount-as-clis-to-cli-proxy", + "sandbox-mcp-container-removal", + "sandbox-mcp-version-removal", + "sandbox-agent-false-removal", } require.Len(t, codemods, len(expectedOrder), "Should have expected number of codemods") diff --git a/pkg/cli/yaml_frontmatter_utils.go b/pkg/cli/yaml_frontmatter_utils.go index 35fa69d7136..17ce9f1deaf 100644 --- a/pkg/cli/yaml_frontmatter_utils.go +++ b/pkg/cli/yaml_frontmatter_utils.go @@ -10,6 +10,18 @@ import ( var yamlUtilsLog = logger.New("cli:yaml_frontmatter_utils") +// isFrontmatterStrictFalse returns true when the frontmatter explicitly sets strict: false. +// These codemods only need to run in strict mode; if the workflow has opted out of strict +// mode, the deprecated keys are still valid and should not be touched. +func isFrontmatterStrictFalse(frontmatter map[string]any) bool { + strictVal, ok := frontmatter["strict"] + if !ok { + return false + } + strictBool, ok := strictVal.(bool) + return ok && !strictBool +} + // reconstructContent rebuilds the full markdown content from frontmatter lines and body func reconstructContent(frontmatterLines []string, markdown string) string { var lines []string