Skip to content

feat(task-run): add bulk progress completion guard (#1041)#1116

Merged
shaun0927 merged 2 commits into
feat/1039-task-run-lifecyclefrom
feat/1041-bulk-progress-contract
May 13, 2026
Merged

feat(task-run): add bulk progress completion guard (#1041)#1116
shaun0927 merged 2 commits into
feat/1039-task-run-lifecyclefrom
feat/1041-bulk-progress-contract

Conversation

@shaun0927
Copy link
Copy Markdown
Owner

@shaun0927 shaun0927 commented May 12, 2026

Progress / Review status

Auto-refreshed 2026-05-13 — owner comments cleaned up to reduce review noise.

Field Value
Branch feat/1041-bulk-progress-contractfeat/1039-task-run-lifecycle
Draft no
CI
Mergeable ❌ CONFLICTING
Review decision
Codex (latest)
Other reviewers (latest)
Head 0cdbef8 — Guard repetitive tasks from premature completion
Commits 2

Owner comment cleanup: 0 issue + 0 inline review comments deleted. Outstanding feedback from automated/external reviewers above is unchanged.


Summary

  • Adds opt-in BulkProgressContract storage/helpers for repetitive item work.
  • Adds oc_bulk_progress_start/update/check tools for expected-count and unknown-total cursor workflows.
  • Links contracts to TaskRuns and makes oc_task_run_complete return a typed guard failure when linked bulk progress is incomplete, unless force:true and force_reason are supplied.

Closes #1041.
Stacked on #1083 / feat/1039-task-run-lifecycle; merge #1083 first.

Direction / duplicate check before implementation

Success criteria

  • Known-total contracts block completion until completed + failed >= expected_total.
  • min_completed blocks completion when successes are below threshold.
  • Unknown-total contracts require explicit stop_satisfied:true.
  • Failed items are retained separately from completed items.
  • Large completed/failed arrays are bounded with truncation metadata.
  • Normal TaskRuns without a contract are unchanged.

Verification performed

  • npm test -- --runTestsByPath tests/core/progress-contract/storage.test.ts tests/tools/bulk-progress-tools.test.ts tests/core/task-run/storage.test.ts tests/tools/task-run-tools.test.ts --runInBand
  • npm run build
  • npm run lint:changed

Real OpenChrome verification after merge

  1. Call oc_task_run_start for goal Visit three URLs and collect titles.
  2. Call oc_bulk_progress_start with returned run_id, expected_total: 3, item_key: "url", and stop_condition: "processed all input urls".
  3. Navigate/read only https://example.com, then call oc_bulk_progress_update with that URL in completed.
  4. Call oc_task_run_complete; verify it returns isError:true, error.code: "bulk_completion_guard_failed", completion_guard.missing_count: 2, and a warning hint suggesting oc_bulk_progress_update.
  5. Navigate/read the remaining two URLs and record them through oc_bulk_progress_update.
  6. Call oc_task_run_complete again; verify the TaskRun reaches COMPLETED.
  7. Repeat with unknown total: create a contract without expected_total, attempt completion before stop_satisfied:true, then verify completion succeeds only after updating stop_satisfied:true.
  8. Restart OpenChrome and call oc_task_run_get; verify cursor/completed/failed state persisted.

Out of scope

  • Automatic page item discovery.
  • LLM evaluation of item quality.
  • Replacing workflow stale circuit breakers or async task ledgers.
  • Default behavior changes for non-bulk TaskRuns.

Repository owner deleted a comment from gemini-code-assist Bot May 13, 2026
Repository owner deleted a comment from qodo-code-review Bot May 13, 2026
Repository owner deleted a comment from chatgpt-codex-connector Bot May 13, 2026
@shaun0927 shaun0927 force-pushed the feat/1039-task-run-lifecycle branch from eae252c to 4ba6bf6 Compare May 13, 2026 09:29
@shaun0927
Copy link
Copy Markdown
Owner Author

Merge rationale (stack consolidation)

Intent. Closes #1041 — adds opt-in BulkProgressContract storage + oc_bulk_progress_start/update/check tools for repetitive item work (expected-count and unknown-total cursor workflows), with a typed completion guard on oc_task_run_complete.

Why this is correct.

  • Opt-in contract; no behavior change unless callers attach a contract to a TaskRun.
  • oc_task_run_complete returns a typed guard failure when linked bulk progress is incomplete, unless force:true is paired with force_reason — explicit override path with audit reasoning preserved.
  • Two workflow shapes (expected-count and unknown-total cursor) cover both deterministic batch and crawler-style patterns.
  • Prerequisite feat(task-run): persist goal-level lifecycle state (#1039) #1083 (task-run lifecycle) is merged.
  • Scope contained: 26 files, +1155/-265. No Codex P0/P1/P2 outstanding.

CI. Targets the task-run lifecycle feature branch; CI workflow only runs on main/develop PRs.

@shaun0927
Copy link
Copy Markdown
Owner Author

Hit merge conflicts after #1110 merged into the same feat/1039-task-run-lifecycle base. The conflicts are stack-consolidation noise (both PRs touched task-run-complete helpers / checkpoint paths). Recommend a quick rebase:

git checkout feat/1041-bulk-progress-contract
git fetch origin && git rebase origin/feat/1039-task-run-lifecycle
git push --force-with-lease

Once the rebase lands, this PR will be ready to merge — the content review pass already cleared it (see the merge rationale comment above).

shaun0927 and others added 2 commits May 14, 2026 00:16
Add an opt-in bulk progress contract that records item progress, cursor state, stop-condition satisfaction, and machine-readable completion guard results. TaskRun completion now rejects incomplete linked contracts unless the caller forces completion with an explicit reason.

Constraint: Stack on #1039 TaskRun lifecycle and avoid duplicating async ledger, scheduler, dashboard, or progress notification work.
Rejected: Changing default completion behavior for all TaskRuns | short non-bulk tasks must remain unaffected.
Confidence: high
Scope-risk: moderate
Directive: Keep future workflow/crawl integrations opt-in by attaching contracts rather than embedding new schedulers.
Tested: npm test -- --runTestsByPath tests/core/progress-contract/storage.test.ts tests/tools/bulk-progress-tools.test.ts tests/core/task-run/storage.test.ts tests/tools/task-run-tools.test.ts --runInBand
Tested: npm run build
Tested: npm run lint:changed
Co-authored-by: OmX <omx@oh-my-codex.dev>
@shaun0927 shaun0927 force-pushed the feat/1041-bulk-progress-contract branch from 9269d64 to fb6cbcb Compare May 13, 2026 15:17
@shaun0927 shaun0927 merged commit ebb004c into feat/1039-task-run-lifecycle May 13, 2026
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