Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
8 changes: 4 additions & 4 deletions .claude/commands/github-review-pr.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ gh pr view <PR_NUMBER> --repo mhenrixon/kamal-proxy --json mergeable,mergeStateS

### Which branch do you merge? (fork-specific — decide BEFORE merging)

The branch model (`.claude/rules/git-workflow.md`) forbids merging `dash` into an upstream-PR-able feature branch — fork-only cert code would contaminate it. So:
`dash` is this fork's main branch and feature branches root off it, so the answer is normally simple:

1. **First try `git merge origin/main`** — the sanctioned forward merge. Most conflicts against `dash` come from upstream code that reached `dash` via `main`; merging `main` forward resolves them without contaminating the branch. Re-check mergeability against `dash` afterwards.
2. **Still conflicting?** The overlap is fork-only code on `dash` (typically the cert overlap zone: `internal/cmd/run.go`, `internal/server/config.go`, `internal/server/router.go`, `internal/server/service.go`). If the branch is fork-internal (never intended for a basecamp PR), merging `origin/dash` is acceptable. If the branch is meant to stay upstream-PR-able, **stop and ask the user** — merging `dash` would be irreversible contamination.
1. **Merge `origin/dash`** — this is the sanctioned forward merge. Branches are no longer kept upstream-PR-able, so there is nothing to contaminate.
2. **Only reach for `git merge origin/main`** on an old branch that still roots off `main`, or when you specifically want upstream fixes that have not yet reached `dash`. Re-check mergeability against `dash` afterwards.

Note `git rerere` is enabled: previously-seen conflicts auto-replay their resolutions — review what rerere staged before trusting it.

Expand All @@ -106,7 +106,7 @@ Note `git rerere` is enabled: previously-seen conflicts auto-replay their resolu

