feat(notify): add dormant response and cleanup foundation - #113
Conversation
Implement Decision 024 message types 6-9 and an unconnected helper core for exact response authorization, immutable response creation, and authenticated digest-targeted cleanup. Keep the foundation unreachable from runtime entrypoints and preserve existing upload, Trigger v1, Relay, app, packaging, and publication behavior.
Add Go and Swift golden/parser/model/fuzz tests plus Linux confinement, crash, restart, replay, rate, expiry, and race coverage for Decision 024 message types 6-9. Document the test-only proof, compatibility, privacy, retention, and rollback limits while keeping download and roundtrip evidence unset.
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
|
Warning Review limit reached
Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a dormant, test-only Decision 024 response and cleanup foundation. It defines signed CBOR message types 6–9, validates causal chains, persists immutable helper responses, performs authenticated digest-targeted cleanup, and adds Go/iOS fixtures, concurrency tests, privacy checks, and documentation. ChangesM6 response and cleanup foundation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✨ 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 |
psimaker
changed the title
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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 (1)
ios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift (1)
163-172: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the shared clock-skew/lifetime bounds The 120s and 600s limits are duplicated here and in
notify/diagnostics_upload_protocol.go; use named constants on both sides so the cross-language contract can’t drift.🤖 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 `@ios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift` around lines 163 - 172, The clock validation in validateClock uses an inline 120-second skew bound that must be shared with notify/diagnostics_upload_protocol.go, along with the protocol’s 600-second lifetime bound. Define named constants for these limits in both implementations and update the validation logic to reference them, keeping the cross-language values identical.
🤖 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 `@docs/architecture.md`:
- Around line 156-160: Clarify the size terminology at both affected sites: in
docs/architecture.md lines 156-160, describe the response as containing exactly
256 random payload bytes; in CHANGELOG.md line 11, replace “256-byte response”
with “response containing exactly 256 random payload bytes.”
---
Nitpick comments:
In `@ios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift`:
- Around line 163-172: The clock validation in validateClock uses an inline
120-second skew bound that must be shared with
notify/diagnostics_upload_protocol.go, along with the protocol’s 600-second
lifetime bound. Define named constants for these limits in both implementations
and update the validation logic to reference them, keeping the cross-language
values identical.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3e22b448-751b-403e-9799-ec9567b3011a
📒 Files selected for processing (15)
CHANGELOG.mdPRIVACY.mddocs/architecture.mddocs/m6-response-cleanup-readiness.mdios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swiftios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftios/VaultSyncTests/Fixtures/diagnostics-response-m6.jsonnotify/README.mdnotify/diagnostics_contract_model_test.gonotify/diagnostics_response_foundation.gonotify/diagnostics_response_foundation_linux_test.gonotify/diagnostics_response_model_test.gonotify/diagnostics_response_protocol.gonotify/diagnostics_response_protocol_test.gonotify/diagnostics_upload_attestor.go
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*
⚙️ CodeRabbit configuration file
**/*: VaultSync syncs private Obsidian notes through Syncthing. Treat data loss,
privacy leaks, security regressions, and broken sync behavior as high priority.
Do not nitpick formatting unless it affects maintainability, correctness, or public API clarity.
Flag any accidental logging, telemetry, crash reporting, or network transfer of note contents,
vault paths, filenames with private context, API keys, APNs tokens, relay keys, or security-scoped bookmark data.
Files:
ios/VaultSyncTests/Fixtures/diagnostics-response-m6.jsonCHANGELOG.mdPRIVACY.mddocs/m6-response-cleanup-readiness.mddocs/architecture.mdnotify/diagnostics_response_model_test.goios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftnotify/README.mdnotify/diagnostics_upload_attestor.goios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swiftnotify/diagnostics_contract_model_test.gonotify/diagnostics_response_foundation.gonotify/diagnostics_response_protocol_test.gonotify/diagnostics_response_protocol.gonotify/diagnostics_response_foundation_linux_test.go
**/*.{md,sh,go,swift}
📄 CodeRabbit inference engine (README.md)
Keep Relay-side request observation and wake-ups received on the iPhone as separate diagnostics evidence; one must not be treated as proof of the other.
Files:
CHANGELOG.mdPRIVACY.mddocs/m6-response-cleanup-readiness.mddocs/architecture.mdnotify/diagnostics_response_model_test.goios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftnotify/README.mdnotify/diagnostics_upload_attestor.goios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swiftnotify/diagnostics_contract_model_test.gonotify/diagnostics_response_foundation.gonotify/diagnostics_response_protocol_test.gonotify/diagnostics_response_protocol.gonotify/diagnostics_response_foundation_linux_test.go
**/*.md
⚙️ CodeRabbit configuration file
**/*.md: Review public documentation for technical accuracy, privacy/security claims, App Store-facing wording,
setup correctness, and consistency with the free app plus optional Cloud Relay subscription model.
Files:
CHANGELOG.mdPRIVACY.mddocs/m6-response-cleanup-readiness.mddocs/architecture.mdnotify/README.md
docs/**/*.md
📄 CodeRabbit inference engine (docs/troubleshooting.md)
docs/**/*.md: After applying each troubleshooting fix, retry from the app to confirm whether the issue is resolved.
For a missing Device ID or “Sync Engine Not Running,” force-close and reopen VaultSync, keep it foregrounded for 20–30 seconds, confirm the Device ID, rescan the vault, and reboot if necessary.
vaultsync-notifymust read the Syncthing API key fromconfig.xml; permission or authentication failures should be diagnosed as wrong-file or read-permission problems before changing credentials.
Runvaultsync-notifyas the user owning Syncthing’s mode-0600config.xml, or configure the appropriate containerPUID/PGID.
SetSYNCTHING_CONFIGwhen the Syncthing configuration is not auto-detected, and remove or correctSYNCTHING_API_KEYoverrides that replace automatic detection.
For relay failures, verify internet access,RELAY_URL, server egress rules, relay health with curl, and the app’s relay diagnostics; confirm delivery through an updated “Last Trigger Received” timestamp.
A relay HTTP 429 from--doctorcounts as success because it proves the trigger endpoint is reachable; an inactive subscription is a warning fixed through app subscription or provisioning.
Peer-state warnings from--doctormust not fail the command: offline peers are normal, and--healthcheckmust skip peer-state diagnostics.
For missing pending shares, verify the desktop folder shares the exact iOS Device ID, ensure Syncthing is online and unpaused, refresh pending shares, and re-share if needed.
For APNs issues, retry APNs registration, verify a token appears, retry provisioning, and confirm delivery with “Last Trigger Received”; notification permission is not required for silent wake-ups.
When connecting Obsidian storage, chooseOn My iPhone → Obsidianor the vault root containing.obsidian, then confirm detected vaults or pending shares.
VaultSync must not move, recreate, or delete vault folders automatically; recovery from moved or deleted folders requires ma...
Files:
docs/m6-response-cleanup-readiness.mddocs/architecture.md
notify/**/*.{sh,go}
📄 CodeRabbit inference engine (README.md)
The optional notify sidecar must support server-side wake-up requests and must not receive notes, file or folder names, or vault structure.
Files:
notify/diagnostics_response_model_test.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_contract_model_test.gonotify/diagnostics_response_foundation.gonotify/diagnostics_response_protocol_test.gonotify/diagnostics_response_protocol.gonotify/diagnostics_response_foundation_linux_test.go
notify/**/*.go
⚙️ CodeRabbit configuration file
notify/**/*.go: Review goroutine lifecycle, context cancellation, HTTP timeouts, signal handling, debounce behavior,
Syncthing REST API polling, relay API calls, error classification, and API-key handling.
Flag leaked request bodies, note metadata, Syncthing API keys, relay keys, or APNs-related secrets.
Files:
notify/diagnostics_response_model_test.gonotify/diagnostics_upload_attestor.gonotify/diagnostics_contract_model_test.gonotify/diagnostics_response_foundation.gonotify/diagnostics_response_protocol_test.gonotify/diagnostics_response_protocol.gonotify/diagnostics_response_foundation_linux_test.go
**/*.swift
📄 CodeRabbit inference engine (Custom checks)
For Swift background execution changes, pass if work is bounded, cancellation-aware, handles expiration callbacks, and records errors without leaking private vault data. Fail only when background work can continue unbounded, miss cleanup, or violate iOS background execution constraints.
Files:
ios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift
ios/**/*.swift
📄 CodeRabbit inference engine (README.md)
ios/**/*.swift: Develop the iOS/iPadOS app using Swift 6 and SwiftUI, targeting iOS/iPadOS 18 or later.
Use VoiceOver and Dynamic Type throughout the iOS/iPadOS app.
Support localization in English, German, Spanish, and Simplified Chinese.
Files:
ios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift
⚙️ CodeRabbit configuration file
ios/**/*.swift: Focus on Swift 6 strict concurrency, Sendable/MainActor correctness, Task cancellation,
retain cycles, memory pressure, SwiftUI observation state, StoreKit/APNs flows, and iOS background execution limits.
Pay special attention to BGAppRefreshTask and BGContinuedProcessingTask behavior, expiration handling,
bounded work, and cleanup when the app is suspended or terminated.
Files:
ios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift
ios/**/*.{swift,plist}
📄 CodeRabbit inference engine (README.md)
ios/**/*.{swift,plist}: Use BGAppRefreshTask and BGContinuedProcessingTask when available for background processing, while allowing iOS to decide whether and when the app runs.
Implement APNs silent push handling for optional Cloud Relay wake-ups.
Files:
ios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Treat Decision 024 as the unchanged canonical contract for message types 6–9.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Accept only exact deterministic-CBOR schemas with the required Ed25519 signature domains, key IDs, epochs, homeserver/folder bindings, operation ID, digest chains, TTL, and clock-skew bounds.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Generate a fresh nonzero 32-byte response nonce and exactly 256 random response payload bytes; sign the complete causal digest chain.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Atomically persist the immutable response before returning the fixed accepted in-process result.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Ensure the exact persisted response wins across duplicate calls, concurrency, and helper restart; a different authorization must not overwrite or reuse it.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Allow a helper response only when the confined namespace contains the exact signed operation request and helper upload attestation, and an app-signed authorization binds both message digests.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Accept cleanup requests containing one to three sorted, unique message digests, and map them only to the three fixed operation artifact paths; the request must never supply a path.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Reopen every cleanup candidate through the M4 confined handle immediately before deletion and validate canonical content, signature, tuple, operation, file identity, and digest.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Remove helper-authored attestation/response artifacts only after the exact app-signed cleanup request; retain live app-authored requests until expiry plus the allowed clock-skew boundary.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Reject expired cleanup requests before candidate reads or deletions because they cannot produce a live acknowledgement.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Return signed `already_absent` only for an unambiguous missing exact artifact; retain changed, invalid, symlinked, hard-linked, swapped, or otherwise unverifiable files as conflicts.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Never target the root, README, manifests, authorizations, credentials, parent paths, user files, backups, versions, or tombstones for cleanup.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Make cleanup crash-safe: after deletion but before acknowledgement, an explicit retry must return `already_absent`; do not add background scans, permanent polling, separate operation databases, or cleanup-derived evidence.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Use the existing bounded per-app and helper-wide direct-request coordinator for response authorization and cleanup, including invalid bodies.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Do not place contract identifiers, keys, bindings, operations, nonces, digests, payloads, results, or cleanup state in Relay, APNs, StoreKit, logs, telemetry, crash annotations, support bundles, UserDefaults, Keychain, or a durable helper operation store.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Preserve existing app/helper and Trigger v1 behavior, and do not construct the foundation from any runtime entrypoint.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Keep deterministic test keys and random-looking test payloads confined to test fixtures; they are not production credentials.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Do not treat backup, Syncthing version, remote-history, or tombstone copies as valid cleanup evidence.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Do not schedule fixed cleanup retry delays or a helper startup scan in M6; these belong to a later runtime milestone.
Learnt from: CR
Repo: psimaker/vaultsync
Timestamp: 2026-07-14T00:38:53.819Z
Learning: Do not authorize helper publication or App runtime until fixed endpoints, TLS/SPKI/application authentication, explicit configuration, least-privilege packaging, upgrade, downgrade, and both rollback directions are proven.
📚 Learning: 2026-07-12T23:03:04.680Z
Learnt from: psimaker
Repo: psimaker/vaultsync PR: 107
File: ios/VaultSyncTests/DiagnosticsContractTests.swift:39-46
Timestamp: 2026-07-12T23:03:04.680Z
Learning: In iOS Swift tests that use CryptoKit’s `Curve25519.Signing.PrivateKey.signature(for:)` (Ed25519), don’t assert that a generated signature’s bytes exactly match deterministic “golden”/fixture signatures. CryptoKit signatures may be randomized (different but valid for the same key+message). Instead, verify correctness by calling `isValidSignature` (or equivalent) against (1) the golden bytes and (2) the freshly generated signature, and avoid byte-for-byte equality assertions between CryptoKit output and reference vectors.
Applied to files:
ios/VaultSyncTests/DiagnosticsResponseM6Tests.swiftios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift
🪛 GitHub Check: GitGuardian Security Checks
ios/VaultSyncTests/Fixtures/diagnostics-response-m6.json
[error] 1-1: GitGuardian detected hardcoded secret(s): Generic High Entropy Secret (GitGuardian IDs: 34802288, 34802290, 34802291, 34802289) in commit 5b197e7.
🔇 Additional comments (23)
PRIVACY.md (1)
84-91: LGTM!Also applies to: 101-103
docs/m6-response-cleanup-readiness.md (1)
1-94: LGTM!notify/README.md (1)
168-190: LGTM!notify/diagnostics_response_protocol.go (2)
302-375: LGTM!
130-140: 🔒 Security & PrivacyNo issue: canonical CBOR ordering is enforced upstream
decodeDiagnosticsCBORre-encodes the parsed value and rejects any byte mismatch, so reordered maps never reach this validator.> Likely an incorrect or invalid review comment.notify/diagnostics_response_protocol_test.go (1)
159-351: LGTM!ios/VaultSyncTests/Fixtures/diagnostics-response-m6.json (1)
1-38: 🔒 Security & PrivacyGitGuardian finding is a false positive — allowlist rather than rotate.
The flagged high-entropy strings are the pinned Ed25519 signatures/nonces/payload of a deterministic protocol golden vector, not live credentials, and they carry no notes, filenames, vault paths, or real API/APNs/relay keys. Add a GitGuardian ignore entry (e.g.
.gitguardian.yamlor an inline# ggignore) scoped to this fixture path so the security gate stops failing; treating these as leaked secrets and rotating anything would be incorrect.Based on learnings: "Keep deterministic test keys and random-looking test payloads confined to test fixtures; they are not production credentials."
Sources: Learnings, Pipeline failures
ios/VaultSyncTests/DiagnosticsResponseM6TestSupport.swift (1)
398-419: LGTM!ios/VaultSyncTests/DiagnosticsResponseM6Tests.swift (1)
5-152: LGTM!notify/diagnostics_response_foundation.go (5)
77-119: LGTM!
121-239: LGTM!
241-294: LGTM!
362-471: LGTM!
295-307: 🩺 Stability & AvailabilityCleanupOwned already revalidates before unlinking. It rereads each candidate with
readImmutableFromParent, compares it against the storedidentityanddigest, and only then callsRemove; any changed or swapped file is returned aserrDiagnosticsNamespaceConflict.> Likely an incorrect or invalid review comment.notify/diagnostics_upload_attestor.go (2)
315-338: LGTM!
354-358: LGTM!notify/diagnostics_response_foundation_linux_test.go (4)
17-206: LGTM!
208-242: LGTM!
244-500: LGTM!
502-593: LGTM!notify/diagnostics_response_model_test.go (1)
5-80: LGTM!notify/diagnostics_contract_model_test.go (2)
291-291: LGTM!Also applies to: 320-323, 348-371
464-514: LGTM!
Clarify that the immutable response contains exactly 256 random payload bytes. Name the Swift lifetime and clock-skew bounds to mirror the existing Go protocol constants.
1 participant
Summary
Scope boundaries
maincallsite, listener, endpoint, capability response, advertised flag, automatic discovery, trust adoption, namespace creation, retry scheduler, or startup scan.ItemFinishedacceptance, causal roundtrip, helper packaging, publication, deployment, or rollout.Local verification
go test ./... -count=1innotify: pass on macOS and network-isolated Linux.go test -race ./... -count=1innotify: pass on macOS and network-isolated Linux.go vet ./...: pass.govulncheck ./...innotify: no vulnerabilities found.govulncheck -tags noassets ./bridge/...: no called vulnerabilities found.xcodebuild build testwithCODE_SIGNING_ALLOWED=NO: pass.Compatibility, privacy, and rollback
The strongest new proof is local/test-only response creation and authenticated cleanup. Upload remains the existing M5 test/mock proof; download and roundtrip remain unset. No operation data enters logs, durable helper state, Relay, APNs, StoreKit, UserDefaults, Keychain, or support/crash data. Live cleanup cannot erase backups, versions, remote history, conflicts, or tombstones.
Source rollback is a normal revert and changes no runtime credentials, mappings, namespace roots, user data, Trigger v1, Relay wire behavior. VaultSync 2.0 remains NO-GO. Issues #91 and #97 remain open and unchanged.
Gate
The standing owner GO authorizes Ready and exact-head squash merge only after green PR CI, completed automated review, no unresolved review threads, and re-verification of the exact head and attribution-free metadata.