Merge assign-to-agent safe output handler into the safe output main handler loop#24934
Merge assign-to-agent safe output handler into the safe output main handler loop#24934
Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9384f0c5-024e-4593-92ef-503f1a2c0742 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…max-skipped tracking Agent-Logs-Url: https://github.com/github/gh-aw/sessions/9384f0c5-024e-4593-92ef-503f1a2c0742 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot convert this smoke-agent-* agentic workflows to use claude |
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/b60c6d5f-3562-42dc-9b84-1ccd15336980 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR integrates the assign_to_agent safe-output handler into the main process_safe_outputs handler-manager loop, removing the dedicated standalone “Assign to Agent” step and updating compiler outputs/env wiring accordingly. It also updates smoke-agent workflows to use the claude engine instead of codex.
Changes:
- Refactors
assign_to_agent.cjsinto a handler-factory invoked by the handler manager, with dedicated Octokit auth viaGH_AW_ASSIGN_TO_AGENT_TOKEN. - Updates Go workflow compiler to route assign-to-agent through
process_safe_outputs, export outputs from that step, and inject the new env var / custom-token setup. - Regenerates multiple workflow lockfiles (including smoke-agent workflows switching
engine: claude).
Show a summary per file
| File | Description |
|---|---|
| pkg/workflow/safe_outputs_integration_test.go | Updates integration expectations to use process_safe_outputs instead of a standalone assign_to_agent step. |
| pkg/workflow/js.go | Removes now-unused getAssignToAgentScript() getter. |
| pkg/workflow/compiler_safe_outputs_steps.go | Injects GH_AW_ASSIGN_TO_AGENT_TOKEN into the handler-manager step env when assign-to-agent is configured. |
| pkg/workflow/compiler_safe_outputs_specialized.go | Deletes the specialized standalone-step builder for assign-to-agent. |
| pkg/workflow/compiler_safe_outputs_job.go | Routes assign-to-agent through handler manager, exports outputs from steps.process_safe_outputs.outputs.*, removes standalone step emission. |
| pkg/workflow/compiler_safe_outputs_job_test.go | Updates tests to validate process_safe_outputs contains GH_AW_ASSIGN_TO_AGENT_TOKEN and no dedicated step exists. |
| pkg/workflow/compiler_safe_outputs_core.go | Ensures assign-to-agent triggers safe-output-custom-tokens setup (for @actions/github). |
| pkg/workflow/assign_to_agent_test.go | Updates workflow compile test to assert handler-manager integration and exported outputs. |
| actions/setup/js/safe_output_handler_manager.cjs | Adds assign_to_agent to HANDLER_MAP, removes it from standalone types, exports assign-to-agent outputs and writes its summary. |
| actions/setup/js/assign_to_agent.cjs | Rewrites assign-to-agent as a handler factory with dedicated Octokit auth, temp-ID deferral, per-message processing, and exported getters/summary writer. |
| actions/setup/js/assign_agent_helpers.cjs | Adds optional githubClient parameter to GraphQL helpers for dedicated Octokit usage. |
| .github/workflows/workflow-generator.lock.yml | Switches assign-to-agent outputs to come from process_safe_outputs, enables safe-output-custom-tokens, adds GH_AW_ASSIGN_TO_AGENT_TOKEN, removes standalone step. |
| .github/workflows/smoke-agent-scoped-approved.md | Changes workflow frontmatter engine from codex to claude. |
| .github/workflows/smoke-agent-scoped-approved.lock.yml | Regenerates lockfile for claude engine and related runtime/tooling changes. |
| .github/workflows/smoke-agent-public-none.md | Changes workflow frontmatter engine from codex to claude. |
| .github/workflows/smoke-agent-public-none.lock.yml | Regenerates lockfile for claude engine and related runtime/tooling changes. |
| .github/workflows/smoke-agent-public-approved.md | Changes workflow frontmatter engine from codex to claude. |
| .github/workflows/smoke-agent-public-approved.lock.yml | Regenerates lockfile; routes assign-to-agent through handler manager and enables custom-token install. |
| .github/workflows/smoke-agent-all-none.md | Changes workflow frontmatter engine from codex to claude. |
| .github/workflows/smoke-agent-all-none.lock.yml | Regenerates lockfile for claude engine and related runtime/tooling changes. |
| .github/workflows/smoke-agent-all-merged.md | Changes workflow frontmatter engine from codex to claude. |
| .github/workflows/smoke-agent-all-merged.lock.yml | Regenerates lockfile for claude engine and related runtime/tooling changes. |
| .github/workflows/issue-monster.lock.yml | Switches assign-to-agent outputs to process_safe_outputs, enables safe-output-custom-tokens, adds GH_AW_ASSIGN_TO_AGENT_TOKEN, removes standalone step. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comments suppressed due to low confidence (1)
actions/setup/js/assign_to_agent.cjs:487
- The step summary labels all skipped results as "(ignore-if-error enabled)" and "assignment failed due to error", but skips can now also occur for reasons like
maxCountbeing reached (and potentially other future skip reasons). This produces misleading summaries. Track a skip reason (e.g.skipped_reason) and render different messaging for ignore-if-error vs max-count vs other skips.
if (skippedResults.length > 0) {
summaryContent += `⏭️ Skipped ${skippedResults.length} agent assignment(s) (ignore-if-error enabled):\n\n`;
summaryContent += skippedResults
.map(r => {
const itemType = r.issue_number ? `Issue #${r.issue_number}` : `Pull Request #${r.pull_number}`;
return `- ${itemType} → Agent: ${r.agent} (assignment failed due to error)`;
})
- Files reviewed: 23/23 changed files
- Comments generated: 2
| async function main(config = {}) { | ||
| // Parse configuration (replaces env vars from the old standalone step) | ||
| const maxCount = parseInt(String(config.max ?? "1"), 10); | ||
| if (isNaN(maxCount) || maxCount < 1) { | ||
| throw new Error(`Invalid max value: ${config.max}. Must be a positive integer`); | ||
| } |
There was a problem hiding this comment.
main() is now a handler factory that returns a per-message function, but the repo still has JS unit tests (e.g. assign_to_agent.test.cjs) that call main() expecting it to read GH_AW_AGENT_OUTPUT and execute immediately. As written, those tests will fail (and any remaining standalone callers would break). Update the tests to invoke the returned handler with messages/temporaryIdMap, or add a small backwards-compatible wrapper entrypoint for standalone execution if that mode is still supported.
| core.info(`⏭ Max count (${maxCount}) reached, skipping agent assignment`); | ||
| const agentNameForSkip = message.agent ?? defaultAgent; | ||
| _allResults.push({ issue_number: message.issue_number || null, pull_number: message.pull_number || null, agent: agentNameForSkip, owner: null, repo: null, success: false, skipped: true }); | ||
| return { success: false, skipped: true }; |
There was a problem hiding this comment.
When maxCount is exceeded, the handler returns { success: false, skipped: true } without an error/reason. The handler manager will log a generic message ("Handler returned success: false with skipped: true"), which makes it hard to understand why the assignment was skipped. Consider returning a specific error/reason string (or using success: true, skipped: true) for this path.
| core.info(`⏭ Max count (${maxCount}) reached, skipping agent assignment`); | |
| const agentNameForSkip = message.agent ?? defaultAgent; | |
| _allResults.push({ issue_number: message.issue_number || null, pull_number: message.pull_number || null, agent: agentNameForSkip, owner: null, repo: null, success: false, skipped: true }); | |
| return { success: false, skipped: true }; | |
| const error = `Max count (${maxCount}) reached, skipping agent assignment`; | |
| core.info(`⏭ ${error}`); | |
| const agentNameForSkip = message.agent ?? defaultAgent; | |
| _allResults.push({ issue_number: message.issue_number || null, pull_number: message.pull_number || null, agent: agentNameForSkip, owner: null, repo: null, success: false, skipped: true, error }); | |
| return { success: false, skipped: true, error }; |
Integrates the
assign-to-agentsafe output handler into the mainprocess_safe_outputshandler manager loop, removing the dedicated standalone step.Changes
JavaScript
assign_to_agent.cjs: Complete rewrite as a handler factory pattern.main(config)returns a per-message handler function. Creates a dedicated Octokit instance fromGH_AW_ASSIGN_TO_AGENT_TOKENenv var (agent token chain) or per-handlergithub-token. Resolves temporary IDs via the handler manager's livetemporaryIdMap. ExportsgetAssignToAgentAssigned/Errors/ErrorCount/Summaryfor the handler manager.assign_agent_helpers.cjs: Added optionalgithubClientparameter (defaulting to globalgithub) to all GraphQL-calling functions. Fully backward compatible.safe_output_handler_manager.cjs: Addedassign_to_agenttoHANDLER_MAP, removed fromSTANDALONE_STEP_TYPES, exports assign_to_agent outputs (assign_to_agent_assigned,assign_to_agent_assignment_errors,assign_to_agent_assignment_error_count) and calls summary writer after processing.Go compiler
compiler_safe_outputs_job.go: Addedassign_to_agenttohasHandlerManagerTypes. Removed dedicated standalone step. Updated outputs to come fromsteps.process_safe_outputs.outputs.*.compiler_safe_outputs_steps.go: AddsGH_AW_ASSIGN_TO_AGENT_TOKENenv var to theprocess_safe_outputsstep when assign-to-agent is configured (uses agent token preference chain:GH_AW_AGENT_TOKEN || GH_AW_GITHUB_TOKEN || GITHUB_TOKEN).compiler_safe_outputs_core.go:hasCustomTokenSafeOutputsreturnstruefor assign_to_agent, triggering@actions/githubinstallation viasafe-output-custom-tokens: true.compiler_safe_outputs_specialized.go: RemovedbuildAssignToAgentStepConfig(no longer needed).js.go: Removed now-unusedgetAssignToAgentScriptfunction.Workflows
smoke-agent-*.md: All fivesmoke-agent-*workflows (smoke-agent-all-merged,smoke-agent-all-none,smoke-agent-public-approved,smoke-agent-public-none,smoke-agent-scoped-approved) updated fromengine: codextoengine: claude.Key behaviors preserved
GH_AW_ASSIGN_TO_AGENT_TOKEN(not the step-level App token) — Copilot assignment API rejects App tokenstemporaryIdMap, with deferred processing when not yet resolvedassign_to_agent_assignment_error_countetc.) preserved for conclusion job