Fix comment handling after the first one

[max-tet]

Look at this comment: https://github.com/FreeshardBase/freeshard-controller/issues/75#issuecomment-4121173013
It was never included in the preliminary plan but it should have. Only the first comment was included.

In general, every visible comment should be included in the prelim or the thorough plan, none should be ignored.

[ClaydeCode]

Preliminary Plan

Overview

The root cause is in run_update in tasks/plan.py. When a plan update fails due to UsageLimitError, InvocationTimeoutError, or a parse error, the code sets status to INTERRUPTED with interrupted_phase=PRELIMINARY_PLANNING (or PLANNING). The orchestrator's retry handler maps INTERRUPTED back to run_preliminary — a full re-plan — instead of retrying the update. Since last_seen_comment_id was never advanced (the early return happens before that line), the new comment is still detectable on the next tick, but the wrong task runs.

The fix is to remove the two update_issue_state(... INTERRUPTED ...) calls from run_update (lines 288–292 and 304–308 in tasks/plan.py). On failure, run_update should just return after logging and accumulating cost. The issue stays in AWAITING_PRELIMINARY_APPROVAL or AWAITING_PLAN_APPROVAL, the next tick re-detects the same new comments, and run_update is retried. This is safe because:

• UsageLimitError: the orchestrator's is_claude_available() check skips the cycle anyway
• InvocationTimeoutError: transient, retry is appropriate
• Parse errors: if persistent, no worse than a full re-plan failing too

No changes needed to safety.py or state.py.

---

This looks like a small issue. React with 👍 to approve this preliminary plan and proceed directly to implementation.

[max-tet]

Your root cause analysis is incorrect. Comment IDs on GitHub are monotonically increasing. You can verify this by looking at the actual comment IDs on FreeshardBase/freeshard-controller#75 — every ID is strictly greater than the previous one.

The real bug is in run_update in tasks/plan.py. When a plan update fails (timeout, usage limit, or parse error), the status is set to INTERRUPTED with interrupted_phase=PRELIMINARY_PLANNING (or PLANNING). The retry handler in the orchestrator maps that to run_preliminary — a full re-plan — instead of retrying the update. Meanwhile, last_seen_comment_id is not advanced (the early return happens before that line), so the new comment is still detectable.

The fix: in run_update, on failure, don't change the status. Just return. The issue stays in AWAITING_*_APPROVAL, and the next tick naturally re-detects the same new comments and retries run_update. This is safe because:

• UsageLimitError: the orchestrator's is_claude_available() check at tick start skips the cycle anyway
• InvocationTimeoutError: transient, retry makes sense
• Parse errors: if persistent, no worse than a full re-plan also failing

Concretely: remove the two update_issue_state(... INTERRUPTED ...) calls in run_update (the ones in the except (UsageLimitError, InvocationTimeoutError) block and the except ValueError block). Keep the accumulate_cost, logging, and return.

Please discard the seen-set approach — it solves a problem that doesn't exist here.

[ClaydeCode]

Plan updated. Root cause analysis was wrong. @max-tet correctly points out that GitHub comment IDs are monotonically increasing, so the watermark approach is not the problem. The actual bug is in run_update in tasks/plan.py: when a plan update fails (timeout, usage limit, or parse error), it sets status to INTERRUPTED with interrupted_phase=PRELIMINARY_PLANNING or PLANNING, which causes the orchestrator to retry with run_preliminary (a full re-plan) instead of retrying the update. The fix is much simpler: remove the update_issue_state(INTERRUPTED) calls from the two exception handlers in run_update so the issue stays in AWAITING_*_APPROVAL and naturally retries the update on the next tick. Discarding the seen-set approach.

[ClaydeCode]

Implementation complete — PR opened: #55