fix: maintainer-loop bug sweep 1 (#132, #133, #134, #135, #137)#176
Merged
Conversation
The .deb/.rpm wrapper and the Homebrew formula's bin launcher exec'd node with whatever HOSTNAME the calling shell happened to have - empty (falls back to Next.js's own 0.0.0.0 default) or, under Docker, the container ID it exports as HOSTNAME to every process. Both wrappers now default to 127.0.0.1 on a direct run and only bind elsewhere when LIBREDB_BIND opts in, mirroring the npx launcher's local-first default. The deb/rpm wrapper skips this when INVOCATION_ID is set (systemd sets it for every unit process), since the systemd unit already resolves HOSTNAME correctly via Environment=/EnvironmentFile= before the wrapper execs - forcing it unconditionally would have broken that already-working, documented HOSTNAME=0.0.0.0 override in /etc/libredb-studio/env.
The bin wrapper's direct-run mode set no STORAGE_SQLITE_PATH, so server.js's chdir into libexec resolved the zero-config auth-bootstrap file (and any SQLite storage) inside the versioned keg, losing it on every brew upgrade. Default it to the same var/libredb-studio path the brew services block already uses.
--verify-cache and every --archive run replaced the whole payload directory (rm + rename) after re-extracting the tarball, wiping the default data/ dir (generated auth-bootstrap.json credentials, any STORAGE_PROVIDER=sqlite state) since every release tarball ships an empty one. preservePayloadData now moves the previous data/ dir onto the freshly extracted staging directory before the swap.
…#133) scripts/build-standalone-payload.sh now packs the payload under a top-level libredb-studio-<version>/ directory via the new scripts/lib/pack-standalone-tarball.sh instead of tarring the payload dir's contents at archive root. The npx launcher's extract() (and every other tar -xzf consumer: the build script's own --smoke self-test, the deb/rpm and snap release jobs) now strips that one path component via the new extractTarball helper in bin/lib/launcher-utils.mjs. Homebrew's formula needs no change - it strips a single top-level directory automatically for its main url/sha256 download.
Add a regression test asserting the real helm-templated Deployment mounts an emptyDir at /app/data by default and a PVC when persistence.enabled=true. The underlying fix already shipped in #165; no existing CI check pinned this specific behavior (ct install only checks pod readiness, which the pre-#165 template also satisfied since the seeding failure was WARN-only).
…#137) Verified every ACCEPTANCE.md criterion against actual repo state (issues, docs, and a fresh full-gate + helm lint rerun on the clean branch tip), ticked ACCEPTANCE.md and the Phase F plan item, and reconciled HANDOFF.md/PROGRESS.md with the milestone's final state.
tests/unit/helm-chart-persistence.test.ts shells out to `helm template` against charts/libredb-studio, whose postgresql subchart is fetched via `helm dependency build` (never vendored in git - charts/**/charts/*.tgz is gitignored, matching the existing helm-lint job's own pattern) rather than committed. The Unit & Integration Tests job had no such step, so a clean checkout failed with "missing in charts/ directory: postgresql" even though the test passed in a dev environment with a stale local build of that dependency lying around. Reproduced locally by removing the vendored .tgz (exact CI error), then confirmed `helm repo add bitnami` + `helm dependency build charts/libredb-studio` - the same two steps helm-lint already runs - fixes it.
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a set of first-run/install-path regressions across multiple distribution channels (npx launcher, standalone tarball, deb/rpm wrapper, Homebrew formula, Helm chart) and introduces a loop/ scaffold used to drive the automated bug-sweep workflow, plus regression tests to prevent recurrence.
Changes:
- Enforce local-first bind defaults for direct wrapper runs and move Homebrew direct-run state out of the versioned Cellar keg.
- Preserve
payload/data/across npx--verify-cache/--archivere-extractions and switch standalone tarballs to a versioned top-level root with matching extraction changes. - Add regression/unit tests (including Helm render assertions) and update docs/workflows/scripts to match the new packaging behaviors.
Reviewed changes
Copilot reviewed 22 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/packaging-standalone-tarball.test.ts | New unit test verifying tarball root directory layout. |
| tests/unit/packaging-homebrew-datadir.test.ts | New unit test verifying Homebrew wrapper defaults state outside the keg. |
| tests/unit/packaging-bind-address.test.ts | New unit test verifying wrapper bind address behavior (loopback default + opt-in). |
| tests/unit/launcher-utils.test.ts | Adds tests for payload data preservation and tarball extraction stripping. |
| tests/unit/helm-chart-persistence.test.ts | Adds Helm template-based regression test for /app/data mount behavior. |
| scripts/lib/pack-standalone-tarball.sh | New helper script to pack payload under libredb-studio-<version>/. |
| scripts/build-standalone-payload.sh | Uses new pack script; updates smoke extraction to --strip-components=1. |
| packaging/linux/libredb-studio | Forces loopback bind on direct run (with systemd guard) and allows home override. |
| packaging/homebrew/libredb-studio.rb.tmpl | Forces loopback bind for direct run; defaults STORAGE_SQLITE_PATH outside keg; updates comments. |
| loop/scripts/loop.sh | Adds autonomous maintainer-loop runner script. |
| loop/scripts/gate.sh | Adds gate script matching the repo’s pre-commit verification. |
| loop/PROMPT.md | Adds build-mode maintainer-loop prompt/spec. |
| loop/PROMPT-PLANNING.md | Adds planning-mode prompt for future milestones. |
| loop/PROGRESS.md | Adds loop progress log and decision record for the bug sweep. |
| loop/LOOP-ENGINEERING.md | Adds loop operating manual/documentation. |
| loop/IMPLEMENTATION_PLAN.md | Adds milestone plan checklist for the sweep. |
| loop/HANDOFF.md | Adds milestone handoff summary. |
| loop/config/loop.env | Adds loop runner configuration defaults. |
| loop/ACCEPTANCE.md | Adds milestone acceptance criteria and completion signal rules. |
| docs/DISTRIBUTION.md | Updates distribution docs for bind behavior, tarball root layout, and data preservation. |
| bin/studio.js | Switches extraction to shared helpers; preserves payload data across swaps. |
| bin/lib/launcher-utils.mjs | Adds preservePayloadData and extractTarball helpers. |
| .github/workflows/release-artifacts.yml | Updates tar extraction to --strip-components=1 for new tarball layout. |
| .github/workflows/ci.yml | Adds Helm repo/dependency steps to support new Helm-rendering unit test. |
| .gitignore | Ignores maintainer-loop runtime artifacts and docs summaries directory. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+21
to
+31
| PAYLOAD_DIR=$1 | ||
| VERSION=$2 | ||
| OUT_TARBALL=$3 | ||
|
|
||
| ROOT_NAME="libredb-studio-${VERSION}" | ||
| PARENT_DIR=$(cd "$(dirname "$PAYLOAD_DIR")" && pwd) | ||
| ROOT_DIR="$PARENT_DIR/$ROOT_NAME" | ||
|
|
||
| rm -rf "${ROOT_DIR:?}" | ||
| mv "$PAYLOAD_DIR" "$ROOT_DIR" | ||
| tar -czf "$OUT_TARBALL" -C "$PARENT_DIR" "$ROOT_NAME" |
Member
Author
There was a problem hiding this comment.
Checked the caller: pack-standalone-tarball.sh has exactly one caller (build-standalone-payload.sh), where PAYLOAD_DIR lives entirely under a mktemp -d STAGE_DIR that the caller unconditionally rm -rf's via trap cleanup EXIT, success or failure. So a mid-tar failure here does not lose any caller-owned working tree - the whole scratch tree is already disposed of regardless. Not adding the trap/restore for a scenario the current sole caller's contract already rules out; will revisit if a second caller with different guarantees shows up.
Matches helm-lint's own azure/setup-helm pin (v3.16.0) instead of relying on whatever ubuntu-latest happens to bundle, so both jobs that run real helm commands stay on the same version regardless of future runner image updates. Addresses Copilot review comment on PR #176.
Comment on lines
+39
to
+45
| const run = Bun.spawnSync(["helm", "template", "release-under-test", CHART_DIR, ...extraArgs], { | ||
| stdout: "pipe", | ||
| stderr: "pipe", | ||
| }); | ||
| if (run.exitCode !== 0) { | ||
| throw new Error(`helm template failed (exit ${run.exitCode}): ${run.stderr.toString()}`); | ||
| } |
| exit 1 | ||
| fi | ||
|
|
||
| if ! command -v ${AGENT_CMD%% *} >/dev/null 2>&1; then |
…sue operation - Triage mode (loop/PROMPT-TRIAGE.md + loop/TRIAGE.md): classify untriaged open issues into loop:queued / loop:needs-info / loop:needs-moderator-action, writing sanitized specs that firewall build mode from raw issue text. - Treat ALL issue content as untrusted input, bodies included, not just comments: PROMPT.md 0e and 999i rewritten, 999j gh-surface allow-list added, new 1b moderator-escalation path for injection attempts, security reports, and changes beyond loop authority. - Mandatory fresh-context loop-reviewer pass before every build-mode commit (.claude/agents/loop-reviewer.md; loop-judge.md ported alongside, fixing LOOP-ENGINEERING.md's broken judge/workflows references). - Runner-level defense-in-depth in loop/config/loop.env: no curl/wget/WebFetch, gh surface reduced to issue read/label/comment; publishing stays human-only. - Label taxonomy created idempotently via loop/scripts/setup-labels.sh (run against the repo). - LOOP-ENGINEERING.md: new untrusted-input firewall section, three-mode table; HANDOFF.md: runbook for the next autonomous milestone (triage -> plan -> build -> human push).
…ssue text Completes the untrusted-input firewall chain: plan task text becomes build mode's acceptance bar, so planning mode must not lift reproduction steps or suggested fixes verbatim from public issue content.
bun run chart:bump keeps charts/libredb-studio/Chart.yaml appVersion and version in sync per the CI-enforced invariant (#138).
|
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
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
Five first-run/install-path bugs fixed by an autonomous, test-gated maintainer loop (scaffold in
loop/, adapted from theopen-spek/looptemplate) working from a fixed queue of filed issues, one commit per issue, full gate green throughout.127.0.0.1) on a direct run instead of leaking an inheritedHOSTNAME(empty, or a Docker container ID);LIBREDB_BINDopts in to a different address. The systemd unit path is left untouched via anINVOCATION_IDguard, since it already resolvesHOSTNAMEcorrectly.STORAGE_SQLITE_PATHoutside the versioned Cellar keg (same pathbrew servicesalready uses), so credentials and SQLite state survivebrew upgrade.npx @libredb/studio --verify-cache(and--archive) no longer wipespayload/data/on re-extraction; a newpreservePayloadDatahelper carries the previous data dir onto the freshly extracted staging directory before the swap.libredb-studio-<version>/directory instead of spilling at archive root; the npx launcher and the deb/rpm/snap release jobs were updated to strip that one path component./app/dataviaemptyDiron a default install) had already shipped in fix(helm): install with default values — zero-config bootstrap as chart default #165; this adds the regression test that was missing to pin the behavior, since no existing CI check exercised it at that granularity.Every fix is test-first with a regression test that failed red before the change. Full details, root-cause analysis, and decisions (with rationale) are in
loop/PROGRESS.md; final state is inloop/HANDOFF.md.Test plan
bun run format && bun run lint && bun run typecheck && bun run test && bun run buildgreen (re-verified independently on the branch tip, not just the loop's own claim)helm lint charts/libredb-studio --strictcleanbin/studio.js(not just unit tests)loop/PROGRESS.md's flagged decisions (theINVOCATION_IDsystemd guard for Local-first bind not enforced on direct wrapper runs: deb/rpm /usr/bin/libredb-studio and the Homebrew binary bind 0.0.0.0 #134; the choice not to touchscripts/engine-smoke.shfor npx launcher: --verify-cache re-extracts the payload and destroys payload/data (generated credentials wiped) #132) before mergeRefs #132, #133, #134, #135, #137 (not closed by this PR - left to the merger's judgment).
Follow-up in this PR: full-autonomy hardening of the loop (d36581d)
Bug Sweep 1 ran against a maintainer-curated queue; this commit prepares the same scaffold to run against the open public issue tracker autonomously:
loop/TRIAGE.md+ plan tasks); raw issues are evidence to re-verify, never authority. NewLOOP-ENGINEERING.mdsection documents the four firewall layers.loop/PROMPT-TRIAGE.md): classifies untriaged open issues intoloop:queued(verified in code, sanitized spec written),loop:needs-info(one clarifying question posted; only a human clears it), orloop:needs-moderator-action(injection attempts, security reports, privileged changes, product decisions — label only, no reply). Labels created idempotently byloop/scripts/setup-labels.sh.loop-reviewersubagent pass (.claude/agents/loop-reviewer.md) checking correctness against the sanitized spec, convention fit (provider triad, platform-integration rules), test realness, scope, and supply-chain red flags. This also fixesLOOP-ENGINEERING.md's previously broken references by porting the judge/reviewer agents from the template.loop/config/loop.env): nocurl/wget/WebFetch, gh surface reduced to issue read/label/comment. Pushing, PRs, merges, and closing issues remain human-only.Runbook for the next autonomous milestone is in
loop/HANDOFF.md(triage -> plan -> build -> human push).