Skip to content

feat(drive,lib,docs): add global settings for the Drive lease (#1677) - #1680

Merged
newhoggy merged 2 commits into
mainfrom
issue-1677-drive-lease-global-settings
Sep 14, 2026
Merged

newhoggy merged 2 commits into
mainfrom
issue-1677-drive-lease-global-settings

Conversation

@newhoggy

Copy link
Copy Markdown
Contributor

Summary

Implements the Global settings scope for the Drive write lease that ADR-0080 §13 designed but deferred (#1664), closing #1677:

  • A new top-level lease section in settings.json (sibling of drive) for default_expiry_minutes, backup_dir, biometrics_only, and allow_headless.
  • A matching OMNI_DEV_DRIVE_LEASE_* env var for each setting.
  • Resolution order: CLI flag → env var → settings.json → hard-coded default (matching resolve_model's precedent), except biometrics_only/allow_headless, which are additive opt-ins (any layer saying "yes" wins).

A second commit closes gaps found in review of the first:

  • The expiry-minutes resolver no longer panics on an out-of-range env/settings value (it now range-checks before constructing the chrono::Duration, instead of relying on a downstream check that never ran for this path).
  • drive lease restore's audit verdict now distinguishes a headless-waived restore (restored-headless-waiver), matching acquire's existing acquired-headless-waiver verdict.
  • Fixed inverted precedence order in the --help text for --backup-dir/--expiry-minutes (it said settings.json beats the env var; the code, docs, and tests all say the opposite).
  • LeaseFlags::resolve() now loads settings.json once instead of twice, using the existing SettingsEnv::from_settings fix built for exactly this (chore(gmail,drive): avoid double JSON parse of error bodies and redundant settings re-reads #1533).
  • Warns when the headless-auth waiver is triggered by an ambient env var or machine-wide setting (not for an explicit per-invocation --allow-headless), and when a malformed env value is silently discarded.

Test plan

  • cargo build, cargo fmt --check, cargo clippy -- -D warnings all clean
  • cargo test — full suite passes (one pre-existing, unrelated daemon-lifecycle flake reproduced and confirmed to pass in isolation)
  • cargo insta snapshot diff reviewed and accepted — only the two intended --help line changes
  • New unit tests for the expiry-minutes range check and the restore headless-waiver verdict

ADR-0080 §13 designed a Global settings scope for `drive lease
acquire`/`restore` (default expiry, backup dir, auth policy,
headless opt-out) alongside the per-account and per-rule scopes,
but only the latter two ever shipped. Add the missing Global
scope: a new `lease` section in settings.json, a matching
OMNI_DEV_DRIVE_LEASE_* env-var layer, and CLI-flag -> env ->
settings.json -> hardcoded-default resolution for all four,
mirroring resolve_model's precedent. The headless/off-macOS
opt-out (§8) is new capability: acquiring without a device-owner
authenticator is now possible via --allow-headless, with the
waiver surfaced in CLI output, JSON output, and the audit log.
Fix the expiry-minutes resolver panicking on an out-of-range
env/settings value instead of erroring, wire the restore path's
headless-waiver flag into its audit verdict for parity with
acquire, correct inverted CLI-help precedence text, load
settings.json once instead of twice in LeaseFlags::resolve, and
warn on ambient or malformed env/settings values that silently
change lease behavior.
@github-actions

Copy link
Copy Markdown

Coverage

Total: 97.26% ⚪ 0 pp vs main

Comparing 8601fe5..fecac7c (merge-base → PR head)

File Before After Δ
src/drive/lease/settings.rs 98.45% 🆕 new
src/cli/drive/lease.rs 99.81% 98.96% 🔴 -0.85 pp
🔇 4 ignored region(s), 0 tolerated region(s)

ignore removes the lines from both reports; tolerate keeps them in the reported percentage but scores them against the baseline, so a cross-run flip cannot move a delta. Regions are read from each revision's own source.

File Kind Lines Rev Reason
src/cli/drive/lease.rs ignore 319-323 both a sheet restore's fresh backup is always a DriveCopy — restore.rs's own recheck refuses unless the target is still a spreadsheet, and acquire() only ever takes a Bytes backup of a non-native target; this arm exists solely for exhaustiveness over the shared LeaseBackup enum
src/cli/drive/lease.rs ignore 793-795 both guards this test helper against misuse; every call site below passes an acquire subcommand
src/drive/lease/ledger.rs ignore 620-624 both mutate_locked refuses before ever calling the closure, so its body never runs — a hit here is a regression, not a coverage gap
src/drive/lease/restore.rs ignore 977-981 both every test using this double refuses before authenticating; a hit here is a regression, not a coverage gap

Patch coverage

Patch: 97.82% (403/412 new lines covered)

File Patch Uncovered new lines
src/cli/drive/lease.rs 94.79% (91/96) 129, 335-338
src/drive/lease/acquire.rs 98% (49/50) 714
src/drive/lease/restore.rs 100% (41/41)
src/drive/lease/settings.rs 98.45% (190/193) 68, 70, 74
src/utils/settings.rs 100% (32/32)
Uncovered new lines (9)
  • src/cli/drive/lease.rs:129
  • src/cli/drive/lease.rs:335
  • src/cli/drive/lease.rs:336
  • src/cli/drive/lease.rs:337
  • src/cli/drive/lease.rs:338
  • src/drive/lease/acquire.rs:714
  • src/drive/lease/settings.rs:68
  • src/drive/lease/settings.rs:70
  • src/drive/lease/settings.rs:74

📦 Full per-file coverage summary · run summary

@newhoggy
newhoggy merged commit 0ac2256 into main Sep 14, 2026
18 checks passed
@newhoggy
newhoggy deleted the issue-1677-drive-lease-global-settings branch September 14, 2026 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant