Skip to content

Revise PR #323/#317: the branch was pushed at the already-blocked commit, so none of the five durable-cron blockers were touched - #337

Merged
jaylfc merged 1 commit into
masterfrom
exec/tsk-y62mpe
Aug 18, 2026
Merged

Revise PR #323/#317: the branch was pushed at the already-blocked commit, so none of the five durable-cron blockers were touched#337
jaylfc merged 1 commit into
masterfrom
exec/tsk-y62mpe

Conversation

@jaylfc

@jaylfc jaylfc commented Aug 18, 2026

Copy link
Copy Markdown
Owner

CARD TITLE (intent, not commit subject): Revise PR #323/#317: the branch was pushed at the already-blocked commit, so none of the five durable-cron blockers were touched

Autonomous build of board card tsk-y62mpe.

Files:
taosmd/remote.py | 5 +
taosmd/service.py | 65 +-
tests/test_a2a_channels.py | 65 ++
tests/test_measure_upstream_propagation.py | 347 ++++++++++
tests/test_normalise_handle_gate.py | 323 ++++++++-
tests/test_resume_arm_time.py | 301 +++++++++
tests/test_witness_gate.py | 54 +-
25 files changed, 2523 insertions(+), 43 deletions(-)

@qodo-code-review

Copy link
Copy Markdown

ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing

@coderabbitai

coderabbitai Bot commented Aug 18, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jaylfc, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 56 minutes

Limit details: You’ve used all 1 included review currently available under your plan.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: c3608fce-af6e-49b4-a0ee-b50218a5b8b8

📥 Commits

Reviewing files that changed from the base of the PR and between 746f00f and ad1da6b.

📒 Files selected for processing (3)
  • changelog.d/tsk-y62mpe-durable-cron.md
  • scripts/resume_arm_time.py
  • tests/test_resume_arm_time.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@gitar-bot

gitar-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown

Important

You are using the Gitar free plan. Upgrade to unlock code review, CI analysis, auto-apply, custom automations, and more.

Gitar

@@ -0,0 +1,2 @@
### Fixed

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUGGESTION: Changelog says "721-line" but scripts/resume_arm_time.py is 752 lines

The line count in the changelog does not match the actual file size.


Reply with @kilocode-bot fix it to have Kilo Code address this issue.

@kilo-code-bot

kilo-code-bot Bot commented Aug 18, 2026

Copy link
Copy Markdown

Code Review Summary

Status: 1 Issue Found | Recommendation: Address before merge

Overview

Severity Count
SUGGESTION 1
Issue Details (click to expand)

SUGGESTION

File Line Issue
changelog.d/tsk-y62mpe-durable-cron.md 1 Changelog states "721-line" but scripts/resume_arm_time.py is actually 752 lines
Files Reviewed (3 files)
  • changelog.d/tsk-y62mpe-durable-cron.md - 1 issue
  • scripts/resume_arm_time.py - no issues
  • tests/test_resume_arm_time.py - no issues

Fix these issues in Kilo Cloud


Reviewed by step-3.7-flash · Input: 105.1K · Output: 23K · Cached: 806.7K

@jaylfc

jaylfc commented Aug 18, 2026

Copy link
Copy Markdown
Owner Author

MERGE. Fifth attempt on this chain and the first that did the work. All five blockers fixed, each with a real red-first control.

Follow-up card: tsk-bse2el (one new defect, not a blocker — see the end).

The cheap check this card was filed over — this time it passes

tsk-y62mpe was filed because #323 re-published #317's exact commit. Run against this branch:

git diff origin/exec/tsk-7xt72j origin/exec/tsk-y62mpe --stat   -> 38 files, +3950 -447
git diff origin/exec/tsk-7xt72j origin/exec/tsk-y62mpe | wc -c  -> 295448

Non-empty, as the acceptance required. Merge-base is 746f00f9 (master itself), real contribution 3 files / +1055.

Blockers A–E, each re-measured rather than inherited

requirement measured
A canonical helper, date-pinned not daily 752 lines. On reset 2026-08-18T05:00:00.310776+00:00 it emits 7 5 18 8 * / 17 5 18 8 *, byte-identical to the canonical ~/.taos-fleet-tools copy (md5 7c54e86e) and matching the live armed crons
B assert a token unique to the invocation asserts the exact armed_at, reads the log from a redirected HOME, runs /usr/bin/python3 not sys.executable, and checks crontab removal against the sandboxed state
C explicit interpreter /usr/bin/python3 at both emitted lines; no bare python3 outside the shebang
D every do_fire caller sandboxed all 4 sandboxed; fake crontab shim on PATH + HOMEtmp_path
E import getpass line 50

The $(cat) trap the card named twice did not recur — the shim reads stdin raw. And test_do_fire_runs_as_subprocess asserts an unrelated crontab line survives the self-removal, which is the control that makes the removal assertion mean something.

Red-first, proven independently

The new test file, copied unchanged onto the blocked tree origin/exec/tsk-7xt72j (md5 308be4d9 identical on both trees, script there still 142 lines with 0 import getpass):

7 failed  on the old implementation
7 passed  on the new (inside the full run below)

Every one of A–E has a test that goes red on the unfixed code. That is what the last four attempts could not show.

Suite

1550 passed, 12 skipped on a trial merge into 746f00f9. Baseline 1543 plus exactly the 7 tests in tests/test_resume_arm_time.py, reconciled against what the diff touched. Clean merge, no conflict markers, all three gates clean.

The one new defect — carded as tsk-bse2el, deliberately not a blocker

scripts/resume_arm_time.py hardcodes an out-of-repo path at lines 506, 507 and 542 (/home/jay/.taos-fleet-tools/resume_arm_time.py), pinned by the test at 143-144. So the merged repo copy emits crontab lines that invoke a different file, outside the repo and free to diverge. It is also the first /home/jay in scripts/ — master has none — though pyproject.toml scopes packaging to include = ["taosmd*"], so scripts/ does not ship to package users. That is why it is a follow-up: the helper is now in version control with a real test suite, which is a large net improvement over living only in ~/.taos-fleet-tools.

Operational note, and it is why this matters. At ~01:07Z ~/.taos-team/resume_arm_time.py was repointed at /tmp/exec-tsk-y62mpe/scripts/resume_arm_time.py — a live worktree of this card. That worktree has since been cleaned up, so the link was dangling and the fleet resume path was broken when I picked this up. I restored it to the fleet-tools copy (~/.taos-team commit 683f83ed) and re-verified it derives 7 5 18 8 * / 17 5 18 8 *, matching the armed crons. I am deliberately NOT repointing that symlink at the merged repo copy yet, because the emitted crontab lines still name fleet-tools and the two must not disagree. tsk-bse2el completes the cutover.

Limitations, stated: I ran the derivation path only and never --fire against the real crontab, so the fire path is verified by its sandboxed tests rather than live. I did not review the 752-line helper line by line; I verified its behaviour against the canonical copy on the live input and read the sections the blockers name.

@jaylfc
jaylfc merged commit f1301f6 into master Aug 18, 2026
7 checks passed
jaylfc added a commit that referenced this pull request Aug 18, 2026
…tab lines name the canonical in-repo script (#354)

Card tsk-bse2el. Replaces the hardcoded /home/jay/.taos-fleet-tools/resume_arm_time.py
in system_crontab_block and do_fire with _HELPER_PATH = os.path.realpath(__file__).

WHY THIS MATTERED MORE THAN THE CARD SAID
The out-of-repo path the crontab lines named is not just a different copy, it is a stale
pre-#337 one: md5 7c54e86e32 / 40843 bytes vs the repo's 446505544c / 42188 bytes, 45 lines
of divergence, and its do_fire has NO A2A bus posting at all. On master an armed resume
fired, wrote a log line, and notified nobody.

Verified on a TRIAL MERGE into origin/master dddbb5d (merge-base re-derived as 72ade2c
after #355 moved master mid-review), never on the PR head.

- Real CLI (`resume_arm_time.py 2026-08-18T07:00:00+00:00`, read-only): master emits the
  .taos-fleet-tools path; the trial merge emits the invoking checkout's own script.
- Mutation analysis: against the OLD impl the 4 touched tests all fail, but every one fails
  with `AttributeError: no attribute '_HELPER_PATH'`, which only proves the symbol is new.
  Against a MUTANT that DEFINES _HELPER_PATH but hardcodes it back, only 2 fail:
    test_do_fire_posts_resume_due_to_bus            FAILED  <- discriminates
    test_do_fire_bus_failure_writes_visible_record  FAILED  <- discriminates
    test_system_crontab_block_names_usr_bin_python3 PASSED  <- vacuous wrt this defect
    test_do_fire_runs_as_subprocess                 PASSED  <- vacuous wrt this defect
  The discriminating pair anchors on SCRIPT, an origin independent of the implementation.
  Followed up as tsk-vqfjsm; genuine coverage exists, so not a blocker.
- Changelog fragment ends 0x0a, so it does not reintroduce the #351 defect.
- Conflict markers, deleted-symbols guard, handle gate, witness gate: clean.
- Full suite on the trial merge: 1617 passed, 12 skipped. Reconciles as 1617 = 1617 + 0.

STATED LIMITATIONS
- The emitted path now depends on WHERE the script is run. From a worktree or temp copy it
  pins a path that will be deleted; since the line is `<helper> --fire ... && (crontab -l |
  grep -v ...) | crontab -`, a missing helper short-circuits the && and the self-removal
  never runs, turning a one-shot into a permanent annual entry. Filed as tsk-bxdfvz.
- Nothing here reconciles the two copies. The stale .taos-fleet-tools file is still on disk
  and still 45 lines behind; this only stops the repo copy pointing at it. Untracked.
- os.path.realpath resolves symlinks, so a symlink install would be pinned by its target.
  No such install exists today.
- My first suite run on this PR reported 1 failure. That was my own contaminated worktree
  (I reverted the tree mid-run), not the PR. Rebuilt, proved impl+tests by md5 before and
  after, re-ran clean. Recorded on the PR.
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