Skip to content

fix(cli): exit non-zero when a resumed task does not complete (#1005 follow-up) - #1021

Merged
frankbria merged 1 commit into
mainfrom
fix/1005-resume-exit-code
Aug 1, 2026
Merged

fix(cli): exit non-zero when a resumed task does not complete (#1005 follow-up)#1021
frankbria merged 1 commit into
mainfrom
fix/1005-resume-exit-code

Conversation

@frankbria

Copy link
Copy Markdown
Owner

Post-merge review finding on #1020.

cf work resume printed "Task execution failed" and exited 0, so cf work resume <id> && next_step proceeded as if the run had succeeded. work start exits 1 for anything short of COMPLETED (app.py:2610), and #1020's body promised parity with it.

Matched to work start exactly — BLOCKED and FAILED both exit 1. The reviewer suggested gating on FAILED alone, on the grounds that blocking again is a normal outcome of a resume. That is true, but it is equally true of work start, which exits 1 anyway; one predictable rule across both commands beats two that differ by a case.

Parametrized test pins all three outcomes (COMPLETED→0, FAILED→1, BLOCKED→1).

tests/ui/test_resume_starts_worker_1005.py: 13 passed, ruff clean.

…follow-up)

Post-merge review finding on #1020. `cf work resume` printed "Task execution
failed" and exited 0, so `cf work resume <id> && next_step` proceeded as if
the run had succeeded. `work start` exits 1 for anything short of COMPLETED.

Matched to start exactly — BLOCKED and FAILED both exit 1. The reviewer
suggested FAILED only, on the grounds that blocking again is a normal outcome
of a resume; that is true, but it is equally true of `work start`, and one
predictable rule across both commands beats two that differ by a case.

Parametrized test pins all three outcomes.
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @frankbria's task in 3m 32s —— View job


Precision bug review — complete

Hunted for concrete defects only (logic errors, null/error paths, security, data loss, race conditions, breaking changes, resource leaks).

  • Gather PR diff + existing comments
  • Read app.py resume + work start context
  • Read the test file
  • Confirm/verify findings against real code
  • Post summary

✅ No concrete defects found. Verdict + reasoning posted as a summary comment below.

Scope note: the BLOCKED→exit-1 design choice (vs. FAILED-only) is a deliberate parity decision with work start and was explicitly defended in the PR body — that's an architecture/design call, out of scope for this defect-only review.

@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

✅ GLM precision review: no defects found.

Reviewed the production hunk in codeframe/cli/app.py (the new raise typer.Exit(1) for BLOCKED/FAILED) against its work start counterpart and the surrounding control flow:

  • The gate state.status in (AgentStatus.BLOCKED, AgentStatus.FAILED) is byte-for-byte identical to work start at app.py:2610 — no inverted/wrong condition.
  • state is guaranteed non-None here (already dereferenced at line 2718), so no None-deref on the new line.
  • The raise typer.Exit(1) sits after the inner try/except Exception and inside the outer try, whose handlers are FileNotFoundError/ValueErrortyper.Exit is neither, so it propagates to the shell correctly (no silent swallow).
  • --no-execute branch is untouched (still exits 0); the existing status=None recorder sentinel also falls through to 0, so no regression to test_cli_resume_executes.
  • Test helper's getattr(AgentStatus, status) is valid for all three parametrized members.

View job run

@frankbria
frankbria merged commit d5ecc95 into main Aug 1, 2026
12 of 13 checks passed
@frankbria
frankbria deleted the fix/1005-resume-exit-code branch August 1, 2026 09:41
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