-
Notifications
You must be signed in to change notification settings - Fork 268
feat: add footer: false support to add-comment safe output
#18942
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -5065,6 +5065,11 @@ | |||||
| "type": "boolean", | ||||||
| "description": "Controls whether the workflow requests pull-requests:write permission for add-comment and includes pull requests in the event trigger condition. Default: true (includes pull-requests:write). Set to false to disable pull request commenting." | ||||||
| }, | ||||||
| "footer": { | ||||||
| "type": "boolean", | ||||||
| "description": "Controls whether AI-generated footer is added to the comment. When false, the visible footer content is omitted but XML markers (workflow-id, metadata) are still included for searchability. Defaults to true.", | ||||||
|
||||||
| "description": "Controls whether AI-generated footer is added to the comment. When false, the visible footer content is omitted but XML markers (workflow-id, metadata) are still included for searchability. Defaults to true.", | |
| "description": "Controls whether AI-generated footer is added to the comment. When false, the visible footer content is omitted but XML markers (workflow-id, tracker-id, metadata) are still included for searchability. Defaults to true.", |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,13 +26,71 @@ func TestFooterConfiguration(t *testing.T) { | |
| assert.Equal(t, "false", *config.CreateIssues.Footer) | ||
| } | ||
|
|
||
| func TestAddCommentFooterConfiguration(t *testing.T) { | ||
| t.Run("footer: false on add-comment", func(t *testing.T) { | ||
| compiler := NewCompiler() | ||
| frontmatter := map[string]any{ | ||
| "name": "Test", | ||
| "safe-outputs": map[string]any{ | ||
| "add-comment": map[string]any{"footer": false}, | ||
| }, | ||
| } | ||
| config := compiler.extractSafeOutputsConfig(frontmatter) | ||
| require.NotNil(t, config) | ||
| require.NotNil(t, config.AddComments) | ||
| require.NotNil(t, config.AddComments.Footer) | ||
| assert.Equal(t, "false", *config.AddComments.Footer, "add-comment footer should be false") | ||
| }) | ||
|
|
||
| t.Run("global footer: false propagates to add-comment", func(t *testing.T) { | ||
| compiler := NewCompiler() | ||
| frontmatter := map[string]any{ | ||
| "name": "Test", | ||
| "safe-outputs": map[string]any{ | ||
| "footer": false, | ||
| "add-comment": map[string]any{}, | ||
| }, | ||
| } | ||
| config := compiler.extractSafeOutputsConfig(frontmatter) | ||
| require.NotNil(t, config) | ||
| require.NotNil(t, config.Footer) | ||
| assert.False(t, *config.Footer, "Global footer should be false") | ||
|
|
||
| workflowData := &WorkflowData{ | ||
| Name: "Test", | ||
| SafeOutputs: config, | ||
| } | ||
| var steps []string | ||
| compiler.addHandlerManagerConfigEnvVar(&steps, workflowData) | ||
|
|
||
| for _, step := range steps { | ||
| if strings.Contains(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG") { | ||
| parts := strings.Split(step, "GH_AW_SAFE_OUTPUTS_HANDLER_CONFIG: ") | ||
| if len(parts) == 2 { | ||
| jsonStr := strings.TrimSpace(parts[1]) | ||
| jsonStr = strings.Trim(jsonStr, "\"") | ||
| jsonStr = strings.ReplaceAll(jsonStr, "\\\"", "\"") | ||
| var handlerConfig map[string]any | ||
| err := json.Unmarshal([]byte(jsonStr), &handlerConfig) | ||
| require.NoError(t, err) | ||
|
|
||
| addCommentConfig, ok := handlerConfig["add_comment"].(map[string]any) | ||
| require.True(t, ok, "add_comment handler config should exist") | ||
| assert.Equal(t, false, addCommentConfig["footer"], "add_comment should inherit global footer: false") | ||
| } | ||
| } | ||
| } | ||
|
Comment on lines
+66
to
+82
|
||
| }) | ||
| } | ||
|
|
||
| func TestGlobalFooterConfiguration(t *testing.T) { | ||
| t.Run("global footer: false applies to all handlers", func(t *testing.T) { | ||
| compiler := NewCompiler() | ||
| frontmatter := map[string]any{ | ||
| "name": "Test", | ||
| "safe-outputs": map[string]any{ | ||
| "footer": false, // Global footer control | ||
| "add-comment": map[string]any{}, | ||
| "create-issue": map[string]any{"title-prefix": "[test] "}, | ||
| "create-pull-request": nil, | ||
| "create-discussion": nil, | ||
|
|
@@ -69,6 +127,9 @@ func TestGlobalFooterConfiguration(t *testing.T) { | |
| require.NoError(t, err) | ||
|
|
||
| // All handlers should have footer: false from global setting | ||
| addCommentConfig, ok := handlerConfig["add_comment"].(map[string]any) | ||
| require.True(t, ok, "add_comment handler config should exist in global footer test") | ||
| assert.Equal(t, false, addCommentConfig["footer"], "add_comment should inherit global footer: false") | ||
| if issueConfig, ok := handlerConfig["create_issue"].(map[string]any); ok { | ||
| assert.Equal(t, false, issueConfig["footer"], "create_issue should inherit global footer: false") | ||
| } | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
footer: false, the other handlers (create_issue.cjs,create_pull_request.cjs,create_discussion.cjs) consistently add agenerateWorkflowIdMarker(fromgenerate_footer.cjs), which produces<!-- gh-aw-workflow-id: ${workflowId} -->. However, the newadd_comment.cjsimplementation callsgenerateXMLMarker(frommessages_footer.cjs), which produces a much more complex<!-- gh-aw-agentic-workflow: ..., workflow_id: ..., run: ... -->marker.This inconsistency also conflicts with the documentation added in
faq.md(line 286), which explicitly states that<!-- gh-aw-workflow-id: ... -->will still be included for searchability. The actual marker format added byadd_comment.cjswithfooter: falseusesgh-aw-agentic-workflow:as the prefix (notgh-aw-workflow-id:), and also includes theworkflow_id:field only conditionally ifGH_AW_WORKFLOW_IDis set.To be consistent with other handlers and with the documentation, the
footer: falsepath inadd_comment.cjsshould use the same approach ascreate_issue.cjsandcreate_pull_request.cjs: checkif (workflowId)and addgenerateWorkflowIdMarker(workflowId)(imported fromgenerate_footer.cjs).