Repository navigation
fix: refresh stale staging clients and keep Termux awake - #4958
Merged
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe changes add build-aware client refresh checks, Termux Android wake-lock management, and desktop end-to-end tests for NovelAI seed persistence. The changelog documents the refresh and wake-lock behavior. ChangesRuntime build compatibility
Termux wake-lock lifecycle
NovelAI persistence validation
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
SpicyMarinara
changed the title
SpicyMarinara
marked this pull request as ready for review
August 12, 2026 18:04
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@e2e/core-flows.e2e.ts`:
- Around line 2006-2018: Extend the test after locating importedConnectionId to
open that imported connection in the connection editor, then assert the NovelAI
Seed spinbutton displays value 73. Keep the existing API defaultParameters
assertion and use the editor’s accessible spinbutton query to verify hydration
into the UI.
- Around line 1887-1902: Move the connection and persona creation requests
inside the existing try block in the test flow, and declare nullable
connection/persona IDs before it. Assign each ID only after its POST succeeds,
then update the finally cleanup to delete whichever created IDs are non-null so
partial setup failures cannot leave records behind.
In `@packages/client/vite.config.ts`:
- Around line 21-24: Update the environment commit selection in
normalizeBuildCommit so blank or whitespace-only higher-priority variables are
skipped before choosing a fallback. Evaluate MARINARA_GIT_COMMIT, GITHUB_SHA,
and BUILD_COMMIT in priority order, selecting the first nonblank value and
preserving the existing normalization and return behavior.
In `@scripts/regressions/launcher-format-guard.regression.mjs`:
- Around line 73-85: Update the wake-lock ordering assertion around
termuxLauncherSource to locate the actual acquisition command, matching the “if
termux-wake-lock ...” invocation rather than the availability-check token.
Compare that match’s position with the lastIndexOf("node dist/index.js") so the
regression fails when acquisition occurs after server startup; preserve the
existing presence and release checks.
In `@start-termux.sh`:
- Around line 563-577: Update the Termux wake-lock setup so trap
release_termux_wake_lock is registered before invoking termux-wake-lock. Keep
TERMUX_WAKE_LOCK_ACQUIRED initialized to 0 so the EXIT trap does not attempt an
unlock when acquisition fails, and set it to 1 only after successful
acquisition.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8750f0aa-59d4-4a8f-9a0e-b29ec5b58ede
📒 Files selected for processing (9)
CHANGELOG.mde2e/core-flows.e2e.tspackages/client/src/App.tsxpackages/client/src/build-env.d.tspackages/client/src/lib/runtime-build.tspackages/client/vite.config.tsscripts/regressions/assigned-issue-sweep.regression.tsscripts/regressions/launcher-format-guard.regression.mjsstart-termux.sh
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@e2e/core-flows.e2e.ts`:
- Line 1925: Add export-path coverage in the test around the connection fetch:
export the saved connection before cleanup and assert that the exported
defaultParameters.imageGeneration.seed equals 50, while preserving the existing
storage and editor hydration assertions.
🪄 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: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: dda0b492-559d-4bd9-9969-6ab13043504d
📒 Files selected for processing (4)
e2e/core-flows.e2e.tspackages/client/vite.config.tsscripts/regressions/launcher-format-guard.regression.mjsstart-termux.sh
1 task
This was referenced Aug 13, 2026
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.
Linked issues
Closes #4957
Closes #4956
Why this change
NovelAI default parameters already survive the current save, navigation, and import paths, but staging commits keep the same release version. A browser still running an older same-version bundle therefore could not detect that the server had moved to a newer commit and could keep exhibiting the pre-fix behavior.
Android may also suspend a backgrounded Termux process unless its wake lock is active, which makes the local Marinara server appear frozen until Termux is foregrounded again.
What changed
/api/healthso same-version staging updates clear stale browser runtime caches and reload once.CHANGELOG.mdUnreleased.Validation
pnpm checkpasses locallyCONTRIBUTING.mdAutomated evidence
pnpm checkbash -n start-termux.shpnpm --filter @marinara-engine/server exec tsx ../../scripts/regressions/assigned-issue-sweep.regression.tsnode scripts/regressions/launcher-format-guard.regression.mjspnpm exec playwright test -c playwright.config.ts e2e/core-flows.e2e.ts --project=desktop-chromium --grep 'NovelAI generation defaults survive'(2 passed)Manual verification still required
Docs and release impact
docs-i18nbranch updated to match, or a[docs-i18n] <paths>follow-up issue openedNo user-facing documentation page changed;
CHANGELOG.mdcontains the release-note entries. No version metadata changed.Summary by CodeRabbit
Bug Fixes
Tests