Skip to content

feat(infra): VM deployment without Docker (systemd + install/deploy scripts) - #3

Merged
hutusi merged 5 commits into
mainfrom
infra/vm-deployment
Jul 17, 2026
Merged

hutusi merged 5 commits into
mainfrom
infra/vm-deployment

Conversation

@hutusi

@hutusi hutusi commented Jul 17, 2026 •

Copy link
Copy Markdown
Contributor

Why

The repo shipped Docker Compose as the only production path, but nothing in the app actually requires containers: both services are plain Bun entry points, the API serves the built SPA via AGRIPPA_WEB_DIST, and migrations + seeding apply on API boot. Operators who run plain VMs (no Docker) had no supported way to deploy. Local dev mode already runs bare and is untouched by this change.

What

New infra/vm/ — the same topology as Compose, installed directly on one Ubuntu 22.04/24.04 host:

  • install.sh — idempotent bootstrap: Bun 1.3.14, Postgres 17 (PGDG), optional Redis 7 (--skip-redis; SSE falls back to DB polling), the worker's OS deps (git, ripgrep, bubblewrap), an agrippa system user, data dirs under /var/lib/agrippa, and /etc/agrippa/agrippa.env with generated secrets. Re-runs never rotate AGRIPPA_SECRET_KEY or the DB password — rotating the key would orphan every stored credential.
  • deploy.sh — update flow shared with first install: git pull --ff-only → bun install --frozen-lockfile → SPA build → restart api → wait for /healthz → restart worker, so schema changes always land before code that expects them. NODE_ENV=production is set only by the units, never during the build (the SPA build needs devDependencies).
  • agrippa-api.service / agrippa-worker.service — repo stays root-owned at /opt/agrippa (mirrors the images' root-owned /app), services run as agrippa. The worker's hardening is deliberately lighter than the api's: bubblewrap needs user/mount/pid/net namespaces, and the SDK sandbox degrades silently without them, so RestrictNamespaces=/SystemCallFilter= stay off. The worker never migrates, so its ExecStartPre gates on the api's /healthz (first boot and upgrades alike).
  • env.example, nginx.conf.example (SSE route with proxy_buffering off), and a pointer README.

Two accepted deviations from Compose, documented in docs/design/08-deployment.md rather than hidden: both services share one user (artifacts are not read-only to the api) and one env file (ANTHROPIC_API_KEY is visible to the api). Splitting either buys little on a single-org box.

Docs move with the change: design doc 08 (retitled "Self-Hosted", now names all three modes), manual 01 + 06 in both locales, README quick-start block, CHANGELOG under [Unreleased]. The ops manual gains troubleshooting rows for the two VM-specific failure modes (worker waiting on /healthz; Ubuntu 24.04 AppArmor userns blocking bwrap, with a one-line probe).

Verification

  • bun run check ✓ · bun test 126 pass / 0 fail, no skipped suites (local Postgres) · bun run templates:validate ✓ · bun run build ✓
  • shellcheck + bash -n clean on both scripts
  • Not yet exercised on a real Ubuntu host — the riskiest unknown is 24.04's apparmor_restrict_unprivileged_userns vs bubblewrap under the unit hardening; a throwaway multipass launch 24.04 smoke run of install.sh is the recommended follow-up before trusting a production VM.

No code changes — dev mode and the Docker Compose path are byte-identical.

Summary by CodeRabbit

  • New Features
    • Added self-hosted deployment on Ubuntu 22.04/24.04 VMs using systemd, without Docker.
    • Added automated installation and update workflows, including PostgreSQL, optional Redis, generated secrets, service setup, and health checks.
    • Added an Nginx reverse-proxy configuration example.
  • Documentation
    • Expanded English and Chinese setup, deployment, operations, backup, upgrade, and troubleshooting guides with VM instructions.

hutusi added 2 commits July 17, 2026 21:09
…ystemd units)

Operators without Docker had no supported production path — the repo shipped
Docker Compose only, while the app itself needs nothing containerized (Bun
entry points, API serves the SPA via AGRIPPA_WEB_DIST, migrate+seed on boot).

infra/vm/ codifies the same topology on one Ubuntu 22.04/24.04 host:

- install.sh: idempotent bootstrap (Bun, Postgres 17 via PGDG, optional
  Redis 7, agrippa system user, /etc/agrippa/agrippa.env with generated
  secrets, units). Re-runs never rotate AGRIPPA_SECRET_KEY or the DB
  password — rotating the key would orphan every stored credential.
- deploy.sh: git pull --ff-only, frozen install, SPA build, restart api,
  wait for /healthz (migrations apply on api boot), then restart worker —
  schema changes always land before code that expects them. The worker
  unit's ExecStartPre gates on /healthz for the same reason on first boot.
- Units mirror the images' security intent: repo root-owned, services run
  as agrippa. The worker's hardening is deliberately lighter than the
  api's — bubblewrap needs user/mount/pid/net namespaces, and the SDK
  sandbox degrades silently without them.

Two accepted deviations from Compose, documented in docs/design/08: both
services share one user (artifacts not read-only to the api) and one env
file (ANTHROPIC_API_KEY visible to the api).
The deployment docs were framed around Docker Compose only; with infra/vm/
in the tree, every page covering deployment needs the third mode so the
design doc stays the living source of truth and both manual locales move
together (repo convention).

- docs/design/08: retitled to Self-Hosted, names the three modes (dev
  bare-metal / Compose / VM systemd), adds the VM section with the layout
  mapping to Compose volumes, boot/upgrade ordering, the bubblewrap
  namespace constraint, and the two accepted deviations from Compose.
- manual 01 (en+zh): VM install/update quick start alongside Compose.
- manual 06 (en+zh): VM section (units, journalctl, layout, update flow);
  backup and upgrade sections now cover both modes; troubleshooting rows
  for the worker gating on /healthz and the Ubuntu 24.04 AppArmor userns
  restriction that silently degrades the sandbox. Also corrects the backup
  section's volume name (workdata never existed; the compose volume is
  artifacts).
- README: VM quick-start block; CHANGELOG under [Unreleased].
@coderabbitai

coderabbitai Bot commented Jul 17, 2026 •

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@hutusi, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 18 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5cdde8eb-1287-4618-90ac-0009156ee06e

📥 Commits

Reviewing files that changed from the base of the PR and between d5602fc and c78a52d.

📒 Files selected for processing (9)
  • AGENTS.md
  • CHANGELOG.md
  • CONTRIBUTING.md
  • README.md
  • apps/web/vite.config.ts
  • infra/vm/agrippa-worker.service
  • infra/vm/deploy.sh
  • infra/vm/install.sh
  • infra/vm/nginx.conf.example
📝 Walkthrough

Walkthrough

Adds Dockerless Ubuntu VM deployment using an idempotent installer, generated configuration, hardened systemd API/worker services, ordered updates, health checks, optional Redis, Nginx proxy guidance, and corresponding English/Chinese deployment and operations documentation.

Changes

VM Deployment

Layer / File(s) Summary
VM provisioning and service configuration
infra/vm/env.example, infra/vm/install.sh, infra/vm/agrippa-*.service, infra/vm/nginx.conf.example
Adds Ubuntu provisioning, PostgreSQL/optional Redis setup, generated secrets, runtime directories, hardened systemd units, and SSE-aware Nginx proxy configuration.
Install and update lifecycle
infra/vm/deploy.sh
Adds initial and subsequent deployment flow with dependency installation, SPA builds, API-first restart ordering, bounded /healthz polling, and worker restart.
Deployment and operations documentation
infra/vm/README.md, docs/design/08-deployment.md, docs/manual/{en,zh-CN}/*, README.md, CHANGELOG.md
Documents VM setup, service behavior, upgrades, backups, troubleshooting, supported self-hosting modes, and the new deployment path.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Operator
  participant deploy.sh
  participant agrippa-api
  participant PostgreSQL
  participant agrippa-worker
  Operator->>deploy.sh: run VM update
  deploy.sh->>agrippa-api: restart API
  agrippa-api->>PostgreSQL: migrate and seed on boot
  deploy.sh->>agrippa-api: poll /healthz
  deploy.sh->>agrippa-worker: restart after API health
Loading
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding a non-Docker VM deployment path with systemd and install/deploy scripts.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch infra/vm-deployment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 17, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds a complete VM deployment path (infra/vm/) for running Agrippa on Ubuntu 22.04/24.04 without Docker — install.sh bootstraps Bun, Postgres 17, optional Redis, the agrippa system user, and hardened systemd units; deploy.sh handles all subsequent updates with the same pull → build → restart ordering. No application code is changed.

  • install.sh is correctly idempotent: secrets and the DB password are generated only when the env file is absent, so re-runs never rotate AGRIPPA_SECRET_KEY.
  • The worker unit's ExecStartPre polls /healthz for up to 120 seconds to ensure migrations have applied before the worker starts, and deploy.sh replicates the same ordering gate at the shell level.
  • The worker's hardening is deliberately lighter than the API's (no RestrictNamespaces, no SystemCallFilter) because bubblewrap needs unrestricted user/mount/pid/net namespaces — the trade-off is documented both in the unit file and in the design doc.

Confidence Score: 4/5

Safe to merge after fixing the missing TimeoutStartSec in the worker unit; everything else is documentation or shell scripts with no application code changes.

The worker's ExecStartPre is designed to wait 120 seconds for the API to become healthy, but without an explicit TimeoutStartSec the unit is subject to systemd's 90-second default — on a slow first boot where migrations run for the first time, systemd will abort the start hook before the API is ready, leaving the worker in a failed state. The fix is a one-line addition to the unit file. All other findings are minor quality improvements (bounding individual curl calls). No application code is touched by this PR.

infra/vm/agrippa-worker.service needs a TimeoutStartSec entry; infra/vm/deploy.sh benefits from --max-time on the curl health-check loop.

Important Files Changed

Filename Overview
infra/vm/agrippa-worker.service Worker systemd unit: ExecStartPre polls /healthz for 120s but no TimeoutStartSec is set, so systemd's default 90s start timeout would abort the wait prematurely on slow first-boot migrations.
infra/vm/deploy.sh Deploy script: well-structured pull → build → restart ordering; health-check loop lacks --max-time per curl call, so a hung (not refused) healthz could stall indefinitely.
infra/vm/install.sh Idempotent bootstrap script; correctly guards secret regeneration behind an ENV_FILE existence check; Bun is installed via curl
infra/vm/agrippa-api.service API systemd unit: hardening is appropriate (NoNewPrivileges, ProtectSystem=full, PrivateTmp, PrivateDevices, RestrictNamespaces); no issues found.
infra/vm/env.example Env template with placeholder tokens substituted by install.sh; well-commented with backup warning for AGRIPPA_SECRET_KEY.
infra/vm/nginx.conf.example Nginx reverse-proxy example: SSE route correctly sets proxy_buffering off + proxy_http_version 1.1 + Connection ""; proxy_read_timeout 1h is appropriate for long-lived SSE connections.
docs/design/08-deployment.md Design doc updated to cover three deployment modes; documents the two accepted deviations from Compose (shared user, single env file) explicitly rather than hiding them.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant Op as Operator (root)
    participant IS as install.sh
    participant DS as deploy.sh
    participant PG as PostgreSQL
    participant API as agrippa-api.service
    participant W as agrippa-worker.service

    Op->>IS: sudo install.sh [--skip-redis]
    IS->>IS: apt-get: bun, pg17, redis, bwrap
    IS->>PG: create role + database agrippa
    IS->>IS: write /etc/agrippa/agrippa.env (secrets)
    IS->>IS: install + enable systemd units
    IS->>DS: delegate first deploy

    DS->>DS: git pull --ff-only
    DS->>DS: bun install --frozen-lockfile
    DS->>DS: bun run build (SPA)
    DS->>DS: copy units → daemon-reload
    DS->>API: systemctl restart
    API->>PG: migrate + seed on boot
    DS->>DS: poll /healthz (≤120 s, 2 s interval)
    API-->>DS: 200 OK
    DS->>W: systemctl restart
    W->>API: ExecStartPre: poll /healthz (≤120 s)
    API-->>W: 200 OK
    W->>W: start pg-boss consumers
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant Op as Operator (root)
    participant IS as install.sh
    participant DS as deploy.sh
    participant PG as PostgreSQL
    participant API as agrippa-api.service
    participant W as agrippa-worker.service

    Op->>IS: sudo install.sh [--skip-redis]
    IS->>IS: apt-get: bun, pg17, redis, bwrap
    IS->>PG: create role + database agrippa
    IS->>IS: write /etc/agrippa/agrippa.env (secrets)
    IS->>IS: install + enable systemd units
    IS->>DS: delegate first deploy

    DS->>DS: git pull --ff-only
    DS->>DS: bun install --frozen-lockfile
    DS->>DS: bun run build (SPA)
    DS->>DS: copy units → daemon-reload
    DS->>API: systemctl restart
    API->>PG: migrate + seed on boot
    DS->>DS: poll /healthz (≤120 s, 2 s interval)
    API-->>DS: 200 OK
    DS->>W: systemctl restart
    W->>API: ExecStartPre: poll /healthz (≤120 s)
    API-->>W: 200 OK
    W->>W: start pg-boss consumers
Loading

Reviews (1): Last reviewed commit: "docs(manual,design): document the VM (sy..." | Re-trigger Greptile

Comment on lines +22 to +25
Restart=on-failure
RestartSec=5
TimeoutStopSec=30
LimitNOFILE=65536

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 The ExecStartPre script is designed to wait up to 120 seconds (60 × 2 s) for /healthz, but no TimeoutStartSec is set. systemd's default DefaultTimeoutStartSec on Ubuntu 22.04/24.04 is 90 seconds. On a slow first boot where initial migrations take a while, systemd will send SIGKILL to the start hook at 90 s — before the 120-second window closes — causing the worker to enter a failed state even though the API would have become healthy. A value of 150 s gives the full 120-second poll plus a comfortable buffer for the API process itself to initialise.

Suggested change
Restart=on-failure
RestartSec=5
TimeoutStopSec=30
LimitNOFILE=65536
Restart=on-failure
RestartSec=5
TimeoutStartSec=150
TimeoutStopSec=30
LimitNOFILE=65536

Comment thread infra/vm/agrippa-worker.service Outdated
Environment=HOME=/var/lib/agrippa
# Wait up to 120s for the API to report healthy (schema migrated + seeded).
# $$ keeps systemd from eating the $ — the shell must see $(seq) and ${PORT}.
ExecStartPre=/bin/sh -c 'for i in $$(seq 1 60); do curl -sf "http://127.0.0.1:$${PORT:-3000}/healthz" >/dev/null && exit 0; sleep 2; done; echo "API /healthz never became ready" >&2; exit 1'

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 The individual curl calls inside ExecStartPre have no per-attempt timeout. On loopback, a refused connection fails instantly, but if the port is open and the process is stalled (e.g. blocked in migration), the call can hang for the OS's TCP keepalive interval (minutes), making the total wait far exceed 120 seconds. Adding --max-time 2 keeps each attempt bounded to 2 seconds and preserves the intended overall ceiling.

Suggested change
ExecStartPre=/bin/sh -c 'for i in $$(seq 1 60); do curl -sf "http://127.0.0.1:$${PORT:-3000}/healthz" >/dev/null && exit 0; sleep 2; done; echo "API /healthz never became ready" >&2; exit 1'
ExecStartPre=/bin/sh -c 'for i in $$(seq 1 60); do curl -sf --max-time 2 "http://127.0.0.1:$${PORT:-3000}/healthz" >/dev/null && exit 0; sleep 2; done; echo "API /healthz never became ready" >&2; exit 1'

Comment thread infra/vm/deploy.sh Outdated
echo "==> wait for /healthz on :${PORT}"
healthy=0
for _ in $(seq 1 60); do
if curl -sf "http://127.0.0.1:${PORT}/healthz" >/dev/null; then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Same concern as the ExecStartPre health loop: curl -sf has no per-attempt timeout. If the API process has opened the port but is blocked mid-migration, a single iteration can stall for minutes and the 120-second budget becomes meaningless. Adding --max-time 2 keeps each attempt bounded.

Suggested change
if curl -sf "http://127.0.0.1:${PORT}/healthz" >/dev/null; then
if curl -sf --max-time 2 "http://127.0.0.1:${PORT}/healthz" >/dev/null; then

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
infra/vm/install.sh (1)

126-126: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Fix alignment in the ASCII art banner.

The right border on this line is off by one space, causing a slight visual misalignment in the output banner.

✨ Proposed fix
-  │  Losing it orphans every credential the platform has stored.      │
+  │  Losing it orphans every credential the platform has stored.       │
🤖 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 `@infra/vm/install.sh` at line 126, Adjust the spacing on the affected ASCII
art banner line so its right border aligns with the surrounding banner lines,
without changing the displayed text.
infra/vm/deploy.sh (1)

19-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Strip carriage returns from the parsed PORT variable.

If an operator edits /etc/agrippa/agrippa.env in a Windows environment, the file might contain \r\n line endings. This would cause port_from_env to end with a hidden \r, which breaks the curl health-check URL.

Consider stripping carriage returns to make the parsing more robust.

💡 Proposed defensive fix
-port_from_env="$(grep -E '^PORT=' "$ENV_FILE" 2>/dev/null | tail -n1 | cut -d= -f2 || true)"
+port_from_env="$(grep -E '^PORT=' "$ENV_FILE" 2>/dev/null | tail -n1 | cut -d= -f2 | tr -d '\r' || true)"
 PORT="${PORT:-${port_from_env:-3000}}"
🤖 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 `@infra/vm/deploy.sh` around lines 19 - 20, Update the port_from_env parsing in
deploy.sh to remove any carriage return characters from the extracted PORT value
before applying the fallback chain. Preserve the existing environment-variable
precedence and default of 3000.
infra/vm/nginx.conf.example (1)

24-30: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Clear the Connection header to enable HTTP/1.1 keep-alive.

For standard API routes, proxy_http_version 1.1; is specified, but the Connection header is not explicitly cleared. By default, NGINX sets Connection: close for proxied connections, which forces a new TCP connection to the backend for every request.

To enable true HTTP/1.1 keep-alive performance to the backend, explicitly clear the Connection header as you did for the SSE route.

💡 Proposed fix
     location / {
         proxy_pass http://127.0.0.1:3000;
         proxy_http_version 1.1;
+        proxy_set_header Connection "";
         proxy_set_header Host $host;
         proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
🤖 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 `@infra/vm/nginx.conf.example` around lines 24 - 30, Update the standard API
proxy location identified by proxy_pass http://127.0.0.1:3000 and
proxy_http_version 1.1 to explicitly clear the upstream Connection header,
matching the SSE route’s keep-alive configuration. Preserve the existing proxy
headers and routing behavior.
🤖 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 `@infra/vm/deploy.sh`:
- Around line 44-48: Update the health-check curl invocation in the polling loop
to include a maximum request timeout, such as 5 seconds, so an unresponsive API
causes the iteration to fail and the loop can eventually time out instead of
hanging indefinitely.
- Around line 24-25: Stop the agrippa-worker service before the git pull and
migration steps in the deployment script, ensuring it is paused before any
application files or database schema are modified. Preserve the existing restart
logic at the end of the script.

In `@infra/vm/install.sh`:
- Around line 115-119: Update the environment-file generation in install.sh to
detect the existing skip-Redis option and comment out the REDIS_URL entry when
Redis is skipped, while preserving the template value for normal installations.
Ensure the generated file enables DB polling fallback without changing unrelated
substitutions.

---

Nitpick comments:
In `@infra/vm/deploy.sh`:
- Around line 19-20: Update the port_from_env parsing in deploy.sh to remove any
carriage return characters from the extracted PORT value before applying the
fallback chain. Preserve the existing environment-variable precedence and
default of 3000.

In `@infra/vm/install.sh`:
- Line 126: Adjust the spacing on the affected ASCII art banner line so its
right border aligns with the surrounding banner lines, without changing the
displayed text.

In `@infra/vm/nginx.conf.example`:
- Around line 24-30: Update the standard API proxy location identified by
proxy_pass http://127.0.0.1:3000 and proxy_http_version 1.1 to explicitly clear
the upstream Connection header, matching the SSE route’s keep-alive
configuration. Preserve the existing proxy headers and routing behavior.
🪄 Autofix (Beta)

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

Run ID: 78a97e26-5e41-4269-a613-e1f01d77fa03

📥 Commits

Reviewing files that changed from the base of the PR and between 5bc7327 and d5602fc.

📒 Files selected for processing (14)
  • CHANGELOG.md
  • README.md
  • docs/design/08-deployment.md
  • docs/manual/en/01-getting-started.md
  • docs/manual/en/06-operations.md
  • docs/manual/zh-CN/01-getting-started.md
  • docs/manual/zh-CN/06-operations.md
  • infra/vm/README.md
  • infra/vm/agrippa-api.service
  • infra/vm/agrippa-worker.service
  • infra/vm/deploy.sh
  • infra/vm/env.example
  • infra/vm/install.sh
  • infra/vm/nginx.conf.example

Comment thread infra/vm/deploy.sh
Comment on lines +24 to +25
echo "==> git pull (--ff-only: refuses if the tree has diverged)"
git pull --ff-only

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Stop the worker before updating files and running migrations.

The script currently leaves agrippa-worker running while git pull updates its files and agrippa-api runs database migrations. This creates a dangerous race condition where the worker could execute new codebase logic against an old database schema, read partially built templates, or write to the database while migrations are actively mutating tables.

Stop the worker before modifying the application files to ensure background jobs are safely paused during the update. The script already correctly restarts the worker at the very end.

🛡️ Proposed fix
 cd "$APP_DIR"
 
+echo "==> stop worker (prevent execution during update)"
+systemctl stop agrippa-worker 2>/dev/null || true
+
 echo "==> git pull (--ff-only: refuses if the tree has diverged)"
 git pull --ff-only
📝 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
echo "==> git pull (--ff-only: refuses if the tree has diverged)"
git pull --ff-only
cd "$APP_DIR"
echo "==> stop worker (prevent execution during update)"
systemctl stop agrippa-worker 2>/dev/null || true
echo "==> git pull (--ff-only: refuses if the tree has diverged)"
git pull --ff-only
🤖 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 `@infra/vm/deploy.sh` around lines 24 - 25, Stop the agrippa-worker service
before the git pull and migration steps in the deployment script, ensuring it is
paused before any application files or database schema are modified. Preserve
the existing restart logic at the end of the script.

Comment thread infra/vm/deploy.sh
Comment thread infra/vm/install.sh
hutusi added 3 commits July 17, 2026 21:51
Review findings from PR #3, each verified before applying:

- agrippa-worker.service: TimeoutStartSec=150 — the ExecStartPre health
  wait is designed for 120s, but systemd's 90s default start timeout
  would SIGKILL it on a slow first boot (initial migrations), leaving
  the worker flapping through restart cycles instead of waiting.
- Both /healthz polls get curl --max-time 2 so a stalled-but-listening
  API bounds each attempt and the overall ceiling stays honest.
- install.sh --skip-redis now comments REDIS_URL out of the generated
  env file; leaving it set pointed ioredis at a dead server to retry
  forever instead of taking the intended DB-polling fallback.
- nginx example: clear the Connection header on the main location so
  upstream keep-alive actually works; align the banner box.

Rejected (with reasons, for the record): stopping the worker before
git pull/migrations — the running worker holds old code in memory and
step-granular resume plus advisory-locked boot migration is the
platform's rolling-upgrade contract (same as Compose); and CRLF
stripping for PORT — the env file is generated and edited on the VM.
… POSTs

better-auth's CSRF check trusts only the baseURL origin
(http://localhost:3000), but in dev the browser talks to the Vite server
on :5173 and the proxy forwarded the Origin header unchanged — every
sign-up/sign-in POST got 403 INVALID_ORIGIN, making dev-mode login
impossible through the documented three-terminal setup.

Fix it in the dev proxy rather than the API: production is same-origin by
design (ADR-0001 — the API serves the SPA, no CORS), and the proxy exists
to simulate exactly that. Rewriting Origin to the API's own completes the
simulation; adding trustedOrigins to the auth config would instead loosen
the production code path for a dev-only problem.
The documented dev flow generated AGRIPPA_SECRET_KEY with openssl on
every export — but that key encrypts credentials stored in dev (repo
tokens, MCP auth), so a fresh key per shell silently orphans them. Bun
loads .env.local natively from the cwd and .gitignore already excludes
it, so a persistent env file is strictly safer and one less setup step.

README keeps the 60-second quick start (now writing .env.local once);
CONTRIBUTING carries the detail and the key-stability warning; AGENTS.md
gets a one-line pointer so all copies of the boot recipe stay in
agreement. Exports still work and take precedence over the file.

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hutusi has reached the 50-credit limit for trial accounts. To continue receiving code reviews, upgrade your plan.

@hutusi
hutusi merged commit 0ee0aa5 into main Jul 17, 2026
5 checks passed
hutusi added a commit that referenced this pull request Jul 18, 2026
Pushes of PR merge commits fail the push-time commitlint step because
"Merge" parses as the commit type (type-case + type-enum), leaving main
red and skipping Test/Build — every PR merge (#2, #3, #4) hit this. The
PR-range step already lints the branch's real commits; the merge subject
is packaging metadata, so commitlint now ignores /^Merge: / (kept narrow
so malformed types still fail).
@hutusi
hutusi deleted the infra/vm-deployment branch July 28, 2026 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant