v1.6.0: 关系洞察与轻触达（Phases 13–18） - #15
Merged
Merged
Conversation
- Lock QUAL-01 soft-delete, D-02 statuses, A1 PROCESSING exclusion - Encode D-03 exact person match, D-04 other, aggregateForPerson shape Co-authored-by: Cursor <cursoragent@cursor.com>
- Lock QUAL-01 soft-delete and QUAL-02 local MM-DD prior-year matching - Encode D-01 leap-day cases, D-02/A1 statuses, optional person filter Co-authored-by: Cursor <cursoragent@cursor.com>
- Lock growthStageFromRate thresholds at 0.2/0.4/0.6/0.8 - Assert null/zero seed and just-below-boundary previous stages Co-authored-by: Cursor <cursoragent@cursor.com>
- Assert localDayKey equals formatDate for local fixtures - Assert localMonthDayKey zero-padded MM-DD including leap Feb 29 Co-authored-by: Cursor <cursoragent@cursor.com>
- Add localDayKey (delegates to formatDate) and localMonthDayKey (MM-DD) - Re-export from shared/formatting barrel; retarget reviewStatsWeather - Mock expo-localization in localDayKeys tests so date module loads Co-authored-by: Cursor <cursoragent@cursor.com>
- Pure GrowthStageId ladder at 0.2/0.4/0.6/0.8 thresholds - Null/undefined rates map to seed; no React/i18n imports Co-authored-by: Cursor <cursoragent@cursor.com>
- Soft-delete first via excludeSoftDeletedEntries (QUAL-01) - D-02/A1 status gate; exact people.includes; other first-class - Stub aggregateForPerson until task 2 Co-authored-by: Cursor <cursoragent@cursor.com>
- PersonTimelineAggregate from entriesForPerson output only - resolveRate / latestTimestamp / growthStage for Phase 14 header Co-authored-by: Cursor <cursoragent@cursor.com>
- Soft-delete first; local MM-DD via localMonthDayKey (QUAL-01/02, D-01) - D-02/A1 status gate; person path reuses entriesForPerson (D-03) - Mock expo-localization in OTD Jest so date.ts import loads Co-authored-by: Cursor <cursoragent@cursor.com>
- Rely on shared growthStageFromRate once services stop importing components Co-authored-by: Cursor <cursoragent@cursor.com>
- getGrowthStage delegates stage id via growthStageFromRate; keep label/icon in UI - gardenMilestone drops services→components import; re-exports GrowthStageId Co-authored-by: Cursor <cursoragent@cursor.com>
- buildShareCardModel resolveGrowthStage uses growthStageFromRate - resolveRevisitSubtitleKey drops private threshold ladder Co-authored-by: Cursor <cursoragent@cursor.com>
reviewStatsWeather imported localDayKey via shared/formatting barrel, which loaded date.ts → i18n → expo-localization and broke Jest. Move pure YYYY-MM-DD / MM-DD helpers to localCalendar.ts. Co-authored-by: Cursor <cursoragent@cursor.com>
- Lock zh-Hans/en-US copy for screen, metrics, empties, a11y, errors - Primary CTA remains 去记一笔 / Go record per UI-SPEC Co-authored-by: Cursor <cursoragent@cursor.com>
- Wire personTimeline into i18n resources and ns array - Add insights.relationship.empty.cta/ctaA11y and potA11yHint Co-authored-by: Cursor <cursoragent@cursor.com>
- Cover null→em dash, relative 0/1/6, ≥7 zh month-day glyphs - Assert never returns raw epoch string Co-authored-by: Cursor <cursoragent@cursor.com>
- Null → em dash; local 0..6 → formatRelativeDayLabel - ≥7 locale month-day with zh-Hans 月/日 fallback - Re-export from shared/formatting barrel Co-authored-by: Cursor <cursoragent@cursor.com>
- Cover undefined/array/empty and no trim/case-fold Co-authored-by: Cursor <cursoragent@cursor.com>
- Coerce string|string[]|undefined without trim/case-fold - Empty string and empty array → undefined for missing person Co-authored-by: Cursor <cursoragent@cursor.com>
- Compose hero/metrics/FlashList EntryCards from entriesForPerson + aggregateForPerson - Empties for missing person and zero rows; audio stop on blur - Register personTimeline keys in i18next CustomTypeOptions (FlashList v2 has no estimatedItemSize) Co-authored-by: Cursor <cursoragent@cursor.com>
- Thin Expo Router entry mirroring recycle-bin - Stack.Screen person-timeline with headerShown false Co-authored-by: Cursor <cursoragent@cursor.com>
- Use listDistinctPeople + aggregateForPerson for pot stats - Keep ascending resolveRate sort and top-5 slice Co-authored-by: Cursor <cursoragent@cursor.com>
- Pot Pressable navigates to /person-timeline with exact person tag - Empty CTA routes to /record without opening EntryEditor - Add pot a11y hint via resolvePeopleLabel display name Co-authored-by: Cursor <cursoragent@cursor.com>
- zh-Hans/en-US Copywriting Contract keys (D-01/D-05) - empty title/body only — no Record CTA key Co-authored-by: Cursor <cursoragent@cursor.com>
- Cover empty, single-year, multi-year header emission, and entry-count preservation Co-authored-by: Cursor <cursoragent@cursor.com>
- Flatten MoodEntry rows into year|entry FlashList items when local year changes - Presentation only — no soft-delete or MM-DD re-filter Co-authored-by: Cursor <cursoragent@cursor.com>
- Cover zh-Hans 月/日 spaced form, en-US human month-day, local getters Co-authored-by: Cursor <cursoragent@cursor.com>
- zh-Hans Memory Lane hero as M 月 D 日 with spaces; en-US via Intl - Re-export from shared/formatting barrel; no slash formatMonthDay Co-authored-by: Cursor <cursoragent@cursor.com>
- Wire zh/en resources + ns beside personTimeline
- Type useTranslation('onThisDay') via i18next.d.ts
Co-authored-by: Cursor <cursoragent@cursor.com>
- AppScreenShell + FlashList year beads / EntryCard spine - Sole data via entriesOnThisDayPriorYears + buildOnThisDaySections - Empty keeps date hero without Record CTA; audio cleanup on blur Co-authored-by: Cursor <cursoragent@cursor.com>
![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: 0e15a7d78c
ℹ️ 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".
Comment on lines
193
to
197
| useEffect(() => { | ||
| if (!isExpanded && isActivePlaybackEntry) { | ||
| const becameCollapsed = wasExpandedRef.current && !isExpanded; | ||
| wasExpandedRef.current = isExpanded; | ||
| if (becameCollapsed && isActivePlaybackEntry) { | ||
| useAppStore.getState().stopAudio(); |
There was a problem hiding this comment.
When FlashList recycles an EntryCard, useRecyclingState resets isExpanded for the new entry.id, but wasExpandedRef still contains the previous item's expanded value. If that recycled instance is reused for the entry whose audio is currently playing, the reset to false satisfies becameCollapsed and calls stopAudio, so scrolling/recycling can still kill playback even though the user did not collapse the active card. Reset the ref in the recycling reset callback or track the previous entry id before treating false as a user collapse.
Useful? React with 👍 / 👎.
| type LocalePreference, | ||
| } from "@/services/localeSettings"; | ||
| import { setOnboardingMetaphorSeen } from "@/services/onboardingMetaphor"; | ||
| import { getStorageKey, saveToStorage } from "@/store/modules/storage"; |
There was a problem hiding this comment.
This new file lives under services/, but it imports @/store/modules/storage, adding a services→store dependency for the dev seed path and breaking the repo's layer boundary. Move the storage-key/read-write helpers to a neutral shared/lib utility, or perform the write from the route/store boundary instead of from a service.
Useful? React with 👍 / 👎.
- Lock QUAL-03 whitelist shape, empty D-09 defaults, and forbid-list/PII absence - Soft-delete exclusion and weather/growth domain-math expectations Co-authored-by: Cursor <cursoragent@cursor.com>
- Add WidgetSnapshot types, SCHEMA_VERSION, FORBIDDEN_KEYS - Pure builder reuses growthStage + soft-delete; inlines weather/rate math - Fix domain-math growthStage expectation (1/2 → seedling) Co-authored-by: Cursor <cursoragent@cursor.com>
- MemorySink round-trip + double-clear idempotency (D-11) - NoOpSink write/clear resolve; read null (D-04) - Whitelist shape via buildWidgetSnapshot (QUAL-03) Co-authored-by: Cursor <cursoragent@cursor.com>
- Interface + WIDGET_SNAPSHOT_KEY / APP_GROUP_ID in sink.ts - MemorySink in-memory slot; clear idempotent (D-08/D-11) - NoOpSink web-safe write/clear/read null (D-04) - Barrel re-exports; no AsyncStorage Co-authored-by: Cursor <cursoragent@cursor.com>
- Cover publish→Memory whitelist write and clear→null - Assert service source has no store/components/AsyncStorage imports Co-authored-by: Cursor <cursoragent@cursor.com>
- Args-in publish via buildWidgetSnapshot + sink.write - Jest Memory / web NoOp / lazy native require with NoOp fallback - Test injection helper; no store/components/AsyncStorage sole sink Co-authored-by: Cursor <cursoragent@cursor.com>
…lding - iOS App Group UserDefaults + Android MODE_PRIVATE SharedPreferences - write/clear/read one JSON key; Privacy Manifest 1C8F.1 documented - app.json entitlements + config plugin; no expo-widgets / SDK bump Co-authored-by: Cursor <cursoragent@cursor.com>
- Hook publishWidgetSnapshot after set({ weather }) in _calculateWeather
- Flush snapshot on AppState background/inactive beside audio teardown
Co-authored-by: Cursor <cursoragent@cursor.com>
- await clearWidgetSnapshot in logout (!user/success/catch) and deleteAccount - clear on SecureStore persist-failure signOut in useAppStore - mock widgetSnapshot in weather unit tests after publish hook import Co-authored-by: Cursor <cursoragent@cursor.com>
- Wave 0 Jest locks cleared-vs-status Soft Stack mapping (PLT-01) - Assert Dashboard deep-link constant emotiondiary:/// (PLT-02) Co-authored-by: Cursor <cursoragent@cursor.com>
- Pure mapSnapshotToChrome with cleared vs status Soft Stack view-model - Weather/growth icon+title tables and WIDGET_DEEP_LINK_URL=emotiondiary:/// Co-authored-by: Cursor <cursoragent@cursor.com>
- SnapshotReader reads App Group suite widget_snapshot_v1 only - Soft Stack A systemSmall chrome + emotiondiary:/// widgetURL - README documents ios-widget layout and dual App ID App Group setup Co-authored-by: Cursor <cursoragent@cursor.com>
- Call WidgetCenter.reloadAllTimelines after App Group mutate - Link WidgetKit in podspec; document logout-safe refresh in README Co-authored-by: Cursor <cursoragent@cursor.com>
- Required so WidgetCenter.reloadAllTimelines resolves at link time Co-authored-by: Cursor <cursoragent@cursor.com>
- WidgetSnapshotProvider reads MODE_PRIVATE prefs + Soft Stack RemoteViews - ~2x2 info XML, Material weather vectors, Dashboard PendingIntent - Document android-widget layout for Plan 17-04 prebuild merge Co-authored-by: Cursor <cursoragent@cursor.com>
- Broadcast ACTION_APPWIDGET_UPDATE for WidgetSnapshotProvider - Share prefs key/suffix constants with Soft Stack provider - Document Android widget refresh parity with iOS timelines Co-authored-by: Cursor <cursoragent@cursor.com>
- Hand-roll iOS EmotionDiaryWidget + Android WidgetSnapshotProvider injection - Declare EAS appExtensions + App Group; document prebuild/EAS policy Co-authored-by: Cursor <cursoragent@cursor.com>
- Create PBXTargetDependency sections when node-xcode skips them - Avoid duplicate Swift file refs from addPbxGroup Co-authored-by: Cursor <cursoragent@cursor.com>
- README records iOS prebuild evidence and android/EAS verification route - 17-UAT.md written locally under .planning (gitignored) Co-authored-by: Cursor <cursoragent@cursor.com>
Wire widget-snapshot via Yarn so Soft Stack stops falling back to NoOp; fill containerBackground edge-to-edge; only publish while signed in so logout clear is not overwritten by background republish. Co-authored-by: Cursor <cursoragent@cursor.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
🧹 Nitpick comments (6)
modules/widget-snapshot/app.plugin.js (3)
196-245: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winShare one list of widget Swift filenames.
normalizeWidgetSourcePathsdeclares the basenames at Lines 199-203.withIosWidgetXcodeTargetdeclares the same three filenames at Lines 241-245. If a Swift file is added toios-widget/, both lists need an update, and a missed update leaves the file reference path un-normalized. Extract one module-level constant.♻️ Proposed refactor
+const WIDGET_SWIFT_FILES = [ + 'EmotionDiaryWidget.swift', + 'EmotionDiaryWidgetBundle.swift', + 'SnapshotReader.swift', +];- const basenames = new Set([ - 'EmotionDiaryWidget.swift', - 'EmotionDiaryWidgetBundle.swift', - 'SnapshotReader.swift', - ]); + const basenames = new Set(WIDGET_SWIFT_FILES);- const swiftFiles = [ - 'EmotionDiaryWidget.swift', - 'EmotionDiaryWidgetBundle.swift', - 'SnapshotReader.swift', - ]; - project.addBuildPhase( - swiftFiles.map((name) => `${WIDGET_TARGET_NAME}/${name}`), + WIDGET_SWIFT_FILES.map((name) => `${WIDGET_TARGET_NAME}/${name}`),🤖 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 `@modules/widget-snapshot/app.plugin.js` around lines 196 - 245, Extract the shared widget Swift filename list into a module-level constant, then reuse it in both normalizeWidgetSourcePaths and withIosWidgetXcodeTarget. Remove the duplicated local basenames and inline swiftFiles declarations while preserving the existing three filenames and behavior.
123-136: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winFail the prebuild when the widget sources are missing, and drop the redundant Info.plist copy.
Two points:
- Lines 123-128: If
ios-widget/is absent, the plugin logs a warning and continues.withIosWidgetXcodeTargetthen still creates the target and a Sources phase that references three Swift files that do not exist. The failure appears later as an Xcode build error with no link to the true cause. The sources are committed in this repository, so their absence indicates a packaging fault. Throw instead.- Lines 132-136:
updateWidgetTargetBuildSettingssetsINFOPLIST_FILEtoEmotionDiaryWidget/Info.plist(Line 150). TheEmotionDiaryWidget-Info.plistcopy is then unused and can drift from the original. Remove the copy, or setINFOPLIST_FILEto the copied name.🔧 Proposed change
if (!fs.existsSync(IOS_WIDGET_SRC)) { - console.warn( - `[widget-snapshot] Missing ios-widget sources at ${IOS_WIDGET_SRC}`, - ); - return cfg; + throw new Error( + `[widget-snapshot] Missing ios-widget sources at ${IOS_WIDGET_SRC}`, + ); } copyDirRecursive(IOS_WIDGET_SRC, widgetDir); - // node-xcode defaults INFOPLIST_FILE to EmotionDiaryWidget/EmotionDiaryWidget-Info.plist - const infoSrc = path.join(widgetDir, 'Info.plist'); - if (fs.existsSync(infoSrc)) { - copyFile(infoSrc, path.join(widgetDir, `${WIDGET_TARGET_NAME}-Info.plist`)); - } -Apply the same fail-fast treatment at Lines 330-335 for
android-widget/.🤖 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 `@modules/widget-snapshot/app.plugin.js` around lines 123 - 136, Update the missing-source checks in withIosWidgetXcodeTarget and the corresponding android-widget handling to throw an error instead of warning and returning the config, so prebuild fails immediately when committed widget sources are absent. Remove the redundant Info.plist copy after copyDirRecursive, keeping the existing INFOPLIST_FILE configuration pointing to Info.plist.
50-98: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider a single source for the App Group and widget identifiers.
APP_GROUP_ID,WIDGET_TARGET_NAME, andWIDGET_BUNDLE_IDare declared here and also hard-coded inapp.json(ios.entitlementsandextra.eas.build.experimental.ios.appExtensions). The plugin already writes both values at prebuild time. If one copy changes, the App Group can silently mismatch between the app and the extension, and the widget reads an empty suite. Keep the duplication only if EAS requires the staticapp.jsonvalues, and add a comment that records the constraint.🤖 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 `@modules/widget-snapshot/app.plugin.js` around lines 50 - 98, Use a single source of truth for APP_GROUP_ID, WIDGET_TARGET_NAME, and WIDGET_BUNDLE_ID across withEasAppExtensions, withWidgetSnapshotAppGroup, and the corresponding app.json configuration. Remove the duplicated hard-coded app.json values if EAS does not require them; otherwise retain them and add a comment documenting that static values are required and must stay synchronized with these constants.scripts/verify-widget-native-link.js (1)
25-53: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding the Android autolinking check.
The script validates
apple.modulesonly. The module also shipsmodules/widget-snapshot/android/src/main/java/expo/modules/widgetsnapshot/WidgetSnapshotModule.kt. Ifexpo-module.config.jsonomits theandroid.modulesentry,requireNativeModule('WidgetSnapshot')throws on Android, and the gate still passes. Add a check for the Android module declaration.Wrap the two
JSON.parsecalls in try/catch as well, so a malformed file produces aFAIL:line instead of a stack trace.🤖 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 `@scripts/verify-widget-native-link.js` around lines 25 - 53, Extend the expoConfig validation to require the Android module declaration, checking cfg.android.modules includes WidgetSnapshotModule alongside the existing apple.modules check. Wrap both package.json and expo-module.config.json JSON.parse operations in try/catch so malformed files call fail with a clear message instead of emitting a stack trace.modules/widget-snapshot/src/index.ts (1)
8-13: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd an automated contract check for widget storage identifiers.
NATIVE_WIDGET_SNAPSHOT_KEYandNATIVE_WIDGET_SNAPSHOT_APP_GROUP_IDduplicate the values inshared/widget/sink.ts. Compare both pairs in a test or static check to prevent future 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 `@modules/widget-snapshot/src/index.ts` around lines 8 - 13, Add an automated contract check covering NATIVE_WIDGET_SNAPSHOT_KEY and NATIVE_WIDGET_SNAPSHOT_APP_GROUP_ID against WIDGET_SNAPSHOT_KEY and WIDGET_SNAPSHOT_APP_GROUP_ID from shared/widget/sink.ts. Assert both identifier pairs remain equal so future changes cannot drift silently.__tests__/unit/store/weather.test.ts (1)
124-142: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAssert the widget snapshot payload.
The signed-in test only proves that publication was invoked. It passes if the module publishes an empty or stale array. Assert that the call includes the current entry with ID
"1"to verify theallEntriescontract instore/modules/weather.tsLines 73-75.Suggested assertion
expect(publishWidgetSnapshot).toHaveBeenCalled(); + expect(publishWidgetSnapshot).toHaveBeenCalledWith( + expect.arrayContaining([ + expect.objectContaining({ id: "1" }), + ]), + );As per coding guidelines, keep the test edit surgical and run
yarn typecheck,yarn lint, and the affected Jest test after the change.🤖 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 `@__tests__/unit/store/weather.test.ts` around lines 124 - 142, Strengthen the signed-in test around createWeatherSlice and _calculateWeather by asserting publishWidgetSnapshot receives the current allEntries payload containing the entry with ID "1", rather than only verifying invocation. Keep the logout test and other test behavior unchanged, then run typecheck, lint, and the affected Jest test.Source: Coding guidelines
🤖 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 `@app/_layout.tsx`:
- Around line 42-48: Update the widget snapshot calls in the layout lifecycle
handler to catch rejected publishWidgetSnapshot and clearWidgetSnapshot
promises, logging each failure through utils/logger. Preserve the existing
signed-in publish and logged-out clear behavior while preventing unhandled
rejections.
In `@modules/widget-snapshot/android-widget/WidgetSnapshotProvider.kt`:
- Around line 79-82: Update mapToChrome in WidgetSnapshotProvider so the
updatedAt validation rejects non-numeric values as well as missing or null
values. Require json.opt("updatedAt") to be a Number before proceeding,
preserving the existing Chrome.Cleared behavior for invalid timestamps and
consistency with the native validation contract.
In
`@modules/widget-snapshot/android/src/main/java/expo/modules/widgetsnapshot/WidgetSnapshotModule.kt`:
- Around line 55-64: Update the writeSnapshot and clearSnapshot AsyncFunction
implementations to capture the boolean result of their SharedPreferences commit
operations, throw when commit returns false, and call notifyAppWidgets() only
after a successful commit. Preserve the existing snapshot write and removal
behavior while ensuring failed clears do not refresh widgets.
In `@modules/widget-snapshot/app.plugin.js`:
- Around line 6-23: Update the ESLint flat configuration to apply a Node
environment override to modules/**/*.plugin.js and scripts/**/*.js, declaring
Node globals for both app.plugin.js and verify-widget-native-link.js; no direct
changes are needed in either JavaScript file.
- Around line 147-180: Update updateWidgetTargetBuildSettings to identify the
widget target using its target-specific configuration or target identifier, not
PRODUCT_NAME alone. Ensure settings are applied only to configurations belonging
to WIDGET_TARGET_NAME, while leaving unrelated targets unchanged.
- Around line 337-383: Update the Android sync logic to copy
WidgetSnapshotProvider.kt and its resources only into the Expo module paths,
removing the corresponding app-source copies while preserving module manifest
registration. Stop rewriting the tracked module AndroidManifest.xml during
prebuild and instead commit the receiver declaration in that manifest, ensuring
there is a single manifest source of truth.
In `@modules/widget-snapshot/ios-widget/SnapshotReader.swift`:
- Around line 51-74: Update numericInt and its use in SnapshotReader so
entryCountActive accepts only non-negative integral values, rejecting booleans,
fractional NSNumber values, and negatives. Ensure invalid counts return .cleared
before constructing .status, while preserving valid integer handling.
In `@modules/widget-snapshot/ios/WidgetSnapshot.podspec`:
- Around line 13-22: Update the platform declaration in the WidgetSnapshot
podspec to remove the unsupported :tvos entry, leaving iOS 15.1 as the supported
platform. Do not add tvOS support unless a separate tvOS implementation for
WidgetSnapshotModule.swift exists.
In `@modules/widget-snapshot/README.md`:
- Line 40: Add the missing .planning/phases/17-widget-native-shell/17-UAT.md
document and ensure it covers the referenced iOS and Android prebuild validation
steps, including the available development/preview APK path. Keep the existing
README references valid and document the PrivacyInfo.xcprivacy verification.
In `@scripts/sim-widget-smoke.sh`:
- Around line 66-80: Add a preflight check before the “Build & install (expo
run:ios)” step in scripts/sim-widget-smoke.sh to verify that Metro is running,
or document the required running-bundler prerequisite in the script header.
Ensure the check clearly reports the missing Metro dependency and exits before
expo run:ios when unavailable, while preserving the existing --no-bundler
workflow.
- Around line 18-27: Add a default branch to the argument case in the script’s
option-parsing loop so any unrecognized argument is reported clearly and causes
the script to exit nonzero; preserve the existing handling for
--open-widget-gallery, --device=*, and help options.
In `@scripts/verify-widget-native-link.js`:
- Around line 55-75: Update the provider lookup in the verification script to
skip the iOS-specific checks with an explicit status when ios/Pods is absent.
Otherwise, resolve ExpoModulesProvider.swift by globbing under ios/Pods/Target
Support Files/Pods-*/ instead of hard-coding Pods-MO, then preserve the existing
registration and import validations against the resolved file.
In `@services/widgetSnapshot.ts`:
- Around line 94-105: Serialize mutations to the shared widget snapshot sink by
routing both publishWidgetSnapshot and clearWidgetSnapshot through a promise
chain that preserves invocation order, including when sink operations are
delayed or reject. Keep snapshot construction within publishWidgetSnapshot, and
add tests covering publish followed by clear and two sequential publishes to
verify the final sink state reflects call order.
In `@shared/widget/chromeMapping.ts`:
- Around line 77-82: Strengthen the numeric validation in the candidate
validation logic around updatedAt and entryCountActive: require updatedAt to be
finite, and require both numeric fields to be non-negative safe integers,
rejecting negative, fractional, and otherwise invalid values before rendering
status chrome. Add regression coverage for negative and fractional inputs.
In `@store/modules/user.ts`:
- Around line 627-628: Introduce one best-effort widget-snapshot cleanup helper
in the relevant user auth module that awaits clearWidgetSnapshot, catches
failures, and logs them via utils/logger. Replace the direct clearWidgetSnapshot
calls in logout, its outer-catch retry, and deleteAccount with this helper so
widget failures never prevent session removal, local-storage reload, or
completion of account deletion.
In `@store/modules/weather.ts`:
- Around line 70-78: Handle rejected promises from publishWidgetSnapshot and
clearWidgetSnapshot in the shown get() branch by attaching catch handlers that
report failures through logger.error, or implement equivalent handling inside
the widget snapshot service. Ensure _calculateWeather cannot produce unhandled
rejections, and add a test using a rejecting sink to verify both snapshot paths
are handled.
- Around line 70-78: Serialize the widget snapshot operations triggered by the
user branch in the weather store so delayed publishWidgetSnapshot writes cannot
complete after clearWidgetSnapshot during logout. Queue both write and clear
operations, or use generation checks to discard stale writes, and add a test
covering the write-then-clear interleaving to ensure widget_snapshot_v1 remains
cleared.
---
Nitpick comments:
In `@__tests__/unit/store/weather.test.ts`:
- Around line 124-142: Strengthen the signed-in test around createWeatherSlice
and _calculateWeather by asserting publishWidgetSnapshot receives the current
allEntries payload containing the entry with ID "1", rather than only verifying
invocation. Keep the logout test and other test behavior unchanged, then run
typecheck, lint, and the affected Jest test.
In `@modules/widget-snapshot/app.plugin.js`:
- Around line 196-245: Extract the shared widget Swift filename list into a
module-level constant, then reuse it in both normalizeWidgetSourcePaths and
withIosWidgetXcodeTarget. Remove the duplicated local basenames and inline
swiftFiles declarations while preserving the existing three filenames and
behavior.
- Around line 123-136: Update the missing-source checks in
withIosWidgetXcodeTarget and the corresponding android-widget handling to throw
an error instead of warning and returning the config, so prebuild fails
immediately when committed widget sources are absent. Remove the redundant
Info.plist copy after copyDirRecursive, keeping the existing INFOPLIST_FILE
configuration pointing to Info.plist.
- Around line 50-98: Use a single source of truth for APP_GROUP_ID,
WIDGET_TARGET_NAME, and WIDGET_BUNDLE_ID across withEasAppExtensions,
withWidgetSnapshotAppGroup, and the corresponding app.json configuration. Remove
the duplicated hard-coded app.json values if EAS does not require them;
otherwise retain them and add a comment documenting that static values are
required and must stay synchronized with these constants.
In `@modules/widget-snapshot/src/index.ts`:
- Around line 8-13: Add an automated contract check covering
NATIVE_WIDGET_SNAPSHOT_KEY and NATIVE_WIDGET_SNAPSHOT_APP_GROUP_ID against
WIDGET_SNAPSHOT_KEY and WIDGET_SNAPSHOT_APP_GROUP_ID from shared/widget/sink.ts.
Assert both identifier pairs remain equal so future changes cannot drift
silently.
In `@scripts/verify-widget-native-link.js`:
- Around line 25-53: Extend the expoConfig validation to require the Android
module declaration, checking cfg.android.modules includes WidgetSnapshotModule
alongside the existing apple.modules check. Wrap both package.json and
expo-module.config.json JSON.parse operations in try/catch so malformed files
call fail with a clear message instead of emitting a stack trace.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bdf24fdf-9676-429f-97ba-9fbb11e8326f
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (44)
__tests__/unit/services/widgetSnapshot.test.ts__tests__/unit/shared/widget/chromeMapping.test.ts__tests__/unit/shared/widget/deepLink.test.ts__tests__/unit/store/weather.test.tsapp.jsonapp/_layout.tsxmodules/widget-snapshot/README.mdmodules/widget-snapshot/android-widget/WidgetSnapshotProvider.ktmodules/widget-snapshot/android-widget/res/drawable/ic_weather_cloudy.xmlmodules/widget-snapshot/android-widget/res/drawable/ic_weather_rainy.xmlmodules/widget-snapshot/android-widget/res/drawable/ic_weather_stormy.xmlmodules/widget-snapshot/android-widget/res/drawable/ic_weather_sunny.xmlmodules/widget-snapshot/android-widget/res/drawable/widget_bg_active.xmlmodules/widget-snapshot/android-widget/res/drawable/widget_bg_cleared.xmlmodules/widget-snapshot/android-widget/res/drawable/widget_bg_empty.xmlmodules/widget-snapshot/android-widget/res/drawable/widget_snapshot_preview.xmlmodules/widget-snapshot/android-widget/res/layout/widget_snapshot_small.xmlmodules/widget-snapshot/android-widget/res/values/colors.xmlmodules/widget-snapshot/android-widget/res/values/strings.xmlmodules/widget-snapshot/android-widget/res/xml/widget_snapshot_info.xmlmodules/widget-snapshot/android/build.gradlemodules/widget-snapshot/android/src/main/AndroidManifest.xmlmodules/widget-snapshot/android/src/main/java/expo/modules/widgetsnapshot/WidgetSnapshotModule.ktmodules/widget-snapshot/app.plugin.jsmodules/widget-snapshot/expo-module.config.jsonmodules/widget-snapshot/ios-widget/EmotionDiaryWidget.swiftmodules/widget-snapshot/ios-widget/EmotionDiaryWidgetBundle.swiftmodules/widget-snapshot/ios-widget/Info.plistmodules/widget-snapshot/ios-widget/SnapshotReader.swiftmodules/widget-snapshot/ios/PrivacyInfo.xcprivacymodules/widget-snapshot/ios/WidgetSnapshot.podspecmodules/widget-snapshot/ios/WidgetSnapshotModule.swiftmodules/widget-snapshot/package.jsonmodules/widget-snapshot/src/index.tspackage.jsonscripts/sim-widget-smoke.shscripts/verify-widget-native-link.jsservices/widgetSnapshot.tsshared/widget/chromeMapping.tsshared/widget/deepLink.tsshared/widget/index.tsstore/modules/user.tsstore/modules/weather.tsstore/useAppStore.ts
Comment on lines
+6
to
+23
| const fs = require('fs'); | ||
| const path = require('path'); | ||
| const { | ||
| withEntitlementsPlist, | ||
| withXcodeProject, | ||
| withAndroidManifest, | ||
| withDangerousMod, | ||
| AndroidConfig, | ||
| } = require('expo/config-plugins'); | ||
|
|
||
| const APP_GROUP_ID = 'group.com.moyunzero.emotiondiary'; | ||
| const APP_GROUPS_KEY = 'com.apple.security.application-groups'; | ||
| const WIDGET_TARGET_NAME = 'EmotionDiaryWidget'; | ||
| const WIDGET_BUNDLE_ID = 'com.moyunzero.emotiondiary.widget'; | ||
| const WIDGET_PROVIDER_CLASS = 'expo.modules.widgetsnapshot.WidgetSnapshotProvider'; | ||
| const DEPLOYMENT_TARGET = '15.1'; | ||
|
|
||
| const MODULE_ROOT = __dirname; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
ESLint reports '__dirname' is not defined in both new CommonJS files. The root cause is one ESLint configuration gap: the config does not declare Node globals for config plugins and repository scripts, so yarn lint fails.
modules/widget-snapshot/app.plugin.js#L6-L23: addmodules/**/*.plugin.jsto a Node-environment override in the ESLint flat config, or add/* eslint-env node */above therequirecalls.scripts/verify-widget-native-link.js#L13-L13: coverscripts/**/*.jswith the same Node-environment override.
🧰 Tools
🪛 ESLint
[error] 23-23: '__dirname' is not defined.
(no-undef)
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 7-7: Prefer node:path over path.
[warning] 6-6: Prefer node:fs over fs.
📍 Affects 2 files
modules/widget-snapshot/app.plugin.js#L6-L23(this comment)scripts/verify-widget-native-link.js#L13-L13
🤖 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 `@modules/widget-snapshot/app.plugin.js` around lines 6 - 23, Update the ESLint
flat configuration to apply a Node environment override to
modules/**/*.plugin.js and scripts/**/*.js, declaring Node globals for both
app.plugin.js and verify-widget-native-link.js; no direct changes are needed in
either JavaScript file.
Sources: Coding guidelines, Linters/SAST tools
Comment on lines
+94
to
+105
| export async function publishWidgetSnapshot( | ||
| entries: readonly MoodEntry[], | ||
| now: number = Date.now(), | ||
| ): Promise<void> { | ||
| const snapshot = buildWidgetSnapshot(entries, now); | ||
| await getWidgetSnapshotSink().write(snapshot); | ||
| } | ||
|
|
||
| /** Clear the same sink/key used by publish (D-08). */ | ||
| export async function clearWidgetSnapshot(): Promise<void> { | ||
| await getWidgetSnapshotSink().clear(); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Serialize snapshot writes and clears.
publishWidgetSnapshot and clearWidgetSnapshot call the same asynchronous sink without ordering. If a clear finishes before an earlier publish write, that write can restore a stale widget snapshot after the clear. Queue sink mutations in invocation order. Add tests for publish → clear and two sequential publishes with delayed sink operations.
Proposed fix
+let sinkOperation: Promise<void> = Promise.resolve();
+
+function enqueueSinkOperation(operation: () => Promise<void>): Promise<void> {
+ const result = sinkOperation.then(operation, operation);
+ sinkOperation = result.catch(() => undefined);
+ return result;
+}
+
export async function publishWidgetSnapshot(
entries: readonly MoodEntry[],
now: number = Date.now(),
): Promise<void> {
const snapshot = buildWidgetSnapshot(entries, now);
- await getWidgetSnapshotSink().write(snapshot);
+ await enqueueSinkOperation(() => getWidgetSnapshotSink().write(snapshot));
}
export async function clearWidgetSnapshot(): Promise<void> {
- await getWidgetSnapshotSink().clear();
+ await enqueueSinkOperation(() => getWidgetSnapshotSink().clear());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| export async function publishWidgetSnapshot( | |
| entries: readonly MoodEntry[], | |
| now: number = Date.now(), | |
| ): Promise<void> { | |
| const snapshot = buildWidgetSnapshot(entries, now); | |
| await getWidgetSnapshotSink().write(snapshot); | |
| } | |
| /** Clear the same sink/key used by publish (D-08). */ | |
| export async function clearWidgetSnapshot(): Promise<void> { | |
| await getWidgetSnapshotSink().clear(); | |
| } | |
| let sinkOperation: Promise<void> = Promise.resolve(); | |
| function enqueueSinkOperation(operation: () => Promise<void>): Promise<void> { | |
| const result = sinkOperation.then(operation, operation); | |
| sinkOperation = result.catch(() => undefined); | |
| return result; | |
| } | |
| export async function publishWidgetSnapshot( | |
| entries: readonly MoodEntry[], | |
| now: number = Date.now(), | |
| ): Promise<void> { | |
| const snapshot = buildWidgetSnapshot(entries, now); | |
| await enqueueSinkOperation(() => getWidgetSnapshotSink().write(snapshot)); | |
| } | |
| /** Clear the same sink/key used by publish (D-08). */ | |
| export async function clearWidgetSnapshot(): Promise<void> { | |
| await enqueueSinkOperation(() => getWidgetSnapshotSink().clear()); | |
| } |
🤖 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 `@services/widgetSnapshot.ts` around lines 94 - 105, Serialize mutations to the
shared widget snapshot sink by routing both publishWidgetSnapshot and
clearWidgetSnapshot through a promise chain that preserves invocation order,
including when sink operations are delayed or reject. Keep snapshot construction
within publishWidgetSnapshot, and add tests covering publish followed by clear
and two sequential publishes to verify the final sink state reflects call order.
Catch widget sink rejections, tighten snapshot validation and audio-mode failures, localize Android widget strings, and strengthen embed/verify/smoke gates. Co-authored-by: Cursor <cursoragent@cursor.com>
Prevent publish/clear races with a thunk queue, harden logout clear and native widget packaging (FQCN, values-en, existing-target PrivacyInfo), and cover the follow-ups with unit tests plus sim smoke gates. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
__tests__/unit/modules/widget-snapshot/nativeHardening.test.ts (2)
156-163: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the locale copy assertion out of the native hardening suite.
This test asserts the exact
locales/en-US/onThisDay.jsoncopy. That resource belongs to the On This Day feature, not the widget native module. A copy edit then fails a widget test and hides the real cause. Move the assertion to an On This Day locale test.🤖 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 `@__tests__/unit/modules/widget-snapshot/nativeHardening.test.ts` around lines 156 - 163, Move the en-US onThisDay hint assertion from the native hardening suite into the existing On This Day locale test suite, preserving the exact expected copy and JSON resource validation while removing it from the widget native module tests.
59-79: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueLoosen the source-text gates so harmless edits do not fail the suite.
These assertions match exact implementation text: a Kotlin expression prefix (
val committed = getPreferences().edit().putString), a plugin statement sequence, and shell command strings. A rename of a local variable, a formatting change by Prettier, or an added flag in the smoke script breaks the test without any behavior change. Assert the intent instead: for the Kotlin module, thatcommit()result is checked beforenotifyAppWidgets(); for the plugin, thatensureWidgetPrivacyInfoResourceruns on the existing-target path.Also applies to: 105-114, 132-145
🤖 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 `@__tests__/unit/modules/widget-snapshot/nativeHardening.test.ts` around lines 59 - 79, Loosen the source-text assertions in the native hardening tests to verify behavior rather than exact formatting or variable names. In the WidgetSnapshotModule checks, assert that the SharedPreferences commit result is evaluated before notifyAppWidgets() in the writeSnapshot flow; in the plugin checks, assert that ensureWidgetPrivacyInfoResource runs on the existing-target path without matching exact statement text or shell command strings.scripts/verify-widget-native-link.js (1)
26-59: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd a timeout and drop the redundant matchers.
Two points for this CI gate:
execFileSyncruns withouttimeout. Ifnpxstalls, for example while it resolves or downloadsexpo-modules-autolinkingfrom the network, the verification job hangs instead of failing. Set an explicittimeout.- The
mentionsModulecheck is redundant./WidgetSnapshot/already matches every string that/WidgetSnapshotModule/matches, so the first test is dead. It also matches any unrelated occurrence of the name in the resolver output. Keepwidget-snapshotandWidgetSnapshotModule.♻️ Proposed change
output = execFileSync( 'npx', ['expo-modules-autolinking', 'resolve', '--platform', platform], { cwd: root, encoding: 'utf8', stdio: ['ignore', 'pipe', 'pipe'], + timeout: 120000, }, ); @@ const haystack = String(output); const mentionsModule = /WidgetSnapshotModule/.test(haystack) || - /widget-snapshot/.test(haystack) || - /WidgetSnapshot/.test(haystack); + /widget-snapshot/.test(haystack);🤖 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 `@scripts/verify-widget-native-link.js` around lines 26 - 59, Update assertAutolinkingResolves to pass an explicit timeout option to execFileSync so stalled npx resolution fails promptly. Simplify mentionsModule to check only /widget-snapshot/ and /WidgetSnapshotModule/, removing the redundant /WidgetSnapshot/ matcher while preserving the existing failure and success handling.modules/widget-snapshot/app.plugin.js (1)
290-306: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSimplify the Resources phase lookup; SonarCloud fails the file on cognitive complexity.
The loop evaluates
objects.PBXResourcesBuildPhase?.[phase.value]twice and thecomment === 'Resources'branch can never setresourcesPhaseUuid. SonarCloud reports a blocking cognitive complexity of 34 forensureWidgetPrivacyInfoResourceat line 234. Simplifying this loop and extracting the file-reference lookup into a helper reduces the score.♻️ Proposed simplification
- let resourcesPhaseUuid = null; - for (const phase of targetEntry.buildPhases) { - const comment = String(phase.comment || ''); - const phaseObj = objects.PBXResourcesBuildPhase?.[phase.value]; - if (phaseObj || comment === 'Resources') { - if (objects.PBXResourcesBuildPhase?.[phase.value]) { - resourcesPhaseUuid = phase.value; - break; - } - } - } + const resourcesPhaseUuid = + targetEntry.buildPhases.find( + (phase) => objects.PBXResourcesBuildPhase?.[phase.value], + )?.value ?? null;🤖 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 `@modules/widget-snapshot/app.plugin.js` around lines 290 - 306, In ensureWidgetPrivacyInfoResource, simplify the Resources build-phase loop by evaluating the PBXResourcesBuildPhase lookup once per phase and assigning resourcesPhaseUuid only when that lookup succeeds; remove the redundant comment-based branch. Extract the phase file-reference lookup into a small helper if needed to reduce cognitive complexity while preserving the existing early-return behavior when no Resources phase is found.Source: Linters/SAST tools
services/widgetSnapshot.ts (2)
151-165: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deriving
enqueueClearWidgetSnapshotfrom the shared scheduler.
enqueueClearWidgetSnapshotrepeats the queue-and-log body ofscheduleWidgetSnapshotOp. Return the queued promise fromscheduleWidgetSnapshotOpand reuse it. This keeps one queue mutation point and one log path.♻️ Proposed refactor
export function scheduleWidgetSnapshotOp( thunk: () => Promise<void>, label: string, -): void { - opChain = opChain.then(async () => { +): Promise<void> { + const run = opChain.then(async () => { try { await thunk(); } catch (error) { logger.warn('widgetSnapshot', label, error); } }); + opChain = run; + return run; } export function enqueueClearWidgetSnapshot(label: string): Promise<void> { - const run = opChain.then(async () => { - try { - await clearWidgetSnapshot(); - } catch (error) { - logger.warn('widgetSnapshot', label, error); - } - }); - opChain = run; - return run; + return scheduleWidgetSnapshotOp(() => clearWidgetSnapshot(), label); }🤖 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 `@services/widgetSnapshot.ts` around lines 151 - 165, Refactor enqueueClearWidgetSnapshot to delegate to scheduleWidgetSnapshotOp, returning its queued promise instead of mutating opChain and logging independently. Move or preserve the clearWidgetSnapshot operation and label-aware failure logging through scheduleWidgetSnapshotOp so there is one queue mutation point and one shared log path.
151-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winOne missing return value forces both the duplicated clear helper and the timer-based test drain.
scheduleWidgetSnapshotOpreturnsvoid, soenqueueClearWidgetSnapshothas to re-implement the queue-and-log body, and the ordering test has to wait on a real timer instead of the queue itself.
services/widgetSnapshot.ts#L151-L165: return the queued promise fromscheduleWidgetSnapshotOp, then implementenqueueClearWidgetSnapshotas a call to it withclearWidgetSnapshot.__tests__/unit/services/widgetSnapshot.test.ts#L142-L182: replaceawait new Promise((r) => setTimeout(r, 30))with an awaited no-op op queued throughscheduleWidgetSnapshotOp.🤖 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 `@services/widgetSnapshot.ts` around lines 151 - 165, Update services/widgetSnapshot.ts lines 151-165 so scheduleWidgetSnapshotOp returns its queued promise, then simplify enqueueClearWidgetSnapshot to delegate to it with clearWidgetSnapshot while preserving failure logging. Update __tests__/unit/services/widgetSnapshot.test.ts lines 142-182 to await a no-op queued through scheduleWidgetSnapshotOp instead of using a timer-based delay.
🤖 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 `@modules/widget-snapshot/app.plugin.js`:
- Around line 243-259: Update the file-reference lookup to resolve the widget
privacy manifest from the EmotionDiaryWidget group’s children instead of
filtering with the hard-coded rawPath.startsWith('MO') host-directory check.
Restrict matching to that group and preserve the existing
privacyBase/privacyRel/rawName matching criteria when assigning fileRefUuid.
In `@scripts/sim-widget-smoke.sh`:
- Around line 140-156: Update assert_snapshot to validate that the snapshot was
freshly published by this run: clear the existing snapshot through the supported
storage path before background_and_publish, then parse the resulting
widget_snapshot_v1 payload and require the expected schema version rather than
merely matching "schemaVersion". Preserve the retry behavior and success output
only for a valid v1 snapshot.
- Around line 82-103: Replace the predictable
`/tmp/emotion-diary-metro-smoke.log` path in the Metro startup and
readiness-error handling with a private file created via `mktemp`, storing its
path in a variable. Use that variable for every related `tail` call and remove
the temporary log during `cleanup`, while preserving the existing Metro process
and timeout behavior.
---
Nitpick comments:
In `@__tests__/unit/modules/widget-snapshot/nativeHardening.test.ts`:
- Around line 156-163: Move the en-US onThisDay hint assertion from the native
hardening suite into the existing On This Day locale test suite, preserving the
exact expected copy and JSON resource validation while removing it from the
widget native module tests.
- Around line 59-79: Loosen the source-text assertions in the native hardening
tests to verify behavior rather than exact formatting or variable names. In the
WidgetSnapshotModule checks, assert that the SharedPreferences commit result is
evaluated before notifyAppWidgets() in the writeSnapshot flow; in the plugin
checks, assert that ensureWidgetPrivacyInfoResource runs on the existing-target
path without matching exact statement text or shell command strings.
In `@modules/widget-snapshot/app.plugin.js`:
- Around line 290-306: In ensureWidgetPrivacyInfoResource, simplify the
Resources build-phase loop by evaluating the PBXResourcesBuildPhase lookup once
per phase and assigning resourcesPhaseUuid only when that lookup succeeds;
remove the redundant comment-based branch. Extract the phase file-reference
lookup into a small helper if needed to reduce cognitive complexity while
preserving the existing early-return behavior when no Resources phase is found.
In `@scripts/verify-widget-native-link.js`:
- Around line 26-59: Update assertAutolinkingResolves to pass an explicit
timeout option to execFileSync so stalled npx resolution fails promptly.
Simplify mentionsModule to check only /widget-snapshot/ and
/WidgetSnapshotModule/, removing the redundant /WidgetSnapshot/ matcher while
preserving the existing failure and success handling.
In `@services/widgetSnapshot.ts`:
- Around line 151-165: Refactor enqueueClearWidgetSnapshot to delegate to
scheduleWidgetSnapshotOp, returning its queued promise instead of mutating
opChain and logging independently. Move or preserve the clearWidgetSnapshot
operation and label-aware failure logging through scheduleWidgetSnapshotOp so
there is one queue mutation point and one shared log path.
- Around line 151-165: Update services/widgetSnapshot.ts lines 151-165 so
scheduleWidgetSnapshotOp returns its queued promise, then simplify
enqueueClearWidgetSnapshot to delegate to it with clearWidgetSnapshot while
preserving failure logging. Update
__tests__/unit/services/widgetSnapshot.test.ts lines 142-182 to await a no-op
queued through scheduleWidgetSnapshotOp instead of using a timer-based delay.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: bfbef896-ac37-4ad8-8063-5bc9c15f3420
📒 Files selected for processing (43)
.maestro/flows/_smoke-widget-publish.yaml__tests__/unit/components/EntryCard/burnedAshA11y.test.ts__tests__/unit/components/Insights/relationshipPotA11y.test.ts__tests__/unit/features/onThisDay/entryCardA11yFollowups.test.ts__tests__/unit/modules/widget-snapshot/nativeHardening.test.ts__tests__/unit/services/devSeedOnThisDayRoute.test.ts__tests__/unit/services/maestroOnThisDaySeed.test.ts__tests__/unit/services/widgetSnapshot.test.ts__tests__/unit/shared/audio/coordinator.audioMode.test.ts__tests__/unit/shared/audio/recordingCoordinator.clipHandler.test.ts__tests__/unit/shared/widget/chromeMapping.test.ts__tests__/unit/store/weather.test.tsapp/_layout.tsxapp/dev-seed-on-this-day.tsxcomponents/EntryCard/EntryCard.tsxcomponents/Insights/RelationshipGarden.tsxcomponents/Insights/relationshipPotA11y.tsfeatures/onThisDay/OnThisDayScreen.tsxlocales/en-US/dashboard.jsonlocales/en-US/onThisDay.jsonlocales/zh-Hans/dashboard.jsonmodules/widget-snapshot/README.mdmodules/widget-snapshot/android-widget/WidgetSnapshotProvider.ktmodules/widget-snapshot/android-widget/res/layout/widget_snapshot_small.xmlmodules/widget-snapshot/android-widget/res/values-en/strings.xmlmodules/widget-snapshot/android-widget/res/values/strings.xmlmodules/widget-snapshot/android/src/main/AndroidManifest.xmlmodules/widget-snapshot/android/src/main/java/expo/modules/widgetsnapshot/WidgetSnapshotModule.ktmodules/widget-snapshot/app.plugin.jsmodules/widget-snapshot/ios-widget/PrivacyInfo.xcprivacymodules/widget-snapshot/ios-widget/SnapshotReader.swiftmodules/widget-snapshot/ios/WidgetSnapshot.podspecscripts/sim-widget-smoke.shscripts/verify-widget-native-link.jsservices/devSeedOnThisDayRoute.tsservices/maestroOnThisDaySeed.tsservices/widgetSnapshot.tsshared/audio/coordinator.tsshared/audio/recordingCoordinator.tsshared/widget/chromeMapping.tsstore/modules/user.tsstore/modules/weather.tsstore/useAppStore.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- locales/en-US/onThisDay.json
- store/modules/weather.ts
- app/dev-seed-on-this-day.tsx
- store/useAppStore.ts
- app/_layout.tsx
- modules/widget-snapshot/android-widget/res/values/strings.xml
- modules/widget-snapshot/ios/WidgetSnapshot.podspec
- modules/widget-snapshot/android-widget/res/layout/widget_snapshot_small.xml
- shared/widget/chromeMapping.ts
- tests/unit/shared/widget/chromeMapping.test.ts
- features/onThisDay/OnThisDayScreen.tsx
- shared/audio/recordingCoordinator.ts
- modules/widget-snapshot/README.md
- components/Insights/RelationshipGarden.tsx
- modules/widget-snapshot/android/src/main/java/expo/modules/widgetsnapshot/WidgetSnapshotModule.kt
- store/modules/user.ts
- modules/widget-snapshot/ios-widget/SnapshotReader.swift
- services/maestroOnThisDaySeed.ts
- shared/audio/coordinator.ts
- modules/widget-snapshot/android-widget/WidgetSnapshotProvider.kt
- tests/unit/store/weather.test.ts
- components/EntryCard/EntryCard.tsx
Comment on lines
+82
to
+103
| # CI=1 avoids interactive "use another port?" prompts when 8081 is briefly raced. | ||
| CI=1 npx expo start --dev-client --port 8081 > /tmp/emotion-diary-metro-smoke.log 2>&1 & | ||
| METRO_PID=$! | ||
| echo " Metro pid=$METRO_PID (log: /tmp/emotion-diary-metro-smoke.log)" | ||
|
|
||
| echo "==> Wait until Metro is ready" | ||
| METRO_READY=0 | ||
| for _ in $(seq 1 90); do | ||
| if ! kill -0 "$METRO_PID" 2>/dev/null; then | ||
| echo "ERROR: Metro exited early - see /tmp/emotion-diary-metro-smoke.log" >&2 | ||
| tail -n 40 /tmp/emotion-diary-metro-smoke.log >&2 || true | ||
| exit 1 | ||
| fi | ||
| if curl -sf "http://127.0.0.1:8081/status" >/dev/null 2>&1; then | ||
| METRO_READY=1 | ||
| break | ||
| fi | ||
| sleep 1 | ||
| done | ||
| if [[ "$METRO_READY" -ne 1 ]]; then | ||
| echo "ERROR: Metro not ready within timeout" >&2 | ||
| tail -n 40 /tmp/emotion-diary-metro-smoke.log >&2 || true |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Use a private Metro log file.
Line 83 redirects output to a predictable path under /tmp. A local user can replace that path with a symlink before the script starts. The shell then writes to the symlink target with the privileges of the script user.
Create the log with mktemp, use that variable for all tail calls, and remove it in cleanup.
Proposed fix
METRO_PID=""
+METRO_LOG="$(mktemp "${TMPDIR:-/tmp}/emotion-diary-metro-smoke.XXXXXX")"
cleanup() {
+ rm -f -- "$METRO_LOG"
if [[ -n "${METRO_PID:-}" ]] && kill -0 "$METRO_PID" 2>/dev/null; then
kill "$METRO_PID" 2>/dev/null || true
wait "$METRO_PID" 2>/dev/null || true
@@
- CI=1 npx expo start --dev-client --port 8081 > /tmp/emotion-diary-metro-smoke.log 2>&1 &
+ CI=1 npx expo start --dev-client --port 8081 > "$METRO_LOG" 2>&1 &
@@
- tail -n 40 /tmp/emotion-diary-metro-smoke.log >&2 || true
+ tail -n 40 "$METRO_LOG" >&2 || true
@@
- tail -n 40 /tmp/emotion-diary-metro-smoke.log >&2 || true
+ tail -n 40 "$METRO_LOG" >&2 || true🧰 Tools
🪛 ast-grep (0.45.1)
[warning] 82-82: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/emotion-diary-metro-smoke.log
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 91-91: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/emotion-diary-metro-smoke.log
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
[warning] 102-102: Writing to or reading from a hardcoded, predictable path under /tmp is vulnerable to symlink and TOCTOU attacks: a local attacker can pre-create the file (or a symlink pointing elsewhere) and hijack or corrupt the contents. Generate a unique, unpredictable temporary file with mktemp instead, e.g. tmpfile="$(mktemp)" (or mktemp -d for directories) and reference "$tmpfile".
Context: /tmp/emotion-diary-metro-smoke.log
Note: [CWE-377] Insecure Temporary File.
(predictable-tmp-file-bash)
🪛 GitHub Check: SonarCloud Code Analysis
[warning] 83-83: "npx" can install packages on-demand and run their lifecycle scripts.
🤖 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 `@scripts/sim-widget-smoke.sh` around lines 82 - 103, Replace the predictable
`/tmp/emotion-diary-metro-smoke.log` path in the Metro startup and
readiness-error handling with a private file created via `mktemp`, storing its
path in a variable. Use that variable for every related `tail` call and remove
the temporary log during `cleanup`, while preserving the existing Metro process
and timeout behavior.
Source: Linters/SAST tools
Comment on lines
+140
to
+156
| assert_snapshot() { | ||
| local found=0 | ||
| local raw="" | ||
| for _ in 1 2 3 4 5 6 7 8 9 10; do | ||
| if [[ -f "$PREFS_PLIST" ]] && plutil -extract "$SNAPSHOT_KEY" raw "$PREFS_PLIST" >/dev/null 2>&1; then | ||
| raw="$(plutil -extract "$SNAPSHOT_KEY" raw "$PREFS_PLIST" 2>/dev/null || true)" | ||
| if echo "$raw" | grep -q '"schemaVersion"'; then | ||
| found=1 | ||
| echo "OK: App Group contains $SNAPSHOT_KEY (schema present)" | ||
| echo "$raw" | head -c 240 | ||
| echo "" | ||
| break | ||
| fi | ||
| fi | ||
| sleep 1 | ||
| done | ||
| return $((1 - found)) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Require a new valid v1 snapshot from this run.
assert_snapshot accepts any retained value that contains "schemaVersion". A snapshot from a previous run, or an unsupported schema version, can pass this check even when background_and_publish did not publish a usable widget_snapshot_v1 payload.
Clear the snapshot through a supported storage path before publishing. Then parse and validate the expected schema version after the background transition.
🤖 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 `@scripts/sim-widget-smoke.sh` around lines 140 - 156, Update assert_snapshot
to validate that the snapshot was freshly published by this run: clear the
existing snapshot through the supported storage path before
background_and_publish, then parse the resulting widget_snapshot_v1 payload and
require the expected schema version rather than merely matching "schemaVersion".
Preserve the retry behavior and success output only for a valid v1 snapshot.
… paths Show Xinqing (not 心晴) on English Soft Stack chrome, localize gallery strings, clear snapshots before sign-out, refresh On This Day on focus, and tighten SnapshotReader/exported/Maestro gates with unit coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
NSNumber bridges to Bool on Apple platforms, so checking `is Bool` first made every schemaVersion/updatedAt fail and Soft Stack stayed cleared. Co-authored-by: Cursor <cursoragent@cursor.com>
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
widget_snapshot_v1白名单契约 + iOS Soft Stack WidgetKit / Android AppWidget + config plugin / EAS appExtensionsTest plan
yarn typecheck && yarn lint && yarn testyarn verify:widget-native+ focused widget Jestemotiondiary:///→ Dashboard17-USER-SETUP.md）Notes
260810-feat-rel-foundation→masterwidget-snapshotYarn autolinking、containerBackground 无白边、登录态才 publish（避免登出后 Soft Stack 被 republish）Summary by CodeRabbit