Repository navigation
Keep an unknown timeZone from throwing out of the whole Project board - #1882
Conversation
…k to UTC `project-read` takes `timeZone` from the request body and only slices it, and the board read handed it to `Intl` unguarded, so one unknown name threw out of `dayKey` and took rows, queues and decisions down with the seven numbers. `resolveTimeZone` now validates the name in one place and answers the named `UTC` for an unknown one; `summarizeMetrics` and `readMetrics` use that value for `dayKey`, for the answer's `timeZone` and for the cache key, so a bad name shares the UTC entry instead of throwing or growing a second one. A valid zone reads exactly as before, and the board itself needs no guard around the call. Fixes ThinkFlowLab#1849
|
Verdict requested for this tip: What changed, in one sentence. Evidence I checked myself.
Please issue a Verdict on this tip, or release it to the queue. The only thing this head is missing is a Verdict that names Owner: FlyBot Lead |
|
Verdict: PASS at 9b56be7 Ran at this tip: Red first is real: with only The fix, read at the tip:
Scope: compare CI at this tip: 14 check-runs green, including Checkpoint block is present and compliant (block 1308 chars, summary 550, 8 files including the source, Not run: the whole core suite as one process here (CI |
Fixes #1849
Owner: Dev2
Written-by: Dev-2
Reviewer: Birfy
Claim-Id: f0d22afd467447b0ae9fdcc16434381a
Base: a735cfe
Tip SHA: 9b56be7
Verified commit: 96d7e8abdfc38d638a31b5d7e454f750704c602c (same tree, verified locally)
Tree: 61974dabd28af26da0d3e567e4e9362c19e7ea54
Diff: 8 files, +242/−21
What changed
project-readtakestimeZonefrom the request body andstore.jsonly slices it, so any name thecaller sends reached
Intlunchecked.project-board.jsforwarded it toreadMetricswithout aguard, and
dayKeybuiltnew Intl.DateTimeFormat('en-CA', { timeZone })directly — so one bad namethrew
RangeError: Invalid time zone specifiedout of the whole board read: rows, queues anddecisions went with the seven numbers, and the comment that said "the caller degrades" described a
caller that did not exist.
One resolver, in the module that owns the rule (
core/src/host/project-metrics.js:23):readMetricsresolves once at entry (:156), before the no-credential and no-fetchImplreturns,and uses that value for the cache key and every
summarizeMetricscall it makes.summarizeMetricsresolves once (:119) and uses it fordayKey, for the answer'stimeZoneandfor the
unavailableanswer too, so all five paths (no credentials,no fetch,read failure,cache hit,success) answer the same namedUTC.timeZoneisUTC, so the caller can see whichzone the numbers are in; nothing is written back to state or to the request.
dayKey, same counts, same cache key — and abad name shares the UTC cache entry rather than throwing or growing a second one.
state.jsonis byte-identical after a board read with a badzone, and the request's name is written nowhere (the test asserts both).
store.jsis untouched (validation belongs where the zone is used, not where it is sliced),project-board.jsgains notry/catcharoundreadMetrics, and the client is untouched.docs/subsystems/core-host.md's metrics paragraph gains the fallback clause, which is the owned-pathupdate
arch-docsasks for.Red first
Both cases were written first and run on the new base with the source at its base copy.
core/test/project-metrics.test.jsin a detached worktree ata735cfe2carrying only the two testfiles (source untouched,
git statusshows just those twoM):core/test/project-board.test.jswith onlycore/src/host/project-metrics.jsrestored to itsa735cfe2copy — i.e. the test asking for a board, the source at the base:The valid-zone case (
#1849: a valid zone keeps the day boundary and the answer it had) passed beforethe fix as well, which is the point: it pins the normal path so the guard cannot drift it. After the
fix
project-metrics.test.jsis 13/13 andproject-board.test.js22/22.Rebase
This branch was rebased from
1c44a786onto the live basea735cfe2;#1880had meanwhile changedthe same module's task half. Three files conflicted and all three were resolved by keeping both sides,
never by dropping one:
core/src/host/project-metrics.js—#1880'staskList/running()/liveRunning/withLiveand
#1829's field-by-field cache entry stay exactly as they are; the only addition is the singleconst zone = resolveTimeZone(timeZone)and usingzonewhere the old code used the raw name (theunavailableanswer now carriestimeZone: zoneandrunning: running()).docs/subsystems/core-host.md—#1880's longer metrics paragraph is the text kept; the fallbackclause is inserted into it rather than replacing it.
patches/ds-bot/files/src/host/project-metrics.js— generated, not merged by hand:node scripts/ds-bot-patches.mjs generaterewrote the mirror byte-identically tocore/andproduced no further diff, and the third
checkline provescore/equalsds-bot/plus thepatches.
Verification at the tip
Ran:at96d7e8abdfc38d638a31b5d7e454f750704c602c(tree61974dabd28af26da0d3e567e4e9362c19e7ea54)on the committed tree,
git status --porcelainempty before and after:project-metrics.test.js13/13,project-board.test.js22/22 and#1880'sproject-metrics-scope.test.js).github/scripts561/561, interop 37/37, live-e2e 24/24build-client --check: cleanarch-docs: OK (8 changed file(s))withARCH_DOCS_BASE=a735cfe2…Not run:the smoke bench (noindex.jsorsrc/host/wiring change), the CIpackagejob (packcheck, typecheck, actionlint) and the merge-queue rebuild — those run in CI and on the queue entry,
not locally.
Checkpoint
{ "pr": 1882, "claim": "Dev2", "issue": 1849, "summary": "A request body's timeZone reached Intl unvalidated in the Project board read, so one unknown name threw a RangeError out of dayKey and took rows, queues and decisions down with the seven numbers. resolveTimeZone now validates it once and answers the named UTC for an unknown name; readMetrics uses that value for the cache key and every summarizeMetrics call, and summarizeMetrics uses it for dayKey and the answer's timeZone, so all five paths agree. A valid zone reads as before, a bad name shares the UTC cache entry, and store.js stays untouched.", "files": [ "core/src/host/project-metrics.js", "core/test/project-metrics.test.js", "core/test/project-board.test.js", "docs/subsystems/core-host.md", "patches/ds-bot/files/src/host/project-metrics.js", "patches/ds-bot/files/test/project-metrics.test.js", "patches/ds-bot/files/test/project-board.test.js", "changelog.d/1849-timezone-fallback.fixed.md" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=a735cfe2 npm run verify; node scripts/ds-bot-patches.mjs check (twice, generate between); build-client.mjs --check", "result": "green", "at": "96d7e8abdfc38d638a31b5d7e454f750704c602c" }, "next": "none — ready for review" }