feat(pr-03): extend LicenseCache to match filaops-pro reader contract#600
Conversation
filaops-pro PR-03b (commit 62ced54, 2026-05-02) added four required fields to LicenseCache — status, last_verified_at, last_server_timestamp, grace_until — on the assumption Core's matching PR-03 would land in parallel. Core's PR-03 was never opened, so the contract has been broken on main for 9 days. The break surfaced when a license-server image rebuild (filaops-ecosystem PR #61, Quoter SPA tarball work) re-baked filaops-pro main into the served wheel for the first time since PR-03b. Subsequent force-recreate on the dogfood VM wiped license.json, re-activation wrote Core's 6-field PR-02 shape, PRO's LicenseCache(**filtered) raised TypeError, load_cache returned None, license_gate returned 403 no_license on every /pro/* route, and admin pages /admin/access-requests + /admin/catalogs broke with a secondary toast-storm amplifier. This PR extends Core to write the full 10-field shape PRO expects. The four new fields come from data already in the license-server's signed /validate response (server returns status + server_timestamp on every call per PR-03a) plus a locally computed grace_until (activation_moment + GRACE_DAYS, matching filaops_pro's compute_grace_until). GRACE_DAYS = 14 is duplicated as a module constant in app.core.license_cache so Core does not import filaops_pro (Sacred Rule); the comment notes the contract must stay in sync with filaops_pro.licensing.cache.GRACE_DAYS. Pre-existing PR-02 cache files on customer instances auto-upgrade on the first /info call after deploy: the endpoint detects missing PR-03 keys via is_pr02_shape() and re-saves with the full shape, deriving sensible defaults from activated_at. No manual deactivate+re-activate required for in-place upgrades. Tests: 39 passed (34 original + 5 new). Covers the upgrade-on-read path, the activation cache shape, server_timestamp fallback to epoch zero, and is_pr02_shape detection. Also verified 46/46 pass in the two other Core test files that import from license_cache (crypto + pro_install). Closes the schema drift documented in Aeonyx memory #148. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Review Council Results0 tests 0 ✅ 0s ⏱️ Results for commit 774524c. ♻️ This comment has been updated with latest results. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughCore adds PR-03 license cache schema with four heartbeat fields, implements idempotent on-read upgrade from PR-02 shape, enriches activation persistence, and provides comprehensive tests validating backward-compatible loading, shape detection, grace-window computation, and endpoint behaviors across upgrade and activation flows. ChangesPR-03 License Cache Migration and Enriched Persistence
Sequence Diagram(s)sequenceDiagram
participant Client
participant GetInfo
participant Upgrade
participant Disk
participant Cache
Client->>GetInfo: GET /system/license/info
GetInfo->>Disk: Check if cache file exists
alt Cache file exists
GetInfo->>Upgrade: Perform upgrade check
Upgrade->>Disk: Read raw JSON
Upgrade->>Upgrade: is_pr02_shape check
alt PR-02 detected
Upgrade->>Cache: from_dict with derivation
Cache-->>Upgrade: Upgraded LicenseCache
Upgrade->>Disk: save_license_cache
end
end
GetInfo->>Client: Return LicenseInfoResponse
sequenceDiagram
participant Client
participant ActivateEndpoint
participant LicenseServer
participant Disk
participant Cache
Client->>ActivateEndpoint: POST with license key
ActivateEndpoint->>LicenseServer: Validate license
LicenseServer-->>ActivateEndpoint: Response with tier/features/status
ActivateEndpoint->>Cache: Build PR-03 shape
Note over Cache: grace_until = activated_at + GRACE_DAYS<br/>last_server_timestamp = server_timestamp or EPOCH_ZERO_ISO
ActivateEndpoint->>Disk: save_license_cache
ActivateEndpoint->>Client: Return LicenseInfoResponse
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates Core’s on-disk license.json contract to match the extended LicenseCache shape expected by filaops-pro, preventing PRO from failing to read a freshly written/legacy PR-02 cache and incorrectly gating all /pro/* routes.
Changes:
- Extend
LicenseCacheto include PR-03-required fields (status,last_verified_at,last_server_timestamp,grace_until) with legacy-safe defaults and PR-02 shape detection. - Update
/system/license/activateto persist the full PR-03 cache shape and/system/license/infoto one-shot upgrade legacy PR-02 files on read. - Expand endpoint/unit tests to cover PR-03 field persistence, timestamp fallback behavior, PR-02→PR-03 upgrade, and idempotency.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
backend/app/core/license_cache.py |
Adds PR-03 cache fields, upgrade helpers, and default-derivation logic for legacy PR-02 files. |
backend/app/api/v1/endpoints/system_license.py |
Persists the full PR-03 cache shape on activation and auto-upgrades PR-02 files during /info. |
backend/tests/endpoints/test_system_license.py |
Adds coverage for PR-03 field persistence, fallback behavior, and PR-02→PR-03 upgrade/idempotency. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Malformed activated_at — fall back to "no grace window". | ||
| # PRO's evaluate_license will treat an unparseable grace_until as | ||
| # equivalent to expired; better than crashing with a TypeError. | ||
| anchor = datetime.now(timezone.utc) |
There was a problem hiding this comment.
Good catch — fixed in 91e... (pushing now). _compute_grace_until now returns EPOCH_ZERO_ISO on ValueError. PRO's evaluate_license checks now < grace_until, so an epoch-zero grace_until deterministically denies with grace_period_expired — the safe, loud failure mode you wanted. The docstring is rewritten to match the actual behavior and explain why fail-closed is intentional (granting a fresh window on garbage input would mask corruption/tampering by silently extending access).
Added test_compute_grace_until_fails_closed_on_malformed_anchor covering empty string, non-timestamp garbage, and wrong format. 40/40 pass.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
backend/app/api/v1/endpoints/system_license.py (1)
170-196: 💤 Low valueDouble read of
license.jsonon every/infocall.
load_license_cachealready parsed the file once;_upgrade_pr02_file_if_presentreads it again to inspect the raw dict. Not a correctness problem, just a small bit of wasted I/O on the hot read path. If you want to keepfrom_dictopaque about what it filled in, an easy alternative is to haveload_license_cachereturn(cache, raw)(or awas_pr02flag) and pass that into the upgrade helper.Happy to leave it as-is if the I/O cost is in the noise for this endpoint — the current implementation is at least correctly fenced behind
OSError/ValueError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/app/api/v1/endpoints/system_license.py` around lines 170 - 196, _load_license_cache currently parses license.json then _upgrade_pr02_file_if_present reads the file again causing redundant I/O; change load_license_cache to return the parsed raw dict (or a was_pr02 flag) along with the LicenseCache and update _upgrade_pr02_file_if_present signature to accept that raw dict/flag so it can call is_pr02_shape on the already-parsed data instead of re-reading the file; update every call site that invokes _upgrade_pr02_file_if_present (and any tests) and keep save_license_cache and the existing logging/exception handling unchanged.backend/tests/endpoints/test_system_license.py (1)
398-423: 💤 Low valueIdempotency check relies on filesystem mtime resolution.
On Linux + ext4 + recent Python this is fine because
st_mtime_nsgives nanosecond resolution and the two/infocalls won't land in the same nanosecond. On filesystems with coarser timestamps (FAT, some macOS configs, certain WSL/Docker bind mounts) two writes inside the same coarse tick could compare equal even if the file was rewritten — which would mask a regression, not produce a false failure. Probably acceptable for CI on Linux, but if you want a stronger signal you could compare file contents (or a hash) instead of mtime, since the assertion you actually care about is "the bytes on disk did not change":- client.get(INFO_URL) # may or may not touch - mtime_after_first = path.stat().st_mtime_ns - client.get(INFO_URL) - mtime_after_second = path.stat().st_mtime_ns - - assert mtime_after_first == mtime_after_second + client.get(INFO_URL) + contents_after_first = path.read_bytes() + client.get(INFO_URL) + contents_after_second = path.read_bytes() + + assert contents_after_first == contents_after_second🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/tests/endpoints/test_system_license.py` around lines 398 - 423, The test test_info_does_not_resave_pr03_file relies on filesystem mtime (path.stat().st_mtime_ns) which can be coarse on some filesystems; instead read the file bytes (the LICENSE_CACHE_FILENAME at path created by save_license_cache) and compare contents (or compute a hash) before and after the second client.get(INFO_URL) to assert the file bytes did not change; keep using the same path variable and INFO_URL and replace the mtime comparisons with a bytes/hash equality check.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@backend/app/core/license_cache.py`:
- Around line 104-118: The loader is setting last_verified_at = activated_at and
computing grace_until from that, which for upgraded PR-02 caches can yield a
past grace window; update the code path that builds the LicenseCache instance
(the constructor call returning cls(...) in license_cache.py) so that
last_verified_at is anchored to the current time when the stored
last_verified_at/activated_at would make grace_until already in the past:
compute last_verified_at = max(parsed_last_verified_or_activated, now_iso) (or
clamp grace_until = max(_compute_grace_until(activated_at), now_iso)) and then
compute grace_until via _compute_grace_until using the adjusted anchor; keep
symbol references: activated_at, last_verified_at, grace_until,
_compute_grace_until and ensure evaluate_license will then see a non-expired
grace window immediately after upgrade.
---
Nitpick comments:
In `@backend/app/api/v1/endpoints/system_license.py`:
- Around line 170-196: _load_license_cache currently parses license.json then
_upgrade_pr02_file_if_present reads the file again causing redundant I/O; change
load_license_cache to return the parsed raw dict (or a was_pr02 flag) along with
the LicenseCache and update _upgrade_pr02_file_if_present signature to accept
that raw dict/flag so it can call is_pr02_shape on the already-parsed data
instead of re-reading the file; update every call site that invokes
_upgrade_pr02_file_if_present (and any tests) and keep save_license_cache and
the existing logging/exception handling unchanged.
In `@backend/tests/endpoints/test_system_license.py`:
- Around line 398-423: The test test_info_does_not_resave_pr03_file relies on
filesystem mtime (path.stat().st_mtime_ns) which can be coarse on some
filesystems; instead read the file bytes (the LICENSE_CACHE_FILENAME at path
created by save_license_cache) and compare contents (or compute a hash) before
and after the second client.get(INFO_URL) to assert the file bytes did not
change; keep using the same path variable and INFO_URL and replace the mtime
comparisons with a bytes/hash equality check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ce316fdf-7382-4748-b118-79521d3650a2
📒 Files selected for processing (3)
backend/app/api/v1/endpoints/system_license.pybackend/app/core/license_cache.pybackend/tests/endpoints/test_system_license.py
Two changes responding to PR #600 review: 1. (CodeRabbit) Anchor pre-PR-03 cache's last_verified_at to load time, not stale activated_at. The previous derivation could yield a grace_until already in the past for a Core that had been installed for weeks before upgrading, putting PRO immediately outside the grace window — exactly the failure mode the upgrade is meant to prevent. The next PRO heartbeat overwrites both fields within ~6h so the upgrade-time anchor only matters until then. 2. (Copilot) _compute_grace_until fails closed on a malformed anchor: returns EPOCH_ZERO_ISO so PRO's evaluate_license denies with grace_period_expired, rather than silently granting a fresh GRACE_DAYS window on garbage input. The old behavior masked potential corruption or tampering by extending access. Tests: 40 passed (was 39 + 1 new fail-closed coverage test). Existing test_load_pr02_file test renamed and updated to assert the new upgrade-time-anchor semantic. test_info_auto_upgrades likewise updated to verify the on-disk last_verified_at is newer than the stale activated_at, not equal to it. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…idempotency Two CodeRabbit nitpicks from the round-1 fix review: 1. /info was reading license.json twice — once via load_license_cache, then again inside _upgrade_pr02_file_if_present to inspect the raw dict for is_pr02_shape. Factored a public load_license_cache_with_raw helper that returns (cache, raw_dict) atomically; load_license_cache stays as the thin Optional[LicenseCache] facade so existing callers (crypto.py, pro_install.py, tests) don't change. /info now calls the with_raw variant and reuses the parsed dict for shape detection. 2. test_info_does_not_resave_pr03_file relied on st_mtime_ns equality. On coarse-resolution filesystems (FAT, some Docker bind-mount setups) two writes inside the same tick could share an mtime and falsely compare equal — masking a regression. Switched to byte equality (path.read_bytes()) which is robust to mtime resolution. Also removed the now-unused json import in system_license.py. Tests: 86 passed (40 system_license + 24 pro_install + 22 crypto). Ruff clean. No public API change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Round-2 review fixes pushed in 774524c. Addressed both CodeRabbit nitpicks from the round-1 review: 1. Double-read of Factored a public
2. mtime-based idempotency assertion (test_system_license.py:398-423 nitpick) Switched Verification: 86/86 pass (40 system_license + 24 pro_install + 22 crypto). Ruff clean. No public API change. |
Blb3D
dismissed
coderabbitai[bot]’s stale review
All three review points addressed across 4288eef (actionable: from_dict anchors last_verified_at to load-time, plus _compute_grace_until fails closed on malformed input) and 774524c (nitpicks: factored load_license_cache_with_raw to drop the double-read in /info; switched the idempotency test from mtime to byte equality). 86/86 tests pass, ruff clean, public API unchanged. CodeRabbit's own follow-up at discussion_r3219550370 confirmed the round-1 approach. Dismissing the stale CHANGES_REQUESTED stamp from before the fix commits.
Summary
Closes the cross-repo schema drift documented in Aeonyx memory #148.
filaops-proPR-03b (2026-05-02) extendedLicenseCachewith four required fields (status,last_verified_at,last_server_timestamp,grace_until) on the assumption Core's matching PR-03 would land in parallel. Core's PR-03 was never opened, leaving the on-disklicense.jsoncontract broken onmainfor 9 days. The break surfaced when alicense-serverimage rebuild (Quoter SPA tarball work) re-bakedfilaops-pro maininto the served wheel; a subsequent--force-recreateon the dogfood VM wipedlicense.json, re-activation wrote Core's 6-field PR-02 shape, PRO'sLicenseCache(**filtered)raisedTypeError,load_cachereturnedNone,license_gatereturned 403no_licenseon every/pro/*route, and/admin/access-requests+/admin/catalogsbroke (with a secondary toast-storm amplifier).This PR extends Core to write the full 10-field shape PRO expects. The four new fields come from data already in the license-server's signed
/validateresponse (server returnsstatus+server_timestampon every call per PR-03a) plus a locally computedgrace_until(activation_moment + GRACE_DAYS, matching PRO'scompute_grace_until).Aeonyx-grounded reconciliation
system_license.pyorlicense_cache.py.filaops_proimports anywhere in this diff.GRACE_DAYS = 14is duplicated as a module constant with a comment noting the contract must stay in sync withfilaops_pro.licensing.cache.GRACE_DAYS.What changed
app/core/license_cache.pyLicenseCachedataclass with safe defaults:status: str = \"active\"last_verified_at: str = \"\"(derived fromactivated_atinfrom_dictif missing)last_server_timestamp: str = EPOCH_ZERO_ISO(sentinel guaranteed older than any real heartbeat — protects the first heartbeat from being rejected as 'rollback' under clock skew)grace_until: str = \"\"(derived fromactivated_at + GRACE_DAYSinfrom_dictif missing)GRACE_DAYS = 14andEPOCH_ZERO_ISOmodule constantsis_pr02_shape(raw)helper for the auto-upgrade detectionfrom_dictnow derives sensible defaults for legacy 6-field cachesapp/api/v1/endpoints/system_license.py/system/license/activatereadsstatus+server_timestampfrom the validate response, computesgrace_until = now + 14d, writes the full 10-field cache/system/license/infocalls_upgrade_pr02_file_if_presentafter load — if the on-disk JSON is missing any PR-03 key, re-save (cache is already PR-03-shaped in memory viafrom_dictdefaults). One-shot, idempotent.LicenseInfoResponsewire schema unchangedtests/endpoints/test_system_license.py_ok_validate_responseto includeserver_timestamp,nonce,signature(matching the real license-server response shape)test_activate_persists_full_pr03_shape— verifies all four fields landtest_activate_falls_back_to_epoch_zero_when_server_omits_timestamptest_activate_carries_server_status_through(e.g.grace_period)test_load_pr02_file_derives_pr03_defaults_from_activated_attest_is_pr02_shape_detects_missing_pr03_fieldstest_info_auto_upgrades_pr02_file_to_pr03_shapetest_info_does_not_resave_pr03_file(idempotency, via mtime)Test plan
pytest tests/endpoints/test_system_license.py— 39 passed (34 original + 5 new), 0 failedpytest tests/core/test_crypto.py tests/endpoints/test_pro_install.py— 46 passed (the other Core test files that import fromlicense_cache)ruff checkclean on all three changed fileslicense.json(now persisted by ecosystem PR Bulk update item type 'Filament' option silently fails #62) should auto-upgrade on first/infocall, after which/admin/access-requestsand/admin/catalogsshould load cleanlyOut of scope (cortex_observe #71 captures these as latent follow-ups)
These three latent issues live on the PRO reader side and need a
filaops-proPR, not a Core PR (Sacred Rule):filaops_pro.licensing.cache.LicenseCache.expires_at: strhas no default — perpetual licenses (Core writesexpires_at = None) would crash construction. Today's BLB3D license has an expiry so this is dormant, but it'll surface for any perpetual customer.tier: Literal[\"community\",\"pro\",\"enterprise\"]doesn't include\"professional\", which is what license-server emits. Python doesn't enforceLiteralat runtime so no crash, but the value semantics are off.status: Literal[\"active\",\"grace\",\"expired\"]doesn't include\"grace_period\"or\"cancelled\", both of which the server can return.Also out of scope: the
AdminAccessRequestsuseEffect toast-storm. Filed as a separate Core PR onfix/toast-storm-usememo(one-lineuseMemowrap on thetoastfactory).🤖 Generated with Claude Code
Agent-Session: a06c11ad-3e32-40f1-9264-a288b7393ea3
Summary by CodeRabbit
New Features
Improvements