Skip to content

fix(harness): verify managed OpenCode cancellation - #2

Closed
tbrandenburg wants to merge 1 commit into
philo-x:codex/opencode-v2-compatibilityfrom
tbrandenburg:fix/3448-opencode-cancellation
Closed

tbrandenburg wants to merge 1 commit into
philo-x:codex/opencode-v2-compatibilityfrom
tbrandenburg:fix/3448-opencode-cancellation

Conversation

@tbrandenburg

Copy link
Copy Markdown

Summary

Verify managed OpenCode cancellation separately from the local DAG outcome. Timeout and Stop retain interruption/verification failures, reconcile the original owning process within a shared bounded budget, and fence unresolved execution instead of silently retrying or replacing it.

This is a separate stacked contribution against codex/opencode-v2-compatibility at 85e9c8b, related to upstream #3367. It does not incorporate the three independent fixes in philo-x/dagu#1.

Changes

  • Validate the V2 interrupt response and explicitly send resume=false; an acknowledgment alone cannot establish settlement.
  • Use one three-second, cancellation-independent budget for same-session, original-process reconciliation, including delayed settlement and bounded Stop results.
  • Preserve cancellation, interruption, verification, and HTTP response-body cleanup errors.
  • Expose cleanup status, acknowledgment, interruption, redacted diagnostics, and owner identity through persisted metadata, lifecycle events, and schema-backed API fields. No new DAG run state.
  • Fence retry, replacement generations, execution, and fallback while cleanup is unresolved, including legacy accepted terminal records without cleanup evidence.
  • Reject restart before changing transcript/generation/status or dispatching work when the cleanup fence applies.
  • Handle accepted/resumed monitoring failures, waiting-session Stop, final-progress/Stop races, repeated Stops, and late persistence snapshots without losing the owner fence.
  • Accept the pinned V1 retry-status variant without treating malformed target status as idle.
  • Add failure-first regressions and real OpenCode V2 delayed-settlement coverage. Reuse the existing SQLite dependency; no dependency changes.

The generated Go diff includes the updated model and the compressed embedded OpenAPI document. Generated Go and TypeScript outputs were regenerated rather than hand-edited.

Related Issues

Closes dagucloud#3448

The author can merge/cherry-pick this independently; upstream incorporation can follow dagucloud#3367. The upstream closing reference is intended for that eventual integration.

Validation

Validated with Go 1.27.3 and the repository-pinned OpenCode 2.1.4 and 1.18.11 executables, using an isolated memory-backed Docker environment because the shared host filesystem was nearly full.

  • Full harness race suite with the pinned V2 executable enabled: PASS, including the real V2 tests (final correction pass: 53.645s).
  • Full IR race suite: PASS (final legacy-fence pass: 1.887s).
  • Full runtime race suite: PASS (23.914s); final focused cleanup/Stop/session race tests also passed (1.731s).
  • Owning API restart/resume/action/session tests with race detection: PASS (1.602s).
  • OpenCode-host race suite with the pinned V1 executable: PASS (6.657s).
  • Binary-level harness conformance: PASS (4.246s), against the rebuilt contribution binary.
  • Original seven adversarial reproductions: 7/7 passed after correction. The subsequent waiting-final-callback race was confirmed failing before correction and passed 200 race-enabled barrier subcases.
  • Response-body cleanup fault tests: failure-first, then PASS with race detection over 20 repetitions.
  • OpenAPI validation and repeated generated-output comparison: PASS. TypeScript generation matched exactly; Go generation was rechecked after the final code changes.
  • Changed Go formatting and staged diff checks: PASS. make fmt completed its Go fix/format phases, but its standard linter phase encountered a package-loading error.
  • Strengthened, validation-only lint configuration with directory-boundary exclusions and a changes-from-base ratchet: 0 new issues in the four affected production scopes. Harness/runtime checks included their tests; IR/API checks used production-only mode because their test-package lint loading failed. The repository lint configuration was not changed.
  • Frontend typecheck and production build: PASS (build emitted two asset-size warnings). Existing agent-session tests: 14 passed across 3 files. The validated snapshot matched 721 frontend source files with zero differences.

Successful focused commands (isolated-runner prefixes omitted):

go test -race -count=1 -timeout=180s ./internal/ir
go test -race -count=1 -timeout=180s ./internal/runtime -run 'Cleanup|Stop|AgentSession'
go test -race -count=1 -timeout=180s ./internal/service/frontend/api/v1 -run 'AgentSession|AgentRestart|AgentResume|AgentAction'
go test -race -count=1 -timeout=180s ./internal/runtime/builtin/harness
DAGU_BIN="$PWD/.local/bin/dagu" go test -p 1 -race -count=1 -timeout=120s -ldflags='-s -w' ./conformance/spec067_harness
pnpm typecheck --noEmit
pnpm build
pnpm exec vitest run src/features/dags/components/agent-session/__tests__ --maxWorkers=1

Real E2E reproduction

Install the pinned executable using the existing .github/opencode manifest, then from the repository root:

DAGU_TEST_OPENCODE_V2_EXECUTABLE="$PWD/.github/opencode/node_modules/@opencode/cli-linux-x64-baseline/bin/opencode" \
go test -p 1 -race -count=3 -timeout=90s \
  ./internal/runtime/builtin/harness -run '^TestRealV2Cleanup$' -v

The test uses the real OpenCode binary and a controlled provider input fixture. A pass-through HTTP spy leaves responses unchanged. A writer transaction on the existing, canonicalized test-owned WAL database is released 500ms after the actual interrupt acknowledgment, deliberately delaying real terminal settlement. Three final coordinator runs waited another 581–602ms after acknowledgment, then verified the same owner PID was idle and shell effects remained unchanged for a further 300ms. An HTTP acknowledgment alone cannot pass this test.

Limits and follow-ups

  • Verification covers the original server process's foreground execution, not detached tools or global inactivity across other hosts.
  • Fencing is intentionally conservative; no new operator override/recovery endpoint is included. Physically stalled storage cannot guarantee a completed disk write; pending/unverified runtime snapshots remain fenced.
  • The full frontend unit suite exceeded a 15-minute constrained-runner budget without a final summary. The full API suite exceeded an eight-minute budget while many unrelated parallel tests were queued; the issue-owning API tests passed. No full repository-wide suite result is claimed.
  • The standard linter's ui/gen path exclusions can conceal builtin/agent findings. New contribution findings exposed by the stronger check were corrected; baseline findings were not opportunistically refactored.
  • Separate follow-ups created: dagucloud/dagu#3458 (frontend runtime/timer diagnostics) and dagucloud/dagu#3465 (linter exclusion boundaries).
  • Cancellation/retry safety is high-risk behavior; leave merging to the maintainer/review process.

Checklist

  • Focused code follows the project style guidelines; broader lint limits documented
  • Self-review and independent adversarial review performed
  • Tests added/updated, including failure-first and real delayed settlement
  • Documentation updated
  • Focused changes tested locally; broader suite limits documented

@tbrandenburg

Copy link
Copy Markdown
Author

Sorry for opening this PR here by mistake and creating unnecessary review work. This follow-up should have been evaluated against dagucloud/dagu, and I failed to recheck the upstream status before publishing it. Dagu maintainer Yota Hamada has since addressed my issue dagucloud#3448 in the merged PR dagucloud#3456, with an explicitly narrower scope. I am closing this provisional PR rather than asking you to merge a competing implementation. Thank you for your OpenCode V2 compatibility work, and apologies for the confusion. This closure concerns PR #2 only; the separate three-fix contribution in PR #1 is unaffected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant