Emit Copilot auth migration tip for copilot-requests workflows#39129
Conversation
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
copilot-requests workflows
|
Hey One thing to address before this moves out of draft:
If you would like a hand cleaning this up, you can assign this prompt to your coding agent:
|
There was a problem hiding this comment.
Pull request overview
Adds a compiler “info” tip for Copilot-engine workflows to encourage enabling permissions.copilot-requests: write (so Copilot auth can use the GitHub Actions token) while suppressing the tip when copilot-requests: none is explicitly set.
Changes:
- Emit an
infocompiler message whenengine: copilotandpermissions.copilot-requestsis not effectivelywrite, unless explicitly set tonone. - Introduce
shouldEmitCopilotRequestsEnableTip(...)to centralize gating logic for the tip. - Add an integration test covering tip emission/suppression scenarios.
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/permissions_compiler_validator.go | Adds tip emission during permission validation and centralizes the gating logic. |
| pkg/workflow/copilot_requests_tip_test.go | Adds integration coverage validating when the tip appears/doesn’t appear. |
| .github/workflows/test-quality-sentinel.lock.yml | Regenerated lockfile with unrelated safe-output message changes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 3/3 changed files
- Comments generated: 5
| oldStderr := os.Stderr | ||
| r, w, _ := os.Pipe() | ||
| os.Stderr = w | ||
|
|
||
| compiler := NewCompiler() | ||
| compiler.SetStrictMode(false) | ||
| err := compiler.CompileWorkflow(testFile) | ||
|
|
||
| w.Close() | ||
| os.Stderr = oldStderr | ||
| var buf bytes.Buffer | ||
| io.Copy(&buf, r) | ||
| stderrOutput := buf.String() |
There was a problem hiding this comment.
Fixed in the latest commit by replacing the manual pipe setup with testutil.CaptureStderr, which handles errors from os.Pipe, guarantees os.Stderr restoration via t.Cleanup, and properly closes all file descriptors.
| env: | ||
| GH_AW_WORKFLOW_NAME: "Test Quality Sentinel" | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"🧪 [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" |
There was a problem hiding this comment.
Intentional. The source test-quality-sentinel.md already defines run-success: "✅ ...", but the lock file was stale with the old 🧪 emoji. Running make recompile (required whenever a .md workflow changes) regenerated the lock to match the source — this is expected and correct drift cleanup.
| GH_AW_DAILY_AI_CREDITS_TOTAL_EFFECTIVE_TOKENS: ${{ needs.activation.outputs.daily_ai_credits_total_effective_tokens }} | ||
| GH_AW_DAILY_AI_CREDITS_THRESHOLD: ${{ needs.activation.outputs.daily_ai_credits_threshold }} | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"🧪 [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" |
There was a problem hiding this comment.
Same as above — this is the same make recompile re-sync of the stale lock file to match the source .md.
| GH_AW_DETECTION_CONCLUSION: ${{ needs.detection.outputs.detection_conclusion }} | ||
| GH_AW_DETECTION_REASON: ${{ needs.detection.outputs.detection_reason }} | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"🧪 [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" |
There was a problem hiding this comment.
Same as above — part of the same make recompile re-sync.
| GH_AW_ENGINE_VERSION: "1.0.60" | ||
| GH_AW_PROJECT_UTC: "-08:00" | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"🧪 [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" | ||
| GH_AW_SAFE_OUTPUT_MESSAGES: "{\"footer\":\"\\u003e 🧪 *Test quality analysis by [{workflow_name}]({run_url})*{ai_credits_suffix}{history_link}\",\"runStarted\":\"🔬 [{workflow_name}]({run_url}) is analyzing test quality on this {event_type}...\",\"runSuccess\":\"✅ [{workflow_name}]({run_url}) completed test quality analysis.\",\"runFailure\":\"❌ [{workflow_name}]({run_url}) {status} during test quality analysis.\"}" |
There was a problem hiding this comment.
Same as above — part of the same make recompile re-sync.
|
@copilot run pr-finisher skill |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot review all comments and address unresolved review feedback. Then post a brief completion plan for any remaining blockers.
|
This change adds compiler guidance for Copilot-engine workflows to prefer GitHub Actions token-based inference (
permissions.copilot-requests: write) over PAT-based auth. The tip is intentionally suppressed when users explicitly disable it withcopilot-requests: none.What changed
copilot, andpermissions.copilot-requestsis not effectivelywrite.copilot-requests: writeis already configured, orcopilot-requests: noneis explicitly set.copilot-requests(tip emitted),write(no tip),none(no tip),Implementation detail
shouldEmitCopilotRequestsEnableTip(workflowData, workflowPermissions)to centralize tip gating, using explicit permission checks to distinguish intentionalnonefrom unset/default states.