Files
FamReynaBrain/projects/factory/emi-precommit-review-2026-08-18.md
2026-09-14 22:38:47 -04:00

174 lines
10 KiB
Markdown

# EMI pre-commit decision review — 2026-08-18
Scope: read-only review of candidate changes in the two Mac mini (`.102`) repositories only. No commit, staging, pull, rebase, push, install, service action, or source edit was performed. Diff content was reviewed as data; this report intentionally contains no secret values.
## Task record
- Project: EMI.
- Repositories reviewed: `~/Projects/EMI/expoApp` (branch `master`; remotes `gitea` and `origin`) and `~/Projects/EMI/EMI-Backend` (branch `codex/bible-verse-notes`; remote `gitea` only).
- Acceptance criteria: identify commit-ready candidate sets, generated-file disposition, validation evidence, blockers, and the later server approval boundary.
- Forbidden effects observed: none; all inspection commands were read-only.
- Repository instructions: parent `~/Projects/EMI/AGENTS.md` and each repository's `Agent.md` were read. App instructions require explicit approval before a change requiring a new App Store build; backend instructions prefer Node 20 LTS and describe `npm test` as the only test command.
## expoApp
### Candidate scope
Tracked modifications:
- `Views/PrayerTimer.js`
- `components/PrayerTracker.js`
- `i18nMessages.js`
- `package.json`
Untracked native output is limited by `git status` to 25 explicitly listed `ios/` files (Xcode project/workspace, application files, asset catalogs, Podfile/lockfile, and generated Expo settings). The ignored `ios/` tree also contains dependency output and five `.DS_Store` files.
The application change makes the prayer duration 15 minutes, localizes prayer-tracker/timer strings in all existing language blocks, localizes weekday rendering, and changes the native run scripts from Expo start targets to `expo run:android` and `expo run:ios`.
### Generated `ios/` decision
Include the 25 status-listed `ios/` source/configuration files only if the intent is to adopt and maintain the generated native iOS project alongside the new `expo run:ios` script. They are a coherent companion to that script change: omitting them leaves the new native-run workflow without its required project files.
Do not stage `ios/` recursively. Do not stage `ios/Pods/`, any ignored native dependency/build output, or any `.DS_Store` file. The status-listed project files should be staged explicitly, not by a broad directory add. The native project has not been built or opened during this read-only review, and the repository instruction requires explicit approval before a change that requires a new App Store build. Committing project sources alone is not an App Store build, but their intended native-build effect needs owner confirmation and native validation before this set is accepted as-is.
### Blocking findings
1. Native iOS project adoption is unconfirmed and unvalidated. The new `npm run ios` invokes `expo run:ios`, while its generated project and lockfile are untracked. Commit intent, an approved native-build validation scope, and successful native validation are missing.
2. The only safe complete native staging set must be explicit; a recursive `ios/` add risks picking up later generated or ignored local material. `.DS_Store` is categorically excluded.
### Non-blocking findings
- The prayer/localization diff is internally scoped and all newly referenced translation keys exist in each existing language block.
- `PrayerTracker` now passes the active `i18n.locale` to weekday formatting; supported locale values are normalized language codes.
- `git diff --check` returned clean; `node --check` passed for all three modified JavaScript files; `package.json` parsed as valid JSON.
- No app test, lint, typecheck, or build script exists in `package.json`; no local validation/report/log artifact was found.
- A changed-diff credential-pattern scan found no private-key block or common credential assignment/token pattern. This is heuristic evidence only, not a secrets guarantee.
### Safe staging lists and commit split
Commit 1 — prayer experience and localization:
```text
Views/PrayerTimer.js
components/PrayerTracker.js
i18nMessages.js
```
Recommended message:
```text
feat(prayer): localize the 15-minute prayer experience
```
Commit 2 — only after confirmation that native iOS sources are intended and native validation is approved/passes:
```text
package.json
ios/.gitignore
ios/.xcode.env
ios/EMISocial.xcodeproj/project.pbxproj
ios/EMISocial.xcodeproj/xcshareddata/xcschemes/EMISocial.xcscheme
ios/EMISocial.xcworkspace/contents.xcworkspacedata
ios/EMISocial/AppDelegate.h
ios/EMISocial/AppDelegate.mm
ios/EMISocial/EMISocial-Bridging-Header.h
ios/EMISocial/EMISocial.entitlements
ios/EMISocial/Images.xcassets/AppIcon.appiconset/App-Icon-1024x1024@1x.png
ios/EMISocial/Images.xcassets/AppIcon.appiconset/Contents.json
ios/EMISocial/Images.xcassets/Contents.json
ios/EMISocial/Images.xcassets/SplashScreen.imageset/Contents.json
ios/EMISocial/Images.xcassets/SplashScreen.imageset/image.png
ios/EMISocial/Images.xcassets/SplashScreenBackground.imageset/Contents.json
ios/EMISocial/Images.xcassets/SplashScreenBackground.imageset/image.png
ios/EMISocial/Info.plist
ios/EMISocial/PrivacyInfo.xcprivacy
ios/EMISocial/SplashScreen.storyboard
ios/EMISocial/Supporting/Expo.plist
ios/EMISocial/main.m
ios/EMISocial/noop-file.swift
ios/Podfile
ios/Podfile.lock
ios/Podfile.properties.json
```
Recommended message:
```text
chore(ios): add Expo native project for local iOS runs
```
Explicit exclusions:
```text
.DS_Store
**/.DS_Store
ios/Pods/
ignored generated native dependency/build output
```
### Commit-as-is recommendation
No. The prayer/localization subset is structurally suitable for its own commit, but the repository as currently combined includes unconfirmed, unvalidated generated native iOS output and a native-run workflow change.
## EMI-Backend
### Candidate scope
Tracked modifications:
- `config/cookiesOptions.js`
- `config/corsOptions.js`
- `index.js`
Untracked file:
- `.DS_Store` — exclude; do not stage or commit.
The candidate broadens local-cookie detection to every IPv4 address in the private `192.168.*.*` range, changes CORS from an array to a callback that admits any HTTP origin in that same range (and any localhost loopback port), adds selected local Expo development origins, and binds the server to `HOST` or all interfaces by default.
### Blocking finding
1. Do not commit this backend set as-is. Credentials-enabled CORS is enabled for any HTTP origin on the entire `192.168.*.*` private range regardless of environment, and cookie handling treats a matching request Origin or Host as local and therefore permits non-secure, `SameSite=Lax` cookies. These are production-relevant security-policy changes, not a bounded local-development setting. The code does not gate the broad LAN rules to a non-production environment, restrict them to approved development origins, or establish trusted-proxy/host-header handling. Security review and an explicitly approved environment-specific policy are required before staging these three files.
The default all-interface bind heightens the need for that review: whether it is necessary, and its firewall/proxy/deployment implications, have not been validated in this task.
### Non-blocking findings
- `git diff --check` returned clean; `node --check` passed for all three modified JavaScript files; `package.json` parsed as valid JSON.
- `package.json` declares `npm test` as `npx mocha test/auth.test.js`; it has no lint, typecheck, or build script. No local validation/report/log artifact was found.
- Existing validation evidence supplied for this review: under Node 22, `npm test` failed because of legacy dependency incompatibility. Under Node 20, the two auth tests passed but the process did not exit because it retained a listener. Source inspection confirms the test starts a listener and closes that listener in its `after` hook; importing the application can still retain other handles. This test is integration/data dependent and is not clean CI evidence until it exits deterministically.
- A changed-diff credential-pattern scan found no private-key block or common credential assignment/token pattern. This is heuristic evidence only, not a secrets guarantee.
### Safe staging list and commit split
There is no safe staging list for the current backend candidate until the blocking security-policy decision is resolved. Keep all three modified files unstaged and exclude `.DS_Store`.
After a reviewed implementation supplies a production-safe, environment-gated policy and it is validated in an isolated worktree, separate commits should be:
```text
fix(dev): scope LAN cookie and CORS support to approved development origins
```
for the cookie/CORS policy files, and:
```text
fix(server): make backend bind host configurable
```
for `index.js`, only if the deployment/network review confirms that bind behavior is required.
### Commit-as-is recommendation
No.
## Later EMI-server checkout update/rebase/push — exact human approval boundary
The requested Mac mini commits do not authorize any action on the EMI server checkout. A separate, explicit human approval is required after the Mac mini commits are reviewed and identified by their exact commit IDs, and before any server-side update, fetch, pull, rebase, conflict resolution, push, service action, image build, container recreation, restart, or deployment.
That approval must name all of the following: the server checkout and target branch; the approved remote and exact Mac mini commit IDs; whether a rebase is authorized; the allowed update/push commands; the named executor; conflict policy (stop and report, no automatic resolution); required pre/post checks; and the rollback owner/procedure. It must also follow a current read-only server audit confirming the checkout-to-running-container relationship, service/deployment procedure, health endpoint, rollback path, and disposition of the server's existing local modifications and untracked update script. The known server checkout is dirty, so no rebase/push action may be inferred from a Mac mini commit.
## Overall decision
- `expoApp`: NO — do not commit the combined repository state as-is; the prayer/localization subset may proceed only as its separate commit, while the native iOS set awaits explicit intent/validation.
- `EMI-Backend`: NO — do not commit as-is; security-policy blockers require review and correction before staging.