diff --git a/.claude/skills/review-dep-pr/SKILL.md b/.claude/skills/review-dep-pr/SKILL.md index aa8c838dd7..fe9e93460c 100644 --- a/.claude/skills/review-dep-pr/SKILL.md +++ b/.claude/skills/review-dep-pr/SKILL.md @@ -365,6 +365,45 @@ Apply the merge-strategy choice from Phase 7 (when Phase 7 was asked). For each PR based on user's choice: +### Approve with rationale (MANDATORY before any merge) + +**Every merge path in this skill funnels through this step first**, including all five strategy paths in Phase 8 (`Lockfile-only batch`, `Wave-based parallel`, `Strict sequential`, `Combine into one PR`, `Defer the conflicting subset`) and all three per-PR action sections below (`Merge as-is`, `Improve and merge`, `Fix CI and merge`). Before invoking `gh pr merge` for any PR, post a PR approval whose body is a **three-part structured rationale** (a one-sentence Decision, a Changelog digest paragraph with 2 to 4 explicit bullets, and a Follow-ups line). This leaves a durable artifact on the PR (visible to future reviewers, audit trails, and bisects) explaining what was scanned and what was deemed relevant; without it, "merged by Renovate label, no comment" becomes the only signal in the timeline. + +The `` body is multi-line by design. `gh pr review --body` accepts newlines (the bash `"..."` quoting preserves them), but for any rationale beyond a single sentence, prefer one of the multi-line forms below over inlining a long quoted string: heredoc piped to `--body-file -`, or write to a temp file first and pass `--body-file `. Inline `"..."` quoting is fine only for the rare patch-bump-with-nothing-relevant case where the entire rationale fits on one line. + +The body MUST contain three sections in this order, each prefixed with its literal label so reviewers across PRs produce a consistent format: + +1. **`Decision:`** one sentence describing the bump type (patch / minor / major / lockfile / digest) and why it's being merged (`CI green`, `no breaking changes affecting us`, `migration applied in this PR`, etc.). +2. **`Changelog digest:`** a short paragraph followed by 2 to 4 markdown bullets (use `-` on a fresh line, indented two spaces if nested under a sub-label) summarising the Phase 2 scan: + - which versions were covered (from -> to) + - **Relevant items** that affect us (new features adopted / deprecations actioned / bug fixes we were hitting / security fixes that matter) + - **Reviewed but not relevant** items (breaking changes in features we don't use, irrelevant platform changes, removed APIs we never imported) +3. **`Follow-ups:`** one line; `none` if clean, otherwise the deferred items the user explicitly accepted in Phase 7 (e.g. "adopt new `--cache-dependency-path` input in a follow-up PR"). + +Recommended invocation patterns (pick whichever matches the rationale length): + +```bash +# Multi-line rationale via heredoc piped through stdin (preferred for typical 3-section bodies) +gh pr review --approve --body-file - <<'EOF' +Decision: Patch bump 0.11.7 -> 0.11.8; CI green; no breaking changes touching our usage. + +Changelog digest: +- Covered 0.11.7 -> 0.11.8 (single release). +- Relevant: bug fix for `uv lock` on `pyproject.toml` files containing only dependency-groups (we hit this on the docs-toolchain refactor). +- Reviewed but not relevant: new `--python-downloads-json-url` flag (we don't customise download sources); `UV_NO_PROJECT` env var (no use case yet). + +Follow-ups: none. +EOF + +# Long rationale via temp file (when heredoc gets unwieldy or you want to review the body before posting) +gh pr review --approve --body-file /tmp/dep-approval-.txt + +# One-liner only when the rationale truly fits on one line +gh pr review --approve --body "Decision: lockfile-only refresh; CI green; no source diffs. Changelog digest: not applicable for lockFileMaintenance. Follow-ups: none." +``` + +**Do NOT skip this step**, even when `--auto` is used and the merge happens asynchronously: the approval must land first so the PR carries the rationale before it auto-merges. Do NOT collapse the rationale into the squash commit message; the approval review is the canonical venue (squash messages get rewritten by maintainers, get truncated, and don't surface in the PR conversation thread). + ### Merge as-is 1. Re-verify CI is passing right before merge (time may have passed since Phase 5): @@ -374,7 +413,8 @@ For each PR based on user's choice: ``` Inspect the JSON output. All checks should have `state: "SUCCESS"`, `"SKIPPED"`, or `"NEUTRAL"`. Do NOT use jq filters with `!=` (escaping breaks on Windows bash). If any checks are failing, inform the user and switch to the "Fix CI and merge" flow instead. -2. Merge: +2. **Approve with rationale** (per the section above): required before merge. +3. Merge: ```bash gh pr merge --squash --auto @@ -383,7 +423,7 @@ For each PR based on user's choice: Note: `--auto` may succeed silently with no stdout. Track which path was used: `auto` or `immediate`. If `--auto` fails (auto-merge not enabled on the repo or branch protection requirements not met), fall back to `gh pr merge --squash` for immediate merge. If that also fails (e.g., required reviews not met), inform the user that manual approval is needed. -3. Verify the merge: +4. Verify the merge: ```bash gh pr view --json state,autoMergeRequest --jq '{state: .state, autoMerge: .autoMergeRequest}' @@ -405,7 +445,8 @@ For each PR based on user's choice: - Close the original bot PR with a comment pointing to the replacement - **Use the replacement PR number for all remaining steps** (CI wait, merge) 5. Wait for CI to pass using `gh pr checks --watch` (use the Bash tool's `timeout` parameter set to 600000ms to cap the wait; if it expires, warn the user that CI may be stuck and ask how to proceed). Use the replacement PR number if step 4 created one. -6. Merge the active PR +6. **Approve with rationale** (per the section above): required before merge. The rationale must additionally describe the improvements applied in this PR (which recommended changes were committed). +7. Merge the active PR. ### Fix CI and merge @@ -414,7 +455,8 @@ For each PR based on user's choice: 3. Fix the issue 4. Commit and push (same bot branch fallback applies; if push fails, open a replacement PR and use that PR number for remaining steps) 5. Wait for CI to pass using `gh pr checks --watch` (use the Bash tool's `timeout` parameter set to 600000ms to cap the wait; if it expires, warn the user that CI may be stuck and ask how to proceed) -6. Merge the active PR when green +6. **Approve with rationale** (per the section above): required before merge. The rationale must additionally describe the CI failure root cause and the fix applied. +7. Merge the active PR when green. ### Close / Skip @@ -432,6 +474,7 @@ After all merges complete, if any PRs were merged, automatically run `/post-merg - **Be specific about what affects us**: don't just list changelog items, cross-reference each one against our actual config and code usage. - **Major version bumps get extra scrutiny**: check for a migration guide. Always fetch it if breaking changes are ambiguous or potentially affect our usage; skip only when all breaking changes are clearly in internal APIs we don't use. - **Don't merge with failing CI**: if CI fails, investigate and fix first. +- **Always approve before merge, with rationale**: every `gh pr merge` invocation in this skill (whether reached via a Phase 8 strategy path - `Lockfile-only batch`, `Wave-based parallel`, `Strict sequential`, `Combine into one PR`, `Defer the conflicting subset` - or via a per-PR action section - `Merge as-is`, `Improve and merge`, `Fix CI and merge`) MUST submit `gh pr review --approve --body ""` first. The rationale records the decision (bump type + why merging), a 2 to 4 bullet changelog digest splitting **Relevant** vs **Reviewed but not relevant**, and any deferred follow-ups. Skipping the approval (even when `--auto` will land the PR asynchronously) leaves the PR with no audit trail of *why* it was accepted; squash commit messages don't substitute because they get rewritten by maintainers and don't surface in the PR conversation thread. - **Grouped updates (Renovate domain groups or Dependabot groups)**: analyze each package in the group separately, then present as one combined report. - **Preserve existing config**: when making improvements, don't refactor unrelated config. Only touch what's relevant to the update. - **If you can't fetch release notes** (private repo, deleted releases, etc.), say so explicitly and recommend the user check manually before merging. diff --git a/README.md b/README.md index baf667e93c..aa3ce1fae9 100644 --- a/README.md +++ b/README.md @@ -56,7 +56,7 @@ synthorg start # pull images + start containers Open [localhost:3000](http://localhost:3000); the **setup wizard** walks you through company config, LLM providers, agent setup with personality presets, and theme selection. Choose **Guided Setup** for the full experience or **Quick Setup** (company name + provider only, configure the rest later). -**Persistence backends:** SQLite (default) for single-node and development, Postgres for multi-instance and production deployments. The CLI orchestrates both. `--persistence-backend postgres` generates a `dhi.io/postgres:18-debian13` DHI service, random credentials, and a named data volume. `synthorg stop` preserves the data volume unless `--volumes` is passed. +**Persistence backends:** SQLite (default) for single-node and development, Postgres for multi-instance and production deployments. The CLI orchestrates both. `--persistence-backend postgres` generates a `dhi.io/postgres` DHI service (image tag pinned via `DefaultPostgresImageTag` in `cli/internal/config/state.go`), random credentials, and a named data volume. `synthorg stop` preserves the data volume unless `--volumes` is passed. ### From source diff --git a/cli/CLAUDE.md b/cli/CLAUDE.md index 2ce95bc9a3..7553054a71 100644 --- a/cli/CLAUDE.md +++ b/cli/CLAUDE.md @@ -75,7 +75,7 @@ The CLI uses four hint tiers with different visibility rules per `hints` mode. W `SYNTHORG_*` env vars without a corresponding flag (settable via env or `config set`) cover four buckets: - **Backend / channel overrides**: `SYNTHORG_LOG_LEVEL`, `SYNTHORG_BACKEND_PORT`, `SYNTHORG_WEB_PORT`, `SYNTHORG_CHANNEL`, `SYNTHORG_IMAGE_TAG`, `SYNTHORG_TELEMETRY_ENABLED`, `SYNTHORG_AUTO_*` (UPDATE_CLI / PULL / RESTART). -- **Image / registry overrides**: `SYNTHORG_REGISTRY_HOST`, `SYNTHORG_IMAGE_REPO_PREFIX`, `SYNTHORG_DHI_REGISTRY`, `SYNTHORG_POSTGRES_IMAGE_TAG`, `SYNTHORG_NATS_IMAGE_TAG`, `SYNTHORG_FINE_TUNE_IMAGE` (any of these disables verification for that invocation). +- **Image / registry overrides**: `SYNTHORG_REGISTRY_HOST`, `SYNTHORG_IMAGE_REPO_PREFIX`, `SYNTHORG_DHI_REGISTRY`, `SYNTHORG_POSTGRES_IMAGE_TAG`, `SYNTHORG_NATS_IMAGE_TAG`, `SYNTHORG_FINE_TUNE_IMAGE` (any of these disables verification for that invocation). `SYNTHORG_POSTGRES_IMAGE_TAG` and `SYNTHORG_NATS_IMAGE_TAG` default to `Default{Postgres,NATS}ImageTag` in `cli/internal/config/state.go`, kept current by a Renovate customManager that watches the `// renovate:` annotations on those constants. - **Timeouts and retry tuning**: `SYNTHORG_BACKUP_*_TIMEOUT`, `SYNTHORG_HEALTH_CHECK_TIMEOUT`, `SYNTHORG_SELF_UPDATE_*_TIMEOUT`, `SYNTHORG_TUF_FETCH_TIMEOUT`, `SYNTHORG_ATTESTATION_HTTP_TIMEOUT`, `SYNTHORG_IMAGE_VERIFY_TIMEOUT` (default 120s, hard min 1s), `SYNTHORG_IMAGE_PULL_ATTEMPTS` (1..100, default 3), `SYNTHORG_IMAGE_PULL_RETRY_DELAY` (default 2s, exponential). - **Byte caps and ports**: `SYNTHORG_MAX_API_RESPONSE_BYTES` (default 4MiB), `SYNTHORG_MAX_BINARY_BYTES` (256MiB), `SYNTHORG_MAX_ARCHIVE_ENTRY_BYTES` (128MiB), `SYNTHORG_DEFAULT_NATS_URL`, `SYNTHORG_DEFAULT_NATS_STREAM_PREFIX`, `SYNTHORG_FINE_TUNE_HEALTH_PORT` (env-only, not in `config set`). @@ -124,7 +124,7 @@ See [docs/reference/cli-config-subcommands.md](../docs/reference/cli-config-subc ## Persistence Backends -`--persistence-backend sqlite` (default, single-node) uses the in-process SQLite store under volume `synthorg-data`. `--persistence-backend postgres` adds a `dhi.io/postgres:18-debian13` DHI service on port `3002` (override via `--postgres-port`) backed by volume `synthorg-pgdata`. Interactive `init` defaults to Postgres + NATS; non-interactive defaults to SQLite + internal bus. +`--persistence-backend sqlite` (default, single-node) uses the in-process SQLite store under volume `synthorg-data`. `--persistence-backend postgres` adds a `dhi.io/postgres` DHI service (tag pinned via `DefaultPostgresImageTag` in `cli/internal/config/state.go`, kept current by Renovate) on port `3002` (override via `--postgres-port`) backed by volume `synthorg-pgdata`. Interactive `init` defaults to Postgres + NATS; non-interactive defaults to SQLite + internal bus. Every generated `compose.yml` includes a one-shot `data-init` helper that chowns each named volume to its non-root owner (`65532:65532` for backend / NATS, `70:70` with mode `0700` for Postgres) before stateful services start. The Postgres / NATS services declare `depends_on: data-init: condition: service_completed_successfully`. diff --git a/cli/cmd/init_postgres_test.go b/cli/cmd/init_postgres_test.go index 1bc42d70c9..be78f32848 100644 --- a/cli/cmd/init_postgres_test.go +++ b/cli/cmd/init_postgres_test.go @@ -273,7 +273,7 @@ func TestPostgresLifecycle_InitGeneratesWritableState(t *testing.T) { if !strings.Contains(composeYAML, "postgres:") { t.Error("compose.yml should contain postgres service") } - if !strings.Contains(composeYAML, "dhi.io/postgres:18-debian13") { + if !strings.Contains(composeYAML, "dhi.io/postgres:"+config.DefaultPostgresImageTag) { t.Error("compose.yml should use DHI postgres image") } if !strings.Contains(composeYAML, "synthorg-pgdata") { diff --git a/cli/cmd/start.go b/cli/cmd/start.go index 1139c72eb0..5e4aa3a3ba 100644 --- a/cli/cmd/start.go +++ b/cli/cmd/start.go @@ -658,10 +658,10 @@ type thirdPartyImage struct { func thirdPartyImages(state config.State) []thirdPartyImage { var images []thirdPartyImage if state.PersistenceBackend == "postgres" { - images = append(images, thirdPartyImage{"postgres", "dhi.io/postgres:18-debian13"}) + images = append(images, thirdPartyImage{"postgres", "dhi.io/postgres:" + config.DefaultPostgresImageTag}) } if state.BusBackend == "nats" { - images = append(images, thirdPartyImage{"nats", "dhi.io/nats:2.12-debian13"}) + images = append(images, thirdPartyImage{"nats", "dhi.io/nats:" + config.DefaultNATSImageTag}) } return images } diff --git a/cli/internal/compose/generate_test.go b/cli/internal/compose/generate_test.go index 4a039c7f81..96d8d9b4ba 100644 --- a/cli/internal/compose/generate_test.go +++ b/cli/internal/compose/generate_test.go @@ -332,7 +332,7 @@ func TestGenerateWithSandboxAndPostgres(t *testing.T) { assertContains(t, yaml, "/var/run/docker.sock:/var/run/docker.sock") assertContains(t, yaml, `SYNTHORG_SANDBOX_IMAGE: "ghcr.io/aureliolo/synthorg-sandbox:latest"`) // Postgres service is still generated alongside the sandbox wiring. - assertContains(t, yaml, "dhi.io/postgres:18-debian13") + assertContains(t, yaml, "dhi.io/postgres:"+config.DefaultPostgresImageTag) assertContains(t, yaml, "SYNTHORG_DATABASE_URL") // SQLite path must not appear when postgres is active. if strings.Contains(yaml, "SYNTHORG_DB_PATH") { diff --git a/cli/internal/config/state.go b/cli/internal/config/state.go index d1612ae0b6..36c078b738 100644 --- a/cli/internal/config/state.go +++ b/cli/internal/config/state.go @@ -130,11 +130,13 @@ type State struct { // Compiled-in default values for the tunables. Exposed so Tunables can detect // customisation (CustomRegistry = any registry/tag field differs from default). const ( - DefaultRegistryHost = "ghcr.io" - DefaultImageRepoPrefix = "aureliolo/synthorg-" - DefaultDHIRegistry = "dhi.io" + DefaultRegistryHost = "ghcr.io" + DefaultImageRepoPrefix = "aureliolo/synthorg-" + DefaultDHIRegistry = "dhi.io" + // renovate: datasource=docker depName=dhi.io/postgres DefaultPostgresImageTag = "18-debian13" - DefaultNATSImageTag = "2.12-debian13" + // renovate: datasource=docker depName=dhi.io/nats + DefaultNATSImageTag = "2.12-debian13" DefaultNATSURLValue = "nats://nats:4222" DefaultNATSStreamPrefixValue = "SYNTHORG" diff --git a/cli/internal/verify/dhi.go b/cli/internal/verify/dhi.go index e437da7545..7885d1b875 100644 --- a/cli/internal/verify/dhi.go +++ b/cli/internal/verify/dhi.go @@ -22,6 +22,7 @@ import ( "runtime" "strings" + "github.com/Aureliolo/synthorg/cli/internal/config" "github.com/google/go-containerregistry/pkg/authn" "github.com/google/go-containerregistry/pkg/name" v1 "github.com/google/go-containerregistry/pkg/v1" @@ -47,9 +48,13 @@ var dhiRegistry = "dhi.io" // postgresImageTag / natsImageTag are the third-party image tags. Used // both for pre-flight auth checks and for consumers (compose template) // that need to reference the exact image without duplicating the string. +// +// Default values come from config.Default*ImageTag (the single source of +// truth that Renovate keeps current); Configure() overrides them with the +// operator-resolved State value at startup. var ( - postgresImageTag = "18-debian13" - natsImageTag = "2.12-debian13" + postgresImageTag = config.DefaultPostgresImageTag + natsImageTag = config.DefaultNATSImageTag ) // dhiEmbeddedPublicKeyPEM is Docker's DHI cosign public key (ECDSA P-256). diff --git a/cli/internal/verify/dhi_test.go b/cli/internal/verify/dhi_test.go index 51aa0a746c..c7daa7f9f9 100644 --- a/cli/internal/verify/dhi_test.go +++ b/cli/internal/verify/dhi_test.go @@ -3,6 +3,8 @@ package verify import ( "encoding/json" "testing" + + "github.com/Aureliolo/synthorg/cli/internal/config" ) func TestParseDHIRef(t *testing.T) { @@ -18,17 +20,17 @@ func TestParseDHIRef(t *testing.T) { }{ { name: "valid postgres", - input: "dhi.io/postgres:18-debian13", + input: "dhi.io/postgres:" + config.DefaultPostgresImageTag, wantReg: "dhi.io", wantRepo: "postgres", - wantTag: "18-debian13", + wantTag: config.DefaultPostgresImageTag, }, { name: "valid nats", - input: "dhi.io/nats:2.12-debian13", + input: "dhi.io/nats:" + config.DefaultNATSImageTag, wantReg: "dhi.io", wantRepo: "nats", - wantTag: "2.12-debian13", + wantTag: config.DefaultNATSImageTag, }, { name: "missing registry", @@ -115,8 +117,11 @@ func TestParseDHIPublicKeyRejectsInvalidPEM(t *testing.T) { func TestDHIPinnedIndexDigest(t *testing.T) { t.Parallel() - // Known pinned images should return digests. - d, ok := DHIPinnedIndexDigest("dhi.io/postgres:18-debian13") + // Known pinned images should return digests. Keys are derived from the + // canonical image-tag constants so a Renovate bump that updates one + // place but misses the other fails this test instead of silently + // dropping verification at runtime. + d, ok := DHIPinnedIndexDigest("dhi.io/postgres:" + config.DefaultPostgresImageTag) if !ok { t.Error("postgres should have a pinned digest") } @@ -124,7 +129,7 @@ func TestDHIPinnedIndexDigest(t *testing.T) { t.Error("postgres digest should not be empty") } - d, ok = DHIPinnedIndexDigest("dhi.io/nats:2.12-debian13") + d, ok = DHIPinnedIndexDigest("dhi.io/nats:" + config.DefaultNATSImageTag) if !ok { t.Error("nats should have a pinned digest") } diff --git a/docs/reference/cli-persistence-backends.md b/docs/reference/cli-persistence-backends.md index 983f969f11..fa24205a75 100644 --- a/docs/reference/cli-persistence-backends.md +++ b/docs/reference/cli-persistence-backends.md @@ -30,7 +30,7 @@ Fresh Docker named volumes are owned by `root:root` at creation, and DHI images When `--persistence-backend postgres` is selected, `synthorg init`: -1. Adds a `dhi.io/postgres:18-debian13` DHI (Docker Hardened Image) service to the generated `compose.yml` (read-only rootfs, minimal capabilities via `cap_add`, `pg_isready` healthcheck, named volume `synthorg-pgdata`). +1. Adds a `dhi.io/postgres` DHI (Docker Hardened Image) service to the generated `compose.yml` (read-only rootfs, minimal capabilities via `cap_add`, `pg_isready` healthcheck, named volume `synthorg-pgdata`). The image tag is pinned via `DefaultPostgresImageTag` in `cli/internal/config/state.go` (kept current by Renovate). 2. Extends the `data-init` helper to also chown `synthorg-pgdata` to `70:70` with mode `0700`. 3. Generates a 32-byte URL-safe random password via `crypto/rand` and persists it to `config.json` (`postgres_password`). Re-init preserves the existing password to avoid breaking the running container. 4. Wires `SYNTHORG_DATABASE_URL=postgresql://synthorg:@postgres:5432/synthorg` into the backend container's environment. The SQLite-only `SYNTHORG_DB_PATH` variable is omitted. diff --git a/renovate.json b/renovate.json index 169881ed93..0a61516c13 100644 --- a/renovate.json +++ b/renovate.json @@ -208,6 +208,16 @@ ], "datasourceTemplate": "docker", "versioningTemplate": "docker" + }, + { + "customType": "regex", + "description": "DHI image tag default constants in CLI config (single source of truth referenced by verify/dhi.go vars, start.go third-party list, and dhi_test.go)", + "managerFilePatterns": ["/cli/internal/config/state\\.go$/"], + "matchStrings": [ + "// renovate: datasource=docker depName=(?[^\\s]+)\\n\\s+Default[A-Za-z]+ImageTag\\s*=\\s*\"(?[^\"]+)\"" + ], + "datasourceTemplate": "docker", + "versioningTemplate": "docker" } ] }