Skip to content

claude-ops/restart-consumer: six post-merge review findings on #1720 never reached triage #1759

Description

@kyle-sexton

This was generated by AI while auditing what the #1648#1658 batch merge carried onto main.

Context

chatgpt-codex-connector posted six review findings on #1720 at 2026-07-29T19:24:03Z46
seconds after the PR merged
at 19:23:17Z. They were never seen by a human or an agent, and the
threads are still unresolved on a merged PR, where nothing surfaces them.

This is not a ruleset bypass: the base ruleset's required_review_thread_resolution was satisfied
at merge time because the threads did not yet exist. The gap is that a post-merge review has no
landing place.

All six are in plugins/claude-ops/skills/lanes/scripts/restart-consumer.sh, shipped in #1720
(claude-ops 0.22.0). None is addressed on main.

Reviewer severities are preserved as given. They have not been independently verified — that is part
of this item's work.

Findings

  • (P1) Distinguish lock-storage errors from held locks:493. An ignored mkdir -p
    failure falls through to the same branch as an existing lock; the absent stamp reads as zero and
    main reports lock-held with exit 0. An unattended consumer with a mistyped path, a permissions
    problem, or an unavailable volume therefore never processes a lane while Task Scheduler records
    successful ticks
    . Return a prerequisite/runtime error when the lock parent or stamp cannot be
    created.
  • (P2) Do not reclaim a lock solely by age:500. A legitimate run lasting over an hour
    has its live lock removed, letting another scheduled run enter the relaunch span concurrently. The
    comment's assumption that confirmation retries bound the run is false, because lane-launcher.sh
    performs an unbounded git pull --ff-only and claude plugin marketplace update before launch.
    Record and verify an owner PID, or maintain a heartbeat.
  • (P2) Propagate offline telemetry parse failures:624. When --telemetry-json is
    missing, unreadable, malformed, or wrongly shaped, jq fails but an unconditional return 0
    converts that into a successful empty read; the lane reports no-state and exits 0. The network
    read now propagates failures, but the offline branch still overwrites jq's status.
  • (P2) Fail closed when the attempt ledger cannot be written:717. If the ledger becomes
    unwritable or the filesystem fills after the lock is acquired, an attempted relaunch is reduced to
    a warning. Those attempts never enter the breaker query, so a launcher that keeps failing is
    retried on every polling tick without ever reaching --max-restarts.
  • (P2) Recheck liveness before invoking unconditional restart:819. A lane starting
    after the session snapshot was loaded but before the predicate runs still reads as stopped, so the
    consumer calls lane-launcher.sh restart — which stops the now-running lane before relaunching.
    The race interrupts a healthy session despite the documented "not currently running" predicate.
  • (P2) Preserve scheduler options in emitted commands:954. print-schedule drops a
    non-default --config or --target-repo from the generated task; the logon, cron, and offline
    forms do the same. The scheduled invocation then falls back to <repo>/.work/lanes.json and the
    checkout's repository, so it can fail at startup or drive a different lane configuration entirely.

Already verified — not part of this item

The handoff-tracked defects D4–D8 are addressed. restart-consumer.test.sh now passes 95
cases, 0 failures
(up from 53 pre-fix), and two of them are deliberate-decision guards rather than
gaps: case 93 asserts the consumer and the morning brief carry the same literal (D5's duplication
is intentional and gate-tested), and case 95 asserts print-schedule's placeholder still says what
to substitute (D7 is intentional). api-error exists as a status distinct from no-request (D8).

Acceptance criteria

  • Each of the six findings is independently verified as real or refuted, with the refutation
    recorded — reviewer severity is not taken on trust.
  • Every confirmed finding is fixed with a regression case in restart-consumer.test.sh, or
    consciously deferred with the reason recorded at the site.
  • The six threads on feat(claude-ops): consume lane restart-requests via scheduled headless reader #1720 are resolved, so the merged PR stops carrying open findings.

Related

Refs #1720
Refs #1653

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions