Skip to content

Add opt-in pad contexts and shared navigation controls - #232

Merged
delano merged 21 commits into
mainfrom
codex/pad-context-implementation
Oct 2, 2026
Merged

delano merged 21 commits into
mainfrom
codex/pad-context-implementation

Conversation

@delano

@delano delano commented Oct 2, 2026

Copy link
Copy Markdown
Member

Summary

  • Add a default-off multiple-pad experiment with a shared picker in the panel and primary editor, Scratch fallback, per-pad tab/file navigation, folder bindings, and optional application-context routing.
  • Add Command-0…9 pad selection that respects explicit keymap overrides, plus independent day and checkpoint ordering shared by the timeline and editor roll.
  • Expose stable tab UUIDs through the FFI and mint UUIDv7 identifiers for new pads and core items.
  • Include the icon work merged into this branch through Create and package the glass app icon with an internal cast shadow #230: saved Icon Composer artwork, compiled release icons, packaging guards, and an Xcode 27 release-packaging CI job.

Review guide

  • Start with PadCatalog.swift and PageModel.swift for ownership, selection restoration, explicit-file routing, and modal/confirmation guards; then review PadPickerView.swift, PadAssociations.swift, and the timeline/roll integration.
  • Implementation trade-offs: the catalog stores pad names, paths, associations, and ownership in unencrypted UserDefaults metadata; path matching is lexical. UUIDv7 generation embeds creation timestamps. These are implementation observations, not accepted privacy or persistence guarantees.
  • Review the new Swift and Rust tests for catalog reload, stable UUID ownership, routing precedence, keymap overrides, display ordering, and UUIDv7 encoding. Icon tests exercise release/debug assembly and missing, empty, stale, or failed compiler outputs.
  • Release icon packaging now requires Xcode 27 or later; debug packaging retains the black development icon. Review build-icons.sh, package-app.sh, and .github/workflows/ci.yml together.
  • The pad specification is an experimental proposal marked needs-review. docs/qa/pad-context-experiment.md tracks outstanding signed-app, activation, accessibility, and release checks; unit coverage does not establish release acceptance.

Validation

  • No validation commands were run during PR preparation. Pre-commit, pre-push, and CI checks are the validation path; manual checks remain tracked in the QA document.

Related to #230

delano and others added 11 commits October 1, 2026 17:57
Keep one core store while grouping stable tab UUIDs and file paths into local pads. Add exclusive folder bindings, bounded recent-app routing, corruption refusal, and independent projection sorting without filtering lifetime state.

Test catalog restore, numeric ID reminting, inactive expiry, file ownership, and routing precedence. Context metadata remains unencrypted local preferences.
Add shared experimental pad chrome, explicit folder and app controls, compact association header, guarded Command-digit switching, and Timeline before Files. Preserve custom key bindings and disabled-mode navigation.

Keep the paper and rail on one sorted projection and anchor Today correctly in ascending mode. Document the feature proposal, verified gate, and native hardware checks still owed.
Use Unix-millisecond UUIDv7 IDs for new pads and core tab, page, chip, and block identities. Preserve stored UUIDv4 identities and the fixed Scratch sentinel.

Document the timestamp and entropy implications, and verify both generators against RFC 9562 plus legacy restoration. Full Swift and Rust test-util suites and Clippy pass.
Fail early on an unavailable release toolchain and report missing generated metadata or payloads at the icon packaging boundary. Scope digest naming to the icns fallback, specify the mac target device, and make compiler failure tests independent of the installed Xcode version.
Link the exported glass preview, distinguish the renderer study from the saved release composition, and record the black premultiplied shadow invariant. Normalize the Composer JSON final newline.
…hadow

Create and package the glass app icon with an internal cast shadow
# Conflicts:
#	docs/README.md
#	shell/Sources/CompanionKit/SettingsSections.swift
#	shell/Sources/OnetimePad/Views/BackdropRootView.swift
@claude

claude Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

Claude finished @delano's task —— View job


Code review: opt-in pad contexts and shared navigation controls

I read the full diff against the PR base (5248c68), including PadCatalog, PadAssociations, PadPickerView, UUIDv7, the PageModel integration, the keymap/FFI/core changes, the new tests, and the icon/packaging scripts. The two Greptile findings (midnight checkpoint reset, sheet-aware shortcut suspension) do look fixed in 4a0e43b/7e75bc6, and the five earlier threads are addressed in the code as docs/qa/pr232-review-followup.md describes.

Overall: the ownership/reconciliation design is careful, the default-off gate is respected on every path I traced, and the test coverage is unusually good for an experiment of this size. Findings below are new (not duplicates of the resolved threads).

Findings

1 · An in-flight conceal loses its visible outcome on a pad switch (shell/Sources/CompanionKit/PageModel.swift:2826, with finishConceal at :6019)

padCatalogChanged clears concealDraft on every real pad/enabled transition. finishConceal now guards on draft.id == draftID, so when the response lands after the switch it returns early — the flash(Self.linkCopiedLine) on success and the inline draft.error on failure are both dropped, while armClipboardClear() and the ledger receipt already happened. The user can therefore authorize a link, switch pads (including via ⌘-digit, create/remove pad, or the Settings toggle), and get no indication that a link was created and placed on the clipboard — or that it failed.

routeFromApplication (:2769) already refuses to route while concealDraft != nil, and activatePad already refuses while pendingFileClose != nil. The symmetric treatment would be to refuse the switch (with the existing flash pattern) while concealDraft?.inFlight == true, or to emit the success/failure notice even when the originating draft is gone. testPadNavigationDismissesConcealForPagesAndChips covers the idle draft only; the in-flight case is untested.

Related: PadShortcutMonitor.allowsPadSwitch (PadPickerView.swift:296) suspends on modals and sheets, but the conceal confirmation is deliberately inline rather than a sheet, so ⌘-digit remains live over it.

2 · Stored metadata now includes individual file paths, which the disclosure copy does not mention (PadCatalog.swift:168, :153; footer at SettingsSections.swift:219)

fileOwners keys and rememberedFiles values are full paths of files opened into named pads. The Settings footer enumerates three categories — "Pad names, folder paths, and associated app identifiers" — and ADR-0039's proposed catalog list is "pad names, directory paths, application bundle identifiers, and stable tab UUID ownership" (lines 75–77). Per-document paths are a fourth category and arguably the most revealing one. Since the ADR is proposed and this is an experiment, the minimal fix is wording (footer + ADR implementation-proposal paragraph); the alternative is keying file ownership by the core's stable file identity instead of the path. Flagging this as an observation about the disclosure's scope, not a claim about the project's privacy guarantees.

3 · toggleCheckpointSort can no-op with no feedback (PageModel.swift:2865)

When padRosterDate == nil (roster read bracketed across midnight or a UTC-offset change) the toggle returns silently and dateKey returns "", so the control shows the default order. Correct and conservative, but the user presses a visible control and nothing happens. Consider disabling the sort control or flashing when the reference is unavailable, so the deferral is observable.

4 · PadApplicationContext can re-apply a stale previous app (PadAssociations.swift:8, :27)

previousBundleID is never cleared after arrivedFrom fires, and it is only updated for .regular apps. Activating the pad app from an accessory/background app (or twice in a row) re-routes to the last regular app's pad, which can override a pad the user just chose manually. pads.activate(target, recordRecency: false) keeps the MRU intact, so the override is quiet. Clearing previousBundleID after routing would make the hint fire once per real arrival.

