Files
jobtrackingapp/docs/phase-0-foundation-report.md
T

254 lines
19 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# 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 (~11.5 wk → ~35 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 12 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.