cesnimda/jobtrackingapp
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

docs(ops): finalize release candidate status

Browse Source 
Restructured into READY / BLOCKED / MANUAL, with accepted limitations
kept separate.

B1 (backup selected the wrong provider and reported success) and N2
(/health always reported version: unknown) are both closed and verified;
their original findings are kept because the failure modes are worth
understanding. N1 closed with the .env.example additions.

One blocker remains and it is external: the CI runner. Stated with what
it needs from the owner, and with the decision it forces -- fix the
runner, or deploy deliberately from a locally verified commit knowing CI
is red.

MANUAL now leads with backup and restore readiness: the mechanism is
verified against containers, but only the owner can prove it works on
production data. Added a note that authenticated smoke testing is not an
automation gap that more work would close -- sign-in needs a password,
and no automated step here should handle one.

Validation only. No application behaviour changed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
cesnimda
2026-07-19 19:01:49 +02:00
parent 8f6f2ba8d6
commit de35947244
1 changed files with 90 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/release-candidate-review.md
+90 -24
View File
@@ -9,17 +9,25 @@
> `docs/infrastructure/database-ownership.md` (who creates which table),
> `docs/release-checklist.md` (state of the build).
**Updated 2026-07-19 (second pass).** **B1 is fixed and verified** — `deploy.sh` now loads the shared
environment before it decides anything, requires `DATABASE_PROVIDER` explicitly, validates the rest of
the deployment configuration before touching the stack, and verifies each backup against its own
format. Details under *Blocking*.
**Updated 2026-07-19 (final pass).** Two findings from the first pass are now fixed and verified —
**B1** (the pre-deploy backup silently backed up the wrong thing) and **N2** (`/health` always reported
`version: unknown`). Both are recorded under *Closed since the first pass*, with their original text
kept because the failure modes are worth understanding.
**Verdict: one blocking item remains — CI (B2), which is external.** The deployment path itself is now
sound.
**Verdict: one blocker remains, and it is external — CI (B2).** Nothing in the application or the
deployment path is now known to be blocking. Everything verified here is *local* verification; CI has
proven none of it.
| Section | Meaning |
|---|---|
| [READY](#ready--completed-technical-checks) | Verified. Nothing further needed before deploying |
| [BLOCKED](#blocked--requires-external-action) | Requires action outside this repository. Deployment is gated on it |
| [MANUAL](#manual--only-the-owner-can-verify) | Only the owner can confirm — sign-in, real data, production smoke tests |
| [Accepted](#accepted-for-the-first-release-non-blocking) | Known limitations, deliberately shipped as-is |
---
## Ready
## READY — completed technical checks
### Database
@@ -72,11 +80,20 @@ sound.
| Missing `AUTH_JWT_KEY` fails the backend | With `Auth__Require=true`, `Program.cs:241` throws `InvalidOperationException`. It does not silently generate an ephemeral key |
| Backup rejects a useless dump | `verify_backup` fails the deploy on an empty file, and on a MariaDB dump lacking `CREATE TABLE` |
| Backups never overwrite | UTC-timestamped filenames; retention is explicitly manual and the script says so |
| Test suite | 393 backend tests pass in Release, run for this review |
| **Deploy script loads its own environment** | `deploy.sh` parses the shared `.env` into its own shell before any decision. Values already in the environment (CI's `APP_VERSION`) still win. No value is echoed |
| **`DATABASE_PROVIDER` is required, not defaulted** | Missing or unrecognised aborts the deploy. Verified: exits 1, names the variable, leaves no backup file |
| **Configuration validated before the stack is touched** | Connection string, `AI_SERVICE_TOKEN` and `AUTH_JWT_KEY` are checked before the backup, and therefore before any build, stop or replace |
| **Backups verified per provider** | A dump needs valid gzip, `CREATE TABLE` and the `Dump completed` trailer; an archive needs `jobtracker.db`. Truncated and trailer-stripped dumps both rejected |
| **SQLite volume resolved by real name** | Project-prefixed, and fails if absent — the bare name would have created an empty volume and backed that up |
| **No secret leakage in deploy output** | Zero occurrences of any password, token or key across every failure-path test |
| **`/health` reports the deployed version** | Reads `App:Version`; verified end to end that `App__Version=9.9.9-test` surfaces as `9.9.9-test`, and that an unset value falls back to the assembly version |
| Test suite | 401 backend tests pass in Release, run for this review |
---
## Blocking
## Closed since the first pass
Both were found by this review, fixed in their own commits, and re-verified. Original findings kept.
### B1. ~~The pre-deploy database backup does not back up the database~~ — **CLOSED 2026-07-19**
@@ -162,26 +179,57 @@ is unset rather than skipping in silence), as does `OLLAMA_MODEL` for the warmup
| Dump with the trailer stripped | ✅ rejected as truncated |
| Secret leakage across every test's output | ✅ zero occurrences of any password, token or key |
### N2. ~~`/health` always reports `version: unknown` under Docker~~ — **CLOSED 2026-07-19**
*Original finding:* the endpoint read the `APP_VERSION` **environment variable**, but compose passes
`App__Version`, which binds to the `App:Version` **configuration key**. No variable by that name
existed in the container, so the version was always `unknown`.
**Closed.** `/health` now reads `App:Version` through `IConfiguration` — the approach
`AdminSystemController` already used for the same value. The resolution rule (configured version, else
assembly version) moved to a shared `BuildMetadata` helper rather than being written twice, so the
admin page and `/health` cannot drift apart.
*Verified against a running backend:* `App__Version=9.9.9-test` reports `9.9.9-test`; unset reports the
assembly version rather than `unknown`. Tests pin the configuration **key**, including that an
`App__Version` environment variable binds to `App:Version` — the original bug failed silently, so a
behavioural test alone would not have caught it.
---
## BLOCKED — requires external action
One item. Nothing in this repository can clear it.
### B2. CI is red — deployment is gated on it
Unchanged and still external to the repository. Commit `8f73548` changed one markdown file and its
test job failed in the same duration band as every other run; a change that cannot affect compilation
cannot fail a test job. The `deploy` job declares `needs: test`, so nothing promotes until this clears.
Everything in this review is therefore **local verification**. Blocked on job logs (a read-scoped
Gitea token), `journalctl -u act_runner`, and the runner container configuration. Evidence in
`docs/infrastructure/runner-investigation.md`.
Everything in this review is therefore **local verification**. Blocked on, and needing you:
- **Job logs** — the Gitea API returns 401 unauthenticated. A read-scoped token would unblock this
- **`journalctl -u act_runner --since '2 hours ago'`** on the runner host
- **The act_runner container/service configuration**
Evidence in `docs/infrastructure/runner-investigation.md`.
**The decision this forces.** Either fix the runner, or deploy deliberately from a locally verified
commit with CI knowingly red. The second is a defensible choice for a self-hosted single-node
deployment — but it should be a decision rather than a default, and it means the 401 backend tests and
128 frontend tests have only ever run on one machine.
---
## Non-blocking
## Accepted for the first release (non-blocking)
Known and accepted for the first release. None of these should stop a deploy; all are worth knowing.
Known limitations, deliberately shipped as-is. None should stop a deploy; all are worth knowing.
| # | Finding | Why it is not blocking |
|---|---|---|
| N1 | ~~**`.env.example` omits `DATABASE_PROVIDER` and `JOBTRACKER_CONNECTION_STRING`**~~ — **CLOSED 2026-07-19** | Both added to the template with the connection-string host caveat. `deploy.sh` now hard-fails without `DATABASE_PROVIDER`, so the template had to name it |
| N2 | **`/health` always reports `"version":"unknown"` under Docker** — the endpoint reads the `APP_VERSION` environment variable, but compose passes it as `App__Version` | Liveness is unaffected; only the version string is wrong. `deploy/first-production-deployment.md` shows a populated version in its expected output, which will not match reality |
| N2 | ~~**`/health` always reports `"version":"unknown"` under Docker**~~ — **CLOSED 2026-07-19** | Fixed; see *Closed since the first pass* above |
| N3 | **The post-deploy gate in `deploy.sh` checks `.State == running`, not health** — a container can be `running` while `starting` or `unhealthy` | Harmless in practice: `compose up` already blocks on `service_healthy` for the frontend's dependency, so an unhealthy backend aborts the deploy before this check is reached. The check is weaker than it looks, not wrong |
| N4 | **Table-count discrepancy across documents** — `database-ownership.md` records 40 tables on MariaDB; `release-checklist.md` and the runbook say ~42 | Both were measured, at different points in Phase 5. Use "the tables listed in `database-ownership.md` all exist" as the check, not a number |
| N5 | **`ai-service` opens completely if `AI_SERVICE_TOKEN` is the empty string** — the middleware is `if AI_SERVICE_TOKEN and …`, so a blank token disables the check rather than failing closed | Compose declares the variable with `:?` on both services, so the stack refuses to start without it. No defence in depth behind that, though |
@@ -193,24 +241,32 @@ Known and accepted for the first release. None of these should stop a deploy; al
---
## Manual verification required
## MANUAL — only the owner can verify
Only the owner can do these. Nothing in this review, and nothing in the test suite, covers any of
them — every automated check stops at the authentication boundary.
Nothing in this review, and nothing in the test suite, covers any of these — every automated check
stops at the authentication boundary, and none of it has touched production data.
### Backup and restore readiness
The automatic backup is fixed and verified against containers (B1). What remains unproven is the one
thing containers cannot prove: that it works **on your database**.
- [ ] **Take a MariaDB dump by hand from production and restore it into a scratch database.** Not a
formality. Container verification says the mechanism is sound; it says nothing about your data
volume, your disk space, or your MariaDB version's dump quirks.
- [ ] **Confirm `/opt/job-tracker/backups` exists and has room.** `deploy.sh` creates it, but a
full disk fails the backup and therefore the deploy.
- [ ] **Know which backup is the restore point** before you start. After the deploy, confirm the newest
file is `jobtracker-<database>-<stamp>.sql.gz` — a `jobtracker-sqlite-*.tar.gz` would mean the
environment is wrong, though the provider check should now stop that first.
### Before deploying
- [ ] **Take a MariaDB dump by hand and restore it into a scratch database.** B1 is fixed and the
automatic backup is verified against containers, but the first production deploy is still the
wrong moment to discover that a backup does not restore *on your data*.
- [ ] **Confirm `/opt/job-tracker/shared/.env` contains `DATABASE_PROVIDER=mariadb` and
`JOBTRACKER_CONNECTION_STRING`.** `deploy.sh` now aborts without them, so a missing value costs
an aborted deploy rather than a bad backup — but check first and skip the round trip.
- [ ] **Confirm the connection-string host resolves *from inside the backend container*** —
`127.0.0.1` there means the container, not the Docker host.
- [ ] **After the deploy, check the backup filename.** It must be
`jobtracker-<database>-<stamp>.sql.gz`. A `jobtracker-sqlite-<stamp>.tar.gz` means the
environment is wrong — though the provider check should now stop that before it happens.
- [ ] **Record the current commit** (`git rev-parse HEAD`) and the current row counts for
`JobApplications` and `Companies`. The row counts are the check that matters most afterwards.
@@ -228,5 +284,15 @@ them — every automated check stops at the authentication boundary.
- [ ] **An AI generation completes end to end** and writes nothing until you save it.
- [ ] **Email and calendar integrations still connect**, if you use them — the OAuth callbacks were
reviewed statically but never exercised against live Google or Microsoft.
- [ ] **`/health` reports the version you deployed**, not `1.0.0.0`. The assembly fallback means
`APP_VERSION` did not reach the container — harmless in itself, but build metadata is then
missing from the admin page too.
- [ ] **The full journey once, deliberately** — empty profile through to a recorded outcome. No one has
walked it. Friction in that path is currently unmapped.
### Why this section cannot shrink
Authenticated smoke testing is not an automation gap that more work would close. Signing in requires a
password, and no automated step in this repository should ever handle one. The 401 backend tests
verify that the authorization boundary *exists* and holds; only you can verify what is behind it, with
your data.