Community 1.1: daemon-owned core and Apple application finalization - #26
Community 1.1: daemon-owned core and Apple application finalization#26bob-codedaptive wants to merge 13 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 940cab3489
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // estate at glk-estate.sqlite in the layout directory. In production this | ||
| // is a fully encrypted estate; in contract tests it is plaintext (both | ||
| // paths share this same code — the key provider handles the difference). | ||
| let glkEstateURL = layoutURL.appendingPathComponent("glk-estate.sqlite") |
There was a problem hiding this comment.
Capture, lifecycle, and review all open layoutURL/estate.sqlite, but this composition creates a separate glk-estate.sqlite and gives its handle to the Obsidian and transfer coordinators. Consequently, captures made through the Community UI are absent from exports and Obsidian sync, while imported records are written to a database that capture, review, and the daemon descriptor never expose. Open the canonical estate through GeniusLocusKit instead of creating a second estate.
Useful? React with 👍 / 👎.
| guard !isOperationInFlight else { return } | ||
| isOperationInFlight = true | ||
| defer { isOperationInFlight = false } | ||
| exportDestinationOutcome = await port.selectExportDestination() |
There was a problem hiding this comment.
When an export plan already exists, selecting or cancelling a different destination only replaces exportDestinationOutcome; exportPlan remains permitted, so the Execute button stays enabled and submits the old token, whose daemon-side record still names the previous destination. This can export data to a location the user just replaced or cancelled. Clear the plan and downstream job state whenever the destination or scope changes; selectImportSource() has the analogous stale-plan path.
Useful? React with 👍 / 👎.
| groups = entitlements.get("com.apple.security.application-groups", []) | ||
| if "group.com.codedaptive.mootx01" not in groups: | ||
| raise SystemExit("profile does not grant group.com.codedaptive.mootx01") |
There was a problem hiding this comment.
Both release jobs pass the daemon provisioning profile through this check, but it requires the bare group.com.codedaptive.mootx01 value while the generated daemon entitlement and this commit's Community artifact verifier require the team-prefixed value (<TEAM>.group.com.codedaptive.mootx01). A correctly provisioned release profile therefore reaches this branch and aborts both macOS package builds before signing. Validate the same prefixed spelling used by the signed entitlement, optionally allowing an explicitly supported alternate form.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8bbfed563
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let urlString = String(data: bookmark, encoding: .utf8), | ||
| let vaultURL = URL(string: urlString), | ||
| vaultURL.isFileURL |
There was a problem hiding this comment.
The shipped macOS UI sends opaque data from URL.bookmarkData(options: .withSecurityScope) in DaemonCommunityFeaturePorts.swift, not a UTF-8 URL string, so this conversion normally fails and every real Obsidian vault selection is denied before the directory check. CommunityTransferCoordinator.resolveBookmarkToURL repeats the same assumption, making import-source and export-destination selection fail as well; resolve the bookmark with the bookmark APIs and retain security-scoped access for the operation instead.
Useful? React with 👍 / 👎.
| // Job is running — it will cancel at the next write-window boundary. | ||
| // Return cancelled{beforeCommit} optimistically; the Task will update to | ||
| // the correct stage (duringCommit or afterCommit) when it terminates. | ||
| // The sidecar update happens asynchronously in the Task. | ||
| log.info("jobCancel: running job cancel requested: \(jobID, privacy: .public)") | ||
| return JobCancelOutcome.cancelled(stage: .beforeCommit).toJSONValue() |
There was a problem hiding this comment.
When cancellation arrives after a job has entered running, this returns cancelled(beforeCommit) even though both job runners perform the entire importSeed or export call before checking the flag again. The UI stores this response verbatim, so it can tell the user that no work was committed while the import or export is still writing and may later finish with afterCommit; wait for the runner's actual stage or otherwise avoid returning the before-commit guarantee.
Useful? React with 👍 / 👎.
| self.plans = [:] | ||
| self.cancelFlags = [:] | ||
| // Load persisted jobs from sidecar; start fresh if missing or corrupt. | ||
| self.jobs = Self.loadJobs(at: layoutURL) |
There was a problem hiding this comment.
If the daemon restarts while a persisted job is queued or running, initialization reloads that state but recreates neither its background task nor its cancellation flag. Subsequent status calls therefore report the nonterminal state forever, and an exact execute retry only returns the existing job ID without restarting work; reconcile loaded nonterminal records by safely resuming them or transitioning them to an explicit interrupted/failed terminal state.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bcd1945249
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| servingState = .active(fd: fd, port: actualPort, credential: credential, serverTask: serverTask) | ||
| return LANStartOutcome.started(endpoint: endpoint, authentication: .valid).toJSONValue() |
There was a problem hiding this comment.
When LAN serving starts, every /records request requires this bearer token, but the production response exposes only the endpoint and authentication state; the only token accessor in the repository is the test-only testToken(). Consequently, even an authority-approved start creates a server that no remote client can authenticate to. Add a secure production pairing or credential-delivery path before reporting the service as usable.
Useful? React with 👍 / 👎.
| if let recovered = allDrawers.first(where: { $0.addedBy == addedByMarker }) { | ||
| // The previous attempt committed to the estate. Rebuild the ledger | ||
| // entry so future retries take the fast path, then return the receipt. | ||
| let contentHash = captureContentHash( | ||
| content: arguments.content, | ||
| subject: arguments.subject | ||
| ) | ||
| let recoveredEntry = LedgerEntry( |
There was a problem hiding this comment.
If the daemon crashes after estate.capture() commits but before the ledger write, a retry with the same request ID but different content, destination, or policy enters this recovery branch. It finds the old drawer by addedBy but constructs the recovered ledger entry from the new arguments without comparing them to the committed drawer, then returns the old record as successfully applied; this defeats the documented request-conflict guarantee and permanently associates the old record with the new payload hash and policy. Validate the recovered drawer against the retry or reconstruct the original receipt without accepting conflicting arguments.
Useful? React with 👍 / 👎.
| let receiptID = metadata?.receiptID ?? UUID().uuidString.lowercased() | ||
| return LifecycleMCPResponse.wrap(LifecycleStateBuilder.ready( | ||
| estate: estateSummary, | ||
| receiptID: receiptID |
There was a problem hiding this comment.
For an existing estate whose new estate-metadata.json sidecar is absent or unreadable, every inspect() call generates a fresh receipt UUID here rather than synthesizing and persisting stable metadata. This is also reached after create() silently fails to write the sidecar, so reconnects present different receipt identities for the same estate and violate the model's stable-receipt contract. Derive or persist one stable receipt ID, and avoid reporting creation success when that persistence fails.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e62237049
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| locusEstate = try await Estate.open( | ||
| storage: storage, | ||
| owner: OwnerCredentials(ownerIdentifier: ownerIdentifier), |
There was a problem hiding this comment.
On every fresh production install, DaemonProvider.activate() calls this host before the community dispatcher becomes reachable, and Estate.open creates the missing estate.sqlite. Consequently, the lifecycle coordinator can never return needsCreation; its create(name:) path is always refused because the file already exists, so the first-run UI cannot create or name the estate. Activation needs a non-creating readiness state, or the automatically created estate must still be surfaced as requiring lifecycle setup.
Useful? React with 👍 / 👎.
| // secret tier: never exports (excluded above by .sensitivityAtMost(.secret)) | ||
| // private tier: excluded unless scope is .believedIncludingPrivate | ||
| let privateExcluded = exportScope.includesPrivateTier ? 0 : | ||
| allInScope.filter { $0.sensitivity == .restricted }.count |
There was a problem hiding this comment.
When an existing or imported secret-tier drawer is also marked exportable, .sensitivityAtMost(.secret) includes it—the ceiling includes every tier through secret—but this partition counts only .restricted drawers as excluded. The plan therefore overstates estimatedTransferCount and may permit execution when every candidate is secret, while VaultBridge.export later excludes those records and writes an empty export. Partition secret-tier records here using the same rules as DrawerMapping.export.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad6045523c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let key = try KeychainKeyStore( | ||
| service: "com.codedaptive.mootx01", | ||
| estateURL: url, | ||
| accessGroup: "com.codedaptive.mootx01.shared" |
There was a problem hiding this comment.
In a signed production daemon, the entitlement and ProviderEligibility.expandedKeychainGroup use <TEAMID>.com.codedaptive.mootx01.shared, but the estate opener passes the bare suffix directly as kSecAttrAccessGroup. Because KeychainKeyStore does not expand this value, the initial estate.openEstate() during provider activation cannot read or create the database key and fails with a Keychain entitlement error, preventing the resident daemon from starting. Thread the already-validated expanded group into both estate key-provider closures.
Useful? React with 👍 / 👎.
| let currentFingerprint = CommunityReviewEngine.estateFingerprint( | ||
| activeDrawers: drawers.filter { $0.tombstonedAt == nil } |
There was a problem hiding this comment.
When captureChoices() has initialized an empty estate, it creates a system:capture_choices sentinel. generateSession() excludes that sentinel when producing sourceEstateState, but this staleness check includes every non-tombstoned drawer, so the fingerprints can never match and every first-time review action returns staleSession even when no user data changed. Apply the same non-system predicate used by session generation and duplicate resolution; the re-apply fingerprint above has the same mismatch.
Useful? React with 👍 / 👎.
| // ideally merges the older drawer's content into the newer before archiving, but | ||
| // Estate.mutate() is internal to LocusKit. Both choices produce the same archive | ||
| // effect here; content merge is deferred pending a public mutation API. |
There was a problem hiding this comment.
When the user selects the advertised “Merge content into the newer record” choice, choiceIndex never affects execution and the coordinator immediately archives every older drawer exactly as it does for the keep-newer choice. For same-subject duplicates whose contents differ, this reports the merge as applied while making the older content disappear without copying it into the retained drawer. Until an actual mutation API is wired, refuse or omit this choice rather than performing the destructive archive-only operation.
Useful? React with 👍 / 👎.
1 participant
Outcome
Publishes the complete Community 1.1 daemon-owned core and macOS Community application projection.
Measured gates
Release gate
No release tag is created by this PR. Final publication remains gated on the physical launchd acceptance ledger.