fix(06): resolve research questions

This commit is contained in:
Jakub Zych
2026-09-19 23:13:24 +02:00
parent 2f90fef830
commit a61c83e614

View File

@@ -459,22 +459,16 @@ Note: the allow-list check (dotted-suffix match, e.g. `evil-discogs.com` must ne
| A3 | The inline `throttle:N,M` key for an authenticated request resolves to `sha1($user->getAuthIdentifier())`, i.e., keyed by user id, not IP, on the JWT group's fixed-throttle routes (`switch`, `collection/share/regenerate`, `wishlist/share/regenerate`, `household/invitations`, `household/invitations/{id}/resend`, `oauth-identities/{provider}` DELETE, `import/csv`) | Common Pitfalls #4 | Verified directly from `ThrottleRequests::resolveRequestSignature()` vendor source — HIGH confidence, not really an assumption, but flagged because the Go port's exact key-string format (raw user id vs. a Go-side hash) is a discretion item the planner must still pin down explicitly |
| A4 | `swag --version` printing `v1.16.4` after a `go install ...@v1.16.6` is a benign embedded-version-string lag, not evidence the wrong module was installed | Standard Stack / Version verification | If wrong, the CI/build-time `swag` binary could be running older-than-expected annotation-parsing logic; low risk since `go list -m` confirmed the module resolves to v1.16.6, but the discrepancy itself was not root-caused (e.g., by inspecting swag's own release-tagging history) |
## Open Questions
## Open Questions (RESOLVED)
1. **Was the Phase 2 parity harness's recorded PHP fixtures captured with `APP_DEBUG=true` or `false`?**
- What we know: dev `.env` defaults to `true`; production runbook mandates `false`; the parity harness (Phase 2) runs against an "Isolated PHP" instance on `127.0.0.1:8423` per STATE.md, whose own `APP_DEBUG` setting was not inspected this session.
- What's unclear: whether the 429/error JSON bodies in already-recorded fixtures (and any new ones the phase 6 planner records) reflect the debug or non-debug shape.
- Recommendation: before finalizing the error-body shape in a plan, grep the actual parity harness's PHP boot config (`check-phase2.sh --fresh-php` env, referenced in STATE.md) for `APP_DEBUG`, or record one live 429 fixture and inspect its body directly.
1. **`APP_DEBUG` contract — RESOLVED: `false`.**
- The Phase 6 implementation pins the isolated PHP parity recorder to `APP_DEBUG=false`, matching the production runbook and establishing the non-debug 429 body `{"message":"Too Many Attempts."}` as the contract. Three older HTML exception fixtures recorded under debug remain explicitly flagged for re-recording when their routes are ported; they are not 429 contract fixtures.
2. **What are the real production `client_max_body_size` / `post_max_size` / `upload_max_filesize` values?**
- What we know: not present anywhere in the `fonoteka` repo; the production nginx vhost is explicitly operator-managed and off-repo per `docs/deploy/plytarium.com.md`.
- What's unclear: the actual numbers.
- Recommendation: `checkpoint:human-verify` — ask the user/operator directly, or SSH-inspect the host per the (out-of-scope-for-this-agent) runbook. Do not ship a guessed default silently as if verified.
2. **Production body limits — RESOLVED: operator-confirmed `128M` for all three limits.**
- On 2026-09-19 the operator confirmed nginx `client_max_body_size=128M`, PHP `post_max_size=128M`, and PHP `upload_max_filesize=128M`. Using binary megabytes, both Go config keys are therefore `134217728` bytes (`http.body_limits.default_bytes` and `http.body_limits.upload_bytes`), as recorded and verified by Plan 06-03.
3. **Does the limiter's `Store` sweep interval need to be configurable per-environment (e.g., faster sweep in tests)?**
- What we know: CONTEXT marks this "Claude's Discretion."
- What's unclear: whether the plan needs a config key (`http.ratelimit.sweep_interval`) or a hardcoded reasonable default (e.g., 1 minute) is sufficient for v1.
- Recommendation: default to a hardcoded interval (e.g., matching the longest bucket's decay, 2 minutes) unless a test needs to control it directly via a constructor parameter — avoid adding a new config surface for something with no current multi-environment need.
3. **Limiter `Store` sweep interval — RESOLVED: fixed two-minute interval.**
- No environment config key was added. `surf.BuildRouter` constructs `NewMemoryStore(2*time.Minute)`, twice the longest one-minute bucket decay; tests may still pass a shorter constructor interval to exercise cleanup without expanding production configuration surface.
## Environment Availability