fix(training-agent): acquire_rights end_date is inclusive (repo-wide CI time-bomb)#5760
Merged
Merged
Conversation
…endar time-bomb)
`acquire_rights` rejected a campaign when `new Date(end_date).getTime() < Date.now()`.
`new Date('YYYY-MM-DD')` parses to 00:00:00Z, so the instant midnight UTC ticks
over on the end date, a campaign ending *today* was wrongly judged "in the past"
and rejected — even though the success path sets valid_until to that day's
23:59:59Z (end_date is inclusive). On 2026-06-30 this turned the
`brand-sandbox-tools` test red and, via the precommit hook + CI, blocked commits
repo-wide.
- Reject only once the whole end day is past: compare against the start of today
(UTC), so a campaign ending today stays licensable.
- Make the `rights_constraint uses date-time format` test use a next-calendar-year
window instead of a hardcoded date, so it exercises the success path durably
rather than rotting into a time-bomb.
Note (follow-up, not fixed here): the hardcoded default `campaign.end_date ||
'2026-06-30'` (brand-handlers.ts) is stale — a no-end_date acquisition gets a
past expiry — but it's applied after the guard so it can't cause a rejection or
break a test; replacing it needs a product call on the default window.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
There was a problem hiding this comment.
Clean fix. The expiry guard now matches the inclusive end_date semantics the success path already emits.
Things I checked
- Boundary math.
new Date('YYYY-MM-DD')parses to00:00:00Zper spec, so a campaign ending today givesendMs == startOfTodayMsandendMs < startOfTodayMsis false — not rejected. Ending yesterday → rejected. Future → fine. (brand-handlers.ts:947-949) - No UTC/local footgun.
Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate())keeps both sides UTC-anchored. The classic mix of localnew Date(y,m,d)against a UTC-parsedendMsis avoided, so the guard flips at UTC midnight in any server timezone. - Internal consistency. Inclusive-through-end-of-day now lines up with the success path's
valid_until: ${endDate}T23:59:59Z. The guard was off by up to a day; it isn't anymore. - Test durability. Window is computed from
getUTCFullYear() + 1, expectations derived from the samestartDate/endDatestrings. Exercises the success path regardless of run date — no more hardcoded-2026-06-30time-bomb. code-reviewer: clean, no findings.
Follow-ups (non-blocking — file as issues)
- The
campaign.end_date || '2026-06-30'andstart_date || '2026-04-01'fallbacks atbrand-handlers.ts:1041-1042are still stale. They sit below the guard, but the guard only runsif (campaign.end_date)— a no-end_dateacquisition skips the guard and flows the stale2026-06-30straight intovalid_until. Out of scope here (needs a product call on the default window, as the PR notes), but it's the next time-bomb in the same file.
Diff is two files, 18/-5, no wire surface, no changeset needed. LGTM.
bokelley
added a commit
that referenced
this pull request
Jun 30, 2026
gap #1 follow-up) Bind-on-verify (#5752) binds a domain to its owner when the origin pointer points at AAO. But domains change hands and publishers remove pointers, so a verified row needs re-checking on a TTL — otherwise a transferred domain keeps its stale owner forever and can never be re-claimed. - Periodic re-verify job (crawler): startPeriodicHostedOriginReverification re-runs verifyHostedPropertyOrigin over verified rows past a 24h TTL, bounded batch + concurrency, mirroring the manager-revalidation pattern. Wired into startup (hourly tick). A permanent failure (origin no longer points at AAO) clears origin_verified_at via the existing verifier path — the owner lock lapses. Transient failures (5xx/429/timeout) leave verified state intact by design. - Lapse → re-claim: bindOwnerFromVerifiedClaim now also binds when origin_verified_at IS NULL, so a lapsed domain's stale workos_organization_id no longer blocks a new owner from re-binding. issueDomainClaim already keys its refusal on origin_verified_at, so a lapsed domain is re-claimable. - Verifier ordering fix: bind BEFORE stamping origin_verified_at, so the bind guard sees the pre-verification (lapsed) state rather than the lock it's about to re-arm. (Found by the re-bind test.) - getHostedPropertiesDueForReverification: the work-list query (verified rows past the TTL; unverified rows are never candidates). - The row is never deleted on lapse — the rid is forever; verification and the owner binding lapse, content stays. Tests: lapse releases the lock and lets a new owner re-claim and re-bind; the due-for-reverification query excludes unverified rows. Existing bind + verifier suites unchanged (7/7 and 10/10). Stacked on #5760 (the time-bomb fix) — that commit drops out on rebase once #5760 merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Jun 30, 2026
gap #1 follow-up) Bind-on-verify (#5752) binds a domain to its owner when the origin pointer points at AAO. But domains change hands and publishers remove pointers, so a verified row needs re-checking on a TTL — otherwise a transferred domain keeps its stale owner forever and can never be re-claimed. - Periodic re-verify job (crawler): startPeriodicHostedOriginReverification re-runs verifyHostedPropertyOrigin over verified rows past a 24h TTL, bounded batch + concurrency, mirroring the manager-revalidation pattern. Wired into startup (hourly tick). A permanent failure (origin no longer points at AAO) clears origin_verified_at via the existing verifier path — the owner lock lapses. Transient failures (5xx/429/timeout) leave verified state intact by design. - Lapse → re-claim: bindOwnerFromVerifiedClaim now also binds when origin_verified_at IS NULL, so a lapsed domain's stale workos_organization_id no longer blocks a new owner from re-binding. issueDomainClaim already keys its refusal on origin_verified_at, so a lapsed domain is re-claimable. - Verifier ordering fix: bind BEFORE stamping origin_verified_at, so the bind guard sees the pre-verification (lapsed) state rather than the lock it's about to re-arm. (Found by the re-bind test.) - getHostedPropertiesDueForReverification: the work-list query (verified rows past the TTL; unverified rows are never candidates). - The row is never deleted on lapse — the rid is forever; verification and the owner binding lapse, content stays. Tests: lapse releases the lock and lets a new owner re-claim and re-bind; the due-for-reverification query excludes unverified rows. Existing bind + verifier suites unchanged (7/7 and 10/10). Stacked on #5760 (the time-bomb fix) — that commit drops out on rebase once #5760 merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
bokelley
added a commit
that referenced
this pull request
Jun 30, 2026
* feat(registry): periodic hosted-origin re-verify + lapse→re-claim (#5749 gap #1 follow-up) Bind-on-verify (#5752) binds a domain to its owner when the origin pointer points at AAO. But domains change hands and publishers remove pointers, so a verified row needs re-checking on a TTL — otherwise a transferred domain keeps its stale owner forever and can never be re-claimed. - Periodic re-verify job (crawler): startPeriodicHostedOriginReverification re-runs verifyHostedPropertyOrigin over verified rows past a 24h TTL, bounded batch + concurrency, mirroring the manager-revalidation pattern. Wired into startup (hourly tick). A permanent failure (origin no longer points at AAO) clears origin_verified_at via the existing verifier path — the owner lock lapses. Transient failures (5xx/429/timeout) leave verified state intact by design. - Lapse → re-claim: bindOwnerFromVerifiedClaim now also binds when origin_verified_at IS NULL, so a lapsed domain's stale workos_organization_id no longer blocks a new owner from re-binding. issueDomainClaim already keys its refusal on origin_verified_at, so a lapsed domain is re-claimable. - Verifier ordering fix: bind BEFORE stamping origin_verified_at, so the bind guard sees the pre-verification (lapsed) state rather than the lock it's about to re-arm. (Found by the re-bind test.) - getHostedPropertiesDueForReverification: the work-list query (verified rows past the TTL; unverified rows are never candidates). - The row is never deleted on lapse — the rid is forever; verification and the owner binding lapse, content stays. Tests: lapse releases the lock and lets a new owner re-claim and re-bind; the due-for-reverification query excludes unverified rows. Existing bind + verifier suites unchanged (7/7 and 10/10). Stacked on #5760 (the time-bomb fix) — that commit drops out on rebase once #5760 merges. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(registry): preserve transient DNS reverify state --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Repo-wide CI blocker — fast-track
As of 2026-06-30,
tests/addie/brand-sandbox-tools.test.ts > rights_constraint uses date-time formatfails onmain, which red-lights the precommit hook and thetest:unitCI job for every branch/PR today.Root cause — calendar time-bomb
acquire_rights(brand-handlers.ts) rejected a campaign when:new Date('2026-06-30')parses to2026-06-30T00:00:00Z, so the instant midnight UTC passes on the end date, a campaign ending today is wrongly judged "in the past" and rejected — even though the success path setsvalid_untilto that day's23:59:59Z.end_dateis meant to be inclusive; the guard was off by up to a day.Fix
valid_untilsemantics.All 38 tests in the file pass; typecheck clean.
Note (follow-up, not fixed here)
The hardcoded default
campaign.end_date || '2026-06-30'inbrand-handlers.tsis stale (a no-end_dateacquisition gets a past expiry), but it's applied after the guard so it can't cause a rejection or break a test. Replacing it needs a product call on the default window.🤖 Generated with Claude Code