254 lines
19 KiB
Markdown
254 lines
19 KiB
Markdown
# Phase 0 — Foundation Corrections Report
|
||
|
||
Date: 2026-07-17
|
||
Scope: the five priorities set after the discovery review. Nothing else was touched.
|
||
Companion to `docs/application-discovery-report.md`, `docs/architecture/current.md`, `docs/decisions/ADR-002-job-application-model.md`.
|
||
|
||
**Nothing was committed.** All changes are in the working tree for review.
|
||
|
||
---
|
||
|
||
## 1. Completed
|
||
|
||
### Priority 1 — Restore accurate architecture documentation ✅
|
||
|
||
The active `docs/` tree was a scaffold: 139 files, median ~480 bytes, while the real documentation sat archived. `docs/AI_SESSION_START.md` sends every future session to read those stubs, so each one started from fiction.
|
||
|
||
- **`docs/architecture/current.md`** — restored from `docs/_archive/SYSTEM_OVERVIEW.md` (20 KB) and re-verified line by line against the code. Was a 3-line "TODO: Complete documentation." stub. **Nine corrections** to the archived original:
|
||
|
||
| Archived claim (2026-07-02) | Reality (verified 2026-07-17) |
|
||
|---|---|
|
||
| "CRA/react-scripts 5, TypeScript 4.9" | Next.js 16 + TypeScript 5.9; CRA survives only as the test runner |
|
||
| Root `Controller/` folder is dead code | Removed in `519c32e` |
|
||
| CI runs "a whitelist of 10 frontend test files" | Whitelist gone; whole suite runs, with a comment forbidding its return |
|
||
| Match score ❌ missing | Exists (`JobCvMatchService`, `/match-score`, `/candidate-fit`) |
|
||
| Salary is a text field | Structured (`SalaryMin`/`Max`/`Currency`/`Period`) |
|
||
| 8 migrations | 11 (after this phase) |
|
||
| ~15 controllers | 19, with exact line counts |
|
||
| Rate limiting: 2 policies | 3 (`auth-login`, `auth-email`, `auth-2fa-challenge`) |
|
||
| 6 hosted services | 7 (`DatabaseBackupHostedService` added) |
|
||
|
||
Also documented for the first time: the three-toolchain frontend, the absence of any state-management layer, the AI provider reality vs. `docs/00-ai-context.md`'s fiction, and six historical decisions that existed only as code comments.
|
||
|
||
- **`docs/research/competitors.md`** — restored from `docs/_archive/PRODUCT_RESEARCH.md` (13 KB, sourced). Was a stub. Six feature-matrix rows corrected against code and marked `[corrected 2026-07-17]`. The scope gap is stated plainly at the top: **Novoresume, Reactive Resume and ElegantCV are analysed nowhere in this repo**, and Reactive Resume is a Phase 4 prerequisite.
|
||
|
||
- **`docs/decisions/ADR-002-job-application-model.md`** — written. Was a 0-byte file naming the single most consequential decision in the system.
|
||
|
||
- **Ten 0-byte files deleted** (verified empty first): the four ADR placeholders, five `09-research/*` analyses, and `known-issues.md`. They advertised content that never existed.
|
||
|
||
### Priority 2 — Secure the AI sidecar ✅
|
||
|
||
The sidecar published port 8001 to the host and had **no authentication of any kind** — the only `Authorization` header in `app.py` was outbound to Groq. Anyone reaching the host could call `/cv/rewrite`, `/summarize`, `/extract-text`: draining the Gemini/Groq API key and running arbitrary text through the model. This matters more than it looks, because `jobtracker_shared` is an *external, shared* Docker network — other containers could reach it too.
|
||
|
||
Two layers, both required:
|
||
|
||
1. **Network** — host port mapping removed; `expose: "8001"` only. The backend reaches it in-network at `http://ai-service:8001`. Local published ports now live only in the explicitly selected `docker-compose.dev.yml`.
|
||
2. **Shared secret** — `X-Ai-Service-Token` required on every endpoint except `/health` (which the backend probe and the compose healthcheck both need, and which exposes no data or generation path). Compared with `hmac.compare_digest` to avoid a timing leak.
|
||
|
||
**Where the enforcement lives matters.** The token is unset → open, so local dev and the existing test suite keep working keyless. Production cannot reach that state: `docker-compose.yml` declares `AI_SERVICE_TOKEN=${AI_SERVICE_TOKEN:?...}`, so **the stack refuses to start without it**. Misconfiguration fails loudly at deploy rather than silently booting open at runtime. Verified: `docker compose config` with no token exits non-zero.
|
||
|
||
The backend side is one place — all seven call sites route through the named `ai-service` `HttpClient`, so the header is set once in `Program.cs`.
|
||
|
||
### Priority 3 — Introduce Job separate from JobApplication ✅ (foundation)
|
||
|
||
`Job` (`JobTrackerApi/Models/Job.cs`) is the opportunity; `JobApplication` is the pursuit of it. `JobApplication.JobId` is a nullable FK (`OnDelete: SetNull`). `Job` carries the same deny-on-null tenant query filter as every other owned entity.
|
||
|
||
**Deliberately additive: nothing reads or writes `Job` yet**, no dual-write, no backfill, no legacy columns dropped. That keeps Priority 3 a pure schema change with zero behavioural risk, which is what "unblock future phases safely" asks for. The Phase 1 cutover plan (dual-write → backfill → flip reads → drop columns) is in ADR-002.
|
||
|
||
`Job` also carries `Source` and `CountryCode`, so market is a data dimension from day one — per the "Norway first, but no hardcoding Norway" decision. The entity is ready; the *plugins* still assume NO (roadmap 6.6).
|
||
|
||
### Priority 4 — Expand pipeline stages beyond Applied ✅
|
||
|
||
`JobPipeline` gained `PipelineCategory.Prospect` and three stages ahead of `Applied`:
|
||
|
||
```
|
||
Saved(1) · Interested(2) · Preparing(3) → Applied(4) · Waiting(5) · Interview(6) → Offer(7) → Rejected(8) · Ghosted(9)
|
||
```
|
||
|
||
No schema change was needed for the stages themselves — `Status` is free-text by deliberate prior design, so custom values survive. Synonyms map in (`bookmarked`/`wishlist`/`to apply` → `Saved`, `shortlisted` → `Interested`, `drafting` → `Preparing`).
|
||
|
||
**This is what unblocks the guide's #2 priority workflow.** The 6-step add-job wizard already existed and walked users through *preparing* an application — with nowhere to put the result but `Applied` with a fabricated date.
|
||
|
||
The supporting change: **`DateApplied` is now nullable**, with `SavedAt` added. The invariant — *`DateApplied` is set if and only if the job has left the Prospect stages* — is enforced in exactly one function, `JobPipeline.SyncAppliedDate`, called from all three status-write paths so they cannot drift.
|
||
|
||
### Priority 5 — Update the roadmap ✅
|
||
|
||
`docs/implementation-roadmap.md` re-scoped against the real architecture, with all six product decisions folded in. Phase 1 shrank (~1–1.5 wk → ~3–5 days) because Phase 0 absorbed most of it; Phase 5 shrank (docs fix, not an abstraction build); Phase 6 shrank and reordered (no scraping; extension before search backend); Phase 3 grew slightly (relational profile is now a committed L). Four new tasks came out of Phase 0 findings.
|
||
|
||
---
|
||
|
||
## 2. Files changed
|
||
|
||
**Backend**
|
||
- `JobTrackerApi/Models/Job.cs` *(new)* — the opportunity entity
|
||
- `JobTrackerApi/Models/JobApplication.cs` — `JobId`, `SavedAt`, nullable `DateApplied`, `DaysSince` → `int?`
|
||
- `JobTrackerApi/Data/JobTrackerContext.cs` — `Jobs` DbSet, tenant filter, FK config, index
|
||
- `JobTrackerApi/Services/JobPipeline.cs` — `Prospect` category, 3 stages, `IsProspect`, `SyncAppliedDate`
|
||
- `JobTrackerApi/Services/RulesEngine.cs` — prospect guard + null-`DateApplied` fail-safe
|
||
- `JobTrackerApi/Services/AnalyticsService.cs` — exclude prospects from applied-volume, average age, response-time; `SavedAt` fallback for stage entry
|
||
- `JobTrackerApi/Services/FollowUpReminderHostedService.cs` — null-safe applied date
|
||
- `JobTrackerApi/Controllers/JobApplicationsController.cs` — `SyncAppliedDate` on all 3 write paths, `SavedAt` in DTO, null-safe analytics/drafts
|
||
- `JobTrackerApi/Controllers/JobApplicationDtos.cs` — `DateApplied` → `DateTime?`, `DaysSince` → `int?`, `SavedAt` added
|
||
- `JobTrackerApi/Controllers/ExportController.cs` — null-safe CSV
|
||
- `JobTrackerApi/Program.cs` — `X-Ai-Service-Token` on the `ai-service` HttpClient
|
||
|
||
**Frontend** (type-driven; the compiler found every site that would have rendered a null)
|
||
- `src/types.ts` — `dateApplied: string | null`, `daysSince: number | null`, `savedAt: string`
|
||
- `src/components/JobFlowBar.tsx` — omits the "Applied" milestone when there is no applied date
|
||
- `src/components/KanbanBoard.tsx` — sort falls back to `savedAt`; "Applied Nd ago" hidden when null
|
||
- `src/components/JobTable.tsx`, `JobDetailsDialog.tsx` — render `—`
|
||
- `src/components/EditJobDialog.tsx`, `ImportExportJobs.tsx` — null handling
|
||
|
||
**AI service**
|
||
- `tools/summarizer/app.py` — token middleware
|
||
- `docker-compose.yml` — `expose` not `ports`; `AI_SERVICE_TOKEN` + `Ai__ServiceToken` with `:?` guards
|
||
- `.env.example` — `AI_SERVICE_TOKEN` documented as required
|
||
|
||
**Tests** (+31)
|
||
- `JobTrackerApi.Tests/RulesEngineProspectTests.cs` *(new)* — 5 cases
|
||
- `JobTrackerApi.Tests/JobPipelineTests.cs` — +21 cases
|
||
- `tools/summarizer/tests/test_app.py` — +5 token-guard cases
|
||
- `job-tracker-ui/src/workflow-trust-signals.test.tsx` — fixture updated
|
||
|
||
**Docs**
|
||
- `docs/architecture/current.md`, `docs/research/competitors.md`, `docs/decisions/ADR-002-job-application-model.md`, `docs/implementation-roadmap.md`, this report. Ten 0-byte files deleted.
|
||
|
||
> **Not mine:** `views/CareerWorkspacePage.tsx` and `views/ProfilePage.tsx` show as modified — that is **your** pre-existing uncommitted work (the inert CV Builder tab). I did not touch either.
|
||
|
||
---
|
||
|
||
## 3. Database changes
|
||
|
||
Migration `20260717071417_AddJobEntityAndProspectStages`:
|
||
|
||
- `JobApplications.DateApplied` → **nullable** (SQLite table rebuild)
|
||
- `JobApplications.JobId` → nullable FK to `Jobs`, `SetNull`
|
||
- `JobApplications.SavedAt` → new, backfilled `= DateApplied`
|
||
- `Jobs` table created (+ `IX_Jobs_OwnerUserId`, `IX_Jobs_CompanyId`, `IX_JobApplications_JobId`)
|
||
|
||
### The migration was hand-edited, and that was necessary
|
||
|
||
`dotnet ef migrations add` **also scaffolded `CreateTable` for `TrustedDevices`, `TwoFactorRecoveryCodes`, `UserSessions` and `AddColumn` for six `AspNetUsers` columns.** Those tables already exist in every real database — the reconciler in `StartupInitializationExtensions` provisioned them, not a migration, so the prior `ModelSnapshot` never knew them and the scaffolder diffed them as missing.
|
||
|
||
**Verified against the live dev database: all three tables are present.** Shipping the scaffolded migration would have failed the deploy with "table already exists". They were removed; the reconciler still creates them on a fresh boot via `CREATE TABLE IF NOT EXISTS`. `IX_JobApplications_OwnerUserId_IsDeleted_Status` was removed for the same reason — the reconciler applies it with the `Status(50)` prefix length MySQL requires and the scaffolded DDL lacks.
|
||
|
||
**The regenerated snapshot now includes those tables, so future migrations will no longer re-scaffold them.** This migration closes that drift permanently.
|
||
|
||
### Verified against real data, not reasoned about
|
||
|
||
EF warned: *"An operation of type 'SqlOperation' will be attempted while a rebuild of table 'JobApplications' is pending."* Rather than trust the reasoning, the migration SQL was applied to a copy of the real dev database (13 rows):
|
||
|
||
| Check | Result |
|
||
|---|---|
|
||
| Rows preserved | 13 → 13 ✅ |
|
||
| All pre-existing columns survive the rebuild | 36 → 38 (+`JobId`, +`SavedAt`), **0 dropped** ✅ |
|
||
| `DateApplied` values preserved | ✅ |
|
||
| `SavedAt` backfilled from `DateApplied` | ✅, no `0001-01-01` sentinels left |
|
||
| `DateApplied` nullable afterwards | ✅ |
|
||
| `Jobs` created | ✅ |
|
||
| Reconciler-era data (`ShortSummary`) preserved | ✅ |
|
||
|
||
The only `DROP TABLE` is EF's standard rebuild of `JobApplications` itself (create temp → copy → drop → rename).
|
||
|
||
---
|
||
|
||
## 4. Security improvements
|
||
|
||
| Finding | Before | After |
|
||
|---|---|---|
|
||
| **AI sidecar unauthenticated** (High) | Port 8001 published to host; zero auth on `/cv/rewrite`, `/summarize`, `/extract-text`; anyone reaching the host could drain the Gemini/Groq key | **Backend-only, verified live (2026-07-17).** No host port; on a private `ai_internal` network with the backend as its only other member; `X-Ai-Service-Token` required on all non-`/health` endpoints, constant-time compared; compose refuses to start without a token. See §4a. |
|
||
| **Auto-ghosting a job nobody applied to** (new risk introduced by Priority 4) | — | Guarded before any date arithmetic; a null `DateApplied` fails safe rather than reading as "infinitely old". Covered by `RulesEngineProspectTests`, including a Saved-a-year-ago job against 1–2 day thresholds |
|
||
| **Tenant isolation on the new entity** | — | `Job` carries the same deny-on-null global query filter as every other owned entity |
|
||
|
||
### 4a. AI service lockdown — completed and verified 2026-07-17
|
||
|
||
The original Phase 0 fix was **half a fix, and it was never applied**. Two gaps:
|
||
|
||
1. **`expose:` only removes the *host* mapping.** `ai-service` was still attached to `shared_services` — which is `external: true` (`jobtracker_shared`). Any other compose stack on the host can join that network and would have reached port 8001. It was also on `default`, which the frontend shares.
|
||
2. **Nothing had been deployed.** The running container still mapped `0.0.0.0:8001` because the compose change had never been applied, and `.env` had no `AI_SERVICE_TOKEN` — so the stack would in fact have **refused to start**.
|
||
|
||
**Now three layers:**
|
||
|
||
| Layer | Mechanism |
|
||
|---|---|
|
||
| No host port | `ports:` removed, `expose: "8001"` only |
|
||
| Private network | New `ai_internal` bridge with exactly two members — `ai-service` and `backend`. Removed from `default` and `shared_services`. Not `internal: true`, so egress to Gemini/Groq still works |
|
||
| Shared secret | `X-Ai-Service-Token` on every endpoint but `/health`, `hmac.compare_digest`. `${AI_SERVICE_TOKEN:?...}` on **both** services so a deploy that forgets it fails loudly |
|
||
|
||
**Verified against the live stack — not just configured:**
|
||
|
||
| Check | Result |
|
||
|---|---|
|
||
| Host → `localhost:8001/health` | **connection refused** ✅ |
|
||
| Frontend container → `ai-service:8001` | **`wget: bad address`** — cannot even resolve it ✅ |
|
||
| `/summarize` no token (from `ai_internal`) | **401** ✅ |
|
||
| `/cv/rewrite` no token | **401** ✅ |
|
||
| `/extract-text` no token | **401** ✅ |
|
||
| Wrong token | **401** ✅ |
|
||
| `/health` no token | **200** ✅ (the probe and compose healthcheck need it; no data, no generation) |
|
||
| **Backend (172.23.0.3) with token → `/summarize`** | **200 OK** ✅ |
|
||
| `docker compose config` with empty token | **rejected** ✅ |
|
||
| App up | frontend 200, backend 200 ✅ |
|
||
|
||
**`AI_SERVICE_TOKEN` was generated and written to `.env`** (64-hex, gitignored, verified). Rotating it requires recreating both containers together — they must agree.
|
||
|
||
Bonus verification: recreating the backend applied migration `20260717071417_AddJobEntityAndProspectStages` to the **container's** database cleanly (`__EFMigrationsHistory` row written). That is a second, independent confirmation on a real database, alongside the dev-file test in §3.
|
||
|
||
**Caveat for shared Ollama:** if `OLLAMA_BASE_URL` points at an Ollama in another compose stack, address it by host IP — `ai-service` can no longer resolve container names on `shared_services`, by design. The bundled `ollama` profile is on `ai_internal` and still resolves by name.
|
||
|
||
### Still open — both need you
|
||
|
||
1. **DataProtection keys remain recoverable from git history** (`519c32e`, `955cae6`). Untracked, but not unrecoverable. Flagged 2026-07-03, still open. **Needs an operator with production access — I cannot rotate these.**
|
||
2. **CORS wildcard landmine — found during this phase.** `Program.cs:96-102`: if `Cors:Origins` contains `*`, the policy uses `SetIsOriginAllowed(_ => true)` **together with** `AllowCredentials()`. That is reflected-origin-with-cookies: any website could make authenticated requests as the signed-in user. **Not currently active** — compose never sets `Cors__Origins`, so it defaults to `localhost:3000` — but it is one config value from session theft. The fix (reject `*`+credentials) is ~3 lines. **Left alone deliberately: it was outside the five priorities and is not currently exploitable.** Now roadmap task 1.5.
|
||
|
||
---
|
||
|
||
## 5. Verification
|
||
|
||
| Suite | Result |
|
||
|---|---|
|
||
| Backend (`dotnet test`) | **232 passed, 0 failed** (was 206 — +26 new) |
|
||
| AI sidecar (`pytest`) | **16 passed** (was 11 — +5 new) |
|
||
| Frontend typecheck (`tsc --noEmit`) | **clean** |
|
||
| Frontend (`react-scripts test`) | 60 passed, **5 failed** — see below |
|
||
| Solution build | clean |
|
||
| Migration against real data | verified (§3) |
|
||
| `docker compose config` without token | correctly rejected |
|
||
|
||
### The 5 frontend failures are pre-existing, not mine
|
||
|
||
`profile-page.test.tsx` and `settings-view.test.tsx` fail. I verified this rather than assumed it: I stashed **all** Phase 0 changes, re-ran both suites against untouched `main`, and got the **identical 5 failures**, then restored the work and re-confirmed 232 backend tests still pass.
|
||
|
||
Neither suite touches anything I changed. Both exercise `ProfilePage`/`SettingsView` — and `ProfilePage.tsx` is part of *your* uncommitted working-tree changes.
|
||
|
||
**This means `main` is currently red.** CI runs the whole frontend suite, so every future change lands on a broken baseline. Now roadmap task 1.8.
|
||
|
||
### Not verified
|
||
|
||
The app was not run end-to-end. Phase 0 changed no UI behaviour by design (the prospect stages are not yet surfaced — that is Phase 1), so there was nothing new to drive in a browser. The migration is verified against real data; a production run is not.
|
||
|
||
---
|
||
|
||
## 6. Remaining decisions
|
||
|
||
Everything blocking is now either a credential or a small engineering call.
|
||
|
||
### Needs you
|
||
|
||
1. **Stripe keys** — gates roadmap 7.5. Tiers are decided; only credentials are missing.
|
||
2. **DataProtection key rotation** — needs production access.
|
||
|
||
### Raised by Phase 0 — needs a call before the phase it touches
|
||
|
||
3. **Orphan database tables.** The dev database contains `CareerProfiles`, `CareerProfileVersions`, `CvVariants`, `CvVersions`, `TailoredApplications`, `InterviewPrepNotes`, `AiWorkspaceNotes` — **in no EF model and no migration**. This is an abandoned Career Workspace attempt, and its table names map almost exactly onto roadmap Phase 3 (`CvVariants` is the glossary's "CV Variant"; `CareerProfileVersions` is section history). **Was there a previous design worth recovering, or is this dead weight?** Answer before Phase 3 re-treads the same ground. (Roadmap 1.9.)
|
||
|
||
4. **Kanban column count.** The pipeline now has 9 stages; the board is built for 6. Do the three Prospect stages get their own columns, collapse into one "Not applied" column, or sit behind a toggle? Needed to finish Phase 1. (Roadmap 1.1.)
|
||
|
||
5. **Dev DB is missing 2 migrations** (`AddCorrespondenceEmailFields`, `AddShortSummary`) yet has `SyncModelSnapshot` — the reconciler patched those columns instead. Local-only, but it means the dev database is not a faithful rehearsal for a production migration. Worth reconciling before the Phase 1 cutover. (Roadmap 1.10.)
|
||
|
||
### Deliberately deferred
|
||
|
||
- **Backward transition clears `DateApplied`.** Moving `Applied` → `Saved` clears the applied date. The alternative — a Saved job still carrying one — silently counts it as applied in analytics and exposes it to the ghosting rules. The original date stays recoverable from `StatusChanged` history. Documented in ADR-002; flag it if you disagree.
|
||
- **`Job` ships empty.** No backfill, by choice: nothing consumes it yet, and a later cutover may shape the data differently.
|
||
- **The inert CV Builder tab** (`careerView` prop, never read) is untouched — it is your uncommitted WIP. Roadmap 1.6.
|