Repository navigation
docs: make RedisForge hardening plan agent-executable #4
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,138 @@ | ||
| # RedisForge: Architecture and Limits | ||
| # RedisForge: Architecture Baseline, Guarantees, and Execution Guardrails | ||
|
|
||
| ## Critical path: `POST /v1/items` | ||
| This file is the **current-state contract** for RedisForge. It exists so future implementation work starts from the repository that actually exists rather than from an imagined target architecture. | ||
|
|
||
| `internal/app/app.go` wires chi handlers to a `CacheItemRepo` whose fallback is a new `MemoryItemRepo`. | ||
| The companion execution queue is [redisforge-principal-engineer-hardening-plan.md](redisforge-principal-engineer-hardening-plan.md). | ||
|
|
||
| 1. `HandleCreateItem` validates name/category. With an idempotency key, `BF.EXISTS` may reject it as a duplicate; a Bloom error fails open. | ||
| ## Current critical path: `POST /v1/items` | ||
|
|
||
| `internal/app/app.go` wires chi handlers to a `CacheItemRepo` whose fallback is a newly constructed `MemoryItemRepo`. | ||
|
|
||
| 1. `HandleCreateItem` validates the request. With an idempotency key, `BF.EXISTS` may reject it as a duplicate; a Bloom error fails open. | ||
| 2. `CacheItemRepo.Create` writes to memory first, then best-effort writes the item as RedisJSON (`item:{id}`). The memory repo assigns version/timestamps. | ||
| 3. After create, Bloom `BF.ADD` is best effort. The handler starts `EmitAsync` and returns `201` without waiting for audit `XADD`. | ||
| 4. `Emitter` uses a five-second background timeout to append to `audit-events`. The stream uses approximate `MAXLEN ~ 100000` trimming. | ||
| 5. `AuditWorker` creates `audit-processors` at stream ID `0`, reads new entries via `XREADGROUP`, and scans stale pending entries via cursor-complete `XAUTOCLAIM` (30-second minimum idle, every five seconds). Processing logs the event then ACKs it; an ACK failure leaves it recoverable. | ||
|
|
||
| ## Important limits | ||
| ## Current guarantees and limits | ||
|
|
||
| | Area | Current guarantee | Current limitation | | ||
| | --- | --- | --- | | ||
| | Item durability | CRUD succeeds against the in-process repository and may populate RedisJSON | The authoritative fallback is memory and is recreated on startup; RedisJSON can outlive memory, so GET/List/search can diverge after restart | | ||
| | Search | RediSearch indexes RedisJSON keys | Update invalidates the cache key, so an item can disappear from search until a later read backfills it; there is no authoritative fallback scan | | ||
| | Idempotency | Bloom may reject a previously seen key | Bloom false positives can reject valid requests; errors fail open; there is no durable request/response receipt | | ||
| | Audit publication | Successful `XADD` gives a recoverable stream entry while retained | Producer failure or shutdown can lose best-effort emits; bounded trimming is not permanent retention | | ||
| | Audit processing | Consumer-group recovery can redeliver retained pending entries | The worker logs and ACKs only; it has no durable audit sink or exactly-once guarantee | | ||
| | Health | `/healthz` proves the HTTP process can answer | It does not prove Redis or any future database dependency is ready | | ||
| | Redis topology | Standalone, Sentinel, and Cluster clients are configurable | Startup also depends on RedisJSON, RedisBloom, and RediSearch module availability | | ||
|
Comment on lines
+19
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚪ Low · Guarantee matrix is undiscoverable When a visitor uses the README or docs index to find RedisForge’s guarantees, the README and docs index link to Link the baseline or its guarantee matrix from the README or docs index. Prompt for AI agentsIn `notes/architecture-limitations.md` at lines 19-27:
**Guarantee matrix is undiscoverable**
When a visitor uses the README or docs index to find RedisForge’s guarantees, the README and docs index link to `docs/failure-semantics.md`, not this matrix or the current-state baseline. Visitors following the documented entry points therefore miss the new guarantee matrix, leaving RF-M0-P01’s requirement for a README- or docs-index-linked matrix unmet.
Link the baseline or its guarantee matrix from the README or docs index. |
||
| | Process lifecycle | Graceful shutdown paths exist | HTTP listener/start failures are not yet treated as a top-level startup failure in the desired hardened contract | | ||
|
|
||
| These limitations are not bugs to hide. They are the baseline the hardening plan is intended to replace deliberately and with evidence. | ||
|
|
||
| ## Chosen target direction | ||
|
|
||
| For the hardening work, the default architecture decision is: | ||
|
|
||
| - **PostgreSQL becomes the item system of record.** | ||
| - A single PostgreSQL transaction owns item mutation, durable idempotency receipt, and outbox event creation. | ||
| - Redis remains important as the cache, search projection, Streams delivery buffer, topology-learning surface, and observability target. | ||
| - Search is an explicitly versioned projection of durable item state. | ||
| - Stream delivery is **at least once**. Consumers must be replay-safe and idempotent. | ||
| - `MemoryItemRepo` remains test/demo infrastructure only; it must not be presented as a production durability layer. | ||
|
|
||
| If this target is intentionally changed later, update this file first and explain the trade-off before implementation begins. | ||
|
|
||
| ## Non-negotiable invariants for hardening work | ||
|
|
||
| 1. **One source of truth.** Do not introduce a second durable item authority or an ambiguous dual-write path. | ||
| 2. **No correctness dependency on Bloom.** Bloom can optimize; it cannot decide durable idempotency alone. | ||
| 3. **No DB + message dual-write gap.** Item mutation and outbox creation must share one database transaction. | ||
| 4. **Duplicates are expected.** Relay and consumer logic must be safe if the same `event_id` appears more than once. | ||
| 5. **Old projection events cannot overwrite newer state.** Item versions must make replay monotonic. | ||
| 6. **ACK follows durable effect.** A consumer must not acknowledge an event before its required durable effect/receipt is established. | ||
| 7. **Retention is explicit.** A bounded Redis Stream is a delivery buffer, not a permanent audit store. | ||
| 8. **Failure semantics stay observable.** Do not silently fall back in a way that changes user-visible guarantees. | ||
| 9. **No invented performance claims.** Benchmarks must record workload, environment, commands, and measured output. | ||
| 10. **No resume-driven infrastructure.** Kafka, Kubernetes, extra Redis modules, or similar additions require a demonstrated need. | ||
|
|
||
| ## Agent execution guardrails | ||
|
|
||
| Automated implementation is allowed only inside the companion hardening plan and must obey these rules. | ||
|
|
||
| ### The agent may | ||
|
|
||
| - inspect the repository and recent Git history; | ||
| - create or switch to a local work branch; | ||
| - implement **one bounded task** from the hardening plan; | ||
| - add or update tests required by that task; | ||
| - update directly affected documentation when behavior changes; | ||
| - run local verification commands; | ||
| - create a **local commit** after verification if configured to do so; | ||
| - produce a final evidence report and proposed/final commit message. | ||
|
|
||
| ### The agent must not | ||
|
|
||
| - push to a remote; | ||
| - merge, rebase, force-push, tag, release, or deploy; | ||
| - open or merge a pull request unless explicitly asked by the user; | ||
| - start a later task because the current one is blocked; | ||
| - redesign the target architecture to make a task easier; | ||
| - weaken or delete a failing test merely to obtain green; | ||
| - change unrelated files for cleanup, style, dependency upgrades, or refactors; | ||
| - add infrastructure not required by the selected task; | ||
| - claim verification that did not actually run. | ||
|
|
||
| ### Mandatory stop conditions | ||
|
|
||
| Stop and report instead of continuing when any of the following is true: | ||
|
|
||
| - the worktree contains unrelated uncommitted user changes; | ||
| - the previous automated change is still unreviewed/unmerged and continuing would stack unrelated work on it; | ||
| - the selected task's prerequisite is not satisfied on the current base branch; | ||
| - implementation requires a product/architecture decision not already made in these notes; | ||
| - a required dependency, Docker service, credential, or local capability is unavailable; | ||
| - a failing test appears unrelated to the task and the root cause is not safely attributable; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium · Docker checks have conflicting instructions When docker is unavailable for an integration check but runnable targeted checks pass, the stop rule requires halting when a Docker service is unavailable, while the verification policy permits committing when Docker prevents a CI-equivalent check. The agent cannot determine whether to stop or proceed with an unverified check. Clarify that the Docker environment-limitation exception permits proceeding when runnable targeted checks pass, consistent with the verification policy. Also at Prompt for AI agentsIn `notes/architecture-limitations.md` at line 94:
**Docker checks have conflicting instructions**
When docker is unavailable for an integration check but runnable targeted checks pass, the stop rule requires halting when a Docker service is unavailable, while the verification policy permits committing when Docker prevents a CI-equivalent check. The agent cannot determine whether to stop or proceed with an unverified check.
Clarify that the Docker environment-limitation exception permits proceeding when runnable targeted checks pass, consistent with the verification policy.
Also at `notes/redisforge-principal-engineer-hardening-plan.md:77`, `notes/architecture-limitations.md:93`. |
||
| - completing the task would require broadening scope beyond its stated acceptance criteria. | ||
|
|
||
| ## Repository verification baseline | ||
|
|
||
| RedisForge CI currently runs the following on Ubuntu: | ||
|
|
||
| ```bash | ||
| test -z "$(gofmt -l .)" | ||
| go vet ./... | ||
| go test -race -count=1 ./... | ||
| go test -count=1 ./internal/redisx -run '^$' -bench '^BenchmarkRedisHotPaths$' -benchtime=1x -benchmem | ||
| go build ./cmd/redisforge | ||
| ``` | ||
|
|
||
| Every automated task must run the smallest relevant tests first, then as much of the CI-equivalent baseline as the local environment supports. | ||
|
|
||
| Rules: | ||
|
|
||
| - If a command cannot run, report **UNVERIFIED** with the exact reason; do not convert it into a pass. | ||
| - Do not run `go mod tidy` unless the selected task changes dependencies or module metadata. | ||
| - Docker/Testcontainers failures caused by Docker being unavailable are environment limitations, not proof of correctness. | ||
| - Local Windows `gofmt -l` results can be affected by CRLF worktree behavior; CI on Ubuntu is the final formatting authority. | ||
| - CI success after the user pushes remains the remote merge gate. | ||
|
|
||
| ## Evidence required from every automated implementation | ||
|
|
||
| The final report must contain: | ||
|
|
||
| - **Item durability:** the app's fallback is in-memory and is recreated on startup. RedisJSON is a cache, not the durable source of truth. Redis can therefore retain cached items after memory data is lost; GET and List may disagree after a restart. | ||
| - **Search/cache coupling:** RediSearch indexes RedisJSON cache keys. Update invalidates the cache entry, so the item can disappear from search until a later GET backfills it. Search has no fallback scan. | ||
| - **Audit durability:** producer failures and in-flight emits at shutdown can lose events. The stream is bounded, and the worker currently only logs and ACKs -- it does not write to an external audit store. At-least-once recovery applies only while the stream retains an entry's payload; it is not permanent retention or exactly-once processing. | ||
| - **Idempotency:** Bloom positives (including false positives) are rejected; lookup errors fail open and add errors are ignored. This is not a durable idempotency record. | ||
| - **Health/topology:** `/healthz` always returns `ok` and does not probe Redis. Standalone, Sentinel, and Cluster clients are configurable, but startup also requires RedisJSON, RedisBloom, and RediSearch modules. | ||
| | Field | Required content | | ||
| | --- | --- | | ||
| | Task | Exact task ID and title from the hardening plan | | ||
| | Base | Base branch and starting commit | | ||
| | Changed files | Exact paths | | ||
| | Behavior changed | Concise before/after explanation | | ||
| | Tests added/changed | What property they prove | | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 Medium · Required report evidence is omitted When an agent follows the hardening plan’s exact report template, the template omits required test-property evidence and the diff summary, so the resulting report does not provide all the evidence the architecture contract requires. Align the exact report template with the required report contents, including tests added or changed and the properties they prove, plus the diff summary. Also at Prompt for AI agentsIn `notes/architecture-limitations.md` at line 129:
**Required report evidence is omitted**
When an agent follows the hardening plan’s exact report template, the template omits required test-property evidence and the diff summary, so the resulting report does not provide all the evidence the architecture contract requires.
Align the exact report template with the required report contents, including tests added or changed and the properties they prove, plus the diff summary.
Also at `notes/redisforge-principal-engineer-hardening-plan.md:886-929`. |
||
| | Verification | Each command and PASS / FAIL / UNVERIFIED | | ||
| | Risks | Remaining uncertainty or follow-up | | ||
| | Diff summary | What the user should inspect first | | ||
| | Commit | Local commit SHA if created, otherwise proposed commit message | | ||
| | Next task | The next queue item only if this task is actually complete | | ||
|
|
||
| ## Verification context | ||
| ## Revalidation rule | ||
|
|
||
| Local inspection used Go 1.27 on Windows. `go vet ./...` and `go build` passed. The race command could not run because local CGO is disabled; non-race integration tests requiring Testcontainers failed because Docker is unavailable here. The CI workflow runs on Ubuntu. `gofmt -l` on this Windows checkout reports CRLF worktree files; Git records them as LF, so that local output does not establish a Linux CI formatting failure. | ||
| Before executing a hardening task, the agent must compare this baseline against the current repository. If the repository has already evolved past a statement here, treat the code and tests as evidence, update this file as part of the relevant task, and do not blindly recreate already-completed work. | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Medium · Pre-cutover items disappear or diverge
When redisJSON contains items absent from PostgreSQL when RF-M1-P03 makes PostgreSQL authoritative, production reads switch to PostgreSQL without specifying how to handle Redis-only items or stale cache entries. Those items disappear from durable reads, or
CacheItemRepo.GetByIDcontinues serving entries thatListomits.Specify how cutover imports or discards Redis-only items and invalidates stale Redis cache entries.
Also at
notes/redisforge-principal-engineer-hardening-plan.md:266-273,notes/architecture-limitations.md:22.Prompt for AI agents