Smaller notes

  • PageModel.swift:3584 — flash("Finish the file close decision before opening another file.") is the only new pad-path message still hard-coded; its sibling at :2786 uses CompanionL10n.string("pad.switch.pendingClose"). (The file's older notices are English literals, so this is an internal inconsistency in the new copy rather than a regression.) That guard is also pad-only: with the experiment off, opening a file during a pending dirty-close is still allowed — worth recording as an intended divergence.
  • PadCatalog.remove shifts every later pad's ⌘-digit, so the shortcut meaning changes silently after a removal. The command-held labels in the picker mitigate this; a line in the QA doc would be enough.
  • refresh() (PageModel.swift:2933) builds Calendar.current twice per refresh and timeUnits/sortedPadProjection rebuilds a tab dictionary and re-sorts on every access. Both are small, but timeUnits is read repeatedly per redraw; caching the calendar (invalidating on NSSystemTimeZoneDidChange/.NSCalendarDayChanged) and memoising the projection per tabs revision would avoid the cost growing with roster size.
  • PadCatalog.isEnabled's didSet reassigns isEnabled inside itself when loadFailure != nil, producing a nested didSet and two publishes. It settles on false correctly, but a computed canEnable check at the call site would be less fragile.
  • PadCatalog.removeFolder / PageModel.removeFolder(_:fromPad:) compare the supplied path literally while addFolder stores a normalized path; a non-normalized argument from a future caller would silently no-op. Normalizing on removal too would make the public API symmetric.
  • rememberSelection is invoked from select/selectFile, so each selection change re-encodes and writes the whole catalog to UserDefaults. Fine at today's sizes; just noting the write pattern.
  • UUIDv7.generate correctly handles NaN/±infinity and the 48-bit bound, and ConcealDraft.id is never persisted — but a plain UUID() would do there, since the embedded creation timestamp serves no purpose for an ephemeral offer ID.
  • scripts/build-icons.sh build_glass_icon reads shell/OnetimePad-Info.plist relative to the cwd; consistent with the rest of the script, but it will fail confusingly if invoked from elsewhere. The --glass/--composer separation and the nonempty-output guards read well.
  • FFI: the additive uuid key plus the renamed existing_summary_fields_and_additive_stable_uuid_are_preserved test, and TabSummary.uuid as a defaulted optional, keep decode compatibility in both directions. The clamping change in sheet.rs replaces two panics with documented bounds and the new test pins the random bits across both bounds — good.

Validation

I could not run any build or test command in this environment: the sandbox has no macOS/Swift toolchain, and cargo/swift invocations are outside my allowed tools here, so the Swift and Rust suites, Clippy, the ADR lint, and the icon-script tests were not re-run. My assessment is a code read only; the counts in docs/qa/pr232-review-followup.md and the CI jobs on this PR remain the evidence for test outcomes, and the signed-app, activation, and VoiceOver checks in docs/qa/pad-context-experiment.md are still owed.

  • Read repo guidelines (AGENTS.md, ADR-0039, QA docs)
  • Review PadCatalog / PadAssociations / UUIDv7
  • Review PageModel integration (ownership, routing, guards, reconciliation)
  • Review PadPickerView + keymap shortcut layer
  • Review Rust FFI/core changes
  • Review packaging/icon scripts
  • Review tests and docs
  • Post review

No files were changed; this is review feedback only.
· Branch: codex/pad-context-implementation

@greptile-apps

greptile-apps Bot commented Oct 2, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5 Tier: plus

[Medium risk] Adds opt-in pad picker and navigation, with UUIDv7 identity scheme.

The PR appears safe to merge, with a non-blocking checkpoint-order reset at midnight and the previously reported shortcut sheet limitation still outstanding.

Fix All in Claude CodeFindings

  1. P2 Midnight resets checkpoint order ▶
  2. P2 Shortcut bypasses another window’s sheet ▶
Fix with agent prompt
### Issue 1
shell/Sources/CompanionKit/PageModel.swift:2920-2923
If midnight passes between reading the tab roster and computing these keys, the roster's day offsets refer to yesterday while `Date()` refers to today. Reconciliation can then delete a live day's saved checkpoint order. The user's ordering preference silently resets on the next refresh.

### Issue 2
shell/Sources/CompanionKit/PadPickerView.swift:306-308
If the pad-name sheet is open on a window that is not key, Command-digit shortcuts can still switch pads: this check looks only at the key window. The workspace then changes beneath an unfinished rename. Check all app windows for attached sheets, as the application-context route does.

```suggestion
                      Self.allowsPadSwitch(isModal: NSApp.modalWindow != nil,
                                           hasAttachedSheet: NSApp.windows.contains { $0.attachedSheet != nil },
                                           isSheet: NSApp.windows.contains { $0.sheetParent != nil }),
```

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Reviews (3) · Last reviewed commit: "docs(pads): record Greptile fixes and fo..."

Comment thread shell/Sources/CompanionKit/PageModel.swift
Comment thread shell/Sources/CompanionKit/PageModel.swift
Comment thread shell/Sources/CompanionKit/PageModel.swift Outdated
Comment thread shell/Sources/CompanionKit/PageModel.swift Outdated
Comment thread shell/Sources/CompanionKit/PadCatalog.swift Outdated
delano added 4 commits October 1, 2026 22:30
Keep new identities UUIDv7 without aborting for pre-epoch or overflowing wall clocks. Preserve secure random bits and legacy restored UUIDs, document the tab-summary seam and initializer compatibility, and exercise clock boundaries in Rust and Swift.
Prune stale metadata only after authoritative restoration, avoid no-op writes, and keep Scratch ownership implicit. Support pad rename/removal, remember selected files, retain ownership through disabled Save As, and compare explicit directory aliases without scanning. Add restore-refusal, pruning, lifecycle, and alias regression coverage.
Show create and rename validation, confirm removal within each pad cell, and avoid duplicate activation. Localize experimental controls and disclosures, share rail width with settings, distinguish current sort order from its next action, and suspend pad shortcuts during sheets.
Document clock and directory identity limits, catalog lifecycle and restore safeguards, remembered files and provisional recency. Preserve an itemized response to every Claude observation, the preview-runner investigation, and final automated results. Correct the missing shadows option in icon usage text.
Comment thread shell/Sources/CompanionKit/PadPickerView.swift Outdated
delano added 3 commits October 1, 2026 22:58
Replace wall-clock recency with bounded logical ranks while preserving legacy relative order on read. Promote a manual visit after automatic routing and keep true MRU reselection write-free. Cover future legacy timestamps and soft-route reselection.
Dismiss outgoing conceal offers on pad transitions and distinguish later offers for the same target by transient UUIDv7 identity. Retain dirty-close visibility across experiment toggles, restore Keep Editing through owner-aware navigation, and preserve existing file ownership on reopen. Key checkpoint preferences and pruning by the displayed calendar day. Cover each transition and late success/failure with regression tests.
Distinguish the three new feedback fixes from the two previously addressed observations. Record logical recency, stable Today sorting, stale same-target conceal responses and cross-pad Keep Editing, with the final 1,528-test Swift result and manual checks still owed.
Comment thread shell/Sources/CompanionKit/PageModel.swift Outdated
delano added 3 commits October 1, 2026 23:23
Validate the calendar interval around the core roster read and share its reference with sorting and controls. Defer date pruning and checkpoint writes on ambiguous reads while reconciling tab ownership; cover midnight, timezone changes, and backwards clock steps.
Check the application window roster before handling Command digits. Exercise a real sheet attached to another window and shortcut resumption after dismissal.
Document both remaining findings, the calendar fallback correction, the 1,530-test gate, and the manual checks for midnight refreshes and sheets in other windows.
@delano
delano merged commit 8d68ced into main Oct 2, 2026
7 checks passed
@delano
delano deleted the codex/pad-context-implementation branch October 2, 2026 06:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant