fix(hook): say the session zone has no DST so the JVM builds it from the offset - #48
Conversation
…the offset The dynamic zone the hook hands out left DynamicDaylightTimeDisabled at FALSE. The JVM (TimeZone_md.c, every line from 8 on) then looks the key name "Chrono Session" up in its own mapping table, misses, and reads ActiveTimeBias from the real registry, so every Java application under a session showed the machine's zone while the verdict said works. MS Learn describes a zone without daylight saving time as this field set with both transition dates cleared, which is exactly the session zone. With it set the JVM builds the zone from Bias (GMT+05:30), and ICU names a whole-hour offset Etc/GMT-N instead of leaving it unnamed. .NET, the C runtime and Go do not read the field, checked in their sources and by the harness. Guard: crates/cli/tests/session_zone.rs reads the field through kernel32 from Windows PowerShell under a session, with a control run without one. With the field reverted it fails on that assertion and on no other. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (9)
Included review availability: Your plan provides up to 10 included reviews per hour; 7 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (6)
🧰 Additional context used📓 Path-based instructions (12)Applies to text shown to the user (labels, buttons, tooltips, placeholders, dialogs, errors, status messages, empty states, translations).⚙️ CodeRabbit configuration file Files:
Verify tests check real behavior and would fail if the implementation were broken.⚙️ CodeRabbit configuration file Files:
Performance is a known weak spot of these projects.⚙️ CodeRabbit configuration file Files:
Applies only to code that builds or styles a GUI.⚙️ CodeRabbit configuration file Files:
User-facing changelog.⚙️ CodeRabbit configuration file Files:
SECURITY, HIGH PRIORITY.⚙️ CodeRabbit configuration file Files:
Source of the public project website (generated output is excluded from review).⚙️ CodeRabbit configuration file Files:
Check that documentation matches the actual code in this PR: commands, flags, config keys, file paths, build steps and examples must exist.⚙️ CodeRabbit configuration file Files:
Rust code.⚙️ CodeRabbit configuration file Files:
All code in this repository is written by an AI coding agent (Claude Code).⚙️ CodeRabbit configuration file Files:
Source excerpt: **Everything inside the repository is English**, including comments.📄 CodeRabbit inference engine (CONTRIBUTING.md) Files:
Scope, duplication and docs: Warn if any of these is true: the PR contains significant changes not mentioned in the title/description, or mixes unrelated refactors with a feature or fix; the PR adds functionality, helpers, UI components, st...📄 CodeRabbit inference engine (Custom checks) Files:
📝 WalkthroughWalkthroughThe dynamic time-zone hook now disables daylight saving time. A Windows integration test checks the session zone key, bias, and daylight-disable field. Documentation describes Java’s fixed-offset zone name and offset-based names for other runtimes. ChangesSession time-zone reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Merge Risk: ⚪ Minimal · up to The session time-zone change has no identified issue requiring a fix before merge; normal checks remain appropriate. 🚥 Pre-merge checks | ✅ 12 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (12 passed)
Full details: Clear User-Facing TextExplanation The PR changes user-facing documentation but uses multiple names for the same concepts. The new changelog calls the same host zone “the machine's time zone” and “the machine's own time zone,” while the documentation alternates between “session time zone” and “session zone.” This violates the check's terminology-consistency condition. Resolution Standardize the new prose on one term for each concept. Use “session time zone” instead of “session zone,” and use one host-zone term, such as “machine time zone,” instead of both “machine's time zone” and “machine's own time zone.” Apply the terms consistently in CHANGELOG.md, README.md, and the English and Polish site pages. Full details: No Resource LeaksExplanation The new Resolution Add an RAII cleanup guard for each scratch directory, with
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
1 participant
What was wrong
A Java application under a session read the session date but showed it in the machine's own time zone, on every Java version from 8 on and on both 32 and 64 bit, while the verdict said
works. The support matrix called it a known gap and blamed the JVM caching its zone.The real cause is one field. The JVM (
TimeZone_md.c, the same logic in the 8u, 11u, 17u and current lines) callsGetDynamicTimeZoneInformation. With a key name present andDynamicDaylightTimeDisabledset, it builds the zone fromBias. With the field clear, it looks the key name up in its own mapping table, misses "Chrono Session", and readsActiveTimeBiasfrom the real registry. The hook left the field at its default, FALSE.The change
h_gdtzisetsDynamicDaylightTimeDisabled = TRUE. MS Learn describes a zone without daylight saving time as this field set with both transition dates cleared, which is exactly the shape of the session zone, so this is the truthful answer rather than a workaround. No new channel, no change to the protocol or the shared control block.Who else reads the field (checked in their sources)
GMT+05:30,GMT-03:00,GMTEtc/GMT-5(andUTCat +00:00) instead of having no name. A half-hour zone keeps its old pathGetTimeZoneInformation, which has no such fieldMeasured before and after
Session zones
+05:00,+05:30,-03:00and+00:00on a host at +02:00:Guards, each with its revert probe measured (field set back to FALSE)
crates/cli/tests/session_zone.rs(CI): Windows PowerShell reads the field through kernel32 under a+05:30session, with a control run without one. Reverted, it fails on the field assertion while the two reach assertions (key name, bias -330) still pass.+05:30: Java reportsGMT+05:30and offset+05:30. Reverted, 8 of 10 on x64 and 8 of 10 on x86, exactly the two zone assertions red.Not a regression, noted while measuring
A command-line tool that ships with Windows and prints the current zone key fails under a session with the old value too. It looks the key name up in the registry, which is the "a target that insists on a registry name will not find one" case the README already states. It was considered as the CI target and dropped for that reason.
Not measured
Qt
QTimeZone, Chromium with the native hook, and an application that sets its own-Duser.timezone.Gates
tools/gates.ps1 -Envran once: 14 of 15, harness 202/202 on x64 and on x86 (S13 grew from 7 to 10 assertions), test-cs 627. The one red gate wastest-ruston the network register, which requires a reason for every process a test starts and had none yet for the new test. With the entry added,test-rustpasses 528 tests and clippy is clean.🤖 Generated with Claude Code
Summary by CodeRabbit
GMT+05:30. Node.js and Deno use clearer names for whole-hour offsets, includingUTCfor zero offset.