Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 35 additions & 4 deletions .claude/skills/review-dep-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,33 @@ 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 below funnels through this step first.** Before invoking `gh pr merge`, post a PR approval that records *why* this update is being accepted and a one-paragraph changelog digest. 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.

Run:

```bash
gh pr review <number> --approve --body "<rationale>"
```

The `<rationale>` MUST cover, in this order:

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**: 2 to 4 bullets summarising the scan from Phase 2:
- 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**: `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").

If the rationale would exceed shell-quoting friendliness, write it to a temp file and pass via `--body-file`:

```bash
gh pr review <number> --approve --body-file /tmp/dep-approval-<number>.txt
```

**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):
Expand All @@ -374,7 +401,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 <number> --squash --auto
Expand All @@ -383,7 +411,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 <number> --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 <number> --json state,autoMergeRequest --jq '{state: .state, autoMerge: .autoMergeRequest}'
Expand All @@ -405,7 +433,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 <active-number> --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

Expand All @@ -414,7 +443,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 <active-number> --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

Expand All @@ -432,6 +462,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 merge path (`Merge as-is` / `Improve and merge` / `Fix CI and merge`) MUST submit `gh pr review <number> --approve --body "<rationale>"` before the merge command. The rationale records the decision (bump type + why merging), a 2–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.
Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 2 additions & 2 deletions cli/CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`).

Expand Down Expand Up @@ -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`.

Expand Down
2 changes: 1 addition & 1 deletion cli/cmd/init_postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
4 changes: 2 additions & 2 deletions cli/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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})
}
Comment on lines 660 to 665

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The thirdPartyImages function now hardcodes the use of config.DefaultPostgresImageTag and config.DefaultNATSImageTag, which ignores any user-provided overrides in state.PostgresImageTag or state.NATSImageTag (e.g., set via environment variables or config.json).

This is a significant issue because thirdPartyImages is used in hasDHIDigests to determine if verification can be skipped based on the cache. If an operator provides a custom image tag, the CLI will check the cache for the default image tag instead. If the default tag is cached, the CLI will proceed to pull and run the custom image without any verification, potentially bypassing security checks. The function should respect the values in the state object, falling back to the defaults only when the state fields are empty.

	if state.PersistenceBackend == "postgres" {
		tag := state.PostgresImageTag
		if tag == "" {
			tag = config.DefaultPostgresImageTag
		}
		images = append(images, thirdPartyImage{"postgres", "dhi.io/postgres:" + tag})
	}
	if state.BusBackend == "nats" {
		tag := state.NATSImageTag
		if tag == "" {
			tag = config.DefaultNATSImageTag
		}
		images = append(images, thirdPartyImage{"nats", "dhi.io/nats:" + tag})
	}

return images
}
Expand Down
2 changes: 1 addition & 1 deletion cli/internal/compose/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
10 changes: 6 additions & 4 deletions cli/internal/config/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
9 changes: 7 additions & 2 deletions cli/internal/verify/dhi.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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).
Expand Down
19 changes: 12 additions & 7 deletions cli/internal/verify/dhi_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ package verify
import (
"encoding/json"
"testing"

"github.com/Aureliolo/synthorg/cli/internal/config"
)

func TestParseDHIRef(t *testing.T) {
Expand All @@ -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",
Expand Down Expand Up @@ -115,16 +117,19 @@ 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")
}
if d == "" {
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")
}
Expand Down
2 changes: 1 addition & 1 deletion docs/reference/cli-persistence-backends.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:<password>@postgres:5432/synthorg` into the backend container's environment. The SQLite-only `SYNTHORG_DB_PATH` variable is omitted.
Expand Down
10 changes: 10 additions & 0 deletions renovate.json
Original file line number Diff line number Diff line change
Expand Up @@ -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=(?<depName>[^\\s]+)\\n\\s+Default[A-Za-z]+ImageTag\\s*=\\s*\"(?<currentValue>[^\"]+)\""
],
"datasourceTemplate": "docker",
"versioningTemplate": "docker"
}
]
}
Loading