Skip to content

Commit 6ab2750

Browse files
Copilotlpcox
andauthored
Remove cli-proxy-writable feature flag and add read-only gh CLI prompt for cli-proxy (#25013)
* Initial plan * remove cli-proxy-writable feature flag — agent gh CLI should be read-only Agent-Logs-Url: https://github.com/github/gh-aw/sessions/1dd22cfc-aa5d-4843-9516-c881f8818dfe Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> * add cli-proxy prompt and skip GitHub MCP server when cli-proxy is enabled Agent-Logs-Url: https://github.com/github/gh-aw/sessions/8f40a5c6-a9d3-4271-bd96-a06d5ed34614 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
1 parent 6f2145b commit 6ab2750

10 files changed

Lines changed: 160 additions & 62 deletions

.changeset/minor-add-cli-proxy-feature-flag.md

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<gh-cli>
2+
The `gh` CLI is pre-authenticated via the CLI proxy sidecar. Use `gh` commands for all GitHub reads: listing and searching issues, pull requests, discussions, labels, milestones; reading workflow runs, jobs, and artifacts; accessing repository contents, code, and metadata. No GitHub MCP server is available.
3+
</gh-cli>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<gh-cli>
2+
The `gh` CLI is pre-authenticated via the CLI proxy sidecar. Use `gh` commands for all GitHub reads: listing and searching issues, pull requests, discussions, labels, milestones; reading workflow runs, jobs, and artifacts; accessing repository contents, code, and metadata. Use safeoutputs tools for GitHub writes and completion signaling. No GitHub MCP server is available.
3+
</gh-cli>

pkg/constants/feature_constants.go

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,17 +38,4 @@ const (
3838
// features:
3939
// cli-proxy: true
4040
CliProxyFeatureFlag FeatureFlag = "cli-proxy"
41-
// CliProxyWritableFeatureFlag enables write operations on the AWF CLI proxy sidecar.
42-
// By default, the CLI proxy sidecar is read-only. When this flag is enabled,
43-
// --cli-proxy-writable is injected into the AWF command, allowing write operations
44-
// such as creating issues or merging PRs via gh CLI.
45-
//
46-
// Requires CliProxyFeatureFlag to also be enabled.
47-
//
48-
// Workflow frontmatter usage:
49-
//
50-
// features:
51-
// cli-proxy: true
52-
// cli-proxy-writable: true
53-
CliProxyWritableFeatureFlag FeatureFlag = "cli-proxy-writable"
5441
)

pkg/workflow/awf_helpers.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -250,12 +250,6 @@ func BuildAWFArgs(config AWFCommandConfig) []string {
250250
awfArgs = append(awfArgs, "--enable-cli-proxy")
251251
awfHelpersLog.Print("Added --enable-cli-proxy for gh CLI proxy sidecar")
252252

253-
// Allow write operations when cli-proxy-writable feature flag is also set
254-
if isFeatureEnabled(constants.CliProxyWritableFeatureFlag, config.WorkflowData) {
255-
awfArgs = append(awfArgs, "--cli-proxy-writable")
256-
awfHelpersLog.Print("Added --cli-proxy-writable for write access via gh CLI proxy")
257-
}
258-
259253
// Generate and pass the guard policy JSON for the cli-proxy.
260254
// Reuses getDIFCProxyPolicyJSON() to build the static policy from tools.github config
261255
// (min-integrity and repos fields), matching the DIFC proxy guard policy semantics.

pkg/workflow/awf_helpers_test.go

Lines changed: 3 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -730,8 +730,8 @@ func TestAWFSupportsExcludeEnv(t *testing.T) {
730730
}
731731
}
732732

733-
// TestBuildAWFArgsCliProxy tests that BuildAWFArgs correctly injects --enable-cli-proxy,
734-
// --cli-proxy-writable, and --cli-proxy-policy based on the cli-proxy feature flags.
733+
// TestBuildAWFArgsCliProxy tests that BuildAWFArgs correctly injects --enable-cli-proxy
734+
// and --cli-proxy-policy based on the cli-proxy feature flag.
735735
func TestBuildAWFArgsCliProxy(t *testing.T) {
736736
baseWorkflow := func(features map[string]any, tools map[string]any) *WorkflowData {
737737
return &WorkflowData{
@@ -758,7 +758,6 @@ func TestBuildAWFArgsCliProxy(t *testing.T) {
758758
argsStr := strings.Join(args, " ")
759759

760760
assert.NotContains(t, argsStr, "--enable-cli-proxy", "Should not include --enable-cli-proxy when feature flag is absent")
761-
assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable when feature flag is absent")
762761
assert.NotContains(t, argsStr, "--cli-proxy-policy", "Should not include --cli-proxy-policy when feature flag is absent")
763762
})
764763

@@ -776,42 +775,6 @@ func TestBuildAWFArgsCliProxy(t *testing.T) {
776775
argsStr := strings.Join(args, " ")
777776

778777
assert.Contains(t, argsStr, "--enable-cli-proxy", "Should include --enable-cli-proxy when cli-proxy feature flag is enabled")
779-
assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable when cli-proxy-writable feature flag is absent")
780-
})
781-
782-
t.Run("includes --cli-proxy-writable when cli-proxy-writable feature flag is enabled", func(t *testing.T) {
783-
config := AWFCommandConfig{
784-
EngineName: "copilot",
785-
WorkflowData: baseWorkflow(
786-
map[string]any{"cli-proxy": true, "cli-proxy-writable": true},
787-
nil,
788-
),
789-
AllowedDomains: "github.com",
790-
}
791-
792-
args := BuildAWFArgs(config)
793-
argsStr := strings.Join(args, " ")
794-
795-
assert.Contains(t, argsStr, "--enable-cli-proxy", "Should include --enable-cli-proxy")
796-
assert.Contains(t, argsStr, "--cli-proxy-writable", "Should include --cli-proxy-writable when feature flag is enabled")
797-
})
798-
799-
t.Run("does not include --cli-proxy-writable without --enable-cli-proxy", func(t *testing.T) {
800-
// cli-proxy-writable alone (without cli-proxy) should not inject any cli-proxy flags
801-
config := AWFCommandConfig{
802-
EngineName: "copilot",
803-
WorkflowData: baseWorkflow(
804-
map[string]any{"cli-proxy-writable": true},
805-
nil,
806-
),
807-
AllowedDomains: "github.com",
808-
}
809-
810-
args := BuildAWFArgs(config)
811-
argsStr := strings.Join(args, " ")
812-
813-
assert.NotContains(t, argsStr, "--enable-cli-proxy", "Should not include --enable-cli-proxy when cli-proxy flag is absent")
814-
assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable when cli-proxy flag is absent")
815778
})
816779

817780
t.Run("includes --cli-proxy-policy with guard policy when tools.github has min-integrity", func(t *testing.T) {
@@ -894,8 +857,7 @@ func TestBuildAWFArgsCliProxy(t *testing.T) {
894857
},
895858
},
896859
Features: map[string]any{
897-
"cli-proxy": true,
898-
"cli-proxy-writable": true,
860+
"cli-proxy": true,
899861
},
900862
Tools: map[string]any{
901863
"github": map[string]any{
@@ -914,7 +876,6 @@ func TestBuildAWFArgsCliProxy(t *testing.T) {
914876
argsStr := strings.Join(args, " ")
915877

916878
assert.NotContains(t, argsStr, "--enable-cli-proxy", "Should not include --enable-cli-proxy for AWF < v0.25.14")
917-
assert.NotContains(t, argsStr, "--cli-proxy-writable", "Should not include --cli-proxy-writable for AWF < v0.25.14")
918879
assert.NotContains(t, argsStr, "--cli-proxy-policy", "Should not include --cli-proxy-policy for AWF < v0.25.14")
919880
})
920881
}

pkg/workflow/mcp_setup_generator.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,13 @@ func (c *Compiler) generateMCPSetup(yaml *strings.Builder, tools map[string]any,
9393
}
9494
// Standard MCP tools
9595
if toolName == "github" || toolName == "playwright" || toolName == "cache-memory" || toolName == "agentic-workflows" {
96+
// When cli-proxy is enabled, agents use the pre-authenticated gh CLI for GitHub
97+
// reads instead of the GitHub MCP server. Skip registering the GitHub MCP server
98+
// so it is not configured with the gateway.
99+
if toolName == "github" && isFeatureEnabled(constants.CliProxyFeatureFlag, workflowData) {
100+
mcpSetupGeneratorLog.Print("Skipping GitHub MCP server registration: cli-proxy feature flag is enabled")
101+
continue
102+
}
96103
mcpTools = append(mcpTools, toolName)
97104
} else if mcpConfig, ok := toolValue.(map[string]any); ok {
98105
// Check if it's explicitly marked as MCP type in the new format

pkg/workflow/prompt_constants.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,8 @@ const (
2626
agenticWorkflowsGuideFile = "agentic_workflows_guide.md"
2727
githubMCPToolsPromptFile = "github_mcp_tools_prompt.md"
2828
githubMCPToolsWithSafeOutputsPromptFile = "github_mcp_tools_with_safeoutputs_prompt.md"
29+
cliProxyPromptFile = "cli_proxy_prompt.md"
30+
cliProxyWithSafeOutputsPromptFile = "cli_proxy_with_safeoutputs_prompt.md"
2931
)
3032

3133
// GitHub context prompt is kept embedded because it contains GitHub Actions expressions

pkg/workflow/unified_prompt_step.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,24 @@ func (c *Compiler) collectPromptSections(data *WorkflowData) []PromptSection {
222222
EnvVars: envVars,
223223
})
224224
}
225+
}
225226

227+
// 9b. GitHub tool-use guidance: directs the model to the correct mechanism for
228+
// GitHub reads (and writes when safe-outputs is also enabled).
229+
// When cli-proxy is enabled, the agent uses the pre-authenticated gh CLI for reads
230+
// instead of a GitHub MCP server (which is not registered). Otherwise, the GitHub
231+
// MCP server is used for reads.
232+
if isFeatureEnabled(constants.CliProxyFeatureFlag, data) {
233+
unifiedPromptLog.Print("Adding cli-proxy tool-use guidance (gh CLI for reads, no GitHub MCP server)")
234+
cliProxyFile := cliProxyPromptFile
235+
if HasSafeOutputsEnabled(data.SafeOutputs) {
236+
cliProxyFile = cliProxyWithSafeOutputsPromptFile
237+
}
238+
sections = append(sections, PromptSection{
239+
Content: cliProxyFile,
240+
IsFile: true,
241+
})
242+
} else if hasGitHubTool(data.ParsedTools) {
226243
// GitHub MCP tool-use guidance: clarifies that the MCP server is read-only and
227244
// directs the model to use it for GitHub reads. When safe-outputs is also enabled,
228245
// the guidance explicitly separates reads (GitHub MCP) from writes (safeoutputs) so

pkg/workflow/unified_prompt_step_test.go

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -374,3 +374,127 @@ func TestCollectPromptSections_GitHubMCPAndSafeOutputsConsistency(t *testing.T)
374374
}
375375
})
376376
}
377+
378+
// TestCollectPromptSections_CliProxy tests that when the cli-proxy feature flag is
379+
// enabled, the cli-proxy prompt is used instead of the GitHub MCP tools prompt,
380+
// and that the GitHub MCP server guidance is never injected.
381+
func TestCollectPromptSections_CliProxy(t *testing.T) {
382+
t.Run("cli-proxy enabled without safe-outputs uses cli_proxy_prompt", func(t *testing.T) {
383+
compiler := &Compiler{}
384+
385+
data := &WorkflowData{
386+
ParsedTools: NewTools(map[string]any{"github": true}),
387+
Features: map[string]any{"cli-proxy": true},
388+
SafeOutputs: nil,
389+
}
390+
391+
sections := compiler.collectPromptSections(data)
392+
require.NotEmpty(t, sections, "Should collect sections")
393+
394+
// Should include cli-proxy prompt
395+
var cliProxySection *PromptSection
396+
for i := range sections {
397+
if sections[i].IsFile && sections[i].Content == cliProxyPromptFile {
398+
cliProxySection = &sections[i]
399+
break
400+
}
401+
}
402+
require.NotNil(t, cliProxySection, "Should include cli_proxy_prompt.md when cli-proxy is enabled")
403+
404+
// Should NOT include GitHub MCP tools prompt
405+
for _, section := range sections {
406+
assert.NotEqual(t, githubMCPToolsPromptFile, section.Content,
407+
"Should not include github_mcp_tools_prompt.md when cli-proxy is enabled")
408+
assert.NotEqual(t, githubMCPToolsWithSafeOutputsPromptFile, section.Content,
409+
"Should not include github_mcp_tools_with_safeoutputs_prompt.md when cli-proxy is enabled")
410+
}
411+
})
412+
413+
t.Run("cli-proxy enabled with safe-outputs uses cli_proxy_with_safeoutputs_prompt", func(t *testing.T) {
414+
compiler := &Compiler{}
415+
416+
data := &WorkflowData{
417+
ParsedTools: NewTools(map[string]any{"github": true}),
418+
Features: map[string]any{"cli-proxy": true},
419+
SafeOutputs: &SafeOutputsConfig{
420+
MissingData: &MissingDataConfig{},
421+
NoOp: &NoOpConfig{},
422+
},
423+
}
424+
425+
sections := compiler.collectPromptSections(data)
426+
require.NotEmpty(t, sections, "Should collect sections")
427+
428+
// Should include the with-safeoutputs cli-proxy prompt
429+
var cliProxySection *PromptSection
430+
for i := range sections {
431+
if sections[i].IsFile && sections[i].Content == cliProxyWithSafeOutputsPromptFile {
432+
cliProxySection = &sections[i]
433+
break
434+
}
435+
}
436+
require.NotNil(t, cliProxySection, "Should include cli_proxy_with_safeoutputs_prompt.md when cli-proxy and safe-outputs are both enabled")
437+
438+
// Should NOT include GitHub MCP tools prompt
439+
for _, section := range sections {
440+
assert.NotEqual(t, githubMCPToolsPromptFile, section.Content,
441+
"Should not include github_mcp_tools_prompt.md when cli-proxy is enabled")
442+
assert.NotEqual(t, githubMCPToolsWithSafeOutputsPromptFile, section.Content,
443+
"Should not include github_mcp_tools_with_safeoutputs_prompt.md when cli-proxy is enabled")
444+
}
445+
})
446+
447+
t.Run("cli-proxy enabled without github tool still adds cli-proxy prompt", func(t *testing.T) {
448+
compiler := &Compiler{}
449+
450+
data := &WorkflowData{
451+
ParsedTools: NewTools(map[string]any{}),
452+
Features: map[string]any{"cli-proxy": true},
453+
SafeOutputs: nil,
454+
}
455+
456+
sections := compiler.collectPromptSections(data)
457+
458+
// Should include cli-proxy prompt even without github tool configured
459+
var cliProxySection *PromptSection
460+
for i := range sections {
461+
if sections[i].IsFile && sections[i].Content == cliProxyPromptFile {
462+
cliProxySection = &sections[i]
463+
break
464+
}
465+
}
466+
require.NotNil(t, cliProxySection, "Should include cli_proxy_prompt.md when cli-proxy is enabled regardless of tools.github")
467+
})
468+
469+
t.Run("cli-proxy disabled uses GitHub MCP tools prompt when github tool is enabled", func(t *testing.T) {
470+
compiler := &Compiler{}
471+
472+
data := &WorkflowData{
473+
ParsedTools: NewTools(map[string]any{"github": true}),
474+
Features: nil,
475+
SafeOutputs: nil,
476+
}
477+
478+
sections := compiler.collectPromptSections(data)
479+
480+
// Should include the standard GitHub MCP tools prompt
481+
var githubMCPSection *PromptSection
482+
for i := range sections {
483+
if sections[i].IsFile && strings.Contains(sections[i].Content, "github_mcp_tools") {
484+
githubMCPSection = &sections[i]
485+
break
486+
}
487+
}
488+
require.NotNil(t, githubMCPSection, "Should include github_mcp_tools file when cli-proxy is not enabled")
489+
assert.Equal(t, githubMCPToolsPromptFile, githubMCPSection.Content,
490+
"Should use the standard github_mcp_tools_prompt when cli-proxy is not enabled")
491+
492+
// Should NOT include cli-proxy prompt
493+
for _, section := range sections {
494+
assert.NotEqual(t, cliProxyPromptFile, section.Content,
495+
"Should not include cli_proxy_prompt.md when cli-proxy is not enabled")
496+
assert.NotEqual(t, cliProxyWithSafeOutputsPromptFile, section.Content,
497+
"Should not include cli_proxy_with_safeoutputs_prompt.md when cli-proxy is not enabled")
498+
}
499+
})
500+
}

0 commit comments

Comments
 (0)