docs(P00): grill — v0.6 adversarial review (D-078+, G-025+)
---ci--- project: oy phase: 0 milestone: v0.6 status: grill ---/ci---
This commit is contained in:
@@ -1093,3 +1093,441 @@ fixes:
|
||||
escalations: []
|
||||
---/ci---
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## v0.6 Grill (Nomad Web UI)
|
||||
|
||||
> **Reviewer**: CIAgent adversarial grill (red-team, full autonomy)
|
||||
> **Date**: 2026-08-18
|
||||
> **Target**: v0.6 Phase 0 artifacts (PROJECT.md D-066..D-073, REQUIREMENTS.md REQ-040..REQ-045 + IDEATE traceability, ROADMAP.md v0.6 phase table, PLANS.md v0.6 plan lines 1692-2152 incl. MVP/UX 3 sections, RESEARCH.md, PERSONAS.md frontend-engineer activation) + v0.1..v0.5 codebase baseline
|
||||
> **Milestone**: v0.6 — Nomad Web UI (feature type; tags run on the v0.5.x patch line)
|
||||
> **Autonomy**: full (decision_confidence_threshold = 0.60)
|
||||
> **Mode**: multi-project (slug `oy`)
|
||||
> **G-NNN sequence**: continues from G-024 (highest prior grill id). New fixes G-025..
|
||||
> **D-NNN sequence**: continues from D-073 (highest prior clarify id). New decisions D-078..
|
||||
|
||||
### Methodology
|
||||
|
||||
v0.6 is the project's first UI milestone. The grill assumes the plan is
|
||||
over-scoped, too costly, and quietly breaks G-006 (zero-dep) or G-003
|
||||
(production firewall) until evidence forces otherwise. Each of the nine
|
||||
review axes was scored against concrete evidence (commit hash, file:line,
|
||||
REQ-ID, D-decision, verified codebase state). Missing or contradictory
|
||||
evidence is itself a finding.
|
||||
|
||||
### Evidence baseline (verified against the actual repo, not the docs)
|
||||
|
||||
- `go.mod`: `module github.com/oy/openyield`, `go 1.22`, **cosmos-sdk v0.50.8
|
||||
+ ibc-go v8.2.1 already present** (the v0.5 D-055 controlled exception
|
||||
landed). v0.6 "go.mod unchanged" therefore means **zero NEW require lines
|
||||
on top of the v0.5 baseline**, NOT "go.mod is empty". This is a subtle but
|
||||
material audit-surface distinction — a naive "no new lines added in
|
||||
P1..P5" check would pass even if an indirect dep was bumped (a `go mod
|
||||
tidy` side-effect). **G-028** (below) forces the P6 audit to diff go.mod
|
||||
against the `v0.5.0` tag (the v0.6 P0 baseline), not just check "no new
|
||||
direct require lines".
|
||||
- `web/` directory: **does NOT exist** (verified — `ls web/` fails). v0.6 is
|
||||
greenfield UI; the plan's "new top-level `web/` dir" is accurate.
|
||||
- `lexicon_meta_web/`: **does NOT exist** (verified). The new firewall is
|
||||
net-new work, mirroring `lexicon_meta_docs/` (which exists and has the
|
||||
G-009 self-test + G-013 walk-coverage + self-exclusion via
|
||||
`runtime.Caller(0)` — all verified present).
|
||||
- `lexicon/lexicon.go`: `BannedTerms()`, `FindBannedTerm()`, and
|
||||
`SyntheticBannedStrings()` ALL exist (verified at lines 49, 73, 111). The
|
||||
G-014 shared helper from v0.4 REQ-029 landed. The new `lexicon_meta_web/`
|
||||
firewall has a proven template to mirror — no detection reimplementation
|
||||
risk.
|
||||
- `x/bread/types/types.go:13`: `GrainsPerBread = 10000` (verified). The docs
|
||||
table `docs/shared/bread-scale.md` says "each 1,000× the previous" (Crumb =
|
||||
1,000 Grain; Bread = 1,000 Crumb). **The code says Grain→Bread is 10,000×
|
||||
(GrainsPerBread=10000), NOT 1,000×.** D-073 (code constants are the source
|
||||
of truth, NOT docs) is CONFIRMED CORRECT — the docs are genuinely outdated.
|
||||
This is not a judgment call; it is a verified numeric contradiction. The
|
||||
doc-fix is correctly deferred (P1+ follow-up, not v0.6 scope — docs were a
|
||||
v0.3 deliverable).
|
||||
- `x/bloom/types/types.go:13-19`: `TargetBloomRateBasisPoints=450`,
|
||||
`MinBloomRateBasisPoints=400`, `MaxBloomRateBasisPoints=500`,
|
||||
`AccrualPeriodBlocks=144` — ALL exist (verified). P5's Bloom screen has
|
||||
real code constants to read.
|
||||
- `x/standing/types/types.go:12-13,39-40,94,99,112,129`: `PriorMean=4.0`,
|
||||
`PriorWeight=10`, `FreeholderMinStandingScore=4.5`,
|
||||
`FreeholderMinCategories=3`, `IsFreeholderEligible()`,
|
||||
`ComputeDiversityBonus()`, `GetVoucherWeight()`, `GetStandingBucket()` —
|
||||
ALL exist (verified). P4's Standing screen has the real locked-formula
|
||||
surface. The plan's "computed from locked constants, NOT hardcoded" claim
|
||||
is grounded.
|
||||
- `x/stash/types/types.go:19,23,32,35,38-39`: `BalanceGrain`, `StashActivity`,
|
||||
`MaturityThresholdDays=90`, `MaxGapForMaturity=30`, `IsMature()` — ALL
|
||||
exist (verified). P2's Stash dashboard has real maturity logic.
|
||||
- `x/window/types/types.go:19,22-24,36,56,100,117,123`: `ScopeKind` enum
|
||||
(ReadStash/ReadStanding/ProcessPassActForStand), `RateLimit`,
|
||||
`AuditEntry`, `Revoke()`, `Expire()`, `Activate()` — ALL exist (verified).
|
||||
P3's Window screen has the real lifecycle methods. The plan's "call
|
||||
`Window.Activate/Revoke/Expire`, NOT a reimplementation" is enforceable.
|
||||
- `x/identity/types/types.go:14-19`: `Reach` struct with `ReachID`,
|
||||
`HolderID`, `IsNomad` — exists (verified). P1's "Create a Reach" form has a
|
||||
real type to instantiate.
|
||||
- **G-003 import-invariant test** exists at `x/window/types/types_test.go`
|
||||
(uses `go/parser` ImportsOnly, scans `x/**/*.go`, prefix
|
||||
`github.com/oy/openyield/x/`). **Confirmed: `web/` is OUTSIDE `x/` so this
|
||||
test does NOT scan `web/`** — the plan's new `web/store/import_test.go`
|
||||
(P1-03-03) is the correct complement. BUT the plan's test only forbids
|
||||
`x/*/keeper` imports; `x/*/module` (the Cosmos runtime wiring) is also a
|
||||
production surface that `web/` must not reach into. **G-025** extends the
|
||||
forbidden-import set.
|
||||
- **No `app.go`, no `cmd/oyd`, no `main.go`** exists in the repo (verified
|
||||
— the plan's "no chain runtime exists" is accurate). v0.6's `web/main.go`
|
||||
is the project's first `main.go`. This is a genuine greenfield.
|
||||
|
||||
These baseline facts confirm the v0.6 plan's architecture claims against the
|
||||
actual codebase. The plan is well-grounded; the binding decisions below are
|
||||
correctness and verification hardening, not scope rework.
|
||||
|
||||
---
|
||||
|
||||
## 1. Per-Axis Adversarial Assessment
|
||||
|
||||
### Axis 1 — Feasibility (Go html/template + HTMX + stdlib ServeMux, no node) — **PASS** (confidence 0.85)
|
||||
|
||||
The stack is genuinely achievable with zero new Go deps. Go 1.22
|
||||
`net/http.ServeMux` supports method-pattern routing (`mux.HandleFunc("GET
|
||||
/reach", ...)` — verified Go 1.22 feature). `html/template` is stdlib with
|
||||
auto-escaping (XSS protection by default). HTMX 1.x minified is ~14KB
|
||||
served as a static asset via `http.FileServer` — no `go get`, no node, no
|
||||
build step. The mock-server-over-real-Go-types approach (D-067) is sound:
|
||||
all six referenced `x/*/types` packages exist and export the structs the UI
|
||||
surfaces (verified — Reach, Stash, StashActivity, Window, FreeholderSignals,
|
||||
BloomRecord all present with the methods the plan calls). No hidden blocker
|
||||
reaches the escalation threshold. The one feasibility risk is **HTMX
|
||||
version drift** (the vendored file has no version-pinned provenance in the
|
||||
plan beyond "HTMX 1.x") — minor, addressed by P1-02-02's "pin the version in
|
||||
a comment in the file header" mitigation.
|
||||
|
||||
### Axis 2 — Scope (5 screens + firewall + mock server in 5 execution phases) — **PASS** (confidence 0.80)
|
||||
|
||||
5 screens + 1 firewall + 1 mock server across 5 execution phases (P1..P5) +
|
||||
1 final phase (P6) is proportionate, not over-scoped. Each screen is a
|
||||
vertical slice (store extension + handler + template + tests) following the
|
||||
proven v0.2..v0.5 pattern. The Nomad happy path (Reach → Stash → Window →
|
||||
Standing → Bloom) is the correct ordering — each screen depends on the
|
||||
prior's data existing. No screen is gratuitous: all 5 map to a user-
|
||||
validated `--ideate` request and a REQ. The "working prototype" depth (mock
|
||||
data, no persistence, no auth) is the right scope for a first UI milestone
|
||||
— deferring auth/persistence to v0.7+ is correct (a UI with mock auth is a
|
||||
real prototype; a UI with broken half-auth is a worse prototype). Reject
|
||||
the "defer a screen" hypothesis — all 5 are on the Nomad critical path and
|
||||
each is one phase of work.
|
||||
|
||||
### Axis 3 — Cost (6 phases: P1..P5 + P6 final) — **PASS** (confidence 0.82)
|
||||
|
||||
33 tasks across 6 phases is proportionate. P1 is the largest (9 tasks —
|
||||
foundation + Reach + firewall, three parallel waves) because it lands the
|
||||
shared substrate; P2..P5 are uniform (5 tasks each — one screen per phase);
|
||||
P6 is light (4 tasks — review/audit/ship). Bundling P2+P3 (Stash + Window)
|
||||
into one phase would save one ship cycle but would couple two independent
|
||||
screens and break the vertical-slice shippability property (each phase
|
||||
ships a patch tag). The 6-phase structure is the right granularity — fewer
|
||||
phases would couple unrelated screens; more would be ceremony. No bloat:
|
||||
every task produces a concrete `.go`/`.html`/`.js` file or a verifiable
|
||||
gate. Reject the "too many phases" hypothesis.
|
||||
|
||||
### Axis 4 — G-006 (zero-dep): HTMX as vendored static asset — **PASS** (confidence 0.92)
|
||||
|
||||
A vendored JS file served over `http.FileServer` is NOT a Go module
|
||||
dependency — it is a static asset, exactly like `docs/images/` or the
|
||||
vendored CSS. The boundary is unambiguous: **`go.mod` require lines = Go
|
||||
deps; static files under `web/static/` = not Go deps.** HTMX has no Go
|
||||
import path; `go build ./web` does not resolve it; it is served byte-for-
|
||||
byte to the browser. G-006's intent (durability of the Go build, no
|
||||
external Go module tree) is fully preserved. The risk is not "HTMX is a
|
||||
dep" but "a future `go mod tidy` accidentally adds a Go dep" — **G-028**
|
||||
makes the P6 audit verify `go.mod` is byte-identical to the `v0.5.0` tag
|
||||
baseline (diff, not just "no new direct require lines"). Confidence holds.
|
||||
|
||||
### Axis 5 — G-003 (production firewall): web/ importing x/*/types — **CONDITIONAL** (confidence 0.78) → fixed by G-025
|
||||
|
||||
`web/` is NOT an `x/` module — it is the application layer (D-070), exactly
|
||||
as a future `cmd/oyd` would be. `web/store` importing `x/identity/types`
|
||||
is app-layer consumption of protocol types, the intended consumption
|
||||
direction. G-003's intent (no cross-module struct coupling INSIDE the
|
||||
protocol layer) is intact: no `x/` module gains a production import of
|
||||
another `x/` module's types via `web/`. The existing G-003 test
|
||||
(`x/window/types/types_test.go`) scans `x/**/*.go` and correctly does NOT
|
||||
scan `web/` (web/ is outside x/). The plan's new `web/store/import_test.go`
|
||||
(P1-03-03) is the right complement. **BUT** the plan's test only forbids
|
||||
`x/*/keeper` imports; `x/*/module` (the Cosmos `module.go` runtime wiring)
|
||||
is ALSO a production surface that `web/` must not reach into — importing
|
||||
`x/bond/module` would couple the UI to the runtime app-module graph,
|
||||
violating the "no keeper, no Cosmos runtime" boundary (D-067). **G-025**
|
||||
extends the forbidden-import set to `x/*/keeper` AND `x/*/module` (and
|
||||
any `x/*/` subpackage other than `types`). Confidence holds after the fix.
|
||||
|
||||
### Axis 6 — REQ-012 (lexicon firewall extension to web/) — **CONDITIONAL** (confidence 0.80) → fixed by G-026
|
||||
|
||||
The firewall extension is sound in structure: a new sibling
|
||||
`lexicon_meta_web/` mirroring `lexicon_meta_docs/` (same
|
||||
`lexicon.FindBannedTerm` + word-boundary, G-009 self-test via
|
||||
`SyntheticBannedStrings()`, G-013 walk-coverage, self-exclusion via
|
||||
`runtime.Caller(0)`). The firewall-first ordering (P1 before P2..P5
|
||||
content) is the proven D-044 pattern. "Create a Reach" (not "Sign up for
|
||||
an account") is the correct label — "account" is banned (REQ-012). The
|
||||
file-scan firewall catches banned terms in `web/templates/**`,
|
||||
`web/static/**`, and `web/**/*.go`. **BUT** the plan's `web/**/*.go` scan
|
||||
does not explicitly state whether it covers **string literals** (error
|
||||
messages, template-fragment strings) or only comments/identifiers. A banned
|
||||
term in a Go string literal (e.g., `errors.New("account not found")`) is
|
||||
user-facing if it surfaces in an error response — the file-scan catches it
|
||||
(the walk reads the file content, not just the AST), but the plan should
|
||||
make this explicit. The deeper gap: the **per-handler rendered-HTML lexicon
|
||||
check** (each phase's handler test scans the HTTP response body) covers the
|
||||
happy-path response, but **error responses** (400/500 pages) are a drift
|
||||
vector — a banned term in an error template or a dynamically-generated
|
||||
error message would not be caught by the happy-path rendered-HTML check.
|
||||
**G-026** requires the rendered-HTML lexicon check to cover at least one
|
||||
error response per handler (e.g., `POST /reach` with empty HolderID → 400
|
||||
response body scanned for banned terms). Confidence holds after the fix.
|
||||
|
||||
### Axis 7 — Bread-scale discrepancy (D-073) — **PASS** (confidence 0.90)
|
||||
|
||||
D-073 is CONFIRMED CORRECT against the actual codebase. The code
|
||||
(`x/bread/types/types.go:13` `GrainsPerBread = 10000`) and the docs
|
||||
(`docs/shared/bread-scale.md` "each 1,000× the previous") are in verified
|
||||
numeric contradiction: the docs say Grain→Crumb is 1,000× and Crumb→Bread
|
||||
is 1,000× (so Grain→Bread is 1,000,000×), while the code says
|
||||
Grain→Bread is 10,000×. The code constants are tested
|
||||
(`x/bread/types/types_test.go` asserts them); the docs are not. Using the
|
||||
code as the source of truth keeps the UI consistent with the protocol
|
||||
layer. The doc-fix is correctly deferred to a P1+ follow-up (not v0.6
|
||||
scope — docs were a v0.3 deliverable; this is a doc-drift fix, not a UI
|
||||
feature). The regression-guard test (P2-03-01 "Bread-scale conversion
|
||||
correctness test — would FAIL if the outdated docs values were used") is
|
||||
the right enforcement. No binding change — D-073 is ratified as-is.
|
||||
|
||||
### Axis 8 — Security (no auth, no sessions, in-memory store, XSS, injection) — **CONDITIONAL** (confidence 0.75) → fixed by G-027
|
||||
|
||||
For a "working prototype" milestone, no auth / no sessions / in-memory
|
||||
store is acceptable — it is explicitly out of scope (PROJECT.md v0.6 OOS)
|
||||
and documented. The XSS risk is LOW: `html/template` auto-escapes by
|
||||
default; the Reach form inputs (HolderID, PublicKey) are rendered through
|
||||
template actions (`{{.HolderID}}`) which escape HTML. The one injection
|
||||
risk the plan does not address: **ReachID/HolderID used as map keys in the
|
||||
mock store without validation.** The plan's P1-04-03 tests "POST /reach
|
||||
with empty HolderID returns 400" (good), but does not specify validation
|
||||
of the map key itself — a HolderID containing path separators (`/`),
|
||||
template syntax (`{{`), or very long strings could cause route confusion,
|
||||
template injection (if unescaped in a URL path), or memory exhaustion.
|
||||
`html/template` escapes on OUTPUT, but the map key is also used in URL
|
||||
construction (`GET /reach/{id}`) and route matching. **G-027** requires
|
||||
`CreateReach` to validate HolderID/PublicKey (non-empty, length-bounded,
|
||||
no path separators, no template syntax) before using them as map keys /
|
||||
URL path segments. This is not a production security gate (the mock store
|
||||
resets on restart); it is a prototype-robustness gate that prevents the
|
||||
happy path from breaking on adversarial input. Confidence holds after the
|
||||
fix.
|
||||
|
||||
### Axis 9 — Testability (≥80% on web/, HTMX fragment rendering, MVP/UX criteria) — **PASS** (confidence 0.82)
|
||||
|
||||
≥80% coverage on `web/store` + `web/handlers` is realistic with
|
||||
`httptest.NewRecorder` + `httptest.NewRequest` (stdlib — no external test
|
||||
deps, G-006 preserved). The handlers are thin (load from store → render
|
||||
template → write response), so coverage is achievable with table-driven
|
||||
tests per route + per lifecycle transition. HTMX fragment rendering is
|
||||
tested WITHOUT a browser: the handler tests assert the response body
|
||||
contains the expected HTML fragments (e.g., the Bread-scale conversion
|
||||
table, the maturity progress bar width) — the HTMX swap is a client-side
|
||||
concern, but the SERVER-SIDE fragment is testable via response-body
|
||||
assertions. The 8 MVP/UX acceptance criteria (PLANS.md lines 2143-2151)
|
||||
are all auto-verifiable EXCEPT criterion 2's "manual browser check at
|
||||
http://localhost:8080" and the happy-path end-to-end (PLANS.md Happy Path
|
||||
section) — these are manual checks, not auto-tests. This is acceptable for
|
||||
a UI milestone (the auto-tests verify the routes return 200 + correct
|
||||
HTML; the manual check verifies the browser renders them correctly), but
|
||||
the P6 audit must record the manual check as a separate verification
|
||||
step, not conflate it with the auto-test green. No binding change — the
|
||||
plan already distinguishes "handler tests" from "manual browser check"
|
||||
throughout.
|
||||
|
||||
---
|
||||
|
||||
## 2. Binding Decisions (D-078..D-081)
|
||||
|
||||
These are **binding** — the orchestrator MUST apply them before EXECUTE
|
||||
begins. Numbered D-078..D-081 (continuing from D-073).
|
||||
|
||||
| ID | Decision | Rationale | Confidence | Binding fix (if any) |
|
||||
|----|----------|-----------|------------|----------------------|
|
||||
| **D-078** | **RATIFY G-006 boundary: HTMX vendored as `web/static/htmx.min.js` is G-006-compliant (static asset, not a Go dep); `go.mod` MUST stay unchanged across the v0.6 milestone range (zero new require lines on top of the v0.5 baseline).** The boundary is: `go.mod` require lines = Go deps; static files under `web/static/` = not Go deps. HTMX has no Go import path; `go build ./web` does not resolve it. | A vendored JS file served over `http.FileServer` is a static asset (like `docs/images/`), not a Go module dependency. Verified: `go.mod` already has cosmos-sdk from v0.5; v0.6 adds nothing. G-006 intent (durability of the Go build, no new external Go module tree) is fully preserved. | 0.92 | **G-028** — the P6 audit MUST diff `go.mod` against the `v0.5.0` tag (the v0.6 P0 baseline), not just check "no new direct require lines were added in P1..P5". A `go mod tidy` side-effect could bump an indirect dep without adding a direct require line. The diff must be EMPTY (or only the expected `// indirect` reordering with no version changes). | None — RATIFIED (with G-028 enforcement) |
|
||||
| **D-079** | **RATIFY G-003 boundary: `web/` importing `x/*/types` is app-layer consumption (D-070), NOT a cross-`x/` production import. The G-003 production firewall stays intact: no `x/` module gains a production import of another `x/` module's types via `web/`.** The existing G-003 test (`x/window/types/types_test.go`) scans `x/**/*.go` and correctly does NOT scan `web/` (web/ is outside x/). The new `web/store/import_test.go` (P1-03-03) is the correct `web/`-scoped complement. | `web/` is the application layer (like a future `cmd/oyd`), not an `x/` module. Consuming protocol types is the intended direction. Verified: the G-003 test prefix is `github.com/oy/openyield/x/` so `web/` imports are outside its scope by construction. | 0.85 | **G-025** — `web/store/import_test.go` MUST forbid imports of `x/*/keeper` AND `x/*/module` (and any `x/*/` subpackage other than `types`), not just `x/*/keeper`. The plan only mentions `keeper`; `module.go` is also a Cosmos runtime surface that `web/` must not reach into (D-067 "no keeper, no Cosmos runtime"). | None — RATIFIED (with G-025 enforcement) |
|
||||
| **D-080** | **RATIFY D-073: Bread-scale source of truth = `x/bread/types` code constants (`GrainsPerBread=10000`, `BreadScaleAll()`), NOT `docs/shared/bread-scale.md`.** The doc is verified outdated (docs say 1,000× ratios; code says 10,000× Grain→Bread). The doc-fix is a P1+ follow-up, NOT v0.6 scope (docs were a v0.3 deliverable; this is a doc-drift fix, not a UI feature). The P2-03-01 Bread-scale conversion correctness test (would FAIL if docs values were used) is the regression guard. | Verified numeric contradiction: `x/bread/types/types.go:13` `GrainsPerBread = 10000` vs `docs/shared/bread-scale.md` "each 1,000× the previous". Code constants are tested (`types_test.go`); docs are not. Using code keeps the UI consistent with the protocol layer. | 0.90 | None — the P2-03-01 regression-guard test is already in the plan. The doc-fix deferral is correct (P1+ follow-up, not v0.6). | None — RATIFIED |
|
||||
| **D-081** | **RATIFY the lexicon firewall extension to `web/` (REQ-045): new sibling `lexicon_meta_web/` mirroring `lexicon_meta_docs/` (same `lexicon.FindBannedTerm` + word-boundary + G-009 self-test via `SyntheticBannedStrings()` + G-013 walk-coverage + self-exclusion via `runtime.Caller(0)`). Firewall-first (P1 before P2..P5 content). "Create a Reach" is the correct label (not "Sign up for an account" — "account" is banned).** The file-scan firewall + per-handler rendered-HTML lexicon checks are the dual firewall (file-scan catches static content; rendered-HTML catches dynamic content). | REQ-012 is `All` phases and UI strings are user-facing. Verified: `lexicon.SyntheticBannedStrings()` exists (G-014 helper); `lexicon_meta_docs/` has the proven template (self-test + walk-coverage + self-exclusion). Firewall-first (D-044 pattern) catches drift at build time, not at P6 audit. | 0.88 | **G-026** — the per-handler rendered-HTML lexicon check MUST cover at least one ERROR response per handler (e.g., `POST /reach` with empty HolderID → 400 response body scanned for banned terms), not just the happy-path 200 response. Error messages and error templates are a drift vector the happy-path check misses. | None — RATIFIED (with G-026 enforcement) |
|
||||
|
||||
---
|
||||
|
||||
## 3. Binding Fixes (G-025..G-028)
|
||||
|
||||
These are **binding** — the orchestrator MUST apply them before the
|
||||
affected phase ships. Numbered G-025..G-028 (continuing from G-024).
|
||||
|
||||
| ID | Binding Fix | Rationale | Confidence | Affects (phase / task) |
|
||||
|----|-------------|-----------|------------|------------------------|
|
||||
| **G-025** | **`web/store/import_test.go` (P1-03-03) MUST forbid imports of `x/*/keeper` AND `x/*/module` (and any `x/*/` subpackage other than `types`), not just `x/*/keeper`.** The plan's P1-03-03 only mentions `x/*/keeper`. `x/*/module` (the Cosmos `module.go` runtime wiring) is also a production surface that `web/` must not reach into — importing `x/bond/module` would couple the UI to the runtime app-module graph, violating D-067 ("no keeper, no Cosmos runtime, no `app.go`"). The import-invariant test should assert that every `github.com/oy/openyield/x/<module>/` import path in a `web/` production `.go` file ends in `/types` (i.e., only `x/<module>/types` is allowed; `x/<module>/keeper`, `x/<module>/module`, `x/<module>/simtest`, etc. are forbidden). | The plan's `web/store/import_test.go` only forbids `x/*/keeper`, leaving `x/*/module` (and other runtime subpackages) as an unguarded import path. D-067's "no Cosmos runtime" boundary is broader than just "no keeper". A single test asserting "only `x/*/types` imports from `web/`" closes the full boundary. | 0.82 | **P1-03-03** (`web/store/import_test.go`). Must land before P1 ships; carries through P2..P5. |
|
||||
| **G-026** | **The per-handler rendered-HTML lexicon check (P1-04-03, P2-03-01, P3-03-01, P4-03-01, P5-03-01) MUST scan at least one ERROR response body per handler, not just the happy-path 200 response.** For each handler, the test must include a case that triggers an error response (e.g., `POST /reach` with empty HolderID → 400; `GET /stash/{nonexistent}` → 404; `POST /window` with invalid scope → 400) and assert the error response body contains no banned terms via `lexicon.FindBannedTerm`. Error messages and error templates are a drift vector: a banned term in an error string (e.g., `"account not found"`, `"deposit failed"`) would not be caught by the happy-path rendered-HTML check. The file-scan firewall catches banned terms in template FILES, but a dynamically-generated error message (constructed in Go code, not a template file) is only caught by scanning the error response body. | The plan's rendered-HTML lexicon checks scan the happy-path 200 response. Error responses are a separate code path (different template, or a Go-constructed error string) that the happy-path check does not exercise. A banned term in an error message is user-facing and would pass the file-scan firewall (the term is in a Go string literal, not a template file) while failing the lexicon intent. Scanning one error response per handler closes this gap. | 0.80 | **P1-04-03, P2-03-01, P3-03-01, P4-03-01, P5-03-01** (rendered-HTML lexicon checks). Must land before each phase ships. |
|
||||
| **G-027** | **`web/store/store.go` `CreateReach` (P1-03-01) MUST validate `holderID` and `publicKey` before using them as map keys / URL path segments.** Validation: non-empty (already tested in P1-04-03), length-bounded (e.g., `len(holderID) <= 128`, `len(publicKey) <= 256`), no path separators (`/`, `\`), no template syntax (`{{`, `}}`). The handler test (P1-04-03) MUST include cases for each validation failure (empty → 400; too long → 400; contains `/` → 400; contains `{{` → 400). This is a prototype-robustness gate, not a production security gate — the mock store resets on restart, but an adversarial input should not break the happy path or cause route confusion (`GET /reach/{id}` with `id` containing `/` would mismatch the route). | The plan tests "empty HolderID returns 400" but does not specify validation of the map key / URL path segment beyond emptiness. `html/template` escapes on OUTPUT, but the HolderID is also used in URL construction (`GET /reach/{id}`) and route matching — a HolderID containing `/` would cause route confusion; a very long string would cause memory exhaustion in the in-memory map. Validation before map-key use is the standard prototype-robustness pattern. | 0.75 | **P1-03-01** (`web/store/store.go` `CreateReach` validation); **P1-04-03** (handler test cases for each validation failure). Must land before P1 ships. |
|
||||
| **G-028** | **The P6 audit (P6-02-01) MUST verify `go.mod` is byte-identical (or diff-empty) against the `v0.5.0` tag (the v0.6 P0 baseline), NOT just check "no new direct require lines were added in P1..P5".** The verification: `git diff v0.5.0..HEAD -- go.mod` must be EMPTY (or contain only `// indirect` reordering with no version changes). A `go mod tidy` side-effect in P1..P5 could bump an indirect dep version (e.g., `github.com/cosmos/cosmos-sdk` stays v0.50.8 but an indirect dep bumps from v1.2.3 to v1.2.4) without adding a direct require line — a naive "no new direct require lines" check would pass while `go.mod` changed. The diff-against-baseline check closes this. | `go.mod` already has the v0.5 cosmos-sdk + ibc-go deps (verified). v0.6 "go.mod unchanged" means unchanged FROM the v0.5 baseline, not "empty". The plan's P6-02-01 says "G-006 go.mod unchanged (HTMX is a vendored static asset, NOT a `go get` — zero new require lines in v0.6)" — "zero new require lines" is necessary but not sufficient; an indirect dep bump is a `go.mod` change that "zero new require lines" would miss. The diff-against-`v0.5.0` check is the complete enforcement. | 0.85 | **P6-02-01** (audit feature purity gate — G-006 verification). Must land before P6 ships (milestone release). |
|
||||
|
||||
---
|
||||
|
||||
## 4. Escalations
|
||||
|
||||
**None.** All nine axes resolved at confidence ≥ 0.60 after the binding
|
||||
fixes G-025..G-028 are applied. No axis required escalation to the human.
|
||||
At full autonomy, the orchestrator applies the binding decisions (D-078..
|
||||
D-081) and binding fixes (G-025..G-028) and proceeds to EXECUTE.
|
||||
|
||||
The single most material finding is **G-028** (the go.mod audit surface):
|
||||
because v0.5 already added cosmos-sdk, the v0.6 "go.mod unchanged" gate is
|
||||
subtler than "no new deps" — it must verify no indirect dep was bumped
|
||||
either. This is not an escalation (the fix is mechanical: diff go.mod
|
||||
against the v0.5.0 tag), but it is the finding most likely to cause a
|
||||
false-green P6 audit if not surfaced now.
|
||||
|
||||
---
|
||||
|
||||
## 5. Overall Verdict
|
||||
|
||||
### **SHIP Phase 0 WITH FIXES** (confidence 0.82)
|
||||
|
||||
The v0.6 Phase 0 plan is fundamentally sound and well-grounded: the
|
||||
Go `html/template` + HTMX + stdlib `ServeMux` stack is genuinely zero-new-
|
||||
dep (HTMX is a vendored static asset, not a Go dep — D-078); the mock-
|
||||
server-over-real-Go-types approach (D-067) is grounded in verified
|
||||
codebase facts (all six `x/*/types` packages exist with the structs and
|
||||
methods the UI surfaces); the 5-screen scope is proportionate (not over-
|
||||
scoped for a first UI milestone); the firewall-first ordering (D-069,
|
||||
P1 before content) is the proven D-044 pattern; D-073 (code constants
|
||||
over outdated docs) is verified correct against the actual numeric
|
||||
contradiction (`GrainsPerBread=10000` vs docs "1,000×").
|
||||
|
||||
The 4 decision ratifications (D-078..D-081) are all **RATIFIED**:
|
||||
- **D-078** (G-006 boundary — HTMX is a static asset) — RATIFY with G-028
|
||||
audit enforcement.
|
||||
- **D-079** (G-003 boundary — web/ is app-layer) — RATIFY with G-025
|
||||
import-invariant enforcement.
|
||||
- **D-080** (D-073 — code constants over docs) — RATIFY as-is (regression-
|
||||
guard test already in the plan).
|
||||
- **D-081** (lexicon firewall extension to web/) — RATIFY with G-026
|
||||
error-response lexicon enforcement.
|
||||
|
||||
The 4 binding fixes (G-025..G-028) are **correctness and verification
|
||||
hardening**, not scope rework:
|
||||
- **G-025** (import-invariant forbids `x/*/module` too) — closes the
|
||||
"no keeper but maybe module" gap in the plan's `web/store/import_test.go`.
|
||||
- **G-026** (rendered-HTML lexicon check covers error responses) — closes
|
||||
the error-message drift vector the happy-path check misses.
|
||||
- **G-027** (ReachID/HolderID validation before map-key use) — prototype-
|
||||
robustness gate; prevents route confusion and memory exhaustion on
|
||||
adversarial input.
|
||||
- **G-028** (go.mod diff against v0.5.0 baseline, not just "no new
|
||||
require lines") — closes the indirect-dep-bump false-green in the P6
|
||||
audit.
|
||||
|
||||
None of these rise to "RETHINK" or "REDUCE SCOPE" — the architecture,
|
||||
scope, ordering, and persona assignments are correct. The security posture
|
||||
(no auth, no sessions, in-memory store) is acceptable for a "working
|
||||
prototype" milestone and explicitly out of scope. Apply the 4 binding
|
||||
fixes and proceed to EXECUTE (P1).
|
||||
|
||||
**Confidence in overall verdict: 0.82**
|
||||
|
||||
---
|
||||
|
||||
## 6. Summary Block
|
||||
|
||||
```
|
||||
Decision ratifications:
|
||||
D-078 (G-006 boundary — HTMX vendored static asset) — RATIFY (0.92)
|
||||
D-079 (G-003 boundary — web/ is app-layer, not x/) — RATIFY (0.85)
|
||||
D-080 (D-073 — code constants over outdated docs) — RATIFY (0.90)
|
||||
D-081 (lexicon firewall extension to web/) — RATIFY (0.88)
|
||||
|
||||
Nine-axis scorecard:
|
||||
1. Feasibility (html/template + HTMX + stdlib) — PASS (0.85)
|
||||
2. Scope (5 screens + firewall + mock, 5 phases) — PASS (0.80)
|
||||
3. Cost (6 phases, 33 tasks) — PASS (0.82)
|
||||
4. G-006 (HTMX vendored, not a Go dep) — PASS (0.92) → enforced by G-028
|
||||
5. G-003 (web/ app-layer consumption) — CONDITIONAL (0.78) → fixed by G-025
|
||||
6. REQ-012 (lexicon firewall extension to web/) — CONDITIONAL (0.80) → fixed by G-026
|
||||
7. Bread-scale (D-073 code constants) — PASS (0.90)
|
||||
8. Security (no auth, XSS, injection) — CONDITIONAL (0.75) → fixed by G-027
|
||||
9. Testability (≥80% web/, HTMX fragments, MVP) — PASS (0.82)
|
||||
|
||||
Feature purity gate: PASS WITH FIXES (G-025, G-028)
|
||||
|
||||
Binding fixes: 4 (G-025..G-028)
|
||||
G-025 — web/store/import_test.go forbids x/*/keeper AND x/*/module — before P1
|
||||
G-026 — rendered-HTML lexicon check covers error responses — before each phase
|
||||
G-027 — CreateReach validates HolderID/PublicKey before map-key — before P1
|
||||
G-028 — P6 audit diffs go.mod against v0.5.0 tag (not just no-new-lines) — before P6
|
||||
|
||||
Escalations: 0
|
||||
Overall: SHIP Phase 0 WITH FIXES (confidence 0.82)
|
||||
```
|
||||
|
||||
---
|
||||
|
||||
## 7. CI Commit Block (for the orchestrator)
|
||||
|
||||
```
|
||||
docs(P00): grill — v0.6 adversarial review (D-078+, G-025+)
|
||||
|
||||
---ci---
|
||||
project: oy
|
||||
phase: 0
|
||||
milestone: v0.6
|
||||
status: grill
|
||||
decisions:
|
||||
- id: D-078
|
||||
decision: RATIFY G-006 boundary — HTMX vendored as web/static/htmx.min.js is G-006-compliant (static asset, not Go dep); go.mod unchanged across v0.6
|
||||
rationale: vendored JS served over http.FileServer is not a Go module dep (no import path); go.mod already has cosmos-sdk from v0.5; v0.6 adds nothing; G-028 enforces diff-against-v0.5.0
|
||||
confidence: 0.92
|
||||
alternatives: [HTMX via go get (breaks G-006); node toolchain + React (breaks Go-only convention)]
|
||||
- id: D-079
|
||||
decision: RATIFY G-003 boundary — web/ importing x/*/types is app-layer consumption (D-070), not cross-x/ production import; G-003 firewall intact
|
||||
rationale: web/ is not an x/ module; existing G-003 test scans x/**/*.go (web/ outside scope by construction); new web/store/import_test.go is the web/-scoped complement; G-025 extends forbidden set to x/*/module
|
||||
confidence: 0.85
|
||||
alternatives: [treat web/ as x/ module (wrong — not protocol); forbid web/ from importing x/*/types (would force TS/JSON fixtures, losing locked-constant grounding)]
|
||||
- id: D-080
|
||||
decision: RATIFY D-073 — Bread-scale source of truth = x/bread/types code constants (GrainsPerBread=10000, BreadScaleAll()), NOT docs/shared/bread-scale.md (outdated: 1,000× vs code 10,000×)
|
||||
rationale: verified numeric contradiction (types.go:13 GrainsPerBread=10000 vs docs "1,000×"); code constants tested, docs not; doc-fix deferred to P1+ follow-up (not v0.6 scope); P2-03-01 regression-guard test enforces
|
||||
confidence: 0.90
|
||||
alternatives: [use docs values (wrong — not tested, disagrees with code); fix docs in v0.6 (out of scope — doc-drift fix, not UI feature)]
|
||||
- id: D-081
|
||||
decision: RATIFY lexicon firewall extension to web/ (REQ-045) — new sibling lexicon_meta_web/ mirroring lexicon_meta_docs/; firewall-first (P1 before content); "Create a Reach" label (not "account")
|
||||
rationale: REQ-012 is All phases; UI strings user-facing; SyntheticBannedStrings() helper exists (G-014); lexicon_meta_docs/ has proven template; G-026 extends rendered-HTML check to error responses
|
||||
confidence: 0.88
|
||||
alternatives: [skip firewall (REQ-012 is All phases); single combined meta-test (loses isolation)]
|
||||
fixes:
|
||||
- id: G-025
|
||||
fix: web/store/import_test.go MUST forbid x/*/keeper AND x/*/module (only x/*/types allowed from web/)
|
||||
affects: P1-03-03
|
||||
before_phase: P1
|
||||
confidence: 0.82
|
||||
- id: G-026
|
||||
fix: per-handler rendered-HTML lexicon check MUST scan at least one ERROR response body (not just happy-path 200)
|
||||
affects: P1-04-03, P2-03-01, P3-03-01, P4-03-01, P5-03-01
|
||||
before_phase: P1 (carries through P2..P5)
|
||||
confidence: 0.80
|
||||
- id: G-027
|
||||
fix: CreateReach MUST validate HolderID/PublicKey (non-empty, length-bounded, no path separators, no template syntax) before map-key use; handler test covers each validation failure
|
||||
affects: P1-03-01, P1-04-03
|
||||
before_phase: P1
|
||||
confidence: 0.75
|
||||
- id: G-028
|
||||
fix: P6 audit MUST diff go.mod against v0.5.0 tag (not just check "no new direct require lines") — catches indirect dep bumps
|
||||
affects: P6-02-01
|
||||
before_phase: P6
|
||||
confidence: 0.85
|
||||
escalations: []
|
||||
---/ci---
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user