Repository navigation
fix(flare-workflow): delete_state deletes workflow_runs by wrong column - #576
Merged
Merged
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 24 minutes Limit details: You’ve used all 2 included reviews currently available. Your 59 included PR review attempts over the past 7 days set your current allowance at 2 reviews per hour. Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?Wait for the limit to reset, then comment An organization admin can change what happens after included review limits in Billing. How do review limits work?CodeRabbit enforces per-developer PR review limits within each organization. For paid Pro and Pro+ reviews, CodeRabbit uses a developer's included PR review attempts over the past 7 days to set the current hourly allowance. At typical activity levels, the full plan allowance applies. Higher sustained activity can lower the allowance until earlier attempts leave the 7-day window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Comment |
getappz
changed the title
delete_state looped over [workflow_runs, journal, step_state, run_vars] assuming all four key on run_id. workflow_runs is keyed on id, not run_id, so every DELETE against it failed with 'no such column: run_id' — this fired on every claim-time resume/cleanup call and blocked all job dispatch for ~33h. Agentflare-Agent: claude-code Agentflare-Branch: task/163-delete-state-column-fix Agentflare-Item: 163
getappz
force-pushed
the
task/163-delete-state-column-fix
branch
from
August 21, 2026 07:37
7b5f5de to
d1231b5
Compare
getappz
added a commit
that referenced
this pull request
Aug 24, 2026
…ck + workflow-store smoke test) (#596) * feat: add dispatch preflight/init validation step (item #164) Duplicate-work detection (pre-claim): before dispatching an item, find_duplicate_pr searches GitHub for a PR already carrying the item's 'for item #N' marker. A merged match self-heals the item to completed and cleans up its worktree instead of letting a redispatch re-do already-merged work (the near-miss from items #122/#156, where tracked state fell out of sync with a merged PR). An open match skips dispatch and flags for human review instead of racing a second PR. Workflow-store smoke test: flare_workflow::smoke_test does a real save/delete/load round-trip against the actual on-disk SqliteStore schema. Wired into daemon boot (dashboard::server::run) and into dev_install::run via a new hidden 'daemon workflow-store-smoke-test' subcommand, run against the freshly built binary right after a binary swap -- the exact trigger (item #576) that let a broken delete_state go undetected for ~33h. On failure, refuses dispatch instead of letting every claimed item retry-and-fail silently into a stuck state. src/cli/work.rs is frozen at the LOC gate's 2100-line limit, so the new duplicate-PR logic and its tests live in their own included files (work_duplicate_pr.rs, work_duplicate_pr_tests.rs) rather than growing work.rs itself. Agentflare-Agent: claude-code Agentflare-Branch: task/164-dispatch-preflight-validation Agentflare-Item: 164 * fix review findings: duplicate-PR selection, claim/completion error handling, smoke-test ordering Addresses 6 CodeRabbit findings on PR #596, all independently verified against the actual code before fixing: - pick_duplicate_pr: a closed-but-unmerged PR was treated the same as a genuinely open one, permanently blocking a legitimate redispatch. Now filters to merged-or-open only. - handle_duplicate_pr: a failed/unconfirmed mark_completed was silently reported as success (comment posted, claim released, exit 0). Now returns a retryable failure and leaves the claim armed instead. - handle_duplicate_pr: item_release's result was discarded and claim_guard was disarmed unconditionally, so a failed release left nothing to retry it. Now only disarms on confirmed release success. - dashboard::server::run: engine().recover() ran before the workflow-store smoke test, so a broken store could still be touched by recovery before validation. Reordered so recovery is gated behind the smoke test too. - dev_install::verify_workflow_store: used a blocking Command::status() with no timeout, unlike its sibling verify_runs. Now uses the same spawn/try_wait/kill-with-deadline pattern. - github::pulls::find_by_item_marker: the search API call was unpaginated (GitHub's default page size is 30), so a matching PR beyond the first page would silently not be found. Now uses the existing get_paginated helper. Also fixes the schema-mismatch regression test in sqlite_store.rs: it put the intentional id/run_id mismatch on workflow_runs itself, so write_state failed on save and delete_state's own failure path was never actually exercised. Moved the mismatch to journal (write_state never touches it for an empty smoke-test state) and strengthened the assertion to confirm the failure comes from there. New tests: pick_duplicate_pr_ignores_a_closed_unmerged_pr, pick_duplicate_pr_selects_a_genuinely_open_pr_when_no_merged_match_exists. Verified: cargo build, cargo test --bin agentflare (1548/1548 passing), cargo test -p flare-workflow (all passing), cargo fmt --check, cargo clippy (CI's exact flags), scripts/loc-gate.sh. Agentflare-Agent: claude-code Agentflare-Branch: task/164-dispatch-preflight-validation Agentflare-Item: 164 * fix: gate work dispatch on successful pipeline registration/recovery engine().register_workflow()/recover() failures were only logged -- the worker pool and both supervisor ticks still started regardless, undermining the smoke-test gate's own fail-closed contract one level up: dispatch could start against pipeline state that's already known to be broken. Also fixes a compile error from the prior commit: recover() returns Result<Vec<WorkflowRunId>, WorkflowError>, not Result<(), _>. Agentflare-Agent: claude-code Agentflare-Branch: task/164-dispatch-preflight-validation Agentflare-Item: 164 --------- Co-authored-by: shiva <shiva@gosysinfo.tech>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
delete_state()incrates/flare-workflow/src/sqlite_store.rslooped over["workflow_runs", "journal", "step_state", "run_vars"]issuingDELETE FROM {table} WHERE run_id = ?1for each.workflow_runsis keyed onid, notrun_id— the other three are per-run child tables keyed onrun_id.Why
This fired unconditionally on every claim-time cleanup/resume call, so every
agentflare workdispatch failed immediately withno such column: run_id in DELETE FROM workflow_runs WHERE run_id = ?1. Confirmed via job logs: 19+ consecutive failures across 5 items, then 3 more failures on this very item (#163) once created — the daemon's entire dispatch pipeline had been dead for ~33h as a result, 0 completions in the trailing 24h.Fix
Per-table id column instead of one hardcoded
run_idfor all four tables.Test
Added
delete_removes_state_from_all_tables— round-trips a state throughsave/delete/load, assertingloadnow returnsNotFound. Fails against the old code (bad column), passes with the fix.Follow-up
Items #122, #156, #159, #161, #162 were stuck retrying against this same bug and exhausted their retries — redispatch them once this merges.
Opened by
claude-codeon flared:51bb8de6c33b for item #163 via agentflare.