feat(gate): execute repair and retry-posts paths#49
Conversation
FindingsMajor: Major: Repair/retry mutate local ledger state before calling Nit: |
|
No findings on the targeted re-review. The prior issues are addressed: repair/retry now perform the base-moved precheck before limiter validation, outbox errors after mutation return a stable execution |
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: f1a54b7
Summary
| Reviewer | Findings |
|---|---|
| harness-engineering:harness-architecture-reviewer | 1 |
| harness-engineering:harness-self-documenting-code-reviewer | 1 |
harness-engineering:harness-architecture-reviewer (1 findings)
internal/gateio/gateio.go:477
In executeRepair, if allocateRepair succeeds but insertRepairSubmitReview subsequently fails, an orphaned run is written to the ledger with no planned actions. The guard at the top of executeRepair checks GetRun and returns 'repair run already exists' on all future attempts, permanently blocking automatic repair for this PR until the ledger row is manually removed. Either wrap allocateRepair+insertRepairSubmitReview in a single transaction, or delete the allocated run on insertRepairSubmitReview failure before returning the error.
harness-engineering:harness-self-documenting-code-reviewer (1 findings)
💡 Suggestion - internal/gateio/gateio.go:601
resetRequiredFailedTerminalActions clears Error, FailureClass, PostedAt, and UpstreamID but deliberately preserves Attempts and AttemptedAt. The name 'reset' implies a full state reset; the selective preservation is only discoverable via the test. A name like resetRequiredFailedTerminalActionsForRetry, or a brief inline comment naming the invariant, would make the behavior explicit at the call site.
3 info-level observations excluded. Run with --verbose to include.
2 PR discussion threads considered.
Completed in 2m 22s | $1.47 | sonnet | daemon 0.2.121 | Glorfindel
| Field | Value |
|---|---|
| Model | sonnet |
| Reviewers | hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor |
| Engine | claude · sonnet |
| Reviewed by | pr-review-daemon · monit-pr-reviewer |
| Duration | 2m 22s wall · 8m 42s compute (Reviewers: 1m 46s · Synthesis: 33s) |
| Cost | $1.47 |
| Tokens | 368.7k in / 44.6k out |
| Turns | 11 |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost |
|---|---|---|---|---|---|---|
| hybrid-synthesis | sonnet | 31.7k | 1.7k | 0 | 31.7k (1h) | $0.15 |
| harness-engineering:harness-architecture-reviewer | sonnet | 70.5k | 14.0k | 29.7k | 40.8k (1h) | $0.39 |
| harness-engineering:harness-enforcement-reviewer | sonnet | 105.4k | 8.7k | 67.5k | 37.9k (1h) | $0.31 |
| harness-engineering:harness-knowledge-reviewer | sonnet | 65.9k | 7.0k | 29.7k | 36.2k (1h) | $0.27 |
| harness-engineering:harness-self-documenting-code-reviewer | sonnet | 65.1k | 6.3k | 29.7k | 35.4k (1h) | $0.26 |
| security:security-code-auditor | haiku | 30.1k | 6.9k | 0 | 30.1k (1h) | $0.09 |
Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.
| } | ||
|
|
||
| desired, event, err := repairOutcomeEvent(decision.Outcome) | ||
| if err != nil { |
There was a problem hiding this comment.
🟡 Medium (harness-engineering:harness-architecture-reviewer): In executeRepair, if allocateRepair succeeds but insertRepairSubmitReview subsequently fails, an orphaned run is written to the ledger with no planned actions. The guard at the top of executeRepair checks GetRun and returns 'repair run already exists' on all future attempts, permanently blocking automatic repair for this PR until the ledger row is manually removed. Either wrap allocateRepair+insertRepairSubmitReview in a single transaction, or delete the allocated run on insertRepairSubmitReview failure before returning the error.
Reply to this thread when addressed.
There was a problem hiding this comment.
Fixed - delete recovery run if repair action insert fails.
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
🔵 Low (harness-engineering:harness-self-documenting-code-reviewer): resetRequiredFailedTerminalActions clears Error, FailureClass, PostedAt, and UpstreamID but deliberately preserves Attempts and AttemptedAt. The name 'reset' implies a full state reset; the selective preservation is only discoverable via the test. A name like resetRequiredFailedTerminalActionsForRetry, or a brief inline comment naming the invariant, would make the behavior explicit at the call site.
Reply to this thread when addressed.
There was a problem hiding this comment.
Fixed - renamed retry reset helper to make attempt preservation explicit.
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: 67cf79b | Previous: f1a54b7 (incremental)
Summary
No issues found.
4 PR discussion threads considered.
Completed in 3m 07s | $0.92 | sonnet | daemon 0.2.121 | Glorfindel
| Field | Value |
|---|---|
| Model | sonnet |
| Mode | Re-review · Cycle 2 · Session resumed |
| Reviewers | hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer, harness-engineering:harness-self-documenting-code-reviewer, security:security-code-auditor |
| Engine | claude · sonnet |
| Reviewed by | pr-review-daemon · monit-pr-reviewer |
| Duration | 3m 07s wall · 4m 31s compute (Reviewers: 2m 44s · Synthesis: 12s) |
| Cost | $0.92 |
| Tokens | 356.2k in / 7.2k out |
| Turns | 6 |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost |
|---|---|---|---|---|---|---|
| hybrid-synthesis | sonnet | 36.3k | 581 | 18.6k | 17.6k (1h) | $0.08 |
| harness-engineering:harness-architecture-reviewer | sonnet | 64.0k | 85 | 0 | 64.0k (1h) | $0.24 |
| harness-engineering:harness-enforcement-reviewer | sonnet | 58.7k | 80 | 0 | 58.7k (1h) | $0.22 |
| harness-engineering:harness-knowledge-reviewer | sonnet | 57.0k | 129 | 0 | 57.0k (1h) | $0.22 |
| harness-engineering:harness-self-documenting-code-reviewer | sonnet | 56.3k | 222 | 56.3k | 0 | $0.02 |
| security:security-code-auditor | haiku | 57.6k | 5.9k | 57.6k | 0 | $0.04 |
| discussion-summarizer | — | 26.3k | 213 | 0 | 26.3k (1h) | $0.10 |
Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.
Summary
Verification
Closes #23