Repository navigation
Conversation
Capture attributed AWF timeouts and failed wrapper exits, preserve failed step metadata, and omit contradictory zero exits when no native failure code is known. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
🟡 Changes recommended
DeepSeek error handling omits detection in generated workflows and can lose timeout or wrapper failures, leaving missing terminal errors and incorrect zero exit codes.
3 open issues
What changed in this PR
This PR improves DeepSeek Harness failure reporting in gh-aw, preserving execution diagnostics without inventing native conversation events or unknown exit codes.
Changes:
- Recognizes attributed timeouts and failed wrapper exits as session errors.
- Adds live failure-metadata fallback and reconciles contradictory exit-zero records.
- Adds regression coverage, documentation, and patch-release metadata.
| File | Description |
|---|---|
docs/src/content/docs/reference/third-party-agent.md |
Documents failure coverage and unknown exit-code semantics. |
actions/setup/js/unified_session.cjs |
Reconciles DeepSeek failures during unified collection. |
actions/setup/js/parse_deepseek_log.test.cjs |
Adds timeout, wrapper-exit, attribution, and collection regressions. |
actions/setup/js/parse_deepseek_log.cjs |
Parses additional failures and reconciles exit codes. |
actions/setup/js/detect_agent_errors.test.cjs |
Tests live timeout classification and persistence. |
actions/setup/js/detect_agent_errors.cjs |
Adds DeepSeek diagnostics and metadata-only fallback errors. |
.changeset/fix-deepseek-failure-capture.md |
Records the patch-release change. |
Review effort: Balanced (auto)
| }); | ||
| writeSessionArtifact(path.join(rootDir, "agent-errors.jsonl"), execution ? [execution] : [], collectAddMaskedValues(logContent)); | ||
| if (deepSeek) execution = reconcileDeepSeekExecution(execution, diagnostics); | ||
| writeSessionArtifact(path.join(rootDir, "agent-errors.jsonl"), [...supplementalErrors, ...(execution ? [execution] : [])], collectAddMaskedValues(logContent)); |
| if (process.env.GH_AW_ENGINE_ID === "deepseek-harness" && parseDeepSeekLog(logContent).logEntries.some(event => event.type === "session.error" && event.data.code === "agentic_engine_timeout")) { | ||
| stdioResults.agenticEngineTimeout = true; |
| if (failureIndex < 0) { | ||
| if (shutdown < 0) return []; |
|
✅ Test Quality Sentinel completed test quality analysis. Test Quality SentinelOutcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
🧠 Matt Pocock Skills Reviewer has completed the skills-based review. ✅
|
|
✅ Ponytail Reviewer completed successfully!
|
|
✅ PR Code Quality Reviewer completed the code quality review.
|
|
✅ Design Decision Gate 🏗️ completed the design decision gate check. See the comment below for the result and any generated ADR draft. Design Decision Gate 🏗️Outcome: The agent completed and reported no action. A no-op output alone does not verify that all required work was completed or that publication was possible. Agent messages are omitted from this comment. The category uses structured run status, not interpretation of agent prose.
|
|
❌ PR Specification Reviewer was cancelled. Please review the logs for details.
|
Integrate merged Goose structured-result and Gemini run-info version fixes at main snapshot 1b6cb00 while preserving DeepSeek failure diagnostics. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
What this change does: It preserves DeepSeek failure diagnostics when the wrapper reports a successful exit. It also adds regression coverage for timeout and failure collection.
- The timeout test can use the existing log fixture and a zero exit artifact; its subprocesses only generate the hard-coded input already exercised by the collection assertions.
Verdict: Ship.
Lean: -16 lines possible.
Generated by ✂️ Ponytail Reviewer for #76104 · codex · routed: gpt61 medium · ~6 AIC · ⌖ ~1 AIC · ⊞ 181
Comment /ponytail to run again
| [ | ||
| 'const { spawnSync } = require("node:child_process");', | ||
| 'console.log("[deepseek-harness] configured provider=github model=auto");', | ||
| 'const child = spawnSync(process.execPath, ["-e", "setTimeout(() => {}, 60000)"], { timeout: 50 });', |
There was a problem hiding this comment.
This timeout only makes the test double print a hard-coded warning; it never exercises the AWF timeout path. Use the existing timedOut fixture plus that warning and a 0 exit artifact, and keep the collection assertions: this removes both subprocesses and their timing dependency without losing coverage of the code under test.
There was a problem hiding this comment.
I am not approving this change. The existing inline findings remain unresolved and are merge-blocking: (1) persistAgentExecution discards parsed DeepSeek session.error events when writing agent-errors.jsonl, so a canonical session containing only session.init still loses the timeout/error; (2) the DeepSeek live-detection branch is not emitted for generated workflows because the engine has no GetErrorDetectionScriptId; and (3) wrapper-exit fallback can misattribute or reject logs when ordinary output precedes a failure. The specialist pass was unavailable due to tool-permission rejection; this verdict is based on the prefetched diff and source inspection.
🔎 Code quality review by PR Code Quality Reviewer · copilot · gpt50 · ~3 AIC · ⌖ ~1 AIC · ⊞ 1.6K
Comment /review to run again
|
🚀 PR Specification Reviewer has started processing this pull request |
|
🔬 Test Quality Sentinel is analyzing test quality on this pull request... |
|
🔎 PR Code Quality Reviewer is reviewing code quality for this pull request... |
|
✂️ Ponytail Reviewer has started processing this pull request |
|
🔍 Design Decision Gate 🏗️ is checking for design decision records on this pull request... |
|
🧠 Matt Pocock Skills Reviewer is reviewing this pull request using Matt Pocock's engineering skills... |
Summary
Fix the DeepSeek Harness failure-capture boundary without inventing a native conversation or changing intentional RPC compaction.
session.errorevents.Evidence
Existing live smoke run 42540358186, revision
4cec482315dd38838403ac3fa83462cc5c669544, failed with[WARN] Agent command timed out after 15 minutes, stopping container...while the exit artifact recorded0. Before this fix the parser retained onlysession.initand the published unified session had empty execution diagnostics with exit zero. A constant local Node subprocess reproduces an actual timeout with a wrapper returning zero; the fixed detector and canonical/fallback unified sessions retain the timeout error and omit the unavailable exit code.The successful conformance run 41423156883 had no published unified session because conclusion was cancelled; this PR does not change that boundary. Raw evidence remains private and is not included in the patch.
Validation
make fmt-cjs && make lint-cjs: passed.make agent-report-progress: passed, including build, lint, schema freshness, type checking and 222 impacted JavaScript tests. No Go or workflow-source changes; the standard gate skipped corresponding impacted tests/recompilation. Two existing lint warnings remain; no lint errors.Scope and remaining coverage
No workflow dispatch, live inference, MCP startup, credentials changes, or new destinations. No native reasoning, tools, IDs, subagents, usage or final-answer telemetry is synthesized. New hosted execution behavior remains unverified; only existing Actions evidence and local doubles were used.
Authored Markdown changes only document failure coverage/unknown exit semantics and patch-release metadata; executable snippets, permissions, credential recipients, sandbox and firewall controls are unchanged.