- The PR reports `MERGEABLE` (or the local `git merge-tree` check is clean), AND the merge commit (if one was needed) is pushed.
- If the merge produced changes, CI is now re-running — that's expected; Phase A reads the fresh run.
- If a conflict cannot be resolved with confidence (both sides rewrote the same logic and the correct combination isn't decidable from the code, or resolving requires merging `dash` into an upstream-PR-able branch), **stop and ask the user** — a guessed resolution that compiles is worse than a question.
- If a conflict cannot be resolved with confidence (both sides rewrote the same logic and the correct combination isn't decidable from the code, or the correct combination is genuinely ambiguous), **stop and ask the user** — a guessed resolution that compiles is worse than a question.

---

Expand Down
4 changes: 2 additions & 2 deletions .claude/commands/lfg.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ Execute a complete engineering workflow with verification at each phase, respect
1. Check the current branch: `git branch --show-current`
2. If NOT on `main`, switch: `git checkout main`
3. Sync with upstream (do not assume `origin/main` is current): `git fetch upstream --tags --prune && git merge --ff-only upstream/main && git push origin main`
4. Create feature branch **off `main`** (keeps it upstream-PR-able): `git checkout -b feature/{description}` (or `fix/{description}`, `issue-{number}-{brief-description}`)
4. Create feature branch **off `dash`** (`dash` is this fork's main branch): `git checkout -b feature/{description}` (or `fix/{description}`, `issue-{number}-{brief-description}`)
5. The branch merges **forward** into `dash` at PR time — never rebase it once pushed. See `.claude/rules/git-workflow.md` and `.claude/rules/upstream-sync.md`.

---
Expand Down Expand Up @@ -327,7 +327,7 @@ The tests prove the CODE is right; this phase keeps the USER's mental model righ
- [ ] `make test` passes
- [ ] `go vet ./...` clean
- [ ] Backwards compatibility maintained (state files, RPC contract, `kamal-proxy` naming untouched)
- [ ] Branch rooted off `main`, PR opened against `dash`
- [ ] Branch rooted off `dash`, PR opened against `dash`
- [ ] PR created with description
- [ ] PR body ends with `## Deviations & judgment calls` (from implementation-notes.md, since deleted)
- [ ] Comprehension close-out delivered (decisions + three merge-gate questions)
Expand Down
5 changes: 3 additions & 2 deletions .claude/commands/plan.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ You are the planning specialist for **dash-proxy**, the Go fork of `basecamp/kam
- **Read-only for source code.** Never edit `.go` files, never commit, never create branches. The only file you may Write is a new plan markdown under `docs/plans/`.
- **Never reproduce secrets** (ACME account keys, DNS provider API tokens, ghcr credentials) in the plan, even redacted ones you encounter while reading config or state files.
- **Dedupe before creating an issue**: `gh issue list --search "<keywords>" --repo mhenrixon/kamal-proxy` — if an existing issue covers this, extend it in your summary instead of duplicating.
- **Respect the fork boundary.** `main` is a fast-forward-only mirror of upstream — never plan work that lands there. Fork-only work (cert batching, wildcard DNS-01, anything not upstreamable) targets a feature branch rooted off `main`, merging forward into `dash`. If the change is generically useful and upstream-clean, say so — it may be worth a PR to `basecamp/kamal-proxy` instead of a fork-only patch.
- **Respect the fork boundary.** `dash` is this fork's main branch; plan work onto a feature branch rooted off `dash`, merging back into `dash`. `main` is a fast-forward-only mirror of upstream — never plan work that lands there. Upstream mergeability is **not** a constraint: design what is best for `dash` and diverge from basecamp where that is better.
- **Check upstream before porting.** When an issue says "port basecamp/kamal-proxy#N", verify that PR is still open and unmerged before planning a port — several have been superseded or merged since the issues were written (#63→#225, #197→#228). Diff against `upstream/main` first.

## Phase 1 — Investigate

Expand Down Expand Up @@ -87,7 +88,7 @@ Use this structure for the issue body or markdown file. Every section is load-be
<Explicit boundaries — the adjacent things an eager executor must NOT do. Always include: no edits to Dockerfile/Makefile/script/release (upstream's), no renaming kamal-proxy module/binary/RPC/socket, no touching main.>

## Execution
Implement on a branch rooted off `main` (or the relevant feature branch — `san-certificate-batching` / `wildcard-certs` — if this extends fork-only cert work), PR against `dash`.
Implement on a branch rooted off `dash` (or the relevant feature branch — `san-certificate-batching` / `wildcard-certs` — if this extends fork-only cert work), PR against `dash`.
```

For GitHub issues: create with `gh issue create --repo mhenrixon/kamal-proxy --title "..." --body-file <tmpfile>`. Write the body to a temp file first; do not use inline heredoc with `--body` (code fences get mangled by shell interpolation).
Expand Down
13 changes: 10 additions & 3 deletions .claude/rules/git-workflow.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,20 @@ This is a fork, not a normal repo — branch roles are fixed. Full sync mechanic

| Branch | Role | Can you commit here? |
|---|---|---|
| `dash` | **This fork's main branch** — every feature lands here | Only via merge from feature branches |
| `main` | Fast-forward-only mirror of `basecamp/kamal-proxy` | **NEVER** |
| `dash` | Long-lived integration + release branch | Only via merge from feature branches |
| `san-certificate-batching` | SAN cert batching feature branch | Yes |
| `wildcard-certs` | DNS-01 wildcard certs feature branch | Yes |
| `feature/*`, `fix/*` | New work | Yes — root off `main` |
| `feature/*`, `fix/*` | New work | Yes — root off `dash` |

**Root new feature branches off `main`, not `dash`** — this keeps them upstream-PR-able (basecamp can merge them without inheriting fork-only cert code). They merge *forward* into `dash`, never the reverse, and published branches are never rebased.
**Root new feature branches off `dash`.** `dash` is this fork's main branch, and upstream
mergeability is not a constraint on our design — we build what is best for `dash` and diverge
from basecamp where that is better. (Same call as the `../kamal` fork.) Rooting off `main`
instead produces PRs that run no CI and conflict on every fork-only file, which is why we
stopped doing it.

`main` still exists so upstream fixes can be merged *forward* into `dash`. It is a source,
never a target. Published branches are never rebased.

## Branch Naming

Expand Down
14 changes: 10 additions & 4 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,10 +53,16 @@ Layer 0: unix socket + state files (~/.config/kamal-proxy, kamal-proxy.sock)

## Branch map

| Branch | Contents | Conflict surface vs main |
**`dash` is this fork's main branch.** New work branches off `dash` and PRs back into `dash`.
Upstream mergeability is **not** a design constraint — we take what is best for `dash` and
diverge where that is better. `main` still exists as an upstream mirror so we can keep pulling
basecamp's fixes forward, but nothing is shaped for their benefit. (Decided alongside the same
call in the `../kamal` fork.)

| Branch | Contents | Conflict surface vs dash |
|---|---|---|
| `main` | basecamp mirror, ff-only | — |
| `dash` | integration + release: publish workflow + cert features merged | — |
| `dash` | **the main branch** — all fork work lands here; publish workflow + cert features merged | — |
| `main` | basecamp mirror, ff-only — a source to merge *from*, never a target | — |
| `san-certificate-batching` | SAN cert batching (`internal/server/san_cert_manager.go`), `--acme-email`/`--acme-directory` | run.go, config.go, router.go, go.mod |
| `wildcard-certs` | DNS-01 wildcard certs (`internal/server/acme/`, cert registry), `--acme-dns-provider` etc. | run.go, config.go, router.go, go.mod |
| `feat/loadbalancing` | SUPERSEDED — upstream absorbed multi-target LB natively (`load_balancer.go`, reader/writer split); the branch only retains a standalone `TargetPool` module. Not merged into `dash`; candidate for deletion. | — |
Expand All @@ -77,7 +83,7 @@ Tag push (`vX.Y.Z.N`) → `.github/workflows/docker-publish.yml` → multi-arch

| Command | Purpose |
|---------|---------|
| `/lfg` | Full autonomous workflow: branch off `main` → understand → plan → TDD → verify → PR into `dash` |
| `/lfg` | Full autonomous workflow: branch off `dash` → understand → plan → TDD → verify → PR into `dash` |
| `/plan` | Read-only planning → GitHub issue or `docs/plans/` markdown (execute with `/lfg`) |
| `/architect` | Coordinate work across the cmd → RPC → server layers |
| `/tdd` | Enforce RED → GREEN → REFACTOR with Go table-driven tests |
Expand Down
2 changes: 1 addition & 1 deletion ROADMAP.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ Proxy-side roadmap for the dash fork. The cross-repo release sequencing, strateg
| Redirect/rewrite rules | #35; kamal discussions #1214/#97 | DONE — `--redirect '<pattern>=<replacement>[;status=<code>]'` and `--rewrite '<pattern>=<replacement>'` on deploy (`internal/server/redirect_rules.go`). Both pieces of evidence only ask for www→apex, which `--canonical-host` already answers; what shipped is the path-level gap the issue names. Patterns are RE2 anchored to the whole path with `$1` expansion. Redirects fold into `redirectURLIfNeeded` alongside the TLS/canonical hop, so a path move on a TLS service costs the client one redirect, not two; a rule resolving to the request's own URL is dropped so a catch-all cannot loop. A relative replacement is always rebuilt as `scheme://host/...`, which is what stops a captured `//evil.com` from becoming a scheme-relative Location. Rewrites apply last in `serviceRequestWithTarget`, after the health-check exemptions and the allow list, and skip `isInternalRequest` — the TLS on-demand probe runs the same chain, and a catch-all pointing it at the app's index would approve a certificate for any host |
| Compression (gzip/zstd/brotli) | rejected #19 | DONE — `--compress`, `--compress-min-length`, `--compress-content-type` on deploy (`internal/server/compression.go`, `compression_middleware.go`), wrapped outermost in `Service.createMiddleware`. Per-service, not per-target, because that is the only chain that sees proxy-written responses too. Encoding is chosen by the client's q values, ties by the order given. The decision is held until the body's size and type are known, which means a flush *before* any body must not settle it — `ReverseProxy` schedules exactly that after the headers of every unknown-length response (`flushInterval` returns -1), and it races the first write. `text/event-stream` is excluded outright, so the streaming bypass (`response_buffer_middleware.go:86`) never sees a compressed stream. The built-in error pages render above the router (`server.go:338`) and stay uncompressed; `--error-pages` ones do not |
| Liveness endpoint for external monitors | #25 | DONE — `GET /.kamal-proxy/ping` → `200` (`internal/server/ping_handler.go`), mounted outermost and **unconditionally** in `Server.buildHandler`. It reuses the `/.kamal-proxy/` namespace but not `DynamicDomainManager.WrapHandler`, which only mounts when dynamic domains are configured — the endpoint has to answer on a proxy with zero services, which is the whole point. Outermost is what keeps it out of the access log: the logging middleware never sees it, at the cost of no request ID and no error page on that path. No readiness variant: `RestoreLastSavedState` runs before `Start` opens a listener and `BeginDrain` closes the listeners, so readiness here could only ever be `200` — the TCP state is the real signal. Not mounted on the metrics port, which is opt-in and IP-restricted |
| Scale-to-zero | port PR #197 (open) | `PauseController` states (`pause_controller.go`) are the natural base |
| Scale-to-zero | port PR #228 (open; supersedes the closed #197 the issue names) | `PauseController` states (`pause_controller.go`) are the natural base. Upstream maintainer blessed the architecture in discussion #222: opt-in direct Docker socket behind a `ContainerLifecycle` interface, so a restricted host-side start/stop service can replace it later. Mounting `docker.sock` into the internet-facing proxy is root-equivalent on the host — opt-in only, and only when the feature is enabled |
| Observability batch — log format selection, OTel traceparent, metrics path excludes | #213 counter-proposal | `logging_middleware.go:81` (fixed JSON today); `request_id_middleware.go` |

## Implementation notes (apply to every feature)
Expand Down
Loading
Loading