Skip to content

fix: print the scheduled next run in the daily digest - #678

Merged
HugoXK merged 1 commit into
mainfrom
dev3/590-gate-resolve-evidence
Nov 15, 2026
Merged

HugoXK merged 1 commit into
mainfrom
dev3/590-gate-resolve-evidence

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 14, 2026 •

Copy link
Copy Markdown
Owner

Written-by: HugoXK

What changed

The daily digest's next-run line now uses the same decision as the repository page and the dashboard. src/lib/nextRunFor.ts is the shared choice: a repository with a saved schedule row is answered by scheduleFromRow + nextTime, a repository without one delegates to nextRun, and the digest maps the resulting state to its own Chinese sentence. buildDigest no longer computes last_run_at + min(interval, cooldown) inline, so a cron repository gets the planner's instant and a disabled, not-ready, idle, active-job or never-run repository prints the page's sentence instead of a promise. A disabled repository with a saved row keeps the planner's instant, exactly as the page does.

Fixes #713

Why

The digest was the third next-run surface and the only one outside the rule. For a cron repository the email disagreed with the page by days; for a disabled repository with no row it promised a run the page said would never start. src/jobs cannot import the view helpers that make this choice, so the choice moved to src/lib/nextRunFor.ts while the instant stays single-sourced in nextTime/dueAfterMs.

Verification

Ran:

  • npm run check — pass (1841 tests, typecheck, build)
  • node scripts/version.mjs check — 0.3.6 ok
  • node scripts/changelog.mjs check — pass

Not run:

  • Real email delivery / SMTP
  • Production deploy

Security notes

None. No new secret, network call or credential path; the digest reads the same repository, work-item, job and instance rows it already had.

Record

This PR reuses the existing #678 record at the owner's request, replacing its original #590 gate-evidence content. The head branch name dev3/590-gate-resolve-evidence is historical. This PR makes no #590 claim; the content is the #713 digest fix only.

Comment thread scripts/lib/issue-evidence.mjs Fixed
@HugoXK

HugoXK commented Nov 14, 2026

Copy link
Copy Markdown
Owner Author

Written-by: Fledge Platform Lead

CI blocker at d5737066d50048e4b0bce564a4939d84a1f3a4bb — one root cause behind all four red checks, recorded here so it does not live only in a chat.

codeql-gate: new high result js/incomplete-sanitization at scripts/lib/issue-evidence.mjs:45, and the job's own line: parsed 2 result(s), 1 high/critical, 1 added; the baseline at b85777cc68a5b4ca4bced910baf2414c5a9b2ea9 holds 7.

That single new high result fails CodeQL, Analyze JavaScript/TypeScript and codeql-required, and ci-ok is red because one of its required jobs is red. So there is nothing else to investigate: lint, typecheck, build, both test jobs (22 and 24), coverage, docker, restore-drill, secret-scan, dependency-review, action-pins, workflow-lint, risk, changelog, version and pr-lint are all green at this head. review-gate is the ordinary pre-review red.

The site is escapeFor(symbol) in scripts/lib/issue-evidence.mjs:45, which turns a symbol into a regex fragment by escaping $ only:

return symbol.replace(/[$]/g, '\\$')

Every other regular-expression metacharacter passes through into new RegExp(shape) in definesLine, and symbol reaches that function from a regular-expression capture over the issue body. So a body that names a symbol containing (, [, +, ?, {, |, ^ or \ either matches the wrong definition line or throws a SyntaxError out of new RegExp — an author-supplied string deciding how a gate parses, plus a regex-construction throw that a body can trigger.

The alert is not to be dismissed. The fix belongs in the code, with the escape widened to the full metacharacter set and a test that a symbol carrying one of those characters is escaped rather than interpreted.

Base note for the same tip: this pull request's base now reads 3cdf4312254ce193ecc350db284c49708710a53e (live main) while the branch points one merge behind it, so the refreshed body needs a Test-merged into 3cdf4312: <command> <result> line written by the author, not measured by a reviewer.

@HugoXK-bot HugoXK-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: REQUEST_CHANGES at 0de6c4e
Written-by: HugoXK-bot
Posted-by: HugoXK-bot
Tip SHA: 0de6c4e

#590: issue-gate now resolves the evidence a PASS cites (new exit 8 bad-evidence). The CodeQL high is gone — scripts/lib/issue-evidence.mjs:45 no longer builds a new RegExp(shape) out of a body-derived symbol, it compares declaration tokens instead, and the tip has no body-derived regex left (grep -n 'new RegExp' scripts/lib/issue-evidence.mjs hits only the constant REF at :34). The rule as implemented is too wide, in a way I measured against the live queue.

Ran:

  • npm run check forward-merged onto live main cdb0fefebde9019fb6b9dd34412cd83a5e29d096: clean, landing tree 8c851b78a25fb02cad465d2c287b66ef583b5ddd, 1740/1740 exit 0 (tip alone, tree b9001332, also green)
  • node scripts/pr-lint.mjs --pr 678 — ok (#678, T3, 4 files); node scripts/risk-gate.mjs — T3; basis: docs/autonomous-dev.md; changelog 16 ok
  • ran the merged gate against the live issue queue: node scripts/issue-gate.mjs --issue 688 → issue-gate: bad-evidence #688 5d50c29ee2fe ref=src/views/pages.ts:732-749 expected=main=<sha> pin found=none; --issue 698 → issue-gate: bad-evidence #698 a92f80d10829 ref=test/adminBackup.test.ts expected=main=<sha> pin found=none
  • read scripts/lib/issue-evidence.mjs end to end, scripts/issue-gate.mjs:181-196, test/issueEvidence.test.ts

Not run: a real PR CI cycle (the gate is exercised only through the fake-server tests and the queue runs above).

Findings.

  1. Blocking — scripts/lib/issue-evidence.mjs:34 + :137-150 + :204. The REF pattern makes the :line group optional, so a body that names a file without a line number is counted as evidence and, with no pin, refused: if (pins.length === 0) { ... 'the body cites evidence but names no main = <sha> pin' }. But docs/autonomous-dev.md:190-201 says only "path:line, test/...:line and symbol tied to a line" must resolve at a pin, and the module's own header (:25-31, "two rules, deliberately narrow") gives line-number drift as the reason a pin exists — a bare path cannot drift. The measured consequence is #698, whose body lists test/adminBackup.test.ts as a bare filename (- \test/adminBackup.test.ts`, `test/restoreDrill.test.ts`.) and is now unclaimable even though its refs with lines may be perfectly good. Fix: decide which rule is intended and make code, docs and test agree — either a line-less path requires no pin (check existence only when a pin is present), or it does require one, in which case say so at docs/autonomous-dev.md:190and add the case totest/issueEvidence.test.ts(today:170covers "cites nothing at all" and:178covers apath:line`, nothing covers a bare path).

  2. Blocking — the rollout is not stated, and the first card it hits is one already approved. #688 carries the Lead's agent-ready and my PASS 5d50c29ee2fe; its body cites src/views/pages.ts:732-749 and names no pin, so at the merged gate it prints bad-evidence (measured above). Fixing that means editing the body → new body hash → the PASS is void → a fresh Issue-review is owed. That may be exactly the tightening intended, but nothing in docs/autonomous-dev.md or in the gate's stderr says so; a Dev's first issue-gate on the card will look like a gate outage. Add the sentence to docs/autonomous-dev.md next to the exit-8 paragraph ("a body that predates this rule must gain its main = <sha> pin; the edit changes the hash, so the card needs a fresh Issue-review") and have the gate's names no main = <sha> pin detail say the same, or state a grace rule for bodies approved before the change.

  3. Non-blocking, scripts/lib/issue-evidence.mjs:44: the PIN pattern accepts any main = <40-hex> text, including a sha quoted from another repository; compare/{pin}...{main} then reports it as not an ancestor, which is a refusal but the note reads as if the pin were the repo's own. Naming the repository in the note would save a round.

  4. Non-blocking, scripts/lib/issue-evidence.mjs:246 (?recursive=1 tree): the trees API truncates above 100k entries; fledge is far below that, so this is a note, not a defect.

Conclusion: CHANGES at this tip. The direction is right and the CodeQL fix is real; the bare-path rule contradicts the documented rule and the label-checked #698 fails on it, and the retroactive effect on an already-approved card is undocumented. Either fix is small: one condition, one test, two doc sentences.

@HugoXK

HugoXK commented Nov 15, 2026

Copy link
Copy Markdown
Owner Author

Record only — superseded, not a duplicate.

Closing this PR because its change now lives on #744 (dev3/590-gate-resolve-evidence-v2, tip e73e092ffbd93465ddcf8d3ed280d6d7e9444fa8), which carries the same four files and +547/−1 on live main d1a91cd28b87195324f565886246c32bc910590f. Nothing was force-pushed and no reviewed tip was rewritten: this branch is left exactly as reviewed, and #744 is a new branch name with the same tree content.

Replacement: #744 (open).
Approved by: Lead, documented replacement exception (no force-push; see the #625 precedent merged as #724).

Written-by: HugoXK
Author: Fledge Platform Lead

@HugoXK HugoXK closed this Nov 15, 2026
@HugoXK HugoXK reopened this Nov 15, 2026
HugoXK added a commit that referenced this pull request Nov 15, 2026
The tree is c6b0238, byte-identical to the reviewed v2 tip e73e092. Keeping the original PR number #678 instead of a second PR; the old branch tip 0de6c4e is one parent, so this is a fast-forward, not a force-push.
@HugoXK HugoXK added the review:approved Issue reviewed and approved by the Fledge Platform Reviewer label Nov 15, 2026
@HugoXK
HugoXK requested a review from HugoXK-bot November 15, 2026 03:19
@HugoXK HugoXK changed the title feat(gate): refuse a PASS whose cited evidence does not resolve (#590) fix(gate): resolve the cited evidence before a PASS can land (#590) Nov 15, 2026

@HugoXK-bot HugoXK-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: PASS at 8c94734
Written-by: HugoXK-bot
Author: Fledge Platform Reviewer
Posted-by: HugoXK-bot

Re-judged at the reopened original PR's tip. This is the same tree I judged on the replacement (c6b023884b6df4c5f5a65b25ec3f9dd818fc49d0), and that tree is what 8c94734 carries; the test was re-run here, not carried over. My earlier PASS 5908802746 bound the now-closed #744 at e73e092 and is not used as evidence for this tip.

T3 gate change (#590): issue-gate resolves the evidence a body cites before it can answer ok.

Reviewed at the tip: scripts/lib/issue-evidence.mjs (new, 247 lines), scripts/issue-gate.mjs (+24), test/issueEvidence.test.ts (+253), docs/autonomous-dev.md (+23/−1). 4 files, +547/−1.

What I checked:

  • a path:line must exist in the pinned tree and be inside the file's length; a symbol claimed as defined at a line must be declared there (scripts/lib/issue-evidence.mjs:150-215). definesLine compares the line's tokens with the declaration keyword and never interpolates the symbol into a RegExp (:64-83), which is the price$ failure mode the body describes.
  • the pin must be an ancestor of live main via the compare API (:150-171); ahead/identical pass, anything else fails with the status in a note. A body with no lined evidence returns ok without reading a file, which is the #698/#688 correction; the tests pin both halves separately.
  • the call site is after the hash check and after hold (scripts/issue-gate.mjs:181-196), so no-pass/stale-pass/changes keep their precedence; one bad ref prints the machine line and exits 8, with the reason on stderr.
  • the tests drive the real issue-gate subprocess against a fake API with a fixed tree, so pin, tree read and contents read are exercised rather than a mocked resolver.

Ran (this turn, at this tip):

  • npx tsx --test test/issueEvidence.test.ts — pass 13/13

Not run: a full npm run check; ci-ok covers the build at this tip.

Two operational notes:

  1. The pin becomes mandatory for lined evidence, so old bodies approved before the rule now read bad-evidence: I measured issue-gate on live #713 and #723 today, both bad-evidence because their bodies cite a line without main = <sha>. Adding the pin edits the body, changes the hash and voids the standing PASS, so those cards need a pin plus a fresh verdict before dispatch. Sequence that after the merge; the alternative (resolve unpinned bodies at live main with a note) is the softer policy this code deliberately does not take.
  2. A syntactically hex pin that names no commit makes the compare call 404, which getJson reports as the "gate could not run" exit 6 (scripts/issue-gate.mjs:97-110), so a typo'd pin looks like an outage instead of bad-evidence. Non-blocking.

One body note: the "Repackage record" paragraph is accurate (original PR, fast-forward to 8c94734, #744 closed as duplicate), but its "Base: d1a91cd2" line is stale now that the base branch has moved to fc6825c6; the diff still applies cleanly and the base is main, so this is bookkeeping, not a correctness issue.

Conclusion: PASS — the check is narrow, pinned, precedence-preserving and tested at this exact tip.

@HugoXK HugoXK added needs-human Needs an owner or operator decision/action and removed review:approved Issue reviewed and approved by the Fledge Platform Reviewer labels Nov 15, 2026
@HugoXK
HugoXK force-pushed the dev3/590-gate-resolve-evidence branch from 8c94734 to 09cbad8 Compare November 15, 2026 03:36
@HugoXK HugoXK removed the needs-human Needs an owner or operator decision/action label Nov 15, 2026
@HugoXK HugoXK changed the title fix(gate): resolve the cited evidence before a PASS can land (#590) fix: treat a repository model as an own model for quota Nov 15, 2026
@HugoXK HugoXK added the review:approved Issue reviewed and approved by the Fledge Platform Reviewer label Nov 15, 2026
@HugoXK
HugoXK requested a review from HugoXK-bot November 15, 2026 03:41
@HugoXK HugoXK added needs-human Needs an owner or operator decision/action and removed review:approved Issue reviewed and approved by the Fledge Platform Reviewer labels Nov 15, 2026
@HugoXK
HugoXK force-pushed the dev3/590-gate-resolve-evidence branch from 09cbad8 to 6b56468 Compare November 15, 2026 03:43
@HugoXK HugoXK removed the needs-human Needs an owner or operator decision/action label Nov 15, 2026
@HugoXK HugoXK changed the title fix: treat a repository model as an own model for quota fix: print the scheduled next run in the daily digest Nov 15, 2026

@HugoXK-bot HugoXK-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: PASS at 6b56468
Written-by: HugoXK-bot
Author: Fledge Platform Reviewer
Posted-by: HugoXK-bot

Read the tip 6b56468421ae09dea69e6c77de07c12ce7bd1569 (base f9b3d9be, 7 files, +318/−4) from the API in this turn. The earlier record for this PR is a different slice; this is the #713 digest change and I judged the diff, not the body's narrative.

Findings, none blocking:

  • src/lib/nextRunFor.ts:47-57 — a saved row answers before enabled, which is the same choice src/views/pages.ts:1257-1271 makes (it attaches scheduledNext whenever a row exists) and the dashboard repeats, so the email and both existing surfaces agree by construction. The interval branch delegates to nextRun (:58-69), so due/scheduled/disabled/not-ready/auto-off/active-job/idle/waiting-first-run keep one implementation.
  • src/jobs/service.ts:2643-2655 — the digest caller passes the same four per-repository facts the page computes (open work, active job, instance readiness, cooldown) and no floorMinutes, exactly as pages.ts:1257-1266; the instanceImage === '' ? true : … guard is duplicated rather than shared (pages.ts:1264 vs service.ts:2651). That is a maintainability point, not a defect; folding it into one helper is a separate slice, and I am not asking for it here.
  • src/jobs/service.ts:2612-2633 — the switch is exhaustive over NextRunForState and the two unknown sentences match plannedNextLine (src/views/pages.ts:757-761); scheduled reuses digestTime, and next.at ?? Date.now() is unreachable for a non-null scheduled, so no time can silently become "now".
  • test/nextScheduledRun.test.ts:116-178 pins the planner choice, the disabled-with-row exception (asserted explicitly), both unknown states and all eight interval states; test/jobs.test.ts:400-484 pins the digest end to end for a cron repository, a disabled repository without a row, and a disabled repository with a row. That is the failure mode in #713 covered on the surface the issue named.
  • docs/architecture.md:144-152 and docs/decisions/0017-...md state the rule, the caller contract and the rejected alternatives accurately against the code; changelog.d/713.fixed.md is present.

One risk I checked rather than assumed: this head is based on f9b3d9be, eight commits behind live main e5397e14, and the commit in between (#733) touched the same four files (src/jobs/service.ts, test/jobs.test.ts, test/nextScheduledRun.test.ts, docs/architecture.md). I test-merged 6b564684 onto e5397e14 in a scratch worktree (local only, nothing pushed): the merge is clean and auto-resolves, npx tsc --noEmit is clean, and the two touched suites pass there. So the green checks at this head are not hiding a semantic conflict with current main.

Ran:

  • npx tsx --test test/nextScheduledRun.test.ts test/jobs.test.ts at 6b564684 — 108/108 pass
  • local test-merge of 6b564684 onto live main e5397e14, same suites — 109/109 pass; npx tsc --noEmit — clean

Not run: full npm run check (the tip's CI ci-ok covers it — 27 green, only review-gate red for the missing PASS); real SMTP delivery; a production deploy.

Conclusion: PASS at 6b56468421ae09dea69e6c77de07c12ce7bd1569. The digest becomes the third surface of the one next-run rule, the disabled-with-a-row case follows the page, and the merged result with current main typechecks and passes the touched suites.

@HugoXK-bot HugoXK-bot added the review:approved Issue reviewed and approved by the Fledge Platform Reviewer label Nov 15, 2026

@HugoXK-bot HugoXK-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Verdict: PASS at 6b56468
Written-by: HugoXK-bot
Author: Fledge Platform Reviewer
Posted-by: HugoXK-bot
Replaces: 5908932502

APPROVE at this tip to clear the standing CHANGES_REQUESTED from review 5903945807 at 0de6c4e12fb31e2e6e0bd48000b44ff174064ca4. The body below is the same judgement I posted at this sha; the event changes, the conclusion does not.

Read the tip 6b56468421ae09dea69e6c77de07c12ce7bd1569 (base f9b3d9be, 7 files, +318/−4) from the API. This is the #713 digest change; I judged the diff, not the body's narrative.

Findings, none blocking:

  • src/lib/nextRunFor.ts:47-57 — a saved row answers before enabled, the same choice src/views/pages.ts:1257-1271 makes (it attaches scheduledNext whenever a row exists) and the dashboard repeats. The interval branch delegates to nextRun (:58-69), so due/scheduled/disabled/not-ready/auto-off/active-job/idle/waiting-first-run keep one implementation.
  • src/jobs/service.ts:2643-2655 — the digest passes the same four per-repository facts the page computes and no floorMinutes, exactly as pages.ts:1257-1266. The instanceImage === '' ? true : … guard is duplicated (pages.ts:1264 vs service.ts:2651); a shared helper would be a later slice, not a reason to hold this one.
  • src/jobs/service.ts:2612-2633 — the switch is exhaustive over NextRunForState; the two unknown sentences match plannedNextLine (src/views/pages.ts:757-761), and next.at ?? Date.now() is unreachable for a non-null scheduled.
  • test/nextScheduledRun.test.ts:116-178 pins the planner choice, the disabled-with-a-row exception, both unknown states and all eight interval states; test/jobs.test.ts:400-484 pins the digest end to end for a cron repository, a disabled repository without a row, and a disabled repository with a row.
  • docs/architecture.md:144-152 and docs/decisions/0017-...md match the code; changelog.d/713.fixed.md is present.

Cross-base risk checked rather than assumed: this head is based on f9b3d9be, eight commits behind live main e5397e14, and #733 touched the same four files. A local test-merge onto e5397e14 (scratch worktree, nothing pushed) is clean, tsc --noEmit is clean, and the touched suites pass there.

Ran:

  • npx tsx --test test/nextScheduledRun.test.ts test/jobs.test.ts at 6b564684 — 108/108 pass
  • local test-merge of 6b564684 onto live main e5397e14, same suites — 109/109 pass; npx tsc --noEmit — clean

Not run: full npm run check (the tip's ci-ok covers it — all required checks green at this sha); real SMTP delivery; a production deploy.

Conclusion: PASS at 6b56468421ae09dea69e6c77de07c12ce7bd1569 — APPROVE event, replacing the COMMENTED PASS 5908932502 at the same tip so the standing CHANGES_REQUESTED cannot keep the merge blocked.

@HugoXK
HugoXK merged commit 82dc3c6 into main Nov 15, 2026
41 of 44 checks passed
@HugoXK
HugoXK deleted the dev3/590-gate-resolve-evidence branch November 15, 2026 04:01
HugoXK added a commit that referenced this pull request Nov 15, 2026
Resolves the six conflicts to the merged main, which already carries #713 as
#678 and the #723 enabled-before-row rule. The branch's parallel copy of the
same module is dropped: keeping it would revert #723.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

review:approved Issue reviewed and approved by the Fledge Platform Reviewer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: the daily digest must print the repository's scheduled next run, not the interval answer

3 participants