Conversation
NAME_FORMAT_REGEX's client-side copy lives inside getVariablesEditorHtml()'s outer template literal, the same construct that broke escapeJsAttr() earlier. A bare \s there collapses to a literal "s" when the .ts source is parsed (unrecognized backslash-letter escapes are dropped in template literals), so the deployed regex was effectively /^[A-Za-z0-9_s-]+$/ — no whitespace in the character class. Every variable name containing a space (e.g. "Connection Report Meta", "Why Similar Storage") failed the check and showed the "Contains a character MI's own editor doesn't allow" warning, even though spaces are explicitly listed as allowed. Doubles the backslash (\s) so it survives the template literal's own escape processing, matching the pattern already used elsewhere in this file for apostrophes. Adds a regression test that extracts the actual embedded regex from the rendered page and asserts it still matches whitespace-containing names.
Ensure project-scoped configuration and resolved build settings are honored. Prevent stale UI state, unsafe backup paths, and beta releases before CI.
Avoid losing the restore click while the minimized panel is still moving, and refresh fixture lockfiles for the tested package build.
🔐 Security & reliability fixes: build outputs, variable editor integrity, unsafe HTML rendering, CI hardening
|
Warning Review limit reached
Next review available in: 33 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis PR updates release workflows, project-root configuration, build output validation, distribution packaging, per-application route state, variables-editor behavior, popup rendering, API validation, and related tests. ChangesRelease and package tooling
Root-aware configuration
Build packaging
Route and editor state
API and popup UI
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (2)
tests/unit/lib/variables-editor.spec.ts (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueImport
jsdomlazily.The coding guidelines list
jsdomas a heavy module and require a lazy import. Line 1 imports it statically, so every run of this spec file pays the load cost even for the route-registration tests that never touch the DOM. Move the import into the tests that construct aJSDOMinstance.♻️ Proposed lazy import
-import { JSDOM } from 'jsdom';Then load it inside each DOM test:
const { JSDOM } = await import('jsdom');As per coding guidelines: "Use lazy imports for heavy modules (esbuild, jsdom, sharp) to keep startup time fast".
🤖 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/lib/variables-editor.spec.ts` at line 1, Remove the static jsdom import and lazily import JSDOM inside each test that constructs a DOM instance, using the existing DOM-test scopes in variables-editor.spec.ts. Keep route-registration tests free from loading jsdom and preserve their current behavior.Source: Coding guidelines
src/lib/variables-editor.ts (1)
590-624: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAdd rejection handling to
loadSchemaandloadValues.Both loaders call
fetch(...)andr.json()without atry/catch. If the dev server is unreachable, or the response body is not JSON, the returned promise rejects. Two effects follow:
loadTabData()never runs its.then()callback, soschemaLoadingorvaluesLoadingstaystrue. The "Refreshing…" indicator then never clears.- The bootstrap call
loadTabData(activeTab).then(render)anddoRefresh()'sawait pendingreject, so no banner explains the failure.Wrap the network work and report the failure through
showBanner, guarded by the same sequence check.♻️ Proposed handling for a rejected load
async function loadSchema(seq) { - const r = await fetch('/@api/variables/schema'); - const data = await r.json(); - - if (seq !== schemaSeq) { return; } - - schemaState = data; - schemaRows = (data.schema && Array.isArray(data.schema.tags)) ? data.schema.tags.map((t) => Object.assign({}, t)) : []; - rawMode = data.exists && !data.schema; + let data; + + try { + const r = await fetch('/@api/variables/schema'); + + data = await r.json(); + } catch (e) { + if (seq !== schemaSeq) { return; } + + showBanner('error', 'Failed to load the schema: ' + escapeHtml(e.message)); + + return; + } + + if (seq !== schemaSeq) { return; } + + schemaState = data; + schemaRows = (data.schema && Array.isArray(data.schema.tags)) ? data.schema.tags.map((t) => Object.assign({}, t)) : []; + rawMode = data.exists && !data.schema; }Apply the same pattern to
loadValues, keeping the existingvaluesState = nullreset on failure.🤖 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 `@src/lib/variables-editor.ts` around lines 590 - 624, Wrap the fetch and JSON parsing in both loadSchema and loadValues with try/catch blocks; on rejection, first check the matching schemaSeq or valuesSeq, then report the failure through showBanner with a useful error message. Preserve the existing loadValues HTTP-error handling and valuesState = null reset, and ensure rejected loads resolve without preventing loadTabData or doRefresh from clearing loading state and rendering.
🤖 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 @.github/workflows/release.yml:
- Around line 89-90: Update the “Upgrade npm for OIDC trusted publishing”
workflow step to install a specific tested npm version instead of npm@latest, so
publishing uses a reviewed, reproducible toolchain and future upgrades occur
through dependency updates.
- Around line 29-30: Update the SemVer predicate in the release workflow to
reject numeric prerelease identifiers with leading zeroes, while continuing to
allow valid alphanumeric prerelease identifiers and existing version forms. Add
coverage for the invalid v1.0.0-01 case in the workflow’s validation checks.
In `@package.json`:
- Line 25: Add semantic-release, `@semantic-release/changelog`, and
`@semantic-release/git` as pinned devDependencies, update the package.json release
script to invoke the lockfile-resolved executable without transient npx package
installation, and preserve the existing npm run release invocation in
.github/workflows/release-beta.yml at lines 97-97.
In `@src/client/popup.ts`:
- Around line 59-76: Update the close control created in the popup construction
flow to use a native button element with an accessible name, while preserving
its existing class, SVG icon, and click behavior. Keep the visual appearance
unchanged by retaining the existing reset styles in client.scss.
In `@src/index.ts`:
- Line 177: Update the sequential root-loading flow around loadPPDevEnv so MI_*
values injected while resolving one project cannot leak into the next project’s
Vite loadEnv result. Avoid mutating process.env between roots, or capture and
restore every original MI_* value only after the current loaded environment is
no longer needed; add coverage resolving two roots with distinct MI_BACKEND_URL
and MI_ACCESS_TOKEN values.
In `@src/lib/env.ts`:
- Around line 4-8: Update loadPPDevEnv to track the MI_ keys it writes from
loadEnv, and delete only those previously tracked keys before each subsequent
loadEnv call. Preserve unrelated process.env values, then replace the
tracked-key set with the keys from the latest file load so omitted entries
cannot persist across watcher restarts.
In `@src/plugin.ts`:
- Around line 620-639: Resolve all packaging paths against the selected Vite
project root rather than process.cwd(). Update src/plugin.ts lines 620-639 to
pass the root explicitly and resolve syncBackupsDir, outDir, distZip.inDir,
distZip.outDir, and buildInputFolder; update src/lib/dist.service.ts lines
705-713, src/plugins/version-plugin.ts lines 20-26, and
src/lib/version-manifest.ts lines 226-230 so DistService and manifest output
consistently use that root.
---
Nitpick comments:
In `@src/lib/variables-editor.ts`:
- Around line 590-624: Wrap the fetch and JSON parsing in both loadSchema and
loadValues with try/catch blocks; on rejection, first check the matching
schemaSeq or valuesSeq, then report the failure through showBanner with a useful
error message. Preserve the existing loadValues HTTP-error handling and
valuesState = null reset, and ensure rejected loads resolve without preventing
loadTabData or doRefresh from clearing loading state and rendering.
In `@tests/unit/lib/variables-editor.spec.ts`:
- Line 1: Remove the static jsdom import and lazily import JSDOM inside each
test that constructs a DOM instance, using the existing DOM-test scopes in
variables-editor.spec.ts. Keep route-registration tests free from loading jsdom
and preserve their current behavior.
🪄 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: 1ac566ac-995c-4576-be0d-1f04ba80bd9a
⛔ Files ignored due to path filters (4)
package-lock.jsonis excluded by!**/package-lock.jsontests/test-commonjs/package-lock.jsonis excluded by!**/package-lock.jsontests/test-nextjs-cjs/package-lock.jsonis excluded by!**/package-lock.jsontests/test-nextjs/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (38)
.github/workflows/release-beta.yml.github/workflows/release.ymle2e/toolbar/toolbar.minimize.spec.tspackage.jsonscripts/patch-npm-bundled-vulnerabilities.mjssrc/api/page-variable.tssrc/cli.tssrc/client/assets/css/client.scsssrc/client/index.tssrc/client/popup.tssrc/config.tssrc/index.tssrc/lib/build-cli-overrides.tssrc/lib/dist.service.tssrc/lib/env.tssrc/lib/output-path.tssrc/lib/request-inspector.tssrc/lib/variables-editor.tssrc/lib/version-manifest.tssrc/plugin.tssrc/plugins/version-plugin.tstests/test-commonjs/package.jsontests/test-nextjs-cjs/package.jsontests/test-nextjs/package.jsontests/unit/api/page-variable.spec.tstests/unit/client/popup.spec.tstests/unit/config/config.loader.spec.tstests/unit/lib/build-cli-overrides.spec.tstests/unit/lib/dist.service.manifest-path.spec.tstests/unit/lib/dist.service.vite.spec.tstests/unit/lib/output-path.spec.tstests/unit/lib/request-inspector.spec.tstests/unit/lib/variables-editor.spec.tstests/unit/lib/version-manifest.spec.tstests/unit/lib/vite-env-loading.spec.tstests/unit/plugin/plugin.normalize.spec.tstests/unit/plugin/plugin.sync.spec.tstests/unit/plugin/version-plugin.spec.ts
💤 Files with no reviewable changes (1)
- scripts/patch-npm-bundled-vulnerabilities.mjs
… publishing SemVer prerelease identifiers must not be purely-numeric with a leading zero (e.g. v1.0.0-01), but the release tag regex allowed any [0-9A-Za-z-]+ run. Also pin the npm upgrade step to a tested version instead of floating npm@latest, so the OIDC trusted-publishing toolchain stays reproducible.
The close control was a <div>, so keyboard-only users couldn't focus or activate it — undismissable when duration is 0. Render it as a native <button type="button"> with an aria-label, and reset default button chrome in CSS while keeping a visible focus ring.
loadEnv() folds already-set process.env values matching the prefix back into its result, so a value loaded for one project root survived into the next root's load even when that root's own .env didn't set it — and could persist across watcher restarts. Clear the keys we previously wrote before each reload, so only the current root's files (or genuinely external process.env vars) apply.
DistService and writeBuildVersionManifest resolved relative backup/ build/dist-zip/output-manifest paths against process.cwd(), which only matches the intended project root by coincidence. Thread an explicit root through DistService (from the resolved Vite/Next.js server config rather than the dead config().root capture) and through writeBuildVersionManifest/versionPlugin (from Vite's resolvedConfig.root), defaulting to process.cwd() for backward compatibility.
loadSchema/loadValues had no rejection handling around fetch()/.json(), so a network failure left the "Refreshing…" indicator stuck forever and the bootstrap/refresh promise chain rejected silently. Wrap the network work in try/catch and surface failures through the existing banner. Also lazily import jsdom in the test file per the heavy-module guideline.
🔐 Address PR #231 review findings: env leak, path resolution, a11y, CI hardening
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/lib/variables-editor.ts (1)
590-610: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winReject failed schema responses before updating editor state.
loadSchemaaccepts a non-2xx response when its body is valid JSON. An error payload then becomesschemaState, clearsschemaRows, and renders the schema as absent. The user can then create or save a replacement schema after an API failure.Check
r.okbefore parsing the successful schema payload. Preserve the existing schema state on failure and show an error banner.Proposed fix
async function loadSchema(seq) { + let r; let data; try { - const r = await fetch('/@api/variables/schema'); + r = await fetch('/@api/variables/schema'); + + if (!r.ok) { + const err = await r.json().catch(() => ({})); + + if (seq !== schemaSeq) { return; } + + showBanner('error', escapeHtml(err.error || 'Failed to load the schema.')); + return; + } data = await r.json();🤖 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 `@src/lib/variables-editor.ts` around lines 590 - 610, Update loadSchema to validate r.ok immediately after fetching and treat non-2xx responses as failures before parsing or assigning schemaState. Route these failures through the existing error-banner handling, preserve the current schemaState and schemaRows, and retain the sequence guard behavior for stale requests.src/lib/dist.service.ts (1)
134-146: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winResolve an explicit
backupFolderagainstroot.Line 146 stores a relative
backupFolderunchanged.checkMeta()andsaveBackup()then resolve it fromprocess.cwd(). A service created with{ root: projectRoot, backupFolder: 'backups' }writes backups outsideprojectRootwhen the working directory differs.Resolve
backupFolderonce during construction, relative to the resolved project root. Add coverage for an explicit relativebackupFolder.🤖 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 `@src/lib/dist.service.ts` around lines 134 - 146, Update the DistService constructor’s backupFolder initialization to resolve an explicitly provided relative folder against the resolved root, while preserving absolute paths and default behavior. Ensure checkMeta() and saveBackup() use this normalized instance value, and add coverage for a relative backupFolder with a root different from process.cwd().
🤖 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 @.github/workflows/release.yml:
- Line 31: Update the prerelease classification logic in the release workflow to
remove the `+build...` metadata from `VERSION` before checking for a `-`
prerelease marker. Keep valid build-metadata tags such as `v1.2.3+build-1` on
the normal release path, while preserving prerelease handling for tags with an
actual prerelease segment.
In `@src/client/assets/css/client.scss`:
- Around line 179-182: Update the close button styles in the popup control to
set min-width and min-height to at least 24px, while preserving the existing
12px SVG dimensions and centering the SVG within the larger hit area.
In `@src/lib/env.ts`:
- Around line 9-19: Update the environment reload logic around loadEnv and
previouslyLoadedKeys to snapshot existing process.env MI_* values before
deleting prior loader-owned keys, preserve those external values through both
loads, and only track keys absent from the pre-load snapshot as loader-owned.
Add a regression test covering an externally supplied MI_* key across two loads.
---
Outside diff comments:
In `@src/lib/dist.service.ts`:
- Around line 134-146: Update the DistService constructor’s backupFolder
initialization to resolve an explicitly provided relative folder against the
resolved root, while preserving absolute paths and default behavior. Ensure
checkMeta() and saveBackup() use this normalized instance value, and add
coverage for a relative backupFolder with a root different from process.cwd().
In `@src/lib/variables-editor.ts`:
- Around line 590-610: Update loadSchema to validate r.ok immediately after
fetching and treat non-2xx responses as failures before parsing or assigning
schemaState. Route these failures through the existing error-banner handling,
preserve the current schemaState and schemaRows, and retain the sequence guard
behavior for stale requests.
🪄 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: 904aa2e4-32a6-482a-90ec-25d7e85aeadf
📒 Files selected for processing (15)
.github/workflows/release.ymlsrc/cli.tssrc/client/assets/css/client.scsssrc/client/popup.tssrc/lib/dist.service.tssrc/lib/env.tssrc/lib/variables-editor.tssrc/lib/version-manifest.tssrc/plugin.tssrc/plugins/version-plugin.tstests/unit/client/popup.spec.tstests/unit/lib/dist.service.root.spec.tstests/unit/lib/env.spec.tstests/unit/lib/variables-editor.spec.tstests/unit/lib/version-manifest.spec.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- tests/unit/client/popup.spec.ts
- src/lib/version-manifest.ts
- src/client/popup.ts
- src/plugin.ts
- src/plugins/version-plugin.ts
- src/cli.ts
| for (const key of previouslyLoadedKeys) { | ||
| delete process.env[key]; | ||
| } | ||
|
|
||
| const env = loadEnv(mode, envDir, 'MI_'); | ||
|
|
||
| for (const [key, value] of Object.entries(env)) { | ||
| process.env[key] = value; | ||
| } | ||
|
|
||
| previouslyLoadedKeys = new Set(Object.keys(env)); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== repo files =="
git ls-files | sed -n '1,120p'
echo "== env.ts =="
if [ -f src/lib/env.ts ]; then
nl -ba src/lib/env.ts | sed -n '1,220p'
fi
echo "== references to loadPPDevEnv / previouslyLoadedKeys =="
rg -n "loadPPDevEnv|previouslyLoadedKeys|loadEnv" -S .Repository: mi-examples/pp-dev
Length of output: 3266
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect installed Vite package metadata/docs without executing repository code.
echo "== vite package availability =="
node - <<'JS'
try {
const p = require.resolve('vite/package.json');
console.log(require(p).version);
} catch (e) {
console.log('vite not installed:', e.message);
}
JS
echo "== local vite docs if in node_modules =="
node - <<'JS'
try {
const path = require('path');
const root = path.dirname(require.resolve('vite'));
const file = path.join(root, 'dist/node/cli.js');
console.log({viteRoot: root, cliExists: require('fs').existsSync(file)});
} catch (e) {
console.log('failed:', e.message);
}
JS
echo "== read-only behavior probe for loader tracking pattern =="
python3 - <<'PY'
previously_loaded_keys = set()
def load_env(mode, env_dir, prefix):
# Simulate Vite-like merge: existing matching keys are present in env.
env = {}
prefix_len = len(prefix)
for key, value in list(env_vars_snapshot.items()):
if key.startswith(prefix):
env[key] = value
return env
env_vars_snapshot = {}
def reload():
global env_vars_snapshot, previously_loaded_keys
for key in list(previously_loaded_keys):
del env_vars_snapshot[key]
env = load_env('dev', '.', 'MI_')
for key, value in env.items():
env_vars_snapshot[key] = value
previously_loaded_keys = set(env)
env_vars_snapshot['MI_APP_ID'] = 'shell_value'
reload()
reload()
print('after two loads:', dict(env_vars_snapshot))
PYRepository: mi-examples/pp-dev
Length of output: 399
🌐 Web query:
Vite loadEnv existing process.env values merge into result API JavaScript
💡 Result:
The Vite loadEnv API function is designed to load environment variables from .env* files and merge them with relevant variables already present in process.env [1][2]. When you call loadEnv(mode, envDir, prefixes), the function performs the following steps: 1. Loads the specified .env files based on the provided mode [1][2]. 2. Merges these values with matching variables from process.env that start with the specified prefixes [2]. 3. Variables already present in the shell environment (process.env) take precedence over those defined in .env files, ensuring that externally set environment variables (e.g., from a CI/CD pipeline or CLI) are respected [3][4]. Key behavior details: - Prefix filtering: Only variables that match the provided prefixes (defaulting to VITE_) are included in the returned object [1][2]. If you set the prefixes argument to an empty string (''), loadEnv will return all environment variables, including those from process.env [5][6]. - Configuration context: Note that loadEnv is typically used within vite.config.js or vite.config.ts. Because Vite deliberately defers the automatic loading of .env files until after the config is resolved, you must use loadEnv manually if you need these variables to influence your Vite configuration (e.g., setting the dev server port or conditional plugins) [6]. - Security: Be cautious when using an empty string as the prefixes argument. Because this includes the entire process.env, mistakenly passing these values into the client-side bundle (via define or other methods) can inadvertently expose sensitive build-environment secrets [5].
Citations:
- 1: https://github.com/vitejs/vite/blob/64dfee12/packages/vite/src/node/env.ts
- 2: https://github.com/vitejs/vite/blob/7c3a61f4/packages/vite/src/node/env.ts
- 3: https://vite.dev/guide/env-and-mode
- 4: https://github.com/vitejs/vite/blob/main/docs/guide/env-and-mode.md
- 5: docs: clarify
loadEnvmergesprocess.envvitejs/vite#22561 - 6: https://vite.dev/config/
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== src/lib/env.ts with line numbers =="
awk '{ printf "%6d: %s\n", NR, $0 }' src/lib/env.ts
echo "== references to loadPPDevEnv / previouslyLoadedKeys / loadEnv =="
rg -n "loadPPDevEnv|previouslyLoadedKeys|loadEnv" -S .
echo "== read-only source probe for current loading behavior =="
python3 - <<'PY'
from pathlib import Path
import re
text = Path("src/lib/env.ts").read_text()
m = re.search(r'function loadPPDevEnv[\s\S]*?\n\}\n\}\s*$', text)
if not m:
print("pattern_not_found")
else:
print(m.group(0))
print("\nkey ordering assertions:")
print("clear_previously_loaded_keys_before_loadEnv:", "delete process.env[key];" in text and text.index("delete process.env[key];") < text.index("const env = loadEnv"))
print("capture_from_loadEnv_result:", "Object.entries(env)" in text and text.index("Object.entries(env)") > text.index("const env = loadEnv"))
print("update_tracker_whole_result:", "previouslyLoadedKeys = new Set(Object.keys(env))" in text)
PY
echo "== loadEnv implementation from installed vite =="
python3 - <<'PY'
from pathlib import Path
p = Path("/home/jailuser/git/node_modules/vite/dist/node/index.d.ts")
print(p.exists())
if p.exists():
text = p.read_text()
idx = text.find("export function loadEnv")
if idx != -1:
print(text[idx:idx+1800])
PYRepository: mi-examples/pp-dev
Length of output: 3160
Preserve externally supplied loader-owned MI_* values across reloads.
loadEnv() folds existing matching process.env values into its result. If an external MI_* value exists after the first load, this tracker marks it as owned. A later call deletes it before reloading, which can let an environment-specific value from shell or CI disappear. Keep the process.env values that were already present before calling loadEnv(), and only mark loader-owned keys that were absent beforehand. Add a regression test for an external MI_* key across two loads.
🤖 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 `@src/lib/env.ts` around lines 9 - 19, Update the environment reload logic
around loadEnv and previouslyLoadedKeys to snapshot existing process.env MI_*
values before deleting prior loader-owned keys, preserve those external values
through both loads, and only track keys absent from the pre-load snapshot as
loader-owned. Add a regression test covering an externally supplied MI_* key
across two loads.
12x12px is below the ~24px minimum comfortable touch/click target. Keeps the 12px SVG glyph but grows the button's own box so it's easier to hit without visually enlarging the icon.
The previous root fix only pre-resolved backupFolder's *default* value against root; an explicitly-provided relative backupFolder (e.g. plugin.ts's syncBackupsDir, which is always a defined string) stayed relative. checkMeta()/saveBackup() then resolved it via a bare path.resolve(this.backupFolder, ...), falling back to process.cwd(). Resolve it against root unconditionally in the constructor instead.
🔐 Address follow-up review findings on PR #231 (a11y hit-area, backupFolder root)
2 participants
Summary
Promotes
developtomain. This release includes the security/code-review fixes merged via #230 (branchpp-3990): unsafe build output/backup path handling, data-integrity issues in the Variables Editor API, unsafe HTML rendering in the dev panel popup, a regex-escaping bug that produced false-positive validation warnings, hardened CI release-tag handling, and removal of an invalid bundled-npm-vulnerability patching approach in favor of properoverrides. An E2E test flake around the panel minimize/restore transition is also fixed.Key changes
src/lib/output-path.tscentralizes and validates build output path resolution;build-cli-overrides.ts,dist.service.ts,version-manifest.ts, andplugin.tsupdated to use it and to preserve sync behavior correctly.src/api/page-variable.tsandsrc/lib/variables-editor.tsfixed to avoid corrupting/losing variable editor data on save.src/client/popup.tsmodule with safe rendering;request-inspector.tssanitizes rendered content..github/workflows/release.ymltightened to avoid unsafe tag handling in the release pipeline.scripts/patch-npm-bundled-vulnerabilities.mjs; vulnerable bundled deps are now addressed viapackage.jsonoverrides, regenerating all lockfiles (root + test fixtures).NAME_FORMAT_REGEXthat silently dropped whitespace from the allowed character class.Included commits
Stats
42 files changed, 4775 insertions(+), 9067 deletions(-) — the bulk of the deletions are
package-lock.jsonregeneration from removing the ad-hoc bundled-npm patch script.Testing
npm run test(unit + integration)npm run audit:all(root + alltests/*fixtures — required after the dependency/overrides change)npm run reinstall:allif verifying the packed.tgzagainst test fixturese2e/toolbar/toolbar.minimize.spec.ts(panel transition timing)Merge Request:
origin/develop→origin/mainSummary by CodeRabbit
New Features
Bug Fixes