Route harness fallback diagnostics through safeoutputs CLI#35934
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 merge main and recompile |
There was a problem hiding this comment.
Pull request overview
Routes Copilot harness fallback diagnostics (missing_tool / report_incomplete) through the safeoutputs CLI instead of appending JSONL directly to GH_AW_SAFE_OUTPUTS, so structured failure signals survive read-only teardown states (EROFS). Denied-command context is folded into the alternatives field with bounded length, and tests are updated to assert the CLI invocation contract.
Changes:
- Adds
runSafeOutputsCLIhelper (with key validation and richer error context) andbuildMissingToolAlternatives(512-char cap with... and N moreoverflow marker). - Switches
emitMissingToolPermissionIssueandemitInfrastructureIncompletefromappendFileSyncto CLI invocation; preserves skip behavior whenGH_AW_SAFE_OUTPUTSis unset. - Updates harness tests for the new CLI contract and adds coverage for empty/capped/overflow alternatives and CLI failure logging.
Show a summary per file
| File | Description |
|---|---|
| actions/setup/js/copilot_harness.cjs | Adds CLI-based safeoutputs emission and bounded alternatives builder; replaces direct file appends in fallback emitters. |
| actions/setup/js/copilot_harness.test.cjs | Updates fallback tests to assert CLI calls and adds edge-case + failure-path coverage. |
| .github/workflows/*.lock.yml (8 files) | Regenerated lock files: checkout-line format flipped to repo X → $GITHUB_WORKSPACE and refreshed heredoc tag hashes; unrelated to harness changes. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 10/10 changed files
- Comments generated: 2
| try { | ||
| childProcess.execFileSync(command, commandArgs, { encoding: "utf8", stdio: ["ignore", "pipe", "pipe"] }); | ||
| } catch (error) { | ||
| const err = /** @type {{message?: string, stderr?: string | Buffer}} */ error || {}; |
| const argSummary = commandArgs.slice(1, 7).join(" "); | ||
| throw new Error(stderr ? `safeoutputs ${argSummary} failed: ${message}: ${stderr}` : `safeoutputs ${argSummary} failed: ${message}`); |
…ix-silent-failure # Conflicts: # .github/workflows/schema-feature-coverage.lock.yml # .github/workflows/smoke-codex.lock.yml Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done — merged |
|
|
| * @param {string} toolName | ||
| * @param {Record<string, string>} args | ||
| */ | ||
| function runSafeOutputsCLI(toolName, args) { |
There was a problem hiding this comment.
@copilot refactor safeoutputs cli helpers to own file and refactor support for all harnesses
There was a problem hiding this comment.
Done in the latest commit. The CLI helpers are now in safeoutputs_cli.cjs which exports runSafeOutputsCLI, buildMissingToolAlternatives, emitMissingToolPermissionIssue, and emitInfrastructureIncomplete. All three harnesses (copilot, claude, codex) now require from this shared module — claude_harness and codex_harness also switch from the old appendFileSync path to the CLI path. A new safeoutputs_cli.test.cjs covers the shared module directly.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Fixed in the latest commit:
|
When agentic runs failed after retry exhaustion, harnesses attempted to append fallback diagnostics directly to
outputs.jsonl; in read-only teardown states this hitEROFS, dropping structured failure signals. This change moves fallback emission to thesafeoutputsCLI path somissing_tool/report_incompletediagnostics are still emitted through the supported channel — for all three harnesses (Copilot, Claude, Codex).Shared CLI helper module (
safeoutputs_cli.cjs)runSafeOutputsCLI,buildMissingToolAlternatives,emitMissingToolPermissionIssue, andemitInfrastructureIncompleteinto a single shared module consumed by all harnessestoolName(key1, key2)format for deterministic, value-free contextFallback emission path
emitMissingToolPermissionIssuenow emits viasafeoutputs missing_tool ...in all harnessesemitInfrastructureIncompletenow emits viasafeoutputs report_incomplete ...GH_AW_SAFE_OUTPUTSfile for these pathsclaude_harnessandcodex_harnessswitch from the oldappendFileSyncpath to the shared CLI pathCLI emission hardening
GH_AW_SAFE_OUTPUTSis unsetPermission-denied diagnostics payload shaping
alternatives... and N more) instead of naive truncationTests
safeoutputs_cli.test.cjswith 15 tests covering the shared module directly