cesnimda
f0f178d77e
Deep, code-grounded audit of Job Tracker producing the mission deliverables under docs/remaster/: system audit, bug report, architecture/data-model/AI/UX reviews, remaster proposal, migration plan, competitor research, and the gated REBUILD_DECISION. Verdict: Incremental Refactor (no full rebuild). Evidence: no Critical defects; hardened cookie/CSRF auth (token never in JS storage), real SSRF defence, enforced multi-tenancy via global query filters, decoupled provider-swappable AI service, 135 backend tests. Debt is localised (god controllers/entity, missing hot-path indexes, prompt-injection hardening, CRA build debt) and reachable by in-place, test-guarded refactors. Also harden .gitignore: exclude agent tooling (.claude/, .bg-shell/, .agent.md) and restore/broaden the runtime-secrets block (**/keys/, **/backups/, exports, CV artifacts) so nested DataProtection keys can't be committed accidentally. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
84 lines
6.0 KiB
Markdown
84 lines
6.0 KiB
Markdown
# Rebuild Decision — Job Tracker
|
||
|
||
**Companion to:** [SYSTEM_AUDIT_REPORT.md](SYSTEM_AUDIT_REPORT.md)
|
||
**This is the gate.** Per the mission, because the recommendation is **Incremental Refactor**, work STOPS
|
||
here pending your approval — no `JobTrackerV2` is created.
|
||
|
||
## Executive summary
|
||
The audit examined product logic, architecture, data model, AI, security, UX, testing, and deployment
|
||
against the actual code. The system is a **mature, working, production-deployed brownfield** with correct
|
||
architectural bones (hardened cookie/CSRF auth, real SSRF defence, enforced multi-tenancy, a cleanly
|
||
decoupled AI service, deterministic scoring, 135 backend integration tests + 23 frontend suites, live at
|
||
`jobs.cesnimda.uk`). Its problems are **concentrated and fixable** — god controllers, a god entity, missing
|
||
indexes, prompt-injection hardening, and CRA build debt — none of which are load-bearing architectural
|
||
failures. A rebuild would discard substantial correct, tested work to re-solve problems that are already
|
||
solved, while re-introducing risk. **The evidence points clearly to incremental refactor.**
|
||
|
||
## Recommendation
|
||
### ✅ Continue with Incremental Refactor
|
||
(A full rebuild is **not** justified.)
|
||
|
||
## Evidence
|
||
**Against rebuild / for refactor:**
|
||
1. **No Critical defects.** No auth bypass, tenant-isolation break, or SSRF hole in audited paths. Rebuilds
|
||
are justified when the foundation is unsafe; this foundation is sound.
|
||
2. **The hard, easy-to-get-wrong things are already right:** SSRF blocklist (post-DNS, all private/CGNAT/
|
||
link-local/IPv6-ULA ranges), HttpOnly-cookie + CSRF auth (token never in JS storage — test-asserted),
|
||
global query-filter tenancy, and a provider-swappable AI boundary that needs **zero** app changes to move
|
||
off the weak prod GPU.
|
||
3. **Strong test harness.** 135 integration tests exercise controllers against a real in-memory DB — they
|
||
lock behaviour so internals can move safely. A rebuild throws this safety net away.
|
||
4. **Debt is localised.** 3 god controllers (~6.7k of ~9k controller lines) and 1 god entity account for
|
||
most of the maintainability pain. Both are reachable by in-place extraction.
|
||
5. **Live in production with a working CI/CD pipeline.** Discarding a deployed, observable system for a
|
||
greenfield reset trades known, bounded debt for unknown, unbounded schedule risk.
|
||
|
||
**Acknowledged weaknesses (all refactorable):** god classes; `JobApplication` god entity + denormalised
|
||
attachment booleans + dual CV source; missing hot-path indexes + load-all analytics + Gmail N+1; prompt
|
||
injection (capped by human-review); CRA transitive-vuln debt; the unexecuted "use next.js" override.
|
||
|
||
## Estimated effort
|
||
| | Incremental Refactor | Full Rebuild |
|
||
|---|---|---|
|
||
| Perf wave (indexes, aggregation, N+1, AI router) | ~1 focused pass | included, re-derived |
|
||
| Safe refactors (extract services/DTOs) | ~1–2 passes | rebuilt from scratch |
|
||
| Data-model evolution (versioning, splits) | ~1–2 passes, additive migrations | rebuilt + data migration anyway |
|
||
| Frontend platform (Vite/Next) | 1 isolated milestone | rebuilt |
|
||
| **Re-earning current parity (auth, SSRF, tenancy, 135 tests, deploy)** | **£0 — already have it** | **large, high-risk, re-tested** |
|
||
| **Total** | **Weeks of bounded, shippable slices** | **Months, mostly to get back to today** |
|
||
|
||
**Long-term maintenance:** after the refactor waves, maintenance cost is *lower than a rebuild's* because
|
||
the domain knowledge, tests, and ops are retained and improved rather than reconstructed.
|
||
|
||
## Risks
|
||
- **Continuing (current architecture):** god classes slow features and invite merge conflicts; unindexed
|
||
hot paths degrade as data grows; prompt injection can mislead drafts; CRA debt ages. **All mitigated by
|
||
the planned waves.**
|
||
- **Rebuilding:** re-introducing already-solved security bugs; long no-value-delivery window; data migration
|
||
is required *either way*; loss of the test harness during transition; opportunity cost.
|
||
- **Migration (refactor path):** additive migrations with backfill checks + staging dry-run on a MariaDB
|
||
copy + already-automated backups keep data-loss risk low.
|
||
- **User impact:** refactor path keeps the app live throughout; rebuild path risks a freeze or a parallel
|
||
system to maintain.
|
||
- **Operational:** single-capacity CI runner → keep slices small, one PR at a time (already the practice).
|
||
|
||
## Reuse analysis
|
||
| Verdict | Items |
|
||
|---|---|
|
||
| **Reuse unchanged** | Cookie/CSRF auth, SSRF ingestion guard, global query filters, `JobCvMatchService`/`JobPipeline`/`StageAnalytics`/`EmailStatusClassifier`, AI HTTP boundary, background-service model (single-node), test suites, deploy pipeline, docs from prior phases |
|
||
| **Refactor before reuse** | `JobApplicationsController`, `ProfileCvController`, `GmailController`, `JobApplication` entity, `tools/summarizer` prompt construction, CRA build setup |
|
||
| **Rewrite** | attachment-boolean logic → computed; tailored-CV/cover-letter storage → versioned tables; analytics read paths → server-side aggregation |
|
||
| **Remove** | denormalised attachment columns (post-backfill), inline `TailoredCvText`/`CoverLetterText` (post-migration), dead `Controller/` folder, scratch files (`temp_job.json`, `temp_post_job.py`) |
|
||
| **Keep out of repo** | `.gsd/`, `.claude/`, keys, backups, exports (all git-ignored — verify `.claude` is added) |
|
||
|
||
## Long-term recommendation
|
||
**Incrementally remaster.** It delivers the best balance of maintainability (retain tests + knowledge),
|
||
scalability (indexes + service extraction + optional queue), engineering velocity (shippable slices, no
|
||
freeze), reliability (behaviour-locked by tests, app stays live), and product quality (UX/AI fixes land
|
||
continuously). Reserve "rebuild" language for the *frontend platform* only, and only if you choose Next.js
|
||
for a public SEO product — that is a scoped migration, not a system rebuild.
|
||
|
||
## Gate
|
||
➡️ **Awaiting your approval.** On approval, I proceed with [MIGRATION_PLAN.md](MIGRATION_PLAN.md) Wave 1
|
||
(the paused Phase 7 performance work) as the first slice. No `JobTrackerV2` will be created.
|