feat: add Polar.sh license activation, deactivation, status #130
feat: add Polar.sh license activation, deactivation, status #130feynon wants to merge 6 commits into
Conversation
Introduce `tiles license activate|deactivate|status` backed by Polar's
public customer-portal license-key endpoints. Activations are persisted
in the encrypted common DB (single-row `polar_license` table) keyed by a
v7 UUID device id. License type is derived from the key prefix
(`BACKER-` / `COMMERCIAL-`); commercial keys carry an expiry, backer
keys are lifetime. The startup banner now prints the license label
("Tiles 0.4.8 Backer License" / "... Commercial License" /
"... Unlicensed") right after the version.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR introduces Polar.sh license management functionality into the application. A new Sequence Diagram(s)sequenceDiagram
participant CLI as CLI/User
participant Main as main.rs
participant License as license module
participant DB as SQLite DB
participant Polar as Polar API
CLI->>Main: license activate <key>
Main->>License: activate(conn, key)
License->>DB: Check existing license
DB-->>License: No active license
License->>Polar: POST /activate (key, device_id, etc.)
Polar-->>License: Activation response
License->>Polar: POST /validate (activation_id)
Polar-->>License: License details
License->>DB: Store license row
DB-->>License: Success
License-->>Main: Result
Main-->>CLI: Status message
sequenceDiagram
participant CLI as CLI/User
participant Main as main.rs
participant License as license module
participant DB as SQLite DB
participant Polar as Polar API
CLI->>Main: license status
Main->>License: status(conn)
License->>DB: Fetch cached license
DB-->>License: License data
License->>Polar: POST /validate (activation_id)
Polar-->>License: Current expiry/limits
License->>DB: Update cached fields
DB-->>License: Success
License-->>Main: License label/status
Main-->>CLI: Display status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
tiles/src/core/storage/db.rs (1)
55-70: Migration looks correct; flagging vestigialactivations_usedcolumn.The migration is appended at the end of the array (good — preserves ordering for existing installs) and the single-row invariant is properly enforced via
CHECK (id = 1). Themigrations_testwill continue to validate this.One nit: per the PR description, the public Polar customer-portal endpoints do not expose the per-key used count, and
activations_usedis always written asNoneinlicense.rs. The column is effectively dead storage today. That's fine if you intend to populate it later (e.g., from an authenticated/admin endpoint), but consider either dropping it from the schema or adding a code comment that documents the forward-compat intent so future readers don't think the writer is buggy.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tiles/src/core/storage/db.rs` around lines 55 - 70, Migration adds a vestigial activations_used column in the polar_license table; either remove that column from the CREATE TABLE statement (delete the activations_used column token) or explicitly document intent so it's not mistaken for a bug—if you keep it, add a clear comment near the migration SQL mentioning polar_license.activations_used is reserved for future admin/portal population and currently always written as None in license.rs, and update any tests (migrations_test) expectations if needed to reflect the schema change.tiles/src/core/account/license.rs (2)
99-105: Heads-up: blocks license-key swap (BACKER → COMMERCIAL upgrades require manual deactivate first).
activaterejects with a clear error if any row exists, which is correct for guarding accidental re-activation, but it also means a user upgrading from a Backer key to a Commercial key has to runtiles license deactivatefirst — which talks to Polar to release the old activation. If they're offline at upgrade time, they're stuck.Acceptable for v1 given the error message points them at the right command, just calling out the workflow gap so it doesn't surprise support. A follow-up could add
tiles license activate --replace <KEY>that performs deactivate-then-activate atomically.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tiles/src/core/account/license.rs` around lines 99 - 105, The activate flow currently rejects if any existing license row is present (see fetch_license in tiles/src/core/account/license.rs), which blocks key swaps; add an explicit --replace flag to the activate command and update the activate handler to, when --replace is set and fetch_license(&conn.common) returns Some(existing), call the existing deactivation routine (e.g., deactivate_license / the function that releases the activation) before continuing to perform the new activation, preserving the existing logging (mask_key, label_for_type) and ensuring errors from the deactivation call are propagated to the user so offline failures remain visible.
439-523: Consider replacing the custom RFC3339 parser withtimeorchrono.The implementation lacks input validation that a standard library would provide:
- No range checks on
month,day,hour,minute,second— e.g.,2027-13-40T25:99:99Zwould parse and produce an unpredictable result instead of rejecting it.- Timezone format support (
±hh:mmand±hhmm) is partially undocumented in the function comment.- Limited scope compared to battle-tested datetime crates.
Neither
timenorchronois currently in the dependency tree, so adding one is straightforward. The current tests cover only valid formats Polar emits, but input validation would improve robustness. Not blocking, but worth scheduling as a follow-up.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tiles/src/core/account/license.rs` around lines 439 - 523, The custom parser parse_rfc3339_to_unix_seconds (and its helpers days_from_civil / civil_from_days / format_unix_date) lacks full validation and should be replaced with a battle‑tested crate: add either the time or chrono dependency and implement parse_rfc3339_to_unix_seconds by delegating to the crate's RFC3339 parser (e.g., chrono::DateTime::parse_from_rfc3339 or time::OffsetDateTime::parse) and returning the UTC unix seconds; remove the manual parsing logic and helpers or keep them only if still needed for formatting, and update/add tests to cover invalid months/days/hours/minutes/seconds and the supported timezone formats.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tiles/src/core/account/license.rs`:
- Line 149: Run rustfmt by executing `cargo fmt -p tiles` to fix the formatting
CI failure; specifically reformat the println! block (the multi-argument
println!("{} {} (key {})", …) area), the polar_activate function parameter list
so it fits one line, the long byte-equality if chain in
parse_rfc3339_to_unix_seconds, and the timezone-offset if/else chain inside the
+/- branch so they reflow per rustfmt rules; locate these by searching for the
println! call, the polar_activate declaration/usage, and the
parse_rfc3339_to_unix_seconds function and then commit the formatted file.
- Around line 111-149: After successful polar_activate and polar_validate but
before returning, ensure that if upsert_license(&conn.common, &row)? fails you
call polar_deactivate(&client, key, &activation_id).await to release the remote
activation; mimic the validate-failure rollback pattern (i.e., capture the
upsert error, attempt let _ = polar_deactivate(&client, key,
&activation_id).await, then return the original upsert error wrapped as before).
Update the code around the upsert_license call (and related activation_id/row
creation) to perform this rollback on persistence failure.
- Line 199: The call to upsert_license(&conn.common, &row) currently swallows
any error (let _ = ...); change this to handle and log failures so cache-write
errors are visible: call upsert_license(&conn.common, &row) and if it returns
Err(e) (or a Result), emit a warn! or info! with context (e.g., "failed to
refresh license cache" and include e, license id or row-identifying fields) so
transient or chronic cache problems can be observed; do this in the same scope
where conn.common and row are available (the upsert_license invocation).
- Around line 208-220: The days calculation in the match arm for (_, Some(exp))
under-reports time by truncating: replace the simple floor division using
unix_now_seconds() with logic that computes rem = exp - now and then if rem <= 0
mark "EXPIRED", else if rem < 86_400 display hours remaining (e.g., round up
rem/3_600) and otherwise compute days by rounding up (e.g., (rem +
86_399)/86_400) so nearly-full days show as "1 day remaining"; update the suffix
construction and println! that uses format_unix_date(exp) accordingly inside the
(_, Some(exp)) branch that currently references unix_now_seconds(), days,
suffix, and format_unix_date.
---
Nitpick comments:
In `@tiles/src/core/account/license.rs`:
- Around line 99-105: The activate flow currently rejects if any existing
license row is present (see fetch_license in tiles/src/core/account/license.rs),
which blocks key swaps; add an explicit --replace flag to the activate command
and update the activate handler to, when --replace is set and
fetch_license(&conn.common) returns Some(existing), call the existing
deactivation routine (e.g., deactivate_license / the function that releases the
activation) before continuing to perform the new activation, preserving the
existing logging (mask_key, label_for_type) and ensuring errors from the
deactivation call are propagated to the user so offline failures remain visible.
- Around line 439-523: The custom parser parse_rfc3339_to_unix_seconds (and its
helpers days_from_civil / civil_from_days / format_unix_date) lacks full
validation and should be replaced with a battle‑tested crate: add either the
time or chrono dependency and implement parse_rfc3339_to_unix_seconds by
delegating to the crate's RFC3339 parser (e.g.,
chrono::DateTime::parse_from_rfc3339 or time::OffsetDateTime::parse) and
returning the UTC unix seconds; remove the manual parsing logic and helpers or
keep them only if still needed for formatting, and update/add tests to cover
invalid months/days/hours/minutes/seconds and the supported timezone formats.
In `@tiles/src/core/storage/db.rs`:
- Around line 55-70: Migration adds a vestigial activations_used column in the
polar_license table; either remove that column from the CREATE TABLE statement
(delete the activations_used column token) or explicitly document intent so it's
not mistaken for a bug—if you keep it, add a clear comment near the migration
SQL mentioning polar_license.activations_used is reserved for future
admin/portal population and currently always written as None in license.rs, and
update any tests (migrations_test) expectations if needed to reflect the schema
change.
🪄 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: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 51487cad-256e-42c1-b8df-f47066d0a4c4
📒 Files selected for processing (5)
tiles/src/commands/mod.rstiles/src/core/account/license.rstiles/src/core/account/mod.rstiles/src/core/storage/db.rstiles/src/main.rs
feynon
changed the title
Mirrors the existing validate-failure rollback so a DB write error no longer leaves an orphaned remote activation. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The status path silently dropped upsert errors, hiding chronic cache write problems. Surface them via warn! with the masked key and activation id. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Floor-dividing seconds-to-expiry by 86400 made nearly-full days display as "0 days remaining". Use ceiling division and fall back to hours when under a day. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Polar's customer-portal API does not expose per-key activation counts, so the column was always written as NULL. Remove it from the schema, StoredLicense struct, upsert/fetch SQL, and tests. Migration is edited in place since this branch hasn't shipped yet and the dev DB has been wiped. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 participant
Summary
tiles license activate|deactivate|statusbacked by Polar.sh's public customer-portal license-key endpoints.polar_licensetable in the encrypted common DB, keyed by a v7 UUID device id.Tiles 0.4.8 Backer License/... Commercial License/... Unlicensed.Notes
BACKER-/COMMERCIAL-); commercial keys carry an expiry, backer keys are lifetime.expires_atfrom Polar; on transport failure duringstatus, falls back to cached values with an "(offline)" note.Activations allowed: Ndisplays Polar'slimit_activations; the public customer-portal endpoints don't expose a per-key activation count, so "used / N" is intentionally omitted.Test plan
cargo build -p tilescargo test -p tiles --lib -- core::account::license core::storage::db(10 tests pass; newpolar_licensemigration validated)tiles license activate <BACKER-…>→ "Activated Backer License (key ****-XXXX)"tiles license status→ "Backer License", Expires: Never, Activations allowed: 5tiles --no-replbanner:Tiles 0.4.8 Backer Licensetiles license activate <COMMERCIAL-…>→ status shows expiry + days remaining; banner:Tiles 0.4.8 Commercial Licensetiles license deactivate→ releases activation slot via Polar🤖 Generated with Claude Code