diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index fdf873f5..87a0e4fb 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -1,5 +1,19 @@ # Engineering Log +## 2026-07-30 (Workflow Failure-Event Test Timeout — Issue #1049) + +- Symptom: the full race gate reached a stored failed workflow state but timed + out after two seconds before its subscriber consumed `workflow.failed`. +- Cause: the fixture measured shared race-runner scheduling latency with an + undersized wall-clock deadline. +- Planned fix: retain the live event assertion behind a stopped ten-second + timer, consistent with the contention class recorded in #958. +- Verification contract: focused normal/race stress, workflows normal/race, + full regression, and GitHub required checks. +- Result: the focused test passed normal/race at `-count=100`, the complete + workflows package passed normal/race, and `./scripts/test-regression.sh` + passed with 85.6% total coverage and zero uncovered functions. + ## 2026-07-30 (Workflow Subscription Cancellation Test — Issue #1035) - Symptom: the full race gate failed in diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index 9c300e4a..cfbaa965 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -1,5 +1,18 @@ # Long-Term Thinking Log +## 2026-07-30 (Workflow Failure-Event Test Timeout — Issue #1049) + +- Command intent: clear the exact full-gate timeout blocking the verified + cron/callback merge chain. +- User intent: distinguish a missing event from a merely contended CI host + without weakening correctness. +- Success definition: still require live `workflow.failed`, preserve failure + details, clean up timers/subscribers, and pass focused through hosted gates. +- Guardrails: issue-first isolated worktree, test-only timer change, no + production workflow or deadline change. +- Outcome: the stopped ten-second timer preserves the exact live-event contract + and diagnostics while focused, package, race, and coverage gates are green. + ## 2026-07-30 (Workflow Subscription Cancellation Test — Issue #1035) - Command intent: clear the race-gate blocker exposed while verifying the diff --git a/docs/plans/2026-07-30-issue-1049-workflow-failure-timeout-impact-map.md b/docs/plans/2026-07-30-issue-1049-workflow-failure-timeout-impact-map.md new file mode 100644 index 00000000..f0e33806 --- /dev/null +++ b/docs/plans/2026-07-30-issue-1049-workflow-failure-timeout-impact-map.md @@ -0,0 +1,66 @@ +# Cross-Surface Impact Map: Issue #1049 Workflow Failure-Event Timeout + +## Task + +- Task / issue: stabilize the live workflow failure-event test, #1049. +- Plan: `2026-07-30-issue-1049-workflow-failure-timeout-plan.md`. +- Owner: Codex. +- Status: implemented and fully verified locally; hosted checks pending. + +## Current Ownership, Callers, and Data Flow + +- Entry: `TestEngineDefinitionSubscribeAndFailureEvents`. +- Production flow: Engine persists failed workflow state and publishes + `workflow.failed` to its subscribers. +- Test flow: subscribe, wait for stored terminal state, then drain the live + stream until the failure event. +- Search conclusion: the two-second timer is test-local; production API, + workflow, and provider deadlines are separate. + +## Config, API, CLI, and Tools + +- Config/env/defaults: none. +- API/CLI/wire/tool behavior: unchanged. +- Event contract: continue requiring exact `workflow.failed` delivery. + +## Persistence and Compatibility + +- Stores/schemas/migrations/caches: unchanged. +- Compatibility: no runtime change. + +## Lifecycle, Security, and Reliability + +- Concurrency: allow race-instrumented shared CI enough scheduling time. +- Cleanup: stop the timer and cancel the subscription. +- Auth/privacy/secrets: none. +- Failure diagnostics: retain the complete observed event list. + +## Product and Integration Surfaces + +- Harness/workflow runtime, API, TUI, web, macOS GUI, providers: no code change. +- Automation: full race and hosted gates become host-speed tolerant. +- UX/accessibility: none. + +## Deployment and Operations + +- Deployment/migration/flags: none. +- Rollback: revert if a genuinely absent failure event no longer fails. +- Observability/operator docs: none. + +## Regression Tests + +- Red: full race gate timed out after two seconds with the first two events. +- Green: focused normal/race `-count=100`. +- Controls: stored status and error remain failed; live stream must still + contain `workflow.failed`. +- Full: workflows normal/race and `./scripts/test-regression.sh`. + +## Documentation and Handoff + +- Update engineering log, long-term log, plan, impact map, and plans index. +- No public docs. + +## Warning Check + +- All runtime/product surfaces are explicitly unaffected because the repair is + confined to one test-owned timer. diff --git a/docs/plans/2026-07-30-issue-1049-workflow-failure-timeout-plan.md b/docs/plans/2026-07-30-issue-1049-workflow-failure-timeout-plan.md new file mode 100644 index 00000000..61354e00 --- /dev/null +++ b/docs/plans/2026-07-30-issue-1049-workflow-failure-timeout-plan.md @@ -0,0 +1,59 @@ +# Plan: Make the Workflow Failure-Event Test Contention-Tolerant + +## Context + +- Governing GitHub issue: #1049. +- Problem: `TestEngineDefinitionSubscribeAndFailureEvents` allows only two + wall-clock seconds for live `workflow.failed` delivery after the stored run + reaches failed. +- Red evidence: the full repository race gate timed out with + `workflow.started` and `workflow.step.started` already observed. +- Constraint: preserve the live-event assertion and production ordering. + +## Scope + +- In scope: a leak-safe, ten-second test deadline for this event assertion. +- Out of scope: workflow runtime ordering, event fanout, storage, production + timeouts, and unrelated timing tests. + +## Documentation Contract + +- Feature status: test-only bug repair. +- Public docs affected: none. +- Evidence: engineering and long-term logs, plan, impact map, plans index. + +## Test Plan + +- Red: retain the exact full-race timeout as regression evidence. +- Green: focused normal and race tests at `-count=100`. +- Package: `internal/workflows` normal and race. +- Full: `./scripts/test-regression.sh` and GitHub required checks. + +## Cross-Surface Impact Map + +- See `2026-07-30-issue-1049-workflow-failure-timeout-impact-map.md`. + +## Implementation Checklist + +- [x] Create contract-complete bug #1049. +- [x] Capture the full-race failure and architecture search evidence. +- [x] Write the plan and impact map before code. +- [x] Replace the short one-shot deadline with a stopped ten-second timer. +- [x] Run focused stress, package, and full local gates. +- [ ] Pass hosted required checks. +- [ ] Merge through a closing PR. + +## Verification + +- Focused normal and race tests passed at `-count=100`. +- The complete `internal/workflows` package passed normal and race tests. +- `./scripts/test-regression.sh` passed normal, race, and the 85.6% coverage + gate with zero uncovered functions. + +## Risks and Mitigations + +- Risk: a longer wait could conceal a missing event. +- Mitigation: the test still fails with the complete event history after a + bounded deadline and still requires `workflow.failed`. +- Risk: `time.After` retains its timer until firing on the fast path. +- Mitigation: use `time.NewTimer` with `defer Stop`. diff --git a/docs/plans/INDEX.md b/docs/plans/INDEX.md index 33a86e1e..ec848957 100644 --- a/docs/plans/INDEX.md +++ b/docs/plans/INDEX.md @@ -1,5 +1,7 @@ # Plans Index +- `2026-07-30-issue-1049-workflow-failure-timeout-plan.md` — Issue #1049 planned contention-tolerant workflow failure-event regression wait. +- `2026-07-30-issue-1049-workflow-failure-timeout-impact-map.md` — Cross-surface impact map for Issue #1049. - `2026-07-30-issue-1035-workflow-subscription-cancel-plan.md` — Issue #1035 planned deterministic workflow subscription-cancellation regression repair. - `2026-07-30-issue-1035-workflow-subscription-cancel-impact-map.md` — Cross-surface impact map for Issue #1035. - `2026-07-30-issue-1026-feedback-direct-publish-plan.md` — Issue #1026 attached-image, publish-by-default GitHub feedback intake (implemented and verified; promotion pending). diff --git a/internal/workflows/engine_test.go b/internal/workflows/engine_test.go index a822ff3b..704cfd3c 100644 --- a/internal/workflows/engine_test.go +++ b/internal/workflows/engine_test.go @@ -213,12 +213,13 @@ func TestEngineDefinitionSubscribeAndFailureEvents(t *testing.T) { } events := append([]Event(nil), history...) - deadline := time.After(2 * time.Second) + deadline := time.NewTimer(10 * time.Second) + defer deadline.Stop() for !hasWorkflowEvent(events, "workflow.failed") { select { case event := <-live: events = append(events, event) - case <-deadline: + case <-deadline.C: t.Fatalf("timed out waiting for workflow.failed event; events=%+v", events) } }