From b6f865590930b4cba75d85d1352503fdc5b0d6d2 Mon Sep 17 00:00:00 2001 From: Aravind Date: Tue, 25 Aug 2026 16:37:05 +0530 Subject: [PATCH] aravind changes --- README.md | 94 +- docs/KROW_BACKEND_COMPLETE_SUMMARY.md | 95 +- docs/api-contract.md | 119 ++ docs/backend-implementation-plan.md | 1566 +++++++++++++++++ go-api/internal/authctx/authctx.go | 10 +- go-api/internal/config/config.go | 5 + go-api/internal/httpserver/api.go | 32 +- go-api/internal/httpserver/auth.go | 59 +- go-api/internal/httpserver/cors.go | 6 + go-api/internal/httpserver/interviews_test.go | 239 +++ go-api/internal/httpserver/owliver.go | 61 + go-api/internal/httpserver/owliver_test.go | 315 ++++ go-api/internal/httpserver/rbac_test.go | 44 + go-api/internal/httpserver/server.go | 29 +- go-api/internal/httpserver/workflows.go | 5 + go-api/internal/httpserver/workflows_test.go | 323 ++++ go-api/internal/orgctx/orgctx.go | 19 +- go-api/internal/owliver/catalog.go | 625 +++++++ go-api/internal/owliver/suggest.go | 359 ++++ go-api/internal/owliver/suggest_test.go | 695 ++++++++ go-api/internal/service/interviews.go | 119 ++ go-api/internal/service/owliver.go | 103 ++ go-api/internal/service/service.go | 29 +- go-api/internal/service/service_test.go | 73 +- go-api/internal/service/workflows.go | 187 +- infrastructure/README.md | 5 +- scripts/oracle.mjs | 5 +- 27 files changed, 5058 insertions(+), 163 deletions(-) create mode 100644 docs/backend-implementation-plan.md create mode 100644 go-api/internal/httpserver/interviews_test.go create mode 100644 go-api/internal/httpserver/owliver.go create mode 100644 go-api/internal/httpserver/owliver_test.go create mode 100644 go-api/internal/owliver/catalog.go create mode 100644 go-api/internal/owliver/suggest.go create mode 100644 go-api/internal/owliver/suggest_test.go create mode 100644 go-api/internal/service/interviews.go create mode 100644 go-api/internal/service/owliver.go diff --git a/README.md b/README.md index db8c92c..7690ed8 100644 --- a/README.md +++ b/README.md @@ -4,13 +4,22 @@ The backend for Krow — a Go API over PostgreSQL, with a Python Owliver service to follow. The Krow frontend lives in a **separate repository** (`krow-demo`) and is not vendored, copied or modified here. -**Status: Phase 3C — session authentication.** The API serves the endpoints in -`docs/api-contract.md` against PostgreSQL, loaded with the frontend's own demo -dataset. Every endpoint except `GET /health` and the two `/api/v1/auth/*` routes -requires a session: sign in with a password, hold an HttpOnly cookie, and the -server resolves it to a real user on every request. Authorization — which roles -may do what — is Phase 3D and is not implemented. No Owliver, no RAG, no Redis, -NATS or S3. +**Status: authenticated, authorized, multi-tenant.** The API serves the +endpoints in `docs/api-contract.md` against PostgreSQL, loaded with the +frontend's own demo dataset. Every endpoint except `GET /health` and the two +`/api/v1/auth/*` routes requires a session: sign in with a password, hold an +HttpOnly cookie, and the server resolves it to a real user on every request. +Roles are enforced — a deny-by-default policy table answers 403 before any query +runs, and organization scope and talent ownership are SQL predicates, so a row +you may not see answers 404. On top of that sits the agent and skill definition +surface: Markdown with YAML frontmatter, parsed and stored, with full CRUD. + +Not built: no Owliver, no RAG, no Redis, no object storage. The agent/skill +runtime is an internal boundary with no executor behind it and no HTTP route +into it. + +`docs/KROW_BACKEND_COMPLETE_SUMMARY.md` is the long-form technical account — +architecture, history, and a frank list of the current limitations. ## Layout @@ -19,27 +28,39 @@ krow-backend/ ├── go-api/ │ ├── cmd/ │ │ ├── api/ the HTTP service entrypoint -│ │ └── seed/ loads the demo dataset +│ │ ├── seed/ loads the demo dataset +│ │ └── setpassword/ the only way a password enters the database │ ├── internal/ │ │ ├── config/ environment loading + validation │ │ ├── db/ pgx pool and the database health check -│ │ ├── domain/ resource descriptors (generated from the schema) +│ │ ├── domain/ resource descriptors (generated) + the policy table │ │ ├── repo/ pgx access layer — every statement built here │ │ ├── service/ validation, org scoping, contract semantics +│ │ ├── auth/ argon2id passwords, sessions, the user store +│ │ ├── authctx/ the authenticated identity a request carries │ │ ├── orgctx/ the organization a request runs as +│ │ ├── definition/ the agent/skill Markdown + YAML frontmatter parser +│ │ ├── runtime/ agent/skill load, validate, resolve, dispatch │ │ ├── httpserver/ router, handlers, /health, graceful shutdown │ │ ├── seeder/ fixture loading + the attendanceSeed.js port │ │ └── testutil/ disposable migrated + seeded test database │ └── go.mod ├── migrations/ SQL migrations — the source of truth for the schema ├── seed/fixtures/ seed.json, generated from the frontend repository -├── docs/api-contract.md the frozen client contract +├── docs/ +│ ├── api-contract.md the frozen client contract +│ └── KROW_BACKEND_COMPLETE_SUMMARY.md the long-form technical account ├── infrastructure/ deployment definitions (empty until later) -├── scripts/ verify_schema.sql, gen_resources.py +├── scripts/ verify_schema.sql, gen_resources.py, oracle.mjs ├── Makefile developer, migration and seed tasks └── .env.example ``` +`migrations/`, `seed/`, `scripts/` and `infrastructure/` sit beside `go-api/` +rather than inside it because none of them is Go: the migrations are run by the +`golang-migrate` CLI, the generator is Python, the parser oracle is Node, and a +second service (Owliver, Python) is expected to consume the same schema. + ## Architecture ``` @@ -82,10 +103,16 @@ make run # http://127.0.0.1:8080/health ## The API -38 endpoints, specified in `docs/api-contract.md`. Every one exists because a -frontend call site exists — a table in the database is never a reason for an -endpoint. `DELETE /job-postings/{id}` and `POST /shift-records` return 405 -because nothing in the frontend deletes a posting or creates a shift record. +54 registered routes: 34 entity routes across 14 resources, 10 agent and skill +definition routes, 4 `/me` routes, 2 `/auth` routes, 2 workflow routes, the +Owliver suggestion route and `GET /health`. The entity routes and the suggestion +route are specified in `docs/api-contract.md` (§2 and §2A); the definition and +workflow routes are not yet in the contract and are documented in the source and +its tests. Every +entity route exists because a frontend call site exists — a table in the +database is never a reason for an endpoint. `DELETE /job-postings/{id}` and +`POST /shift-records` return 405 because nothing in the frontend deletes a +posting or creates a shift record. ```bash curl localhost:8080/api/v1/job-postings @@ -110,8 +137,19 @@ Set a password before signing in: the seeded user's `password_hash` is NULL unti curl -b jar localhost:8080/api/v1/me curl -b jar -X POST localhost:8080/api/v1/auth/logout -Authorization is **not** implemented. `users.role` is carried on the identity and -consulted nowhere: any signed-in user reaches every endpoint. That is Phase 3D. +**Authorization is enforced.** `users.role` — `admin`, `employer` or `talent`, +never the self-editable `account_type` — is the only authority. Entity routes +are gated by the deny-by-default policy table in `internal/domain/policy.go`: +a resource with no policy permits nothing to anyone, and `Server.authorize` +answers 403 before any query runs. Row visibility is a SQL predicate rather than +a filter — the organization scope always, plus an ownership clause for talent +callers — so a row outside it is never fetched and answers 404, which does not +distinguish "exists but not yours" from "does not exist". + +The ten definition routes are the exception: they are not `domain.Resource` +values, so the descriptor machinery does not reach them and their role checks +are written inline in `internal/service/definitions.go`. Tenancy and ownership +*are* enforced for them, in the repository predicates. ## Seeding @@ -135,16 +173,30 @@ records created through the API survive a re-seed. make test # go test ./... ``` -55 tests. The database-backed ones build a disposable database — dropped, -recreated, migrated and seeded per run, named `krow_backend_autotest_` so -concurrent test packages cannot collide. They skip rather than fail when -PostgreSQL is unreachable. +175 test functions. The database-backed ones build a disposable database — +dropped, recreated, migrated and seeded per run, named +`krow_backend_autotest_` so concurrent test packages cannot collide. They +skip rather than fail when PostgreSQL is unreachable. Seed assertions compare the database against the fixture field-by-field rather than against numbers typed into a test. The two ordering guarantees (`NULLS LAST` in both directions, and the `, id` tiebreaker) are covered by tests verified to fail when the guarantee is removed. +**Parser conformance.** The Go definition parser must agree with the frontend's +JavaScript one. `internal/definition/conformance_test.go` asserts against +`internal/definition/testdata/oracle.json`, which is not written by hand: it is +captured by running the real frontend module graph through Vite, so the fixture +records what the JS parser actually does rather than what anyone believes it +does. Regenerating it needs a `krow-demo` checkout, and is not part of +`make test`: + +```bash +node scripts/oracle.mjs go-api/internal/definition/testdata/oracle.json +``` + +`scripts/cases.mjs` holds the adversarial corpus that file is captured over. + `GET /health` is public and says only whether traffic should be sent here: `{"status": "ok"}` with `200`, `{"status": "degraded"}` with `200` when the database is up but unmigrated or left dirty, and `{"status": "unavailable"}` diff --git a/docs/KROW_BACKEND_COMPLETE_SUMMARY.md b/docs/KROW_BACKEND_COMPLETE_SUMMARY.md index 2307d29..6290c5a 100644 --- a/docs/KROW_BACKEND_COMPLETE_SUMMARY.md +++ b/docs/KROW_BACKEND_COMPLETE_SUMMARY.md @@ -1840,7 +1840,7 @@ Recorded in `docs/api-contract.md` §12 and still accurate against the current c | **GCS-compatible object storage** | — | **Future direction.** No reference in the repository; `infrastructure/README.md` names MinIO as a "later" candidate | — | | **Docker deployment** | — | **Future direction.** `infrastructure/` contains only a README; `Dockerfile.api` and `Dockerfile.owliver` are listed there as later work | — | | **Redis** | Shared state for rate limiting across instances | **Future direction.** `ratelimit.go` names Redis or the database as the seam; nothing is wired | Needed before multi-instance deployment for the limiter to mean anything | -| **NATS is not part of the target architecture** | — | **Future direction / stated rule.** Note the discrepancy: `infrastructure/README.md` currently lists NATS as a later docker-compose candidate. See §17. | Remove NATS from that table if the rule stands | +| **NATS is not part of the target architecture** | — | **Future direction / stated rule.** `infrastructure/README.md` previously listed NATS as a later docker-compose candidate; that table has been corrected. See §17.17. | Keep it out | | **RAG / pgvector** | — | **Future direction.** `pgvector` is not installed in the local database and is named only once, in `infrastructure/README.md` | Introduce when the architecture calls for it | --- @@ -1850,35 +1850,31 @@ Recorded in `docs/api-contract.md` §12 and still accurate against the current c Everything below was checked against the current repository. Items the request asked about that could not be verified are marked as such rather than guessed at. -### 17.1 Documentation is behind the code +### 17.1 Documentation is behind the code — resolved -**Issue.** `README.md` describes the repository as being at an earlier stage than the -code is. It states "Authorization — which roles may do what — is Phase 3D and is not -implemented", "Authorization is **not** implemented. `users.role` is carried on the -identity and consulted nowhere: any signed-in user reaches every endpoint", "38 -endpoints" and "55 tests". +**Issue as recorded.** `README.md` described the repository as being at an earlier +stage than the code is. It stated "Authorization — which roles may do what — is Phase +3D and is not implemented", "Authorization is **not** implemented. `users.role` is +carried on the identity and consulted nowhere: any signed-in user reaches every +endpoint", "38 endpoints" and "55 tests". The same staleness appeared in two source +comments: the `internal/httpserver/server.go` package documentation ("Authorization is +NOT here"), and `internal/authctx/authctx.go` ("It is NOT consulted anywhere in Phase +3C: authentication only"). **Cause.** Authorization, the definition tables, the CRUD surface and the runtime all landed after the README was last revised. -**Impact.** A reader trusting the README would conclude that any signed-in user -reaches every endpoint, which is not what the code does. - **Current behaviour.** `internal/domain/policy.go`, `Server.authorize`, `builder.ownership` and `internal/httpserver/rbac_test.go` (730 lines) all exist and -run. 51 routes are registered. 175 test functions run. -`docs/api-contract.md` §9A *is* current and documents the authorization contract -accurately. +run. 51 routes are registered. 175 test functions run. `docs/api-contract.md` §9A +*is* current and documents the authorization contract accurately. -**Possible future handling.** Revise `README.md` against the code. - -The same staleness appears in two source comments: - -- `internal/httpserver/server.go` package documentation: "Authorization is NOT here. - A signed-in user reaches every endpoint they could reach before". -- `internal/authctx/authctx.go`: "It is NOT consulted anywhere in Phase 3C: - authentication only" — `Identity.Role` is now consulted by `Server.authorize`, - `builder.ownership`, `guardInsert` and `service/definitions.go`. +**Resolution.** `README.md` was revised against the code: the status paragraph, the +layout tree (which was missing `cmd/setpassword`, `internal/auth`, `internal/authctx`, +`internal/definition` and `internal/runtime`), the route count, the test count and the +authorization section. The `server.go` and `authctx.go` package comments were +corrected to describe the authorization that exists. `internal/orgctx/orgctx.go`, +which still framed itself as pre-authentication, was corrected in the same pass. ### 17.2 The definitions endpoints are not in the API contract @@ -2000,23 +1996,26 @@ the effective limit multiplies by instance count, resets on restart, and degrade a global budget behind a reverse proxy. **Possible future handling:** move the limiter behind shared state before deploying more than one instance. -### 17.10 The repository has no commits +### 17.10 The repository has almost no history -**Issue.** `git log` reports `fatal: your current branch 'main' does not have any -commits yet`. Every file is untracked. +**Issue as recorded.** `git log` reported `fatal: your current branch 'main' does not +have any commits yet`, and every file was untracked. -**Impact.** There is no history, no recovery point, no blame, and no record of when -any of the work described in §2 happened. The chronology in this document was -reconstructed from migration headers, package documentation and the contract, not -from version control. +**Current behaviour.** The tree is now committed: a single commit on `main` +(`7d12ebe`, "first commit") holds the whole repository. -**Note.** This document does not change that; no commit was made. +**Remaining impact.** One commit is not history. There is still no blame, no +incremental recovery point, and no record of when any of the work described in §2 +happened. The chronology in this document was reconstructed from migration headers, +package documentation and the contract, not from version control, and that remains +the only source for it. -### 17.11 A minor documentation defect in the source +### 17.11 A minor documentation defect in the source — resolved -`internal/httpserver/api.go` — the doc comment describing `decodeBody` sits -immediately above `decodeInto`, so `decodeInto` carries two doc comments and -`decodeBody` carries none. +`internal/httpserver/api.go` — the doc comment describing `decodeBody` sat +immediately above `decodeInto`, so `decodeInto` carried two doc comments and +`decodeBody` carried none. The `decodeBody` comment has been moved to sit above +`decodeBody`. ### 17.12 Badge endpoint mismatch — verified @@ -2091,8 +2090,9 @@ among the things that do not exist yet. **Impact.** A reader of `infrastructure/README.md` would take NATS to be planned. -**Possible future handling.** Amend that table if the rule stands. Nothing was changed -here. +**Resolution.** The rule stands, so the table was amended: `infrastructure/README.md` +no longer lists NATS as a candidate and says explicitly that it is not part of the +target architecture. `README.md` no longer names it either. ### 17.18 Deliberate contract behaviours that read as defects @@ -2169,8 +2169,8 @@ deliberately and should be done knowingly. ### NATS -**NATS is not part of the target architecture.** Note that `infrastructure/README.md` -currently lists it as a later candidate; see §17.17. +**NATS is not part of the target architecture.** `infrastructure/README.md` no longer +lists it as a candidate; see §17.17. ### Nearest-term work implied by the code itself @@ -2281,9 +2281,9 @@ disposable `krow_backend_autotest_` database per test process. **Current Known Limitations:** no AI execution and no public runtime endpoint; login rate limiting is per-process and in-memory; CORS does not permit credentials; definitions endpoints bypass the policy table; the runtime package is unreachable -from the running service; `README.md` and two source comments are behind the code; -the definitions endpoints are absent from the API contract; attendance data is empty -without a rostering source; the repository has no commits. +from the running service; the definitions endpoints are absent from the API contract; +attendance data is empty without a rostering source; the repository has a single +commit and so no usable history. **Current Development Focus:** the most recent work in the tree is the definition system — schema, parser conformance, CRUD — and the runtime boundary that sits on top @@ -2392,13 +2392,12 @@ is how this codebase would stop being trustworthy. - **The main current limitations** are: no AI execution and no runtime endpoint; login rate limiting that does not survive scale-out; CORS that does not yet permit credentials; ten definition endpoints that sit outside the policy table and outside - the written contract; documentation that is behind the code; and a repository with - no commits. + the written contract; and a repository with a single commit and so no usable + history. - **The direction** is real execution behind the existing executor interfaces (Owliver / an LLM provider abstraction), then retrieval, then tooling, then deployment infrastructure — none of which exists here today. **NATS is not part of - the target architecture**, and the one place in the repository that still names it - should be corrected. + the target architecture**, and the documentation no longer implies otherwise. - **The rules that must not be broken** are in §21. The two that matter most in daily work: never trust a client-supplied identity or tenant, and never rewrite stored Markdown. @@ -2409,3 +2408,9 @@ is how this codebase would stop being trustworthy. behaviour above was read from the current checkout or from read-only queries against the local development database. Nothing in the repository was modified to produce this document.* + +*Revised on 2026-08-24 by a structure-and-dead-code cleanup pass, which changed no +schema, no migration, no database row and no runtime behaviour. What it did change is +recorded in §17.1, §17.10, §17.11 and §17.17: documentation and source comments that +had fallen behind the code were corrected. The counts above were re-verified against +the checkout and are unchanged.* diff --git a/docs/api-contract.md b/docs/api-contract.md index c2cab4b..3afd3f7 100644 --- a/docs/api-contract.md +++ b/docs/api-contract.md @@ -129,6 +129,125 @@ these would leave the shim with methods that 404. See §11 (D6). --- +## 2A. Owliver suggestions + +`GET /api/v1/owliver/suggestions?page={surface}&query={typed}` + +The one endpoint here that serves no resource. It answers "what could I usefully +ask on this page?" for the Owliver panel, which calls it while the user types — +so it reads no table, opens no transaction, calls no model, and its whole answer +is computed from a static catalogue in `internal/owliver`. + +It is not on the public allowlist. Which readings exist depends on the caller's +role, so there is no anonymous answer to give. + +### Request + +| Parameter | Required | Meaning | +| --- | --- | --- | +| `page` | yes | A surface id from the closed page vocabulary — the same one `internal/definition` validates a definition's `pages:` against. Aliases (`hired`, `forge`, `new-position`) and loose spellings (`Talent Pool`) resolve to the canonical id. | +| `query` | no | What the user has typed so far. | + +Any other parameter is `invalid_query`. **Nothing about the caller is accepted +here** — role, organization and user are read from the session, and a request +that names one is refused rather than ignored. + +An unknown `page` is `invalid_query`, with the frontend's own wording: +`Unsupported page: {value}. Supported pages: {…}.` An absent, blank or +too-short `query` is **not** an error — there is simply nothing to rank yet. + +### Response + +```json +{ + "data": { + "suggestions": [ + { "text": "Which position has the strongest pipeline?", "intent": "position-strength" }, + { "text": "Show hiring activity as a flow", "intent": "hiring-operations", "capability": "flow" } + ] + } +} +``` + +`data.suggestions` is always an array — `[]` when nothing matches, never `null` +and never an error. There is no `meta`: the list is capped rather than paged. + +| Field | Meaning | +| --- | --- | +| `text` | The question, as the user reads it. | +| `intent` | The **frontend capability id** the panel dispatches on, verbatim from the manifests in `src/components/ai-assistant/capabilities/`. Not a backend identifier, and never invented here. | +| `capability` | The section type to draw the answer as, from the closed `OWLIVER_CAPABILITIES` vocabulary. Present only when the query asked for one ("as a flow", "summarize"); absent otherwise. | + +Nothing internal is exposed: no matching terms, no resource names, no scores, no +policy detail. `text` is always catalogue wording — no part of the query is +echoed back into a suggestion. + +### Selection + +Five stages, each of which only ever removes: + +``` +the page's catalogue → permission → relevance → deduplicate → top 3 +``` + +- **Page.** Intents are keyed by surface, so a suggestion from another page + cannot appear. The same word answers differently per page by construction: + `pipeline` on `positions` is about which role converts, on `candidates` it is + the funnel the applicants are in. +- **Permission.** Each intent declares the resources it reads, and those are + checked against the policy table in §9A — *before* ranking, so a refused + reading is never scored. Two gates, not one: the role must be allowed the + operation, and for an organization-wide reading the role's rows must not be + narrowed. Talent may list job applications; talent may not be offered "which + position has the strongest pipeline?", because their view of that resource is + their own rows. No role list is written down here — see §9A.1. +- **Relevance.** Deterministic keyword ranking over the intent's own terms. + Exact token, then multi-word phrase, then prefix (so a half-typed word still + matches), then extension. Ties break on catalogue order, so the same request + always answers identically. A query naming only a section type ranks the + page's readings; once it names a subject, readings that merely *support* that + shape are dropped rather than used as padding. +- **Deduplicate.** One suggestion per intent id, and no two with the same text. +- **Cap.** Three. Nothing is added to reach three. + +A query with fewer than two letters or digits after normalization returns `[]`. + +### Normalization + +The query is truncated to 200 characters, lower-cased, and reduced to letters, +digits and single spaces — every other character becomes a space rather than +being stripped, so nothing can be glued into a token that was not typed as one. + +There is no injection surface to defend: the normalized text is compared against +a fixed table of literals and never reaches SQL, a template, a shell or a log +message. Hostile input is ranked like any other text, and can only ever produce +entries the catalogue already holds. + +### Where the catalogue comes from + +The frontend owns the vocabulary. Owliver's capabilities are declared per page +context in `src/components/ai-assistant/capabilities/`; `internal/owliver` +transcribes the id and the page, and adds the two things a manifest does not +carry — the words that mean a user is reaching for that reading, and the records +it reads. Same pattern as `internal/definition/vocabulary.go`, and for the same +reason: one vocabulary, named from both ends, with a test on this side that +fails when an id here names no capability there. + +**No table, no migration.** The suggestions are derived from definitions that +already exist. Persistence would only be warranted if suggestions became +admin-managed, and nothing in the product asks for that today. + +### Not covered + +The seven workspace and configuration surfaces — `settings`, `workspace`, +`workspace-agents`, `workspace-skills`, `workspace-skill-configure`, +`skill-development`, `workspace-agent-configure` — are valid pages with no +entries. Their panel answers from the registries rather than from workforce +records, so there is nothing to rank a typed query against. They return `[]`, +which is the honest answer, not a validation error. + +--- + ## 3. Request schemas ### 3.1 Create — `POST /{resource}` diff --git a/docs/backend-implementation-plan.md b/docs/backend-implementation-plan.md new file mode 100644 index 0000000..4ca76ea --- /dev/null +++ b/docs/backend-implementation-plan.md @@ -0,0 +1,1566 @@ +# Krow backend — implementation plan + +**Status:** blueprint. No code has been changed to produce it. +**Backend audited:** `/Users/apple/Krow/krow-backend` @ `cadea4b` (branch `main`). +**Frontend audited:** `/Users/apple/Krow/krow-demo`, normative inventory at +`krow-demo/docs/api-audit.md`. + +--- + +## 0. How to read this document + +Every row is traced. A claim about the server names a Go file and, where it +matters, a line; a claim about the browser names a file in `krow-demo/src`; a +claim about the schema names a migration. Nothing here is inferred from a +README — where a README and the code disagree, the code won, and the +disagreement is recorded in §13. + +Four labels are used throughout, and they are the whole of the taxonomy: + +| Label | Meaning | +| --- | --- | +| **EXISTS** | Registered and working on the server, and the frontend calls it. Nothing to do. | +| **WIRE** | Registered and working on the server. The frontend does not call it. **Frontend-only work; zero server changes.** | +| **CHANGE** | The endpoint exists but its behaviour is wrong, incomplete, or disagrees with the frontend. **Server change to existing code.** | +| **NEW** | No server capability exists at all. Proposed only where §10 justifies it. | + +Two rules were held to while writing this: + +- **No route is invented.** Every recommendation either reuses a registered + route, changes the behaviour behind one, or is listed in §10 with an explicit + justification, request/response shape, service, repository, DB and migration + impact. +- **No duplicate API is proposed for functionality that already has one.** Where + a need could be met either by a new route or by changing an existing handler, + the plan takes the existing handler. §5.2 (interview completion) is the + clearest case: it is solved by extending `POST /api/v1/ai-interviews`, not by + adding an endpoint. + +--- + +## 1. Every registered backend endpoint + +### 1.1 The exact route count, derived from source + +`Server.New` registers `GET /health` first and then sums five helpers +(`go-api/internal/httpserver/server.go` (`Server.New`, lines 146-148)): + +```go +mux.HandleFunc("GET /health", s.handleHealth) +s.endpoints = s.routeAuth(mux) + s.routeResources(mux) + s.routeMe(mux) + + s.routeDefinitions(mux) + s.routeWorkflows(mux) +``` + +| Group | Routes | Counted from | +| --- | ---: | --- | +| Health | 1 | `internal/httpserver/server.go:146` | +| Auth | 2 | `internal/httpserver/auth.go:129-130` (`return 2`) | +| Entity CRUD | 34 | `Ops` bits over 14 resources, `internal/domain/resources_gen.go` | +| `/me` | 4 | `internal/httpserver/me.go:83-86` (`return 4`) | +| Agent + skill definitions | 10 | `internal/httpserver/definitions.go:11-21` (`return 10`) | +| Workflows | 2 | `internal/httpserver/workflows.go:87-88` (`return 2`) | +| **Total registered** | **53** | | +| `Server.Endpoints()` returns | **52** | health is registered before the sum | + +The 34 entity routes, by `Ops` bit: + +| Resource | List | Get | Create | Update | Delete | = | +| --- | :-: | :-: | :-: | :-: | :-: | ---: | +| JobPosting | ✓ | ✓ | ✓ | ✓ | | 4 | +| JobApplication | ✓ | | ✓ | ✓ | ✓ | 4 | +| AIInterview | ✓ | | ✓ | | | 2 | +| Staff | ✓ | | ✓ | ✓ | | 3 | +| WorkerProfile | ✓ | | ✓ | ✓ | | 3 | +| Course | ✓ | ✓ | ✓ | ✓ | | 4 | +| LearningPath | ✓ | | | | | 1 | +| RoleCategory | ✓ | | ✓ | | | 2 | +| Certification | ✓ | | ✓ | | ✓ | 3 | +| UserActivity | ✓ | | ✓ | | | 2 | +| Evidence | ✓ | | ✓ | ✓ | | 3 | +| Assignment | ✓ | | ✓ | | | 2 | +| ShiftRecord | ✓ | | | | | 1 | +| Badge | | | | | | **0** | +| | | | | | | **34** | + +`Badge` carries `Ops: 0` and an empty policy on purpose +(`resources_gen.go` Badge block; `internal/domain/policy.go` `"badges": {}`). +The descriptor exists so the seeder can populate the table. + +### 1.2 Transport contract, common to every route + +- **Envelope in:** a bare JSON object. `decodeBody` + (`internal/httpserver/api.go` `decodeBody` (:203)) caps the body at `maxBodyBytes = 4 MiB` + (`api.go:15`) and reads it into an open `domain.Record`. `decodeInto` + (`api.go` `decodeInto` (:183)) is the closed-struct variant used by login and assign. +- **Envelope out, single record:** `{"data": {...}}` — `writeRecord`, + `internal/httpserver/response.go:48`. +- **Envelope out, collection:** `{"data": [...], "meta": {total, limit, offset, + returned, truncated}}` — `writePage`, `response.go` `writePage` (:52). +- **Errors:** `{"error": {code, message, details}}` — `writeError`, + `response.go` `writeError` (:91). Status mapping in `statusFor`, `response.go` `statusFor` (:66): + `unauthorized`→401, `forbidden`→403, `rate_limited`→429, `not_found`→404, + `validation_failed`→422, `invalid_query`→400, `conflict`→409, else 500. +- **404/405 from the mux** are rewritten into the same envelope by `jsonErrors` + (`server.go` `jsonErrors`). +- **Auth:** every path outside `publicPaths` (`auth.go` `publicPaths` (:289-293): `/health`, + `/api/v1/auth/login`, `/api/v1/auth/logout`) requires the `krow_session` + cookie. `authenticate` (`auth.go` `authenticate`) resolves it, re-reads the user row + on **every** request, revokes on inactive, and puts `authctx.Identity` + + `orgctx` on the context. +- **Tenancy:** `org_id` is taken from the session user's row, never the request + (`auth.go` `authenticate`, `internal/repo/repo.go` `Repo.Insert` (:367)). +- **Authorization:** `Server.authorize` (`api.go` `Server.authorize` (:64)) reads `users.role`, + consults `domain.Policy.Allows`, and answers **403 before any query runs**. + Row visibility is a SQL predicate in `internal/repo`, so an invisible row + answers **404**. + +### 1.3 Health + +| Method | Path | Auth | Role | Request | Response | Tables | +| --- | --- | --- | --- | --- | --- | --- | +| GET | `/health` | **public** | — | — | `{"status": "ok"\|"degraded"\|"unavailable"}` (200/200/503) | `db.Check` reads `schema_migrations`, `information_schema` | + +Handler `server.go` `handleHealth`. The body is deliberately one field; all diagnostic +detail goes to the log (`logHealth`, `server.go` `logHealth`). + +### 1.4 Auth (2) + +| Method | Path | Auth | Role | Request | Response | +| --- | --- | --- | --- | --- | --- | +| POST | `/api/v1/auth/login` | public | — | `{email, password, remember_me}` (`loginRequest`, `auth.go` `loginRequest`) | 200 `{data: }` + `Set-Cookie: krow_session` | +| POST | `/api/v1/auth/logout` | public | — | — | 200 `{data:{status:"signed_out"}}` + cookie cleared | + +- Login order (`auth.go` `handleLogin`): shape validation → **two** rate limiters + (per-email and per-address, `internal/httpserver/ratelimit.go`) → constant-time + credential verify (`internal/auth/credentials.go`) → session issue → + `MarkLoggedIn` best-effort. +- Every credential failure returns the identical 401 (`domain.Unauthenticated()`). +- The token is **never** in the body — `auth.go` `handleLogin` states this explicitly. +- Cookie attributes: `HttpOnly`, `Path=/`, `SameSite` from + `sessionSameSite()` (`auth.go` `sessionSameSite` — `None` when a CORS allowlist is + configured, else `Lax`), `Secure` when not development or when cross-site. +- Tables: `users`, `sessions` (migration `000004`), `user_preferences`. + +### 1.5 Current user (4) + +| Method | Path | Auth | Role | Request | Response | +| --- | --- | --- | --- | --- | --- | +| GET | `/api/v1/me` | session | any | — | `{data: user}` with `preferences` embedded | +| PATCH | `/api/v1/me` | session | any | `{full_name?, account_type?}` | `{data: user}` | +| GET | `/api/v1/me/preferences` | session | any | — | `{data: preferences}` | +| PATCH | `/api/v1/me/preferences` | session | any | any JSON object | `{data: merged preferences}` | + +- Projection: `userColumns`, `me.go` `userColumns` (:33) — `id, legacy_id, full_name, email, + role, account_type, status, created_date, updated_date`, plus `preferences`. +- `PATCH /me` may write **only** `full_name` and `account_type` + (`updatableUserFields`, `me.go` `updatableUserFields` (:54)). `role` was deliberately removed — + the comment at `me.go` `updatableUserFields` (:54) records that leaving it in was a live + privilege-escalation path. Attempts to write anything in + `serverOwnedUserFields` (`me.go` `serverOwnedUserFields` (:69)) are ignored **and logged**. +- Preferences are split: `owliverDefault`, `compactDensity`, `emailDigest` are + columns (`preferenceColumns`, `me.go` `preferenceColumns` (:26)); **everything else lands in + `user_preferences.extra` jsonb** — which is where `customSkills`, + `customAgents`, `disabledSkills` and `removedSkills` live today. +- `PATCH /me/preferences` is a shallow merge with an upsert + (`me.go` `handlePreferencesPatch`). Booleans are type-checked; every other key is stored as-is, + **with no size bound and no schema**. +- Tables: `users`, `user_preferences`. + +### 1.6 Entity CRUD (34) + +All 34 share four handlers — `handleList`, `handleGet`, `handleCreate`, +`handleUpdate`, `handleDelete` (`internal/httpserver/api.go` `handleList`/`handleGet`/`handleCreate`/`handleUpdate`/`handleDelete`) — over one +generic service (`internal/service/service.go`) and one generic repository +(`internal/repo/repo.go`), driven by the descriptors in +`internal/domain/resources_gen.go`. + +**Request shape, create/update:** an open JSON object of column names. +Validation is `Service.validate` (`service.go` `Service.validate` (:204)): + +- an unknown field is **rejected** (`"unknown field"` → 422), not ignored; +- a `ReadOnly` column in the body is **ignored**, not rejected; +- `null` on a `NotNull` column is rejected; +- an enum value outside `Column.Enum` is rejected with the permitted list; +- on create, every `Required` column must be present and non-blank **unless + `serverSupplies` says the session fills it** (`service.go` `serverSupplies` (:266)). + +**Response shape:** the full stored row, every column projected through +`Column.SelectExpr` (`internal/domain/resource.go` `Column.SelectExpr` (:115)) — uuid→text, +numeric→float8, date→`YYYY-MM-DD`, timestamptz→`YYYY-MM-DDTHH:MM:SS.mmmZ`, +citext→text, bigint→text. + +**List parameters** (`Service.ParseList`, `service.go` `ParseList` (:55)): `sort` +(prefix `-` for DESC; explicit empty `?sort=` means no ordering), `limit` +(capped at `MaxLimit = 1000`, `service.go:28`), `offset`, and every other +parameter is a field filter. Array and JSON columns are **not** filterable +(`Resource.Filterable`, `resource.go` `Resource.Filterable` (:92)). Repeated params become +`col = ANY(...)`. + +**Delete** always answers 200 with `{id}` whether or not a row matched +(`service.go` `Service.Delete` (:185)) — deliberate, matching the old client store. + +Per-resource authorization, row scope and derived columns, all from +`internal/domain/policy.go`: + +| Path | List | Get | Create | Update | Delete | Talent row scope | Server-derived on insert | +| --- | --- | --- | --- | --- | --- | --- | --- | +| `job-postings` | all | all | operators | operators | — | `ScopeActivePostings` on `status` | `created_by` ← user id | +| `job-applications` | all | — | all | operators | operators | `ScopeEmail` on `email` | `email` ← session email (**talent only**) | +| `ai-interviews` | all | — | all | — | — | `ScopeOwnApplications` via `application_id` | — (insert guarded, `repo.go:332`) | +| `staff` | operators | — | operators | operators | — | n/a | — | +| `worker-profiles` | all | — | all | all | — | `ScopeUserID` on `user_id` | `user_id` ← user id (**talent only**) | +| `assignments` | all | — | operators | — | — | `ScopeEmail` on `worker_email` | — | +| `shift-records` | all | — | — | — | — | `ScopeEmail` on `worker_email` | — | +| `courses` | all | all | **admin** | **admin** | — | none | — | +| `learning-paths` | all | — | — | — | — | none | — | +| `role-categories` | all | — | operators | — | — | none | — | +| `certifications` | all | — | operators | — | **admin** | none | — | +| `user-activity` | all | — | all | — | — | `ScopeEmail` on `user_email` | `user_id`, `user_email`, `user_name`, `account_type` | +| `evidence` | all | — | operators… | operators | — | `ScopeEmail` on `worker_email` | `worker_email` ← session email (**talent only**) | +| `badges` | — | — | — | — | — | — | — | + +(`evidence` Create is `everyone`; Update is `operators`.) +`all` = `{admin, employer, talent}`; `operators` = `{admin, employer}`. + +Backing tables are one-to-one with `Resource.Table` in `resources_gen.go`; all +are created in `migrations/000001_initial_schema.up.sql`, except +`job_applications.interview_id` (`000002`) and the dropped +`job_applications_screened_consistent` CHECK (`000003`). + +### 1.7 Agent and skill definitions (10) + +| Method | Path | Auth | Role | +| --- | --- | --- | --- | +| GET | `/api/v1/agent-definitions` | session | any (rows scoped) | +| POST | `/api/v1/agent-definitions` | session | any; `visibility: organization` needs non-talent | +| GET | `/api/v1/agent-definitions/{id}` | session | any (rows scoped) | +| PATCH | `/api/v1/agent-definitions/{id}` | session | any own; organization rows need non-talent | +| DELETE | `/api/v1/agent-definitions/{id}` | session | same as PATCH | +| GET | `/api/v1/skill-definitions` | session | any (rows scoped) | +| POST | `/api/v1/skill-definitions` | session | any; `visibility: organization` needs non-talent | +| GET | `/api/v1/skill-definitions/{id}` | session | any (rows scoped) | +| PATCH | `/api/v1/skill-definitions/{id}` | session | any own; organization rows need non-talent | +| DELETE | `/api/v1/skill-definitions/{id}` | session | same as PATCH | + +Handlers `internal/httpserver/definitions.go`; service +`internal/service/definitions.go`; repository `internal/repo/definitions.go`; +parser/validator `internal/definition/`; schema +`migrations/000005_agent_skill_definitions.up.sql`. + +**These are NOT `domain.Resource` values.** They are not in the policy table; +their role checks are written inline in `internal/service/definitions.go` +(`server.go` package doc states this explicitly). + +- **Create request:** `{"markdown": "", "visibility": + "personal"|"organization"}`. `visibility` defaults to `personal` + (`definitions.go` `CreateAgent` (:138)). **No projection field is accepted from the body** — + `name`, `description`, `status`, `version`, `pages`, `definition_id` are all + parsed out of the markdown (`definitions.go` `CreateAgent` (:138)). +- **Update request:** `{"markdown": ...}` **or** `{"status": ...}`. Note the + `else if` at `definitions.go:241` / `definitions.go:415`: **if `markdown` is + present, a sibling `status` in the same PATCH is silently ignored.** + `visibility` is immutable after creation (`definitions.go` `UpdateAgent` (:194)). +- **Delete:** idempotent — a non-UUID or an invisible row returns + `{"id": }` and 200 (`definitions.go` `DeleteAgent` (:253)). +- **Response:** all columns — `id, definition_id, org_id, visibility, + owner_user_id, created_by, markdown, status, version (agents only), name, + description, pages, created_date, updated_date` + (`repo/definitions.go` `agentDefinitionColumns` (:77) / `skillDefinitionColumns` (:92)). +- **List parameters:** `visibility`, `status`, `definition_id`, `sort`, `limit`, + `offset` — and nothing else; an unknown parameter is a 400 + (`definitions.go` `allowedDefinitionFilters` (:17), `:44-49`). Sortable columns: `created_date`, + `updated_date`, `name`, `definition_id`, `status`, and `version` for agents. + Default `-created_date`, limit 100. +- **Row visibility predicate**, applied on every read, update and delete + (`repo/definitions.go` `GetAgent` (:182) and siblings): + `org_id = :org AND (visibility='organization' OR (visibility='personal' AND owner_user_id = :user))`. +- **Validation:** `definition.ValidateAgent` (`internal/definition/agent.go` `ValidateAgent` (:489)) + and `definition.ValidateSkill` (`internal/definition/skill.go` `ValidateSkill` (:279)). + +### 1.8 Workflows (2) + +| Method | Path | Auth | Required operations (all-or-nothing) | +| --- | --- | --- | --- | +| POST | `/api/v1/job-applications/{id}/hire` | session | `job-applications:Update` + `staff:Create` + `user-activity:Create` | +| POST | `/api/v1/job-postings/{id}/assignments` | session | `assignments:Create` + `job-applications:Update` + `user-activity:Create` | + +Every pair resolves to `operators` today. Authorization is `authorizeAll` +(`internal/httpserver/workflows.go` `authorizeAll` (:41)) — checked **before** any `BEGIN`. + +**Hire** (`internal/service/workflows.go` `WorkflowService.Hire` (:123)) + +- Request: optional `{phone?, role?, hire_date?, profile_tier?, status?, + reviewer_name?}`. Anything else is ignored; every other staff field is carried + from the application. +- Response `201 {"data": {"application": {...}, "staff": {...}}}`. +- Behaviour: reads the application **inside** the transaction; a second hire is + a **409** (`workflows.go` `Hire`, the already-hired branch); missing name or email is a 422; writes staff, + patches the application to `hired`, and writes a `user_activity` row — all in + one transaction, all through `repo.New(res, tx)`. +- Tables: `job_applications`, `staff`, `user_activity`. + +**Assign** (`internal/service/workflows.go` `WorkflowService.Assign` (:261)) + +- Request: `{"workers": [{worker_email, worker_name, starts_at, ends_at?, + application_id?, worker_profile_id?, match_score?, source?}]}` — the closed + struct `service.AssignRequest` (`workflows.go` `AssignWorker`/`AssignRequest` (:233/:245)). +- Batch capped at `maxAssignmentBatch = 200` (`workflows.go:47`); the whole batch + is validated before `BEGIN`. +- Response `201 {"data": {"assignments": [...], "count": n}}`. +- Behaviour per worker: insert `assignments`; **if and only if + `application_id` was supplied**, patch that application to `assigned`; insert a + `user_activity` row. Errors are annotated with the batch index + (`workflows.go` `annotate` (:433)). +- Tables: `job_postings` (read), `assignments`, `job_applications`, + `user_activity`. + +--- + +## 2. Frontend → existing backend mapping + +Traced from `krow-demo/docs/api-audit.md` §3–§9 against the router. + +### 2.1 Already wired — EXISTS (39 routes exercised) + +| Frontend call site | Endpoint | Note | +| --- | --- | --- | +| `krowHooks.js:65,72` `useJobPostings/useJobPosting` | `GET /job-postings`, `GET /job-postings/{id}` | | +| `KrowAssistant.jsx:366`, `CreatePosition.jsx:33-34` | `POST /job-postings`, `PATCH /job-postings/{id}` | | +| `krowHooks.js:80` `useApplications` | `GET /job-applications` (± `job_posting_id`) | | +| `krowHooks.js:202,213` | `POST`/`PATCH /job-applications/{id}` | | +| Candidate removal paths | `DELETE /job-applications/{id}` | | +| `krowHooks.js:90`, `AIInterviewModal.jsx:159` | `GET`/`POST /ai-interviews` | | +| `krowHooks.js:112`, hire path | `GET`/`POST`/`PATCH /staff` | | +| `krowHooks.js:583,589` | `GET`/`POST`/`PATCH /worker-profiles` | | +| `krowHooks.js:345,349,362,371` | `GET`/`POST`/`PATCH /courses`, `GET /courses/{id}` | | +| `krowHooks.js:533` | `GET /learning-paths` | | +| `CreatePosition.jsx:38-39` | `GET`/`POST /role-categories` | | +| `CertificationManager.jsx:10-11` | `GET`/`POST`/`DELETE /certifications` | | +| `userTracking.js:45`, `UserTracking.jsx:10` | `GET`/`POST /user-activity` | | +| `krowHooks.js:629,639` | `GET`/`POST`/`PATCH /evidence` | | +| `krowHooks.js:388`, `useAssignWorkers` | `GET`/`POST /assignments` | | +| `krowHooks.js:105` | `GET /shift-records` | | +| `base44Client.js` the shared hydration promise (:130) + 11 call sites | `GET /me` | one shared hydration promise | +| `auth.updateMe`, `Profile.jsx` | `PATCH /me` | | +| `auth.updatePreferences`, `useUpdatePreferences` | `PATCH /me/preferences` | | +| `base44Client.js` `auth.login/logout` | `POST /auth/login`, `POST /auth/logout` | | + +### 2.2 Backend exists, frontend does not use it — WIRE (13 routes) + +| Endpoint | What the frontend does instead | Section | +| --- | --- | --- | +| `POST /job-applications/{id}/hire` | `useHireCandidate` — two un-transactional calls, `krowHooks.js` `useHireCandidate` (:296) | §3.1 | +| `POST /job-postings/{id}/assignments` | `useAssignWorkers` — 3n calls in a loop, `krowHooks.js` `useAssignWorkers` (:406) | §3.2 | +| `GET`/`POST`/`GET {id}`/`PATCH {id}`/`DELETE {id}` `/agent-definitions` (5) | `preferences.customAgents`, `lib/agents/useAgents.js` `stored` (:29), `save` (:62), `remove` (:75) | §3.3 | +| `GET`/`POST`/`GET {id}`/`PATCH {id}`/`DELETE {id}` `/skill-definitions` (5) | `preferences.customSkills`, 104 references across 23 files | §3.4 | +| `GET /me/preferences` | reads preferences off `GET /me` + the `krow_demo_user` mirror | **not a gap** — `auth.preferences()` must answer synchronously (`api-audit.md` §5.3) | + +Also unused but **not** a gap: `GET /health` (registered and proxied by +`vite.config.js`; no frontend code calls it). + +### 2.3 Frontend dummy / localStorage / client-only + +| Thing | Where | Server today | Verdict | +| --- | --- | --- | --- | +| `integrations.Core.InvokeLLM` | `src/api/aiEngine.js`; 10 consumers in `krowAi.js` + `provingGround.js` | none | **NEW** — §10.1 | +| `integrations.Core.UploadFile` → `blob:` URL | `aiEngine.js`; `KrowIdentity.jsx`, `MediaChallenge.jsx`, `VideoRecorder.jsx` | none | **NEW** — §10.2 | +| Assistant stream | `components/ai-assistant/provider.js:205`; `createLocalProvider()` runs because `VITE_ASSISTANT_ENDPOINT` is unset | none | **NEW** — §10.3 | +| Web search | agent `webSearch:` flag; the UI states plainly that no provider is configured | none | **NEW** — §10.4 | +| `analytics.track` | `base44Client.js` `analytics.track` (:287), DEV-only `console.debug` | none | **not a gap** — real audit logging is `POST /user-activity` | +| `customSkills`, `customAgents` | `user_preferences.extra` via `PATCH /me/preferences` | tables exist and are unused | **WIRE** — §3.3/§3.4 | +| `disabledSkills`, `removedSkills` | `user_preferences.extra` | correct location | **not a gap** — migration `000005` header says so verbatim | +| `krow_demo_user`, `krow_assistant:history`, `krow_assistant_open/expanded/width`, `krow_assistant:agent`, per-context thread keys | localStorage / sessionStorage | none | **not a gap** — UI state; `api-audit.md` §9 | +| `ROLE_CATEGORIES` (7 strings) | `src/lib/roleCategories.js:9` | `GET /role-categories` returns the org's own | **not dummy data bypassing the API** — the static list is merged *on top of* the fetched list at both call sites; see §4.2 | +| `src/api/seed.js` (1,928 lines) | retained for `DEMO_USER.preferences` only | entity data is in PostgreSQL | dead but harmless | +| `src/api/store.js` (173 lines) | imported by nothing | — | dead | +| `useBadges` (`krowHooks.js:537`) | zero consumers; `Ops: 0` server-side | — | dead; would 404 | +| `User: 'users'` in `RESOURCE_PATHS` (`httpClient.js:96`) | no call site; no backend resource | — | dead | +| `meta.truncated` | `httpClient.js:226` returns `payload.data` and **drops `meta` entirely** | server already sends it | **WIRE** — §9.3 | + +### 2.4 Genuinely missing backend capability + +Only four, and all four are in §10: LLM inference, file storage, assistant +streaming, web search. **Nothing the frontend calls today is missing from the +server.** + +--- + +## 3. Existing APIs that must be adopted + +### 3.1 Hire — `POST /api/v1/job-applications/{id}/hire` + +**What the frontend does now** (`krowHooks.js` `useHireCandidate` (:296), `useHireCandidate`): + +1. computes `tier` from `application.ai_score` (`>=80` Skilled, `>=60` + Cross-Trained, else Beginner); +2. `PATCH /job-applications/{id}` `{status: 'hired'}`; +3. `POST /staff` with name, email, phone, `role: job?.title || job?.role_category + || 'Staff'`, `profile_tier: tier`, today's `hire_date`, `application_id`, + `job_posting_id`, `ai_score`, `status: 'onboarding'`; +4. `logActivity('hire_candidate')` → `POST /user-activity`. + +A failure between 2 and 3 leaves an application marked `hired` with no staff +row, and nothing notices. + +**What the backend already supports** (`internal/service/workflows.go` `WorkflowService.Hire` (:123)): +all four writes in one transaction, a 409 on a double hire, and an +all-or-nothing role check before `BEGIN`. + +**Three behavioural differences that must be resolved before adoption:** + +| # | Difference | Resolution | +| --- | --- | --- | +| H-1 | Backend writes `event_type: "candidate_hired"` (`workflows.go:212`). The frontend vocabulary is `hire_candidate` — and `PRIVILEGED_EVENTS = ['hire_candidate', 'create_position']` (`src/lib/activitySignals.js:31`), read by `activity.signals` and `activity.breakdown`. The seed fixture also uses `hire_candidate` (`seed/fixtures/seed.json:4763`). Adopting as-is **silently breaks anomaly detection and the activity feed's labels.** | **CHANGE** `workflows.go:212` to `hire_candidate`. | +| H-2 | Backend defaults `profile_tier` to `"Beginner"` (`workflows.go` `Hire`, the staff record literal); the frontend derives it from `ai_score`. | Frontend sends `profile_tier` in the body — the field is already accepted by `pick()`. No server change. | +| H-3 | Backend sets `role` from `app["job_title"]`; the frontend prefers the posting's `title`, then `role_category`, then `"Staff"`. | Frontend sends `role` in the body. No server change. | + +**Adoption plan:** replace the body of `useHireCandidate` with one +`POST /job-applications/{id}/hire` carrying `{role, profile_tier}`; drop the +separate `logActivity('hire_candidate')` (the workflow writes it); keep the same +`onSuccess` invalidations plus `['userActivity']`. + +### 3.2 Assign — `POST /api/v1/job-postings/{id}/assignments` + +**What the frontend does now** (`krowHooks.js` `useAssignWorkers` (:406), `useAssignWorkers`), +per worker, sequentially, with no transaction: + +1. `POST /assignments`; +2. look for an existing application on `(job_posting_id, lower(email))`; if none, + **`POST /job-applications`** with the worker's profile fields and + `status: 'assigned'`; if one exists, `PATCH` it to `assigned`; +3. `logActivity('assign_employee', {position_id, application_id, worker_email})`. + +**What the backend already supports** (`internal/service/workflows.go` `WorkflowService.Assign` (:261)): +the whole batch in one transaction, capped at 200, validated before `BEGIN`, +errors annotated by index. + +**Two blocking differences:** + +| # | Difference | Resolution | +| --- | --- | --- | +| A-1 | **The backend never creates a `JobApplication`.** It patches one only when `application_id` is supplied (`workflows.go` `Assign`, the application branch). The frontend's own comment (`krowHooks.js` `useAssignWorkers`, the create-if-absent comment) explains why creation is required: an assigned worker with no application is unreachable — nothing to open, and no subject for an interview. Adopting the endpoint as-is **loses that behaviour**. | **CHANGE** — see below. This is the single largest existing-code change in the plan. | +| A-2 | Backend writes `event_type: "worker_assigned"` (`workflows.go:373`); the frontend vocabulary is `assign_employee`. | **CHANGE** `workflows.go:373` to `assign_employee`. | + +**Recommended resolution for A-1 — extend the existing endpoint, do not add one.** + +`service.AssignWorker` gains one optional field, `application: {...}`, carrying +the same fields the frontend builds at `krowHooks.js` `useAssignWorkers`, the application literal. Inside the +transaction, per worker: + +- `application_id` supplied → patch it to `assigned` (unchanged); +- `application_id` absent **and** `application` supplied → look the application up + by `(job_posting_id, email)` — the pair is already `UNIQUE` + (`job_applications_posting_email_key`, migration `000001`) and already indexed + (`job_applications_org_email_idx`) — patch it if found, insert it if not, then + link the assignment to it; +- neither → insert the assignment with no application link (today's behaviour, + preserved for the talent-pool case the code comments describe). + +Because the handler may now insert a `job_applications` row, the authorization +requirement list at `internal/httpserver/workflows.go` `handleAssign` (:127) must gain +`{"job-applications", domain.OpCreate}`. It resolves to `operators`, so no +caller's effective permissions change — but the check must be written down or +the workflow performs a write it was not authorized for. + +**Files to change:** `go-api/internal/service/workflows.go`, +`go-api/internal/httpserver/workflows.go`. **No migration. No new route.** + +### 3.3 Agent definitions — 5 endpoints + +**What the frontend does now.** `useAgents` (`src/lib/agents/useAgents.js`) is the +single writer for every agent screen. It reads +`preferences.customAgents` (`:29`) — an array of `{path: 'custom/.md', raw: +}` — runs `readAgentRegistry(stored, {customSkills})`, and every +mutation (`save`, `remove`, `publish`, `archive`, `restore`, `duplicate`) ends in +`updatePreferences.mutateAsync({customAgents: next})` → `PATCH /me/preferences` +→ `user_preferences.extra`. + +Consumers: `pages/admin/Workspace.jsx`, `pages/admin/AgentDetail.jsx`, +`components/ai-assistant/AgentContext.jsx`, `lib/agents/customAgents.js`. + +**What the backend already supports.** Everything above, with tenancy and +ownership: + +| Frontend operation | Endpoint | Server-side equivalent | +| --- | --- | --- | +| `save(source)` — new | `POST /agent-definitions` `{markdown, visibility}` | validates, parses, projects `definition_id/name/description/status/version/pages` | +| `save(source)` — existing | `PATCH /agent-definitions/{id}` `{markdown}` | re-validates and re-projects every column | +| `remove(id)` | `DELETE /agent-definitions/{id}` | idempotent | +| `publish(id)` | `PATCH` with the republished markdown | `agentLifecycle.publishAgent` already writes `status`/`version` into the frontmatter; the server re-parses both | +| `archive`/`restore` | `PATCH` with the rewritten markdown, or `{status}` | server accepts `draft`/`published`/`archived` | +| `sourceFor(id)` | `GET /agent-definitions/{id}` or list + `markdown` | markdown is returned verbatim | +| shadowing shipped ids | `definition_id` is unique **per owner** and **per org**, never globally | `agent_definitions_personal_key` / `_org_key`, migration `000005` | + +`validateAgentSource` in the browser and `definition.ValidateAgent` +(`internal/definition/agent.go:489`) are held to byte-identical messages by +`internal/definition/conformance_test.go`, which replays the real JavaScript +parser's output over all 37 shipped definitions plus `testdata/oracle.json`. + +**One conflict-detection gap to be aware of when wiring.** `publish` compares +against `live.version` held in the browser (`useAgents.js` `publish` (:90)). The server +has no optimistic-concurrency predicate on `PATCH` — `UpdateAgent` +(`repo/definitions.go` `UpdateAgent` (:270)) writes unconditionally. Two tabs publishing +concurrently: last write wins. This is **not** a regression (preferences had no +concurrency control either) and is listed in §12 Phase 3 as an optional +follow-up, not a blocker. + +### 3.4 Skill definitions — 5 endpoints + +Identical shape, minus `version`. The frontend writer is +`src/lib/skills/customSkills.js` (`upsertCustomSkill`, `removeCustomSkill`, +`customSkillSource`) plus `usePageSkills` / `useWorkforcePaths` +(`src/lib/skills/usePageSkills.js`), which read `preferences.customSkills` and +`preferences.disabledSkills` on **every page render**. + +104 references across 23 files write or read `customSkills`: +`pages/admin/{Workspace,WorkspaceSkills,SkillEditor,OwliverSkillEditor,AgentDetail}.jsx`, +`components/skills/SkillSurface.jsx`, `components/agents/*`, +`components/ai-assistant/{KrowAssistant,useAssistant,routing,AgentContext}.jsx`, +`lib/skills/{catalog,tools,usePageSkills}.js`, `lib/agents/{registry,capabilityTest}.js`. + +Mapping is one-to-one with §3.3, with `status` ∈ `{active, inactive}` +(`SkillStatuses`, `internal/definition/vocabulary.go` `SkillStatuses` (:128)) and no `version` +column — migration `000005` states verbatim that inventing one would be +inventing a concept the frontend does not have. + +**What stays in preferences, deliberately:** `disabledSkills` and +`removedSkills`. They are arrays of skill **ids**, mostly *shipped* ids, so they +are suppression preferences over a namespace rather than definitions. Migration +`000005`'s header says exactly this. **Do not migrate them.** + +### 3.5 What adoption of §3.3/§3.4 buys, per migration `000005` + +| Column | What the jsonb blob had | What the table gives | +| --- | --- | --- | +| `markdown` | a string in an array | the authoritative artefact, `length BETWEEN 1 AND 65536` | +| `definition_id` | derived by re-parsing every entry | a column, `CHECK (~ '^[a-z0-9][a-z0-9-]*$')`, indexed, unique per tier | +| `visibility` | none — everything was personal | `personal` \| `organization` with a CHECK | +| `owner_user_id` / `org_id` | none | tenancy + ownership; personal rows CASCADE with their owner | +| `status` | parsed | a column, CHECK-constrained, partial-indexed for the runtime's own query | +| `version` (agents) | parsed | a column, `CHECK (version >= 1)`, sortable | +| `name`/`description`/`pages` | parsed on every read | server-parsed projections, never accepted from a body | +| size | unbounded, and **returned in full by `GET /me` on every page load** | bounded, and off the `/me` hot path | + +--- + +## 4. Create Position / Jobs + +### 4.1 The complete CRUD flow, verified + +| Step | Frontend | Endpoint | Server | +| --- | --- | --- | --- | +| List | `useJobPostings` (`krowHooks.js:65-70`) `list('-created_date', 100)` | `GET /job-postings` | default sort `-created_date`, default limit 100 — `resources_gen.go` JobPosting | +| Read one | `useJobPosting(id)` (`krowHooks.js:72-78`) | `GET /job-postings/{id}` | `Ops` includes `OpGet` | +| Create | `useCreateJobPosting`; `CreatePosition.jsx:33`; also `KrowAssistant.jsx` `createPosition` (:366) (Owliver's guided flow) | `POST /job-postings` | `created_by` derived from the session (`policy.go`); `title` is the only `Required` column | +| Update | `useUpdateJobPosting`; `CreatePosition.jsx:34`; `useGenerateJobDescription` patches a draft (`krowHooks.js` `useGenerateJobDescription` (:276)) | `PATCH /job-postings/{id}` | shallow merge, `service.go` `Service.Update` (:161) | +| Close | status transition to `closed` | `PATCH` | `posting_status` enum, migration `000001:34` | +| Delete | **no call site** | — | `Ops` has no `OpDelete` → the mux answers 405. Correct. | + +Talent callers see only `status = 'active'` rows — +`TalentScope: {Kind: ScopeActivePostings, Column: "status"}` (`policy.go`), so a +draft is a 404 to them, never a 403. + +`vetting_criteria` and `skill_requirements` are `jsonb NOT NULL` with a GIN index +on `skill_requirements` (`migrations/000001_initial_schema.up.sql:281`). The +Create Position form writes both through `toPositionPayload` +(`src/lib/positionModel.js`). + +### 4.2 Dummy data still bypassing the real API + +**One candidate, and it is not a bypass.** `ROLE_CATEGORIES` +(`src/lib/roleCategories.js:9`) is a static array of seven strings. Its own +docstring records that custom categories come from the `RoleCategory` entity and +are merged on top at both call sites — `CreatePosition.jsx:38` reads +`useRoleCategories()` and `KrowAssistant.jsx:271` does the same. The static list +is a **seed vocabulary offered as options**, not a replacement for the fetched +list, and `POST /role-categories` is wired (`CreatePosition.jsx:39`). + +*If* the seven defaults are wanted as real rows, the correct move is a **seed +change** to `seed/fixtures/seed.json` plus deleting the constant in the +frontend — not a backend change. That is optional and is listed in Phase 1 as +such. + +No other dummy data was found on this flow. `Overview.jsx:48-57`, +`Positions.jsx`, `PositionDetail.jsx` and `CreatePosition.jsx` all read live +query hooks. + +### 4.3 Are any backend changes required for Create Position? + +**No.** The four registered routes cover every call site, the defaults match, the +enums match, `created_by` is derived, and delete is correctly absent. The only +work on this flow is the **AI description generation** — `useGenerateJobDescription` +calls the local `generateJobDescription` stub (`src/lib/krowAi.js`) before it +PATCHes the draft. That is §10.1, not a job-posting problem. + +--- + +## 5. Applications / Interviews / Staff / Talent / Assignments + +### 5.1 Applications + +| Flow | Endpoint | Status | +| --- | --- | --- | +| Talent applies (`Apply.jsx:17`) | `POST /job-applications` | **EXISTS.** `email` is derived from the session for talent (`policy.go`), so an applicant cannot file under someone else's address. | +| Operator screens one (`useScreenCandidate`, `krowHooks.js` `useScreenCandidate` (:223)) | `PATCH /job-applications/{id}` | **EXISTS.** Writes `status: 'ai_screened'` plus seven `ai_*` fields. | +| Operator screens all (`useScreenAllCandidates`, `krowHooks.js` `useScreenAllCandidates` (:245)) | n × `PATCH` | **EXISTS, and correctly not a workflow** — `internal/service/workflows.go` file header records the reasoning: a partial screen is not a corrupt state. | +| Move to interview (`useMarkInterviewReady`, `krowHooks.js` `useMarkInterviewReady` (:507)) | `PATCH` + `POST /user-activity` | **EXISTS.** | +| Remove | `DELETE /job-applications/{id}` | **EXISTS**, operators only, idempotent 200. | + +**Two defects found.** + +- **AP-1 — missing validation, wrong layer. CHANGE.** + `Service.serverSupplies` (`go-api/internal/service/service.go` `serverSupplies` (:266)) iterates + `Policy.Derived` and **ignores `Derived.TalentOnly`**. For + `job-applications`, `email` is `Required` *and* `Derived{TalentOnly: true}`. + Consequence: an **operator** who POSTs an application with no `email` skips the + service's required-field check entirely, reaches SQL, and trips + `email citext NOT NULL`. `translate` (`repo/repo.go` `translate` (:673)) turns that into a + 422 `"email must not be null"`, so the status code is right by accident, but + the message and the `details` key differ from every other required-field + refusal and the check is one layer too late. + **Fix:** give `serverSupplies` the caller's role and skip a `TalentOnly` + derivation for non-talent callers. Same defect applies to + `evidence.worker_email` and would apply to any future `TalentOnly` required + column. One file: `go-api/internal/service/service.go`. No migration. + +- **AP-2 — `screened_at` is written by nobody.** Migration `000003` dropped + `job_applications_screened_consistent` and its header explains why: the column + came from the blueprint, not from repository evidence, and 9 of 24 seeded + applications were being rejected by it. The column survives, nullable and + unused. **No action.** Recorded so nobody re-adds the constraint. + +### 5.2 Interviews — one real RBAC break + +`AIInterviewModal.finishInterview` (`src/components/krow/AIInterviewModal.jsx` `finishInterview` (:155)) +does two writes with no transaction: + +1. `POST /ai-interviews` — the transcript, scores and verdict; +2. `PATCH /job-applications/{id}` `{status:'interview', interview_id, ai_score}`. + +**IV-1 — a talent user cannot complete an interview. CHANGE.** +`Apply.jsx` renders this modal for the talent flow. The policy table allows +`ai-interviews:Create` to **everyone** but `job-applications:Update` to +**operators only** (`internal/domain/policy.go`). So step 1 succeeds and step 2 +returns **403**: the interview row exists, the application still says `applied`, +and `interview_id` is never set. `InterviewAnalytics.jsx:9` and +`AnalyticsMetrics.jsx:8` both count `status === 'interview' || a.interview_id`, +so the interview is invisible to analytics afterwards. + +**IV-2 — the two writes are not atomic**, even for an operator. A failure +between them leaves an orphan `ai_interviews` row. + +**Recommended fix — reuse the existing route, do not add one.** Make +`POST /api/v1/ai-interviews` link its application in the same transaction: +after inserting the interview, update the named `application_id` to +`{status:'interview', interview_id: , ai_score: overall_interview_score}` +— performed by the **server** on the row the interview already names, so no +widening of `job-applications:Update` is needed and a talent caller still cannot +patch an application directly. + +The insert is already guarded: `Repo.guardInsert` +(`go-api/internal/repo/repo.go` `guardInsert` (:332)) refuses a talent caller creating an +interview for an application that is not theirs, answering 404. That guard is +exactly the ownership proof the linked update needs. + +- **Files:** `go-api/internal/service/service.go` (or a small + `go-api/internal/service/interviews.go` holding the special-cased Create for + this one resource), and `go-api/internal/httpserver/api.go` only if the handler + needs a transaction begun for it. Prefer routing this resource's `Create` + through the transactional path already built in + `go-api/internal/service/workflows.go` (`WorkflowService.inTx`). +- **Alternative, rejected:** adding `talent` to `job-applications:Update` with a + column allowlist. Rejected because it puts a second authorization mechanism + (per-column) beside the existing per-operation one, which + `internal/httpserver/workflows.go` file header explicitly argues against. +- **No new route. No migration.** + +`ai_interviews` has no `OpUpdate` and no `OpGet` — correct: the record is written +once, and every consumer reads it from the list. + +### 5.3 Staff + +`GET`/`POST`/`PATCH /staff`, operators only. `HiredHistory.jsx:41-42` reads staff +and postings and writes ratings/reviews through `PATCH`. **EXISTS, complete.** +Adopting §3.1 makes the create half transactional. + +### 5.4 Talent / worker profiles + +| Flow | Endpoint | Status | +| --- | --- | --- | +| `useWorkerProfile` — read-or-create on first access (`krowHooks.js` `useWorkerProfile` (:548)) | `GET /worker-profiles?email=&limit=1` then `POST` | **EXISTS.** `user_id` derived for talent. | +| `useWorkerProfiles` (`krowHooks.js` `useWorkerProfiles` (:580)) | `GET /worker-profiles` `-krow_score` 500 | **EXISTS**, matches the server default exactly. | +| `useUpdateWorkerProfile`, `useCompleteCourse`, `useSubmitChallenge` | `PATCH /worker-profiles/{id}` | **EXISTS.** Shallow merge replaces whole arrays, which `useSubmitChallenge` relies on (`repo/repo.go` `Repo.Update` (:429)). | +| Evidence submit (`useSubmitChallenge`, `krowHooks.js` `useSubmitChallenge` (:639)) | `POST /evidence` then `PATCH /worker-profiles/{id}` | **EXISTS**, but two writes with no transaction. `workflows.go` file header names this as deliberately deferred. Low risk: a failed profile patch loses XP, not the evidence. **No change proposed.** | +| Supervisor verify (`useVerifyEvidence`) | `PATCH /evidence/{id}` | **EXISTS**, operators only. | + +**TP-1 — a profile created by an operator locks its own subject out. CHANGE (deferred).** +`worker_profiles.user_id` is derived **talent-only** (`policy.go`), so a profile +an admin creates for a candidate has `user_id = NULL`. The talent row scope is +`ScopeUserID` on `user_id`, so that person's `GET /worker-profiles?email=...` +returns **empty** — the row is invisible to them. `useWorkerProfile` +(`krowHooks.js` `useWorkerProfile` (:548)) reads that empty list as "no profile yet" and POSTs one, +which trips `worker_profiles_org_email_key UNIQUE (org_id, email)` +(`migrations/000001_initial_schema.up.sql:334`). `translate` +(`repo/repo.go` `translate` (:673)) turns the unique violation into a **409 conflict**, and +the query throws. + +So the failure is not a duplicate row — the constraint prevents that correctly — +it is a talent user who can neither see nor create their profile, with no path +out from the UI. It is latent today only because the seed fixture attaches +`user_id` to every profile it writes. + +The fix needs a product decision (may an operator claim a profile on someone's +behalf, and does signing in adopt an unclaimed one?), so it is scheduled in +Phase 7 rather than earlier. Two candidate shapes, both without a new route: +(a) widen the talent read scope to `user_id = :user OR (user_id IS NULL AND +email = :email)` in `internal/domain/policy.go` + `internal/repo/repo.go`, and +adopt the row on first write; or (b) claim it at login in +`internal/httpserver/auth.go`. (a) is preferred — it is a predicate change in +the layer that already owns predicates. See §11 for the DB impact of each. + +**TP-2 — no role gate on the admin console. Frontend.** `AdminRoute.jsx` `AdminRoute` (:23) +checks `isAuthenticated` and explicitly notes that a role check "will go in Phase +3D". It has not. An `employer` who signs in at `/admin/login` reaches KROW Forge +(`University.jsx:48-49`), whose `POST`/`PATCH /courses` are **admin-only** +(`policy.go` — employer is excluded on purpose, because a NULL-`org_id` course is +the shared platform library). The same applies to `DELETE /certifications`. +Result today: a 403 toast where the UI offers an action. **Frontend work; +the server is right.** + +### 5.5 Assignments + +| Flow | Endpoint | Status | +| --- | --- | --- | +| List | `GET /assignments` `-created_date` 500 | **EXISTS**, matches. | +| Create | `POST /assignments` in a 3n loop | **WIRE** → `POST /job-postings/{id}/assignments`, after the A-1/A-2 changes in §3.2. | +| Update / cancel | — | **Not supported and not called.** `Ops` is `List|Create`; nothing in the frontend cancels an assignment. Do not add one speculatively. | + +One cosmetic difference to settle while wiring §3.2: the column default is +`source = 'owliver'` (`migrations/000001_initial_schema.up.sql:511`) and the +frontend sends `'owliver'`, but `WorkflowService.Assign` substitutes +`'manual'` for an empty value (`internal/service/workflows.go:336`). Harmless +while the frontend always sends the field; worth aligning in the same edit. + +The schema is ahead of the API here in a good way: `assignments_period_gist` +(`migrations/000001_initial_schema.up.sql:523`) already indexes the +`[starts_at, ends_at)` period, which is what an overlap check would need if +double-booking ever becomes a rule. No such rule exists in the frontend today, +so none is proposed. + +--- + +## 6. Owliver / Agents / Skills — full audit + +### 6.1 The two tables + +`migrations/000005_agent_skill_definitions.up.sql` creates `agent_definitions` +and `skill_definitions`. Two tables and not one, per its own header: agents have +`status ∈ {draft, published, archived}` plus a monotonic integer `version`; +skills have `status ∈ {active, inactive}` and **no version at all**. + +Deliberately absent, and named as such in the migration: +`definition_versions`, `definition_permissions`, `agent_skills`, +`agent_subagents`, `agent_knowledge`, `conversations`. **Do not add any of them +in this plan.** `permissions:` is parsed +(`internal/definition/agent.go` `normalizePermissions`) and unenforced by design. + +### 6.2 Ownership and tenancy — verified + +| Property | Where enforced | Verdict | +| --- | --- | --- | +| `org_id NOT NULL` on **both** tiers, including personal | migration `000005`; `service/definitions.go` `CreateAgent` (:138) sets it from `ident.OrgID` | correct — a personal definition is always inside a tenant | +| `owner_user_id` set **iff** `visibility='personal'` | `CHECK ((visibility='personal') = (owner_user_id IS NOT NULL))` | correct, and stated in one line so it cannot be half-satisfied | +| personal definitions cascade with the owner | `owner_user_id ... ON DELETE CASCADE` | correct | +| shared definitions survive their author | `created_by ... ON DELETE SET NULL` | correct | +| every read/write carries the org + ownership predicate | `repo/definitions.go` — `ListAgents/GetAgent/UpdateAgent/DeleteAgent` and the four skill twins | correct; an invisible row answers 404, never 403 | +| a talent user cannot author an organization definition | `service/definitions.go` `CreateAgent` (:138) (create), `:207-212` (update), `:264-269` (delete), ×2 for skills | correct | +| `visibility` is immutable after create | `service/definitions.go` `UpdateAgent` (:194), `:415-419` | correct | +| a definition cannot name its own tenancy | `internal/definition/definition.go` package doc — the frontend ignores `visibility:` in frontmatter entirely | correct | + +### 6.3 Shadow-by-id — verified + +`definition_id` is **not** globally unique. Two partial unique indexes: +`(owner_user_id, definition_id) WHERE visibility='personal'` and +`(org_id, definition_id) WHERE visibility='organization'`. So a personal +definition may carry the same id as an organization one, which may carry the same +id as a *shipped* one in `krow-demo/src/skills/` — which is exactly the override +mechanism `useAgents.isOverridden` (`useAgents.js:41`) already reports as +`shadowed`. + +Resolution precedence is implemented: +`GetAgentByDefinitionID` / `GetSkillByDefinitionID` +(`repo/definitions.go` `GetAgentByDefinitionID` (:209), `:430-461`) order by +`CASE WHEN visibility='personal' THEN 1 ELSE 2 END LIMIT 1` — personal wins. +**These two functions are reachable only from `internal/runtime`, which is not +wired to any route** (see §6.7). + +### 6.4 Status, versioning + +- Agents: `status` CHECK `('draft','published','archived')`; `version integer + NOT NULL DEFAULT 1 CHECK (version >= 1)`; partial index + `agent_definitions_published_idx ... WHERE status='published'`. +- Skills: `status` CHECK `('active','inactive')`; partial index + `skill_definitions_active_idx ... WHERE status='active'`. No version. +- Both are **projections parsed out of the markdown**, never accepted from a + body — `service/definitions.go` `CreateAgent` (:138) builds the insert input entirely from + `definition.ParseAgent` / `ParseSkill` output. +- `PATCH {status}` is accepted as a shortcut *only when `markdown` is absent* — + the `else if` at `service/definitions.go:241` and `:415`. + **Gap D-1 (CHANGE, minor):** a PATCH carrying both `markdown` and `status` + silently drops the `status`. It should either apply the markdown's parsed + status (which it does) *and say so*, or refuse the combination as a 422. A + silent drop is the one outcome that teaches nobody anything. +- **No optimistic concurrency.** `UpdateAgent` (`repo/definitions.go` `UpdateAgent` (:270)) + writes unconditionally; `useAgents.publish` compares versions in the browser + (`useAgents.js` `publish` (:90)). Two tabs publishing concurrently: last write wins. + Not a regression from preferences. Optional Phase 3 follow-up: add + `AND version < :new_version` to the agent update predicate and answer 409 on + zero rows — a change to `repo/definitions.go` only, no migration. + +### 6.5 Markdown parsing and validation + +`internal/definition/` is a deliberate port of the browser's parsers: + +| Concern | Backend | Frontend counterpart | +| --- | --- | --- | +| document layer (BOM, line endings, fences, body) | `frontmatter.go` | `src/lib/skills/registry.js` | +| YAML subset (no YAML dependency, on purpose) | `yaml.go` | `src/lib/skills/yaml.js` | +| JS value semantics (`jsString`, `jsTruthy`, `jsNumber`, `jsTrim`) | `jsvalue.go` | JavaScript itself | +| agent parse + normalize | `agent.go` — port of `parseAgent` + `normalizeAgent` | `src/lib/agents/registry.js`, `agentConfig.js` | +| skill parse | `skill.go` — port of `parseSkill` | `src/lib/skills/registry.js` | +| closed vocabularies | `vocabulary.go` | `src/lib/skills/surfaces.js`, `src/lib/agents/vocabulary.js` | + +Compatibility is enforced, not asserted: `internal/definition/conformance_test.go` +replays the **actual** JavaScript parser output (captured into +`testdata/oracle.json` by `scripts/oracle.mjs`) against the Go parser over all 37 +shipped definitions plus every adversarial case. + +Deliberate asymmetries, all documented in source and **not to be "fixed"**: + +- `Skill.Pages` keeps the strings **as the author wrote them**; + `Agent.Pages` are **canonicalised** (`skill.go` `Skill.Pages` (:28), `agent.go` `Agent.Pages` (:66)). The + two frontend parsers differ the same way; reconciling here would make each side + disagree with its own editor. +- `pages: candidates` (a bare scalar) is accepted for an **agent** and refused for + a **skill**, because `normalizeAgent` coerces and `parseSkill` requires a + sequence (`agent.go` `asList` (:112)). +- A skill's `status:` is a **coercion**, not a check — anything that is not + `inactive` reads as `active` (`skill.go` `ParseSkill` (:107)). + +Three rules are the backend's own, all listed as `BackendOnly` on the +`Rejection`: + +1. `MaxMarkdownLength = 65536` characters — the `markdown_size` CHECK, which the + editor does not enforce (`definition.go:73`, `skill.go` `ValidateSkill` (:279), + `agent.go` `ValidateAgent` (:489)). +2. `MaxVersion = 2147483647` — `agent_definitions.version` is a PostgreSQL + `integer`; the editor accepts any whole number ≥ 1 (`agent.go` `ValidateAgent` (:489)). +3. "A definition with a `ui:` block needs an explicit `pages:` list" + (`skill.go` `ValidateSkill` (:279)) — because `parseSkill` falls back to the `ui:` block's + own pages, a fallback the backend cannot compute (§6.6). + +### 6.6 Backend validation vs. the frontend Owliver vocabulary + +This is the sharpest boundary in the system, and it is drawn on purpose. + +**What the backend mirrors exactly.** `SKILL_SURFACES` — all **18** ids and their +aliases (`create-position`←`new-position`, `hired-history`←`hired`, +`krow-forge`←`university`/`forge`) — are transcribed into +`internal/definition/vocabulary.go` `pageSurfaces` (:18), and `normalizeKey` +(`vocabulary.go` `normalizeKey` (:70)) reproduces `surfaces.js`'s own normalization +(lower-case, spaces and underscores read as dashes). Verified against the live +frontend: `python3` over `src/lib/skills/surfaces.js` yields exactly those 18 ids +in that order. The agent vocabularies — statuses, reasoning modes, knowledge +kinds, access, permission roles, the 10 icons — are transcribed at +`vocabulary.go` the agent vocabulary block (:94-113). + +**What the backend does not read at all.** `internal/definition/skill.go` `deferredBlocks` (:235) +(`deferredBlocks`) records the presence of `ui:` and `owliver:` on +`Skill.Deferred` and checks **nothing inside them**. + +| Frontend vocabulary | Count | Source of truth | Backend equivalent | +| --- | ---: | --- | --- | +| Page surfaces | 18 | `src/lib/skills/surfaces.js` `SKILL_SURFACES` | **mirrored** — `vocabulary.go` `pageSurfaces` (:18) | +| Owliver capabilities | 10 | `surfaces.js` `OWLIVER_CAPABILITIES` | **none** | +| Data sources | 25 | `surfaces.js` `DATA_SOURCES` | **none** | +| Section types | 9 | `surfaces.js` `SECTION_TYPES` | **none** | +| Periods | 6 | `surfaces.js` `PERIODS` | **none** | +| Placement→context rules | — | `surfaces.js` `placementProvides()` | **none** | +| `owliver:` normalization | — | `src/lib/skills/owliverConfig.js` | **none** | +| section normalization | — | `src/lib/skills/uiConfig.js` `normalizeSection()` | **none** | + +Consequence, stated precisely: **if §3.3/§3.4 are adopted today, the server will +store and round-trip an `owliver:` block byte-for-byte and validate the `pages:` +list, but it will not reject an unknown capability, an unknown data source, an +unknown period, or a capability that resolves no source.** That validation stays +in `owliverConfig.js`. + +The one known divergence is pinned by a test: +`krow-demo/skill-examples/board-invalid-context.md` is refused by the frontend +(on a placement/data-source rule) and accepted by the backend. The conformance +suite asserts it remains the **only** such case. + +**This is acceptable for Phase 3.** The browser is the only renderer, so +browser-side refusal is sufficient to keep a bad definition off a page. It stops +being acceptable the moment the *server* answers an Owliver question — which is +§7. + +### 6.7 `internal/runtime` — built, tested, and not wired + +`go-api/internal/runtime/` (loader 237 lines, executor 129, types 103, tests 890) +loads a definition by uuid **or** `definition_id`, re-validates it, resolves +skill dependencies, refuses draft/archived agents and inactive skills, detects +circular dependencies, and hands off to an `AgentExecutor` / `SkillExecutor` +boundary. The default is `UnavailableExecutor`, which returns +`ErrExecutorUnavailable` — "AI executor is unavailable (deferred to Phase 5)" +(`runtime/types.go` `ErrExecutorUnavailable`). + +`grep -rn "internal/runtime" go-api --include='*.go'` outside the package and its +tests returns **nothing**. It is not constructed in `Server.New` +(`internal/httpserver/server.go` `Server.New`) and has no route. + +**This is the designed seam for §7 and §10.3.** Do not build a parallel one. + +### 6.8 What is still frontend-only, definitively + +1. All `owliver:` and `ui:` block semantics — capabilities, sources, periods, + shapes, placements, inheritance, `editable` requiring both halves. +2. Every one of the 25 data-source resolvers + (`src/lib/skills/dataResolver.js`, 1,214 lines). +3. `disabledSkills` / `removedSkills` suppression — correctly so. +4. Agent conversation state, history and panel layout + (`components/ai-assistant/*`, `localStorage`/`sessionStorage`). +5. Shipped definitions themselves — `krow-demo/src/agents/**`, + `src/skills/**`. Migration `000005`'s header states they must stay in Git: + putting them in a table would trade `git log`, review and atomic deploy for + nothing. + +--- + +## 7. Owliver skill responses — what backend support would actually require + +**Nothing here is proposed for Phases 1–3.** This section answers "what would it +take", so the decision can be made with the cost visible. + +### 7.1 The requirement + +Today, `provider.stream(request)` (`components/ai-assistant/provider.js`) yields +**snapshots** — the whole block array so far, not deltas, because a table or a +KPI row has no meaningful half-state. `createAssistantProvider()` returns +`createHttpProvider` only when `VITE_ASSISTANT_ENDPOINT` is set; it is unset, so +`createLocalProvider()` runs and computes deterministic answers from the +dashboard's own already-loaded data. + +For a response to become *backend-supported*, three things must move server-side, +and they are separable: + +| Layer | What it is | Cost | +| --- | --- | --- | +| **L1 — vocabulary** | The closed tables in §6.6: 10 capabilities, 25 sources, 9 section types, 6 periods, and the placement→context rules | ~600 lines of transcription + a conformance oracle, mirroring what `vocabulary.go` already does for the 18 surfaces | +| **L2 — resolvers** | 25 data sources rewritten as SQL against `job_applications`, `job_postings`, `worker_profiles`, `assignments`, `shift_records`, `staff`, `courses`, `user_activity` | the real cost — `dataResolver.js` is 1,214 lines and depends on `lib/workforce.js`, `lib/attendance.js`, `lib/activitySignals.js`, `lib/positionModel.js`, `lib/talentHome.js`, `lib/admin/positionInsights.js` | +| **L3 — transport** | one streaming endpoint | small, and specified in §10.3 | + +### 7.2 The security argument for doing it + +`api-audit.md` §7 records that the fact sheet is **deliberately not sent** to the +provider: dashboard data is to be read server-side from the caller's own session, +so a client cannot ask about records it is not entitled to see, and an agent +cannot widen that by being named in the body. + +That property is **only achievable server-side.** L2 done in Go inherits the +tenancy and talent-scope predicates that already live in +`internal/domain/policy.go` and `internal/repo/repo.go` — the same predicates +every list endpoint uses. Done any other way it has to be re-derived. + +### 7.3 The one endpoint, if it is built + +Specified in full in §10.3. **No other route is required** — a capability +resolves to a section, a section resolves to a source, and the answer streams +back as blocks. There is no second shape for "the chat version", by design +(`api-audit.md` §6.1). + +### 7.4 Order + +L1 and L2 are useless without L3, and L3 without L2 answers nothing. But L2 can +be built **one source at a time** behind a capability list the server advertises: +a capability whose source has no server resolver is simply not offered, which is +exactly the rule `owliverConfig.js` already applies (`api-audit.md` §6.1: "a +capability that resolves no source is dropped, with a named error"). That makes +this incremental rather than all-or-nothing, and it is why §12 Phase 4 is a +phase rather than a single task. + +--- + +## 8. Flow-chart data + +### 8.1 Where it comes from today — traced + +A `flow` is one of the 9 `SECTION_TYPES` and one of the 10 +`OWLIVER_CAPABILITIES`. Its data is produced by `resolveSkillData(section, +context)` (`src/lib/skills/dataResolver.js:1196`), which dispatches on +`section.source` into the `RESOLVERS` table. + +`context` is assembled in exactly two places, and both assemble it from **live +query hooks over registered endpoints**: + +- `src/components/skills/SkillSurface.jsx:81-119` — `useApplications`, + `useJobPostings`, `useInterviews`, `useCourses`, `useWorkerProfiles`, + `useStaff`, `useUserActivity`, `useAssignments`, `useShiftRecords`, + `useCurrentUser`, plus `trainingPaths` derived from courses + the profile. +- `src/components/ai-assistant/KrowAssistant.jsx:300-343` — the same set for the + panel. + +`ResponseBlocks.jsx:479` re-resolves the same section live for a chat answer. + +### 8.2 Can existing backend resources provide it? + +**Yes — entirely, and they already do.** Every collection above is a registered +list endpoint with a matching default sort and limit: + +| Context key | Endpoint | Default sort / limit | +| --- | --- | --- | +| `applications` | `GET /job-applications` | `-ai_score` / 200 | +| `positions` | `GET /job-postings` | `-created_date` / 100 | +| `interviews` | `GET /ai-interviews` | `-created_date` / 100 | +| `courses` | `GET /courses` | `-created_date` / 200 | +| `workerProfiles` / `profiles` | `GET /worker-profiles` | `-krow_score` / 500 | +| `assignments` | `GET /assignments` | `-created_date` / 500 | +| `staff` | `GET /staff` | `-created_date` / 100 | +| `activity` | `GET /user-activity` | `-created_date` / 500 | +| `shifts` | `GET /shift-records` | `-created_date` / 500 | + +Periods are computed at read time from `created_date` +(`dataResolver.js` `periodRange` (:48), `inPeriod` at `:77-84`) — never stored, never +hard-coded. The server already returns `created_date` in the exact millisecond +ISO-8601 form the resolvers parse (`domain/resource.go` `Column.SelectExpr` (:115)). + +### 8.3 Recommendation + +**No new endpoint. No backend change.** Flow-chart data is a client-side reading +over records the API already serves, and the reading must stay wherever the +renderer is. The only backend involvement flow data would ever need is §7 L2 — +and that is a *relocation* of `dataResolver.js`, not a new resource. + +One honest caveat, and it is §9.3's caveat too: a flow over `shift-records` reads +at most 500 rows because that is the limit the frontend asks for, and the +frontend discards the `meta.truncated` flag the server already sends. Fix that in +the frontend. + +--- + +## 9. Dashboard / analytics / activity + +### 9.1 What is real + +| Surface | Values | Source | +| --- | --- | --- | +| `pages/Overview.jsx:48-57` | active positions, total applications, screened, new, average AI score, hires | `useJobPostings` + `useApplications` — **all real** | +| `pages/Overview.jsx:60-61` | top candidates, active postings | same two, sorted/sliced client-side | +| `pages/Analytics.jsx:13-16` | funnel, interview analytics, hire metrics | `useApplications`, `useInterviews`, `useStaff`, `useJobPostings` — **all real** | +| `pages/UserTracking.jsx:10` | activity feed | `useUserActivity` — **real** | +| `pages/TalentPool.jsx:9` | pool, score bands | `useWorkerProfiles` — **real** | +| `pages/HiredHistory.jsx:41-42` | hires, tiers, reviews | `useStaff` + `useJobPostings` — **real** | +| `lib/activitySignals.js` | anomaly signals, privileged-event share | `user_activity` rows — **real** | +| `lib/attendance.js` | attendance, overtime, weekly trend | `shift_records` — **real** | + +**Nothing on the dashboard is dummy or localStorage-backed.** The only local +values are UI state (§2.3). + +### 9.2 Are the existing list APIs sufficient? + +**Yes.** Every figure is a count, a filter, an average or a sort over one of the +nine collections. All nine are registered; the frontend's requested limits match +the server's declared defaults exactly (`api-audit.md` §3, verified against +`resources_gen.go`). + +**No aggregation endpoint is proposed.** Adding one would duplicate arithmetic +that already exists and has to keep existing for the offline/local provider, and +would create a second place for "how many were screened" to be defined. + +### 9.3 The one real risk — and it is already solvable, in the frontend + +`writePage` (`internal/httpserver/response.go` `writePage` (:52)) returns +`meta: {total, limit, offset, returned, truncated}` on **every** list response, +and `truncated` is computed as `total > offset + len(records)`. + +`krow-demo/src/api/httpClient.js:226` returns `payload.data` and **drops `meta` +entirely.** So when an organization exceeds 500 shift records or 200 +applications, every dashboard figure silently under-reports and nothing says so. + +- **WIRE** — surface `meta` through `httpClient.request()` and have the hooks + refuse to render a total when `truncated` is true. Frontend only. +- **No backend change.** The server already answers the question; the client + throws the answer away. `internal/httpserver/response.go` `meta` (:20) records that + `truncated` exists precisely so this is fixable without a contract change. + +### 9.4 Activity vocabulary — the one backend change + +Covered as H-1 and A-2 in §3. Restated because it lands here: the two workflow +endpoints write `candidate_hired` and `worker_assigned`; every other producer and +consumer in the system — 9 frontend event types, `PRIVILEGED_EVENTS`, the seed +fixture — uses `hire_candidate` and `assign_employee`. Two string literals in +`go-api/internal/service/workflows.go` (`:212`, `:373`). + +--- + +## 10. Real missing backend capabilities + +Four, exactly as `api-audit.md` §13 concludes. Each is specified here to the +level the "do not invent APIs" rule demands: why it is necessary, request, +response, authorization, service, repository, DB impact, migration impact. **None +of them is required for Phases 1–3.** + +### 10.1 LLM / AI inference — NEW capability, route optional + +**Why.** `src/api/aiEngine.js` implements `integrations.Core.InvokeLLM` in the +browser with **no network and no key**: it recognises a workflow by the stable +phrase its prompt opens with, reads the structured fields the prompt already +carries, and scores deterministically. Ten consumers: +`generateJobDescription`, `buildResumeFromText`, `screenCandidate`, +`generateInterviewQuestion`, `matchTalentForJob`, `evaluateInterview`, +`owliverQuestion`, `buildProfileFromConversation` (`src/lib/krowAi.js`), and +`challengeFollowUp`, `evaluateChallenge` (`src/lib/provingGround.js`). + +There is no route, no key and no config for this anywhere in the backend — +`internal/config/config.go` has `AppEnv`, `Log`, `HTTP`, `DB`, `Seed` and nothing +else. + +**Is a route necessary?** Not for its own sake. Two of the ten consumers already +have a natural home: + +- `screenCandidate` → the screening PATCH it already performs. +- `evaluateInterview` → the interview creation in §5.2. + +The rest are interactive authoring aids. **The recommendation is: build the +service first, no route.** `internal/llm` with a single +`Complete(ctx, req) (Response, error)` boundary and a deterministic +`StubProvider` mirroring `aiEngine.js`, wired the same way +`runtime.UnavailableExecutor` is — an interface with an honest default. Then +each caller that needs it gets it *inside a handler that already exists*. + +If a general-purpose route is later required it should be **one**, and it should +be the assistant endpoint in §10.3, not a family of per-workflow routes. + +- **Service:** new `go-api/internal/llm/` (`llm.go`, `stub.go`, `anthropic.go`). +- **Repository:** none. +- **DB:** none. **Migration: none.** +- **Config:** `go-api/internal/config/config.go` gains an `LLM` block — + provider, model id, API key, timeout, max tokens. The key must be loaded the + same way `DB.Password` is (env only; no default; fail loudly). +- **Wiring:** `go-api/cmd/api/main.go`, `go-api/internal/httpserver/server.go`. +- **Risk to state now:** an LLM call inside a request handler makes that + handler's latency unbounded. `HTTPConfig.WriteTimeout` must be revisited, and a + per-call `context.WithTimeout` is mandatory. + +### 10.2 File storage / upload — NEW, one route genuinely required + +**Why.** `integrations.Core.UploadFile({file})` returns +`{file_url, file_name, file_size}` where `file_url` is a `blob:` URL from +`URL.createObjectURL`. It is valid for the rest of the session and **dies on +reload**. Consumers: `src/pages/KrowIdentity.jsx` (selfie), +`src/components/krow/proving/MediaChallenge.jsx` and `VideoRecorder.jsx` +(challenge media). + +That URL is then **persisted**: into `worker_profiles.selfie_url` and +`evidence.media_url`, both `text NOT NULL` columns +(`migrations/000001_initial_schema.up.sql`). Every such row already in the +database holds a dangling reference. This is the one missing capability that is +actively corrupting stored data. + +**A route is genuinely necessary** because there is no existing endpoint that +accepts bytes: every registered handler reads a ≤4 MiB JSON body +(`internal/httpserver/api.go:15`). + +| Field | Specification | +| --- | --- | +| **Route** | `POST /api/v1/files` | +| **Auth** | session required (not in `publicPaths`) | +| **Authorization** | any authenticated role. It is the *reference* that carries meaning, and the resources holding references are already scoped. | +| **Request** | `multipart/form-data`, one part `file`. Bounded by a new `HTTP.MaxUploadBytes` — **not** `maxBodyBytes`, which must stay 4 MiB for JSON. | +| **Response** | `201 {"data": {"file_url": "...", "file_name": "...", "file_size": 12345, "content_type": "image/jpeg", "id": ""}}` — `file_url`, `file_name`, `file_size` verbatim so `aiEngine.UploadFile`'s three call sites need no change beyond swapping the implementation. | +| **Errors** | 413 over the size bound; 422 on a rejected content type; the existing envelope. | +| **Content types** | an allowlist derived from the three call sites: `image/jpeg`, `image/png`, `image/webp`, `video/mp4`, `video/webm`. Never trust the client's declared type — sniff. | +| **Service** | new `go-api/internal/service/files.go` | +| **Repository** | new `go-api/internal/repo/files.go` | +| **Storage** | an interface in new `go-api/internal/storage/` with a local-filesystem implementation for development and an object-store implementation for deployment. Bytes must **not** go in PostgreSQL. | +| **Read path** | `file_url` should be a URL this API can serve or redirect from, so access stays behind the session. A public object-store URL would make every uploaded selfie world-readable — do not do that. | + +**DB impact — one new table, one new migration.** + +`migrations/000006_files.up.sql` / `.down.sql` (next free number; depends on +`000001` for `organizations` and `users`): + +``` +files + id uuid PRIMARY KEY DEFAULT gen_random_uuid() + org_id uuid NOT NULL REFERENCES organizations(id) ON DELETE CASCADE + uploaded_by uuid REFERENCES users(id) ON DELETE SET NULL + storage_key text NOT NULL -- opaque; never the original filename + file_name text NOT NULL + content_type text NOT NULL + file_size bigint NOT NULL + created_date timestamptz NOT NULL DEFAULT now() + + CHECK (file_size > 0) + CHECK (length(btrim(file_name)) > 0) + UNIQUE (storage_key) + INDEX files_org_created_idx (org_id, created_date DESC) +``` + +- **Foreign keys:** `org_id` CASCADE (tenancy, matching every other table); + `uploaded_by` SET NULL (attribution survives the author leaving, matching + `job_postings.created_by` and `agent_definitions.created_by`). +- **No new enum.** `content_type` is text with an application-level allowlist; + the vocabulary will grow, and `000005`'s header already argues text+CHECK over + `ALTER TYPE ... ADD VALUE` for exactly this reason. +- **No FK from `worker_profiles.selfie_url` or `evidence.media_url`.** They are + `text` URLs today and every existing row holds a dead `blob:` value; adding a + FK would require back-filling data that cannot be recovered. Leave them as + URLs. +- **Seed:** none. **Rollback:** `DROP TABLE files;` — safe, because nothing + references it. Blobs in the object store are **not** removed by the rollback; + that is deliberate and must be documented in the down migration, as `000005` + documents its own choices. + +### 10.3 Assistant streaming — NEW, one route, and only after §7 L2 + +**Why.** `createHttpProvider({endpoint})` +(`src/components/ai-assistant/provider.js`) is written, tested and unused, +because `VITE_ASSISTANT_ENDPOINT` is unset. Setting it at a route that does not +exist would replace working local answers with `Assistant request failed: 404`. + +**Do not build this before §7 L2 has at least one real resolver.** The endpoint +without resolvers can only proxy an LLM, and the whole security argument for +moving Owliver server-side (§7.2) is that the *data* is read from the caller's +session. + +| Field | Specification | +| --- | --- | +| **Route** | `POST /api/v1/assistant/stream` | +| **Auth** | session required | +| **Authorization** | any authenticated role. Row visibility comes from the resolvers, which reuse `internal/domain/policy.go` scopes — a talent caller asking for `candidates.pipeline` gets their own rows or nothing. | +| **Request** | exactly what `createHttpProvider` already sends: `{"contextId": "admin.positions", "capability": "flow", "question": "...", "agent": {...}, "owliverContext": {...}}`. **The fact sheet is deliberately absent and must stay absent.** | +| **Response** | `text/event-stream`. Lines the client already parses: `data: {"block": {...}}`, `data: {"delta": "..."}`, `data: [DONE]`. Non-2xx throws client-side — so an error must be a status code, not an SSE frame. | +| **Headers** | `Content-Type: text/event-stream`, `Cache-Control: no-store`, `X-Accel-Buffering: no`. | +| **Service** | new `go-api/internal/service/assistant.go`, built **on `internal/runtime`** — `runtime.Engine` already loads a definition, checks eligibility, resolves dependencies and hands to an executor. Replace `UnavailableExecutor` with a real one; do not build a parallel loader. | +| **Repository** | reuses `internal/repo/definitions.go` (already has `GetAgentByDefinitionID`/`GetSkillByDefinitionID` with personal-over-organization precedence) and `internal/repo/repo.go` for the data reads. | +| **DB impact** | **none for the endpoint itself.** Conversation persistence is explicitly out of scope — `migrations/000005`'s header lists `conversations` as deferred, and the frontend keeps history in `localStorage` under `krow_assistant:history`. Do not add a table until a product requirement asks for cross-device history. | +| **Migration** | none. | +| **Config** | `HTTP.WriteTimeout` must not apply to a stream — use `http.ResponseController` or a per-route timeout, or a long answer is cut mid-block. | + +### 10.4 Web search — NEW, lowest priority, no route of its own + +**Why.** An agent may declare `webSearch: true` +(`internal/definition/agent.go` parses it into `Agent.WebSearch`; migration +`000005` stores nothing for it — it lives in the markdown). The frontend is +honest about it: an agent with `webSearch` set gets a note saying no search +provider is configured in this deployment, rather than an answer that quietly +came from nowhere. + +**No route.** Search is a tool the assistant executor calls, not a resource a +client fetches. It belongs behind §10.3 as one more capability of the executor. + +- **Service:** new `go-api/internal/search/` with a provider interface and a + `NotConfigured` default that returns the same honest refusal the UI already + shows. +- **Config:** provider + key in `go-api/internal/config/config.go`. +- **DB:** none. **Migration:** none. + +### 10.5 Other gaps found during this audit + +| # | Gap | Class | Where | +| --- | --- | --- | --- | +| G-1 | Activity event-type mismatch (`candidate_hired`/`worker_assigned`) | **CHANGE** | `internal/service/workflows.go:212,373` | +| G-2 | Assign cannot create the application the frontend requires | **CHANGE** | `internal/service/workflows.go`, `internal/httpserver/workflows.go` | +| G-3 | Talent cannot complete an interview (403 on the linked PATCH) | **CHANGE** | §5.2 | +| G-4 | `serverSupplies` ignores `Derived.TalentOnly` | **CHANGE** | `internal/service/service.go` `serverSupplies` (:266) | +| G-5 | PATCH on a definition silently drops `status` when `markdown` is present | **CHANGE** | `internal/service/definitions.go:241,415` | +| G-6 | No optimistic concurrency on agent publish | **CHANGE (optional)** | `internal/repo/definitions.go` `UpdateAgent` (:270) | +| G-7 | An operator-created worker profile locks its subject out | **CHANGE (deferred)** | §5.4 TP-1 | +| G-8 | `internal/runtime` is complete and unwired | not a defect — the seam for §10.3 | `internal/runtime/` | +| G-9 | `meta.truncated` discarded by the client | **WIRE** | `krow-demo/src/api/httpClient.js:226` | +| G-10 | Admin console has no role gate | **WIRE** | `krow-demo/src/pages/admin/AdminRoute.jsx` `AdminRoute` (:23) | +| G-11 | `README.md:106` says "51 registered routes"; the router registers 53 | doc drift | `README.md` | +| G-12 | `docs/api-contract.md` §1 says "Auth: None in v1" | doc drift | `docs/api-contract.md` | + +--- + +## 11. Database impact, per required change + +The schema is at **`000005`**. The next free number is **`000006`**. +Every migration below depends on `000001` for `organizations` and `users`. + +### 11.1 Changes requiring NO database work + +This is most of the plan, and it is the point. + +| Change | Existing tables touched | New table | New column | FK | Index | Enum | Migration | Seed | Rollback | +| --- | --- | :-: | :-: | :-: | :-: | :-: | :-: | :-: | --- | +| §3.1 Hire adoption (H-1/H-2/H-3) | `job_applications`, `staff`, `user_activity` | no | no | no | no | no | **none** | no | revert two string literals | +| §3.2 Assign adoption + application creation (A-1/A-2) | `assignments`, `job_applications`, `user_activity`, `job_postings` | no | no | no | **no — reuses `job_applications_posting_email_key` and `job_applications_org_email_idx`** | no | **none** | no | revert the handler | +| §3.3/§3.4 Definition adoption | `agent_definitions`, `skill_definitions`, `user_preferences` | no | no | no | no | no | **none** — `000005` already created everything | no | data written to the tables would need re-mirroring into `user_preferences.extra`; see 11.4 | +| §5.2 Interview link (G-3) | `ai_interviews`, `job_applications` | no | no | **no — `job_applications.interview_id` exists since `000002`** | no | no | **none** | no | revert the handler | +| §5.1 `serverSupplies` fix (G-4) | none | no | no | no | no | no | **none** | no | trivial | +| §6.4 definition PATCH semantics (G-5) | none | no | no | no | no | no | **none** | no | trivial | +| §6.4 publish concurrency (G-6) | `agent_definitions` | no | no | no | no | no | **none** | no | trivial | +| §8 Flow data | none | no | no | no | no | no | **none** | no | n/a | +| §9 Dashboard | none | no | no | no | no | no | **none** | no | n/a | +| §10.1 LLM service | none | no | no | no | no | no | **none** | no | n/a | +| §10.3 Assistant stream | reads only | no | no | no | no | no | **none** | no | n/a | +| §10.4 Web search | none | no | no | no | no | no | **none** | no | n/a | + +### 11.2 `000006_files` — the only migration this plan requires + +Full shape in §10.2. + +- **Existing tables:** `organizations`, `users` (referenced only). +- **New table:** `files`. +- **New columns on existing tables:** none. +- **Foreign keys:** `files.org_id → organizations(id) ON DELETE CASCADE`; + `files.uploaded_by → users(id) ON DELETE SET NULL`. +- **Indexes:** `UNIQUE (storage_key)`; `files_org_created_idx (org_id, + created_date DESC)`. No index on `uploaded_by` — attribution only, matching the + reasoning already written into `000005` for `created_by`. +- **Enums:** none. `content_type` is `text` + an application allowlist. +- **Seed:** none. `seed/fixtures/seed.json` is untouched; the seeder + (`go-api/internal/seeder/`) needs no change. +- **`make gen-resources`:** only if `files` is to become a `domain.Resource`. + **It should not be** — it has a non-JSON request body and no list surface. Keep + it hand-written, exactly as the definition endpoints are. +- **Rollback:** `DROP TABLE files;`. Blobs already written to the object store + survive the rollback and must be removed out of band; state that in the `.down` + file. + +### 11.3 `000007_worker_profile_claim` — conditional, Phase 7 only + +Only if TP-1 (§5.4) is resolved by *claiming* rather than by widening the read +predicate. Widening the predicate needs **no migration at all**, which is why it +is the recommended shape. + +If claiming is chosen: no new table, no new column — the change is a `UPDATE +worker_profiles SET user_id = ... WHERE user_id IS NULL AND email = ...` executed +at login, plus a partial index `(org_id, email) WHERE user_id IS NULL` to make +the lookup cheap. Rollback drops the index; the claimed `user_id` values are not +reversible, which is the reason this needs a product decision first. + +### 11.4 Rollback consideration for §3.3/§3.4 — the one that is not free + +Adopting the definition endpoints creates rows in `agent_definitions` and +`skill_definitions` that have **no counterpart** in `user_preferences.extra`. +Reverting the frontend afterwards would make every definition authored in the +meantime invisible. + +Mitigations, in order of preference: + +1. **Dual-write during the cutover window.** The frontend writes both the table + and `customSkills`/`customAgents` for one release, reads from the table, then + stops writing preferences. No server change; the merge is a client concern. +2. **A one-off backfill.** Read every `user_preferences.extra.customSkills` / + `customAgents` entry, POST each to the corresponding endpoint. This is a + **script, not a migration** — it must run through the API so that validation, + parsing and the projections are applied by the code that owns them. Put it in + `scripts/`, beside `oracle.mjs`. +3. Do **not** write a SQL migration that copies jsonb into the tables. It would + have to re-implement `internal/definition`'s parser in SQL to fill + `definition_id`, `name`, `description`, `status`, `version` and `pages`. + +`user_preferences.extra` keys **must not be deleted** by the cutover. Leaving the +old blob in place costs a few kilobytes on `GET /me` and is the entire rollback +plan. + +--- + +## 12. Implementation order + +Seven phases. Phases 1–3 need **no migration and no new route**. Phases 1, 2 and +5 are largely frontend work against a server that is already correct. + +### Phase 1 — Existing API / frontend wiring (no server change) + +| # | Task | Files | Class | +| --- | --- | --- | --- | +| 1.1 | Surface `meta` through `request()` and stop dropping `truncated` | `krow-demo/src/api/httpClient.js:226` and the hooks in `src/lib/krowHooks.js` | WIRE | +| 1.2 | Role-gate the admin console — `role ∈ {admin, employer}` for operator screens, `admin` for KROW Forge authoring and certification deletion | `krow-demo/src/pages/admin/AdminRoute.jsx` | WIRE | +| 1.3 | Delete `useBadges` (`krowHooks.js:537`) and `User: 'users'` (`httpClient.js:96`) — both would 404 | `krow-demo` | WIRE | +| 1.4 | Delete `src/api/store.js` (imported by nothing) | `krow-demo` | WIRE | +| 1.5 | Fix the two documentation drifts | `README.md:106` (51→53), `docs/api-contract.md` §1 (auth) | doc | +| 1.6 | *Optional:* seed the seven `ROLE_CATEGORIES` as rows and delete the constant | `seed/fixtures/seed.json`, `krow-demo/src/lib/roleCategories.js` | seed | + +**Exit criterion:** no frontend call reaches a 404 or 405; a truncated list is +visibly truncated; a non-admin cannot open an admin-only action. + +### Phase 2 — Workflow adoption + +Server first, then frontend, in this order — the frontend cannot adopt Assign +until A-1 lands. + +| # | Task | Files | Class | +| --- | --- | --- | --- | +| 2.1 | `candidate_hired` → `hire_candidate`; `worker_assigned` → `assign_employee` | `go-api/internal/service/workflows.go:212,373` | CHANGE | +| 2.2 | Align the assign `source` default with the column default (`owliver`) | `go-api/internal/service/workflows.go:336` | CHANGE | +| 2.3 | Extend `AssignWorker` with an optional `application:` payload; upsert by `(job_posting_id, email)` inside the transaction; link the assignment | `go-api/internal/service/workflows.go` | CHANGE | +| 2.4 | Add `{"job-applications", domain.OpCreate}` to the assign requirement list | `go-api/internal/httpserver/workflows.go` `handleAssign` (:127) | CHANGE | +| 2.5 | Fix `serverSupplies` to honour `Derived.TalentOnly` (G-4) | `go-api/internal/service/service.go` `serverSupplies` (:266) | CHANGE | +| 2.6 | Link the application from `POST /ai-interviews`, transactionally (G-3) | `go-api/internal/service/service.go` or new `internal/service/interviews.go`; reuse `WorkflowService.inTx` | CHANGE | +| 2.7 | Replace `useHireCandidate` with one call to `POST /job-applications/{id}/hire`, sending `{role, profile_tier}`; drop the separate `logActivity` | `krow-demo/src/lib/krowHooks.js` `useHireCandidate` (:296) | WIRE | +| 2.8 | Replace `useAssignWorkers` with one call to `POST /job-postings/{id}/assignments` | `krow-demo/src/lib/krowHooks.js` `useAssignWorkers` (:406) | WIRE | +| 2.9 | Remove the now-redundant `PATCH` from `AIInterviewModal.finishInterview` | `krow-demo/src/components/krow/AIInterviewModal.jsx` `finishInterview` (:155) | WIRE | + +**Exit criterion:** a hire is one request and one transaction; an assign of *n* +workers is one request and one transaction; a talent user can complete an +interview; `activity.signals` still recognises `hire_candidate` as privileged. + +**No migration. No new route.** + +### Phase 3 — Agent / Skill migration + +| # | Task | Files | Class | +| --- | --- | --- | --- | +| 3.1 | Fix the silent `status` drop on a markdown PATCH (G-5) | `go-api/internal/service/definitions.go:241,415` | CHANGE | +| 3.2 | *Optional:* optimistic concurrency on agent publish (G-6) | `go-api/internal/repo/definitions.go` `UpdateAgent` (:270) | CHANGE | +| 3.3 | Point `useAgents` at `/agent-definitions` — `save`/`remove`/`publish`/`archive`/`restore`/`duplicate`/`sourceFor` | `krow-demo/src/lib/agents/useAgents.js` | WIRE | +| 3.4 | Point `customSkills` readers/writers at `/skill-definitions` — the single writer is `src/lib/skills/customSkills.js`; the readers are `usePageSkills` / `useWorkforcePaths` | `krow-demo/src/lib/skills/{customSkills,usePageSkills}.js` and the 21 consumers | WIRE | +| 3.5 | Dual-write for one release, then stop writing `customSkills` / `customAgents` | `krow-demo` | WIRE | +| 3.6 | Backfill script: read `user_preferences.extra`, POST each definition **through the API** | new `scripts/backfill-definitions.mjs` | script | +| 3.7 | **Leave `disabledSkills` and `removedSkills` in preferences** — migration `000005` says so | — | — | + +**Exit criterion:** an authored agent or skill survives a sign-in on another +device; `GET /me` no longer carries an unbounded definition blob; an +organization-visible definition is visible to colleagues and refused to talent +authors. + +**No migration.** Rollback plan in §11.4 — do not delete the preference keys. + +### Phase 4 — Owliver backend support + +Only after Phase 3, because the server must hold the definitions before it can +answer from them. Incremental by construction (§7.4). + +| # | Task | Files | +| --- | --- | --- | +| 4.1 | Transcribe L1: capabilities (10), data sources (25), section types (9), periods (6), placement→context rules | new `go-api/internal/definition/owliver.go`, `ui.go`; extend `vocabulary.go` | +| 4.2 | Extend `scripts/oracle.mjs` to capture `owliverConfig.js` / `uiConfig.js` output; extend `internal/definition/conformance_test.go` | `scripts/oracle.mjs`, `go-api/internal/definition/` | +| 4.3 | Stop deferring — validate `owliver:` and `ui:` where L1 covers them; keep `Skill.Deferred` for whatever remains | `go-api/internal/definition/skill.go` `deferredBlocks` (:235) | +| 4.4 | L2, one source at a time, reusing `internal/domain/policy.go` scopes and `internal/repo` | new `go-api/internal/service/sources/` | +| 4.5 | Advertise only the capabilities whose sources have a server resolver | `go-api/internal/service/assistant.go` | + +**No migration.** No route until 4.5 has something to answer with. + +### Phase 5 — Dashboard / flow data + +Nothing to do on the server (§8.3, §9.2). This phase exists so the decision is +recorded rather than revisited. + +| # | Task | +| --- | --- | +| 5.1 | Confirm no aggregation endpoint is added. Every dashboard figure is a count/filter/average over one of nine registered lists. | +| 5.2 | Confirm no flow-data endpoint is added. `resolveSkillData` reads the collections `SkillSurface.jsx:81-119` already loads. | +| 5.3 | If a figure is wrong, the cause is Phase 1.1 truncation, not a missing endpoint. Check `meta.truncated` first. | + +### Phase 6 — AI / file / assistant integrations + +Ordered by how much damage the absence is doing. + +| # | Task | Files | New route? | Migration | +| --- | --- | --- | :-: | --- | +| 6.1 | **File storage** — the only capability currently corrupting stored data (`blob:` URLs in `worker_profiles.selfie_url`, `evidence.media_url`) | new `internal/storage/`, `internal/service/files.go`, `internal/repo/files.go`, `internal/httpserver/files.go`; `internal/config/config.go`; `cmd/api/main.go` | **yes — `POST /api/v1/files`** | **`000006_files`** | +| 6.2 | Swap `aiEngine.UploadFile` to call it; the return shape is unchanged | `krow-demo/src/api/aiEngine.js` | no | — | +| 6.3 | **LLM service** with a deterministic stub default | new `internal/llm/`; `internal/config/config.go`; per-call `context.WithTimeout` | **no** | none | +| 6.4 | Use it inside handlers that already exist — screening, interview evaluation | `internal/service/` | no | none | +| 6.5 | **Assistant streaming**, built on `internal/runtime` by replacing `UnavailableExecutor` | new `internal/service/assistant.go`, `internal/httpserver/assistant.go`; `cmd/api/main.go` | **yes — `POST /api/v1/assistant/stream`** | none | +| 6.6 | Set `VITE_ASSISTANT_ENDPOINT` — **not before 6.5 ships**, or working local answers become 404s | `krow-demo` deployment config | no | — | +| 6.7 | **Web search** as an executor tool, `NotConfigured` by default | new `internal/search/`; `internal/config/config.go` | **no** | none | + +**Two new routes total for the whole plan** — 53 → 55. Both are justified above: +one because no handler accepts bytes, one because the client already speaks a +streaming protocol nothing serves. + +### Phase 7 — Tests and integration verification + +| # | Task | Files | +| --- | --- | --- | +| 7.1 | Assert the route count. Nothing does today — `Endpoints()` is only logged (`cmd/api/main.go:65`). Add a test pinning 52/53 so a route cannot be added or lost silently. | `go-api/internal/httpserver/server_test.go` (new) | +| 7.2 | Workflow tests for the Phase 2 changes: assign-creates-application, assign-links-existing, talent completes an interview, event types | `go-api/internal/httpserver/workflows_test.go`, `api_test.go` | +| 7.3 | RBAC matrix regression — every (role, resource, op) triple against `policy.go` | `go-api/internal/httpserver/rbac_test.go` (exists, 730 lines — extend) | +| 7.4 | Conformance suite stays green through Phase 4 L1 | `go-api/internal/definition/conformance_test.go` | +| 7.5 | Definition CRUD: shadow-by-id precedence, visibility immutability, talent refusal, size bound, idempotent delete | `go-api/internal/httpserver/definitions_api_test.go` (exists, 848 lines — extend) | +| 7.6 | Upload tests: size bound, content-type sniffing, tenancy on read | new | +| 7.7 | Resolve TP-1 (§5.4) — needs the product decision first | `internal/domain/policy.go`, `internal/repo/repo.go`, possibly `000007` | +| 7.8 | End-to-end against a seeded database: `make migrate-up && make seed`, then the full hire → assign → interview → evidence loop | `Makefile` targets exist | + +--- + +## 13. Drift found, and where it belongs + +| Where | Issue | Fix in | +| --- | --- | --- | +| `README.md:106` | "51 registered routes" — the router registers **53**; the two workflow routes are missing from the tally | this repo, Phase 1.5 | +| `docs/api-contract.md` §1 | "Auth: None in v1. Every endpoint is unauthenticated." — sessions, argon2id and cookie middleware have existed since `000004` / `internal/auth/` | this repo, Phase 1.5 | +| `docs/api-contract.md` §2 | lists 34 Live + 4 "unreachable today" = the "38 endpoints" figure. It is a table of *entity* rows, not a count of registered routes. | clarify wording, Phase 1.5 | +| `krow-demo/src/api/store.js` | 173 lines, imported by nothing | krow-demo, Phase 1.4 | +| `krow-demo/src/api/seed.js` | 1,928 lines retained for one export, `DEMO_USER.preferences` | krow-demo — leave; the synchronous accessor needs it | +| `krow-demo/src/lib/krowHooks.js:537` | `useBadges`, zero consumers, no route | krow-demo, Phase 1.3 | +| `krow-demo/src/api/httpClient.js:96` | `User: 'users'` names a resource that does not exist | krow-demo, Phase 1.3 | +| `krow-demo/src/pages/admin/AdminRoute.jsx` `AdminRoute` (:23) | comment promises a Phase-3D role check that was never added | krow-demo, Phase 1.2 | + +--- + +## 14. Backend file index — everything this plan would touch + +| File | Phase | Why | +| --- | --- | --- | +| `go-api/internal/service/workflows.go` | 2 | event types (`:212`, `:373`), assign `source` (`:363`), `AssignWorker.application` + upsert | +| `go-api/internal/httpserver/workflows.go` | 2 | add `job-applications:Create` to the assign requirements (`:141-145`) | +| `go-api/internal/service/service.go` | 2 | `serverSupplies` must honour `Derived.TalentOnly` (`:266-276`); transactional `Create` for `ai-interviews` | +| `go-api/internal/service/interviews.go` | 2 | *(new, optional)* holds the interview→application link if it does not belong in `service.go` | +| `go-api/internal/service/definitions.go` | 3 | `status`-with-`markdown` PATCH semantics (`:230`, `:415`) | +| `go-api/internal/repo/definitions.go` | 3 | *(optional)* version predicate on agent update (`:261-305`) | +| `go-api/internal/definition/vocabulary.go` | 4 | extend with the Owliver/UI vocabularies | +| `go-api/internal/definition/owliver.go`, `ui.go` | 4 | *(new)* L1 transcription | +| `go-api/internal/definition/skill.go` | 4 | stop deferring what L1 now covers (`:237`) | +| `go-api/internal/definition/conformance_test.go` | 4 | extend to the new oracle cases | +| `scripts/oracle.mjs` | 4 | capture `owliverConfig.js` / `uiConfig.js` output | +| `go-api/internal/service/sources/` | 4 | *(new)* L2 resolvers | +| `go-api/internal/storage/` | 6 | *(new)* object-store boundary | +| `go-api/internal/service/files.go` | 6 | *(new)* | +| `go-api/internal/repo/files.go` | 6 | *(new)* | +| `go-api/internal/httpserver/files.go` | 6 | *(new)* `POST /api/v1/files` | +| `go-api/internal/llm/` | 6 | *(new)* inference boundary + deterministic stub | +| `go-api/internal/search/` | 6 | *(new)* search boundary + `NotConfigured` default | +| `go-api/internal/service/assistant.go` | 6 | *(new)* built on `internal/runtime` | +| `go-api/internal/httpserver/assistant.go` | 6 | *(new)* `POST /api/v1/assistant/stream` | +| `go-api/internal/runtime/executor.go` | 6 | replace `UnavailableExecutor` | +| `go-api/internal/config/config.go` | 6 | `LLM`, `Storage`, `Search`, `HTTP.MaxUploadBytes` | +| `go-api/cmd/api/main.go` | 6 | wire the new services | +| `go-api/internal/httpserver/server.go` | 6 | register the two new routes; keep `Endpoints()` honest | +| `migrations/000006_files.up.sql` / `.down.sql` | 6 | the only required migration | +| `migrations/000007_worker_profile_claim.*` | 7 | conditional — only if TP-1 is resolved by claiming | +| `go-api/internal/httpserver/server_test.go` | 7 | *(new)* pin the route count | +| `go-api/internal/httpserver/{workflows,api,rbac,definitions_api}_test.go` | 7 | extend | +| `README.md`, `docs/api-contract.md` | 1 | drift | + +--- + +## 15. Summary + +- **53 routes are registered.** 39 are exercised by the frontend, 13 exist and are + ignored, 1 (`GET /health`) is public infrastructure. +- **Nothing the frontend calls today is missing from the server.** +- **12 of the 13 ignored routes should be adopted** — 2 workflow endpoints that + make hire and assign transactional, and 10 definition endpoints that are where + authored agents and skills were always meant to live. The 13th + (`GET /me/preferences`) is correctly unused. +- **Phases 1–3 require no migration and no new route.** They require six + small server changes (§10.5 G-1…G-6) and a body of frontend wiring. +- **Flow-chart data and every dashboard figure are already fully served** by the + nine existing list endpoints. No aggregation endpoint is proposed. The one real + risk is silent truncation, and the server already reports it — the client + discards the report. +- **Four capabilities genuinely do not exist:** LLM inference, file storage, + assistant streaming, web search. **Two new routes** are proposed for them — + `POST /api/v1/files` and `POST /api/v1/assistant/stream` — and **one migration**, + `000006_files`. File storage is first, because it is the only one currently + writing dead references into `worker_profiles` and `evidence`. +- **`internal/runtime` is already built for the assistant** — loader, eligibility, + dependency resolution, executor boundary, 890 lines of tests — and wired to + nothing. It is the seam. Do not build a second one. diff --git a/go-api/internal/authctx/authctx.go b/go-api/internal/authctx/authctx.go index 66da635..18e03f8 100644 --- a/go-api/internal/authctx/authctx.go +++ b/go-api/internal/authctx/authctx.go @@ -28,10 +28,12 @@ var ErrNoIdentity = errors.New("no authenticated identity in context") // Identity is who the request is, as resolved from the session row. // -// Role is carried because Phase 3D will need it, and because carrying it now -// means the middleware reads it once per request instead of every future -// authorization check re-querying the user. It is NOT consulted anywhere in -// Phase 3C: authentication only. +// Role is read once per request by the middleware, out of the user row, so an +// authorization check never has to re-query. It is the authorization authority: +// httpserver.Server.authorize gates operations on it, the repository's ownership +// predicate narrows a talent caller's rows by it, and service/definitions.go +// checks it on every definition write. AccountType is NOT an authority — a user +// can change their own through PATCH /me. type Identity struct { UserID string OrgID string diff --git a/go-api/internal/config/config.go b/go-api/internal/config/config.go index b8d839e..c1f63b9 100644 --- a/go-api/internal/config/config.go +++ b/go-api/internal/config/config.go @@ -259,8 +259,13 @@ func (c *Config) validate() error { // devCORSOrigins are the origins the Vite dev server can occupy. Vite binds // localhost by default and 127.0.0.1 when asked, and a browser treats those two // as different origins, so both are listed. 4173 is `vite preview`. +// 5174 is where Vite lands when 5173 is already taken, which happens whenever a +// second dev server is started; an origin missing from this list is refused at +// the preflight with a bare 403 and no CORS headers, which reads as a server +// fault rather than a misconfigured port. var devCORSOrigins = []string{ "http://localhost:5173", "http://127.0.0.1:5173", + "http://localhost:5174", "http://127.0.0.1:5174", "http://localhost:4173", "http://127.0.0.1:4173", } diff --git a/go-api/internal/httpserver/api.go b/go-api/internal/httpserver/api.go index fd2eb03..7919a8a 100644 --- a/go-api/internal/httpserver/api.go +++ b/go-api/internal/httpserver/api.go @@ -1,6 +1,7 @@ package httpserver import ( + "context" "encoding/json" "io" "net/http" @@ -130,7 +131,7 @@ func (s *Server) handleCreate(svc *service.Service) http.HandlerFunc { writeError(w, s.log, err) return } - rec, err := svc.Create(r.Context(), ident, body) + rec, err := s.create(r.Context(), svc, ident, body) if err != nil { writeError(w, s.log, err) return @@ -139,6 +140,25 @@ func (s *Server) handleCreate(svc *service.Service) http.HandlerFunc { } } +// create inserts through the resource's own service, except where creating a +// record has a consequence in another table. +// +// One resource has one: an AI interview is only half of completing an +// interview, and the application it names has to be linked in the same +// transaction — see internal/service/interviews.go for why the server performs +// that write and the caller may not. Routing it here rather than registering a +// second endpoint keeps POST /api/v1/ai-interviews the only way to write one, +// which is what the client already calls and what the ownership guard already +// covers. +func (s *Server) create(ctx context.Context, svc *service.Service, + ident authctx.Identity, body domain.Record) (domain.Record, error) { + + if svc.Resource().Path == service.InterviewsPath { + return s.workflows.CreateInterview(ctx, ident, body) + } + return svc.Create(ctx, ident, body) +} + func (s *Server) handleUpdate(svc *service.Service) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { ident, ok := s.authorize(w, r, svc, domain.OpUpdate) @@ -174,11 +194,6 @@ func (s *Server) handleDelete(svc *service.Service) http.HandlerFunc { } } -// decodeBody reads a JSON object body. -// -// DisallowUnknownFields is not used — the target is a map, so every field is -// "known" here. Unknown *columns* are rejected in the service, where the -// resource's schema is available to say which those are. // decodeInto reads a JSON body into a typed struct. // // Beside decodeBody rather than replacing it: the resource handlers genuinely @@ -200,6 +215,11 @@ func decodeInto(r *http.Request, dst any) error { return nil } +// decodeBody reads a JSON object body. +// +// DisallowUnknownFields is not used — the target is a map, so every field is +// "known" here. Unknown *columns* are rejected in the service, where the +// resource's schema is available to say which those are. func decodeBody(r *http.Request) (domain.Record, error) { defer func() { _ = r.Body.Close() }() raw, err := io.ReadAll(http.MaxBytesReader(nil, r.Body, maxBodyBytes)) diff --git a/go-api/internal/httpserver/auth.go b/go-api/internal/httpserver/auth.go index b2f891e..7f592f8 100644 --- a/go-api/internal/httpserver/auth.go +++ b/go-api/internal/httpserver/auth.go @@ -42,27 +42,30 @@ const sessionCookieName = "krow_session" // that never authenticate anything. func (s *Server) secureCookies() bool { return s.cfg.AppEnv != "development" } -// sameSite resolves the configured SameSite mode. +// sessionSameSite reports the SameSite mode the session cookie must carry. // -// Lax remains the default and the recommendation. "none" exists for the one -// deployment shape that cannot work without it: a frontend on a different -// registrable domain from the API. In that case Lax withholds the cookie on -// every cross-site fetch, so the sign-in succeeds, the Set-Cookie arrives, and -// the next request carries nothing — which reads as a broken session rather -// than as a cookie policy. +// Lax is the default and the safer value: it closes the CSRF hole by refusing +// to travel on cross-site subresource requests. That is exactly right when the +// page and the API share an origin, which is the supported deployment. // -// An unrecognised value falls back to Lax rather than to None. config.validate -// rejects those before startup, so this is only a belt-and-braces default in -// the safe direction. -func (s *Server) sameSite() http.SameSite { - switch s.cfg.HTTP.CookieSameSite { - case "none": +// When the API is configured with a CORS allowlist, the deployment is by +// definition the other one: a page on some other origin calls this API +// directly. A Lax cookie is never sent on those requests, so login would +// succeed once and every request after it would arrive anonymous. None is the +// only mode a browser will send cross-site, and it requires Secure — which is +// why an origin allowlist forces Secure on regardless of AppEnv. +func (s *Server) sessionSameSite() http.SameSite { + if len(s.cfg.HTTP.CORSOrigins) > 0 { return http.SameSiteNoneMode - case "strict": - return http.SameSiteStrictMode - default: - return http.SameSiteLaxMode } + return http.SameSiteLaxMode +} + +// crossSiteCookies reports whether the cookie must be marked Secure because it +// has to travel cross-site. SameSite=None without Secure is rejected outright +// by every current browser. +func (s *Server) crossSiteCookies() bool { + return s.sessionSameSite() == http.SameSiteNoneMode } // setSessionCookie writes the raw token to the browser. @@ -82,15 +85,13 @@ func (s *Server) setSessionCookie(w http.ResponseWriter, token string, lifetime Path: "/", // HttpOnly: script cannot read it. HttpOnly: true, - // Lax by default, and Strict/None available through - // HTTP_COOKIE_SAMESITE. Strict would drop the cookie on any cross-site - // navigation, so following a link into the app would land on a login - // page despite a live session. None sends it on cross-site requests, - // which is the CSRF hole Lax exists to close — and is nonetheless the - // only workable value when the frontend is on a different registrable - // domain. See Server.sameSite. - SameSite: s.sameSite(), - Secure: s.secureCookies(), + // Lax, not Strict and not None. Strict would drop the cookie on any + // cross-site navigation, so following a link into the app would land on + // a login page despite a live session. None would require Secure and + // would send the cookie on cross-site POSTs, which is the CSRF hole Lax + // exists to close. + SameSite: s.sessionSameSite(), + Secure: s.secureCookies() || s.crossSiteCookies(), MaxAge: int(lifetime.Seconds()), }) } @@ -107,10 +108,8 @@ func (s *Server) clearSessionCookie(w http.ResponseWriter) { Value: "", Path: "/", HttpOnly: true, - // Must match the attributes it was set with, SameSite included, or the - // browser treats this as a different cookie and leaves the original. - SameSite: s.sameSite(), - Secure: s.secureCookies(), + SameSite: s.sessionSameSite(), + Secure: s.secureCookies() || s.crossSiteCookies(), MaxAge: -1, }) } diff --git a/go-api/internal/httpserver/cors.go b/go-api/internal/httpserver/cors.go index 4568d42..68e9308 100644 --- a/go-api/internal/httpserver/cors.go +++ b/go-api/internal/httpserver/cors.go @@ -72,6 +72,12 @@ func cors(origins []string) func(http.Handler) http.Handler { } w.Header().Set("Access-Control-Allow-Origin", origin) + // The frontend sends `credentials: "include"`, and a browser + // discards any response to such a request that does not carry this + // header — preflight included. Safe only because the origin was + // matched exactly above and is echoed back one at a time; "*" is + // never sent, which is the pairing the spec forbids. + w.Header().Set("Access-Control-Allow-Credentials", "true") // Authentication is a cookie, so the browser will neither send it // nor expose the response without this. It is set for allowlisted diff --git a/go-api/internal/httpserver/interviews_test.go b/go-api/internal/httpserver/interviews_test.go new file mode 100644 index 0000000..66eca46 --- /dev/null +++ b/go-api/internal/httpserver/interviews_test.go @@ -0,0 +1,239 @@ +package httpserver_test + +import ( + "context" + "net/http" + "testing" +) + +// Completing an AI interview. +// +// The endpoint is unchanged — POST /api/v1/ai-interviews, the one the modal +// already calls — but finishing an interview is two writes, and the second one +// is a write the caller who most often makes the request may not perform. The +// tests below are about that seam: the interview and the link land together, +// they land for a talent user, and nothing about talent's own permissions has +// widened to make it possible. + +// interviewCount counts the organization's interview rows. +func interviewCount(t *testing.T, r *rbac) int { + t.Helper() + return countRows(t, r, "ai_interviews") +} + +// talentApplication files an application through the API as the talent user, so +// its email is whatever the server derived rather than what a test asked for. +func talentApplication(t *testing.T, r *rbac, who actor) string { + t.Helper() + return mustCreate(t, r, who, "/api/v1/job-applications", map[string]any{ + "job_posting_id": r.activePosting, + "applicant_name": who.name, + }) +} + +func interviewBody(applicationID, postingID string, score any) map[string]any { + body := map[string]any{ + "application_id": applicationID, + "job_posting_id": postingID, + "job_title": "Open Role", + "candidate_name": "Candidate", + "messages": []map[string]any{ + {"role": "assistant", "content": "Tell me about a difficult shift."}, + {"role": "user", "content": "We were two people short and I re-planned the passes."}, + }, + "verdict": "hire", + "hire_recommendation": "Hire", + "summary": "Composed under pressure.", + } + if score != nil { + body["overall_interview_score"] = score + } + return body +} + +/* ── The RBAC break this fixes ──────────────────────────────────────────── */ + +// A talent user completing their own interview is the whole talent flow, and it +// could not finish: ai-interviews:Create is open to everyone, job-applications: +// Update is operators only, so the interview was written and the application +// never learned about it. Both writes now happen server-side, in one +// transaction, on the row the interview already names. +func TestTalentCompletesTheirOwnInterview(t *testing.T) { + r := newRBAC(t) + app := talentApplication(t, r, r.talA) + + got := r.as(r.talA, "POST", "/api/v1/ai-interviews", + interviewBody(app, r.activePosting, 88)) + if got.code != http.StatusCreated { + t.Fatalf("talent interview: got %d, want 201 (%v)", got.code, got.body) + } + interview := got.body["data"].(map[string]any) + interviewID, _ := interview["id"].(string) + if interviewID == "" { + t.Fatalf("the response carries no interview id: %v", got.body) + } + + // The response is still the interview record, unchanged. + if interview["application_id"] != app { + t.Errorf("data.application_id = %v, want %s", interview["application_id"], app) + } + + stored := applicationByID(t, r, app) + if stored["status"] != "interview" { + t.Errorf("application.status = %v, want interview — the analytics count "+ + "status === 'interview' || interview_id", stored["status"]) + } + if stored["interview_id"] != interviewID { + t.Errorf("application.interview_id = %v, want %s", stored["interview_id"], interviewID) + } + if score, ok := stored["ai_score"].(float64); !ok || int(score) != 88 { + t.Errorf("application.ai_score = %v, want the interview's 88", stored["ai_score"]) + } +} + +// And the permission itself has NOT widened. The server writes that one row on +// the caller's behalf; the caller still cannot patch an application. +func TestCompletingAnInterviewDoesNotWidenApplicationUpdate(t *testing.T) { + r := newRBAC(t) + app := talentApplication(t, r, r.talA) + + if got := r.as(r.talA, "POST", "/api/v1/ai-interviews", + interviewBody(app, r.activePosting, 70)); got.code != http.StatusCreated { + t.Fatalf("talent interview: got %d, want 201 (%v)", got.code, got.body) + } + if got := r.as(r.talA, "PATCH", "/api/v1/job-applications/"+app, + map[string]any{"status": "hired"}); got.code != http.StatusForbidden { + t.Fatalf("talent PATCH of their own application: got %d, want 403 (%v)", got.code, got.body) + } +} + +// An operator's interview links the same way. The atomicity half of the fix is +// not talent-specific: a failure between the two writes left an interview +// attached to an application that did not know about it, whoever ran it. +func TestOperatorCompletingAnInterviewLinksTheApplication(t *testing.T) { + r := newRBAC(t) + app := applicationFor(t, r, r.activePosting, "Operator Candidate", "opcand@example.test") + + got := r.as(r.empA, "POST", "/api/v1/ai-interviews", + interviewBody(app, r.activePosting, 64)) + if got.code != http.StatusCreated { + t.Fatalf("operator interview: got %d, want 201 (%v)", got.code, got.body) + } + interviewID := got.body["data"].(map[string]any)["id"].(string) + + stored := applicationByID(t, r, app) + if stored["status"] != "interview" || stored["interview_id"] != interviewID { + t.Errorf("application = status %v, interview_id %v; want interview / %s", + stored["status"], stored["interview_id"], interviewID) + } +} + +/* ── What the link must not do ──────────────────────────────────────────── */ + +// A body that says nothing about the score must not overwrite the screening +// score with the interview column's default of 0. The field the caller never +// mentioned is not a value they asked to store. +func TestInterviewWithoutAScoreLeavesTheApplicationScore(t *testing.T) { + r := newRBAC(t) + app := applicationFor(t, r, r.activePosting, "Scored", "scored@example.test") // ai_score 77 + + got := r.as(r.admin, "POST", "/api/v1/ai-interviews", + interviewBody(app, r.activePosting, nil)) + if got.code != http.StatusCreated { + t.Fatalf("interview: got %d, want 201 (%v)", got.code, got.body) + } + + stored := applicationByID(t, r, app) + if score, ok := stored["ai_score"].(float64); !ok || int(score) != 77 { + t.Errorf("application.ai_score = %v, want the screening score 77 left alone", + stored["ai_score"]) + } + // The status and the link still move — those are what completing an + // interview means. + if stored["status"] != "interview" || stored["interview_id"] == nil { + t.Errorf("application = status %v, interview_id %v; want interview and a link", + stored["status"], stored["interview_id"]) + } +} + +// Somebody else's application is not a subject a talent user may interview for, +// and the refusal must leave nothing behind — not the interview, and not a +// changed application. +func TestInterviewForAnotherPersonsApplicationWritesNothing(t *testing.T) { + r := newRBAC(t) + app := talentApplication(t, r, r.talA) + before := interviewCount(t, r) + + got := r.as(r.talB, "POST", "/api/v1/ai-interviews", + interviewBody(app, r.activePosting, 95)) + if got.code != http.StatusNotFound { + t.Fatalf("interview for another person's application: got %d, want 404 (%v)", + got.code, got.body) + } + if after := interviewCount(t, r); after != before { + t.Errorf("ai_interviews: %d -> %d, want no row", before, after) + } + + stored := applicationByID(t, r, app) + if stored["status"] != "applied" || stored["interview_id"] != nil { + t.Errorf("application = status %v, interview_id %v; want it untouched", + stored["status"], stored["interview_id"]) + } +} + +// An interview that cannot be written must not move the application either. +// Both writes are in one transaction, so a refusal at the first is the whole +// request rolled back rather than a partial completion. +func TestARefusedInterviewLeavesTheApplicationAlone(t *testing.T) { + r := newRBAC(t) + app := applicationFor(t, r, r.activePosting, "Unfinished", "unfinished@example.test") + before := interviewCount(t, r) + + body := interviewBody(app, r.activePosting, 80) + body["verdict"] = "definitely" // outside the interview_verdict enum + + got := r.as(r.admin, "POST", "/api/v1/ai-interviews", body) + if got.code == http.StatusCreated { + t.Fatalf("an invalid verdict was accepted: %v", got.body) + } + if after := interviewCount(t, r); after != before { + t.Errorf("ai_interviews: %d -> %d, want no row", before, after) + } + + stored := applicationByID(t, r, app) + if stored["status"] != "shortlisted" || stored["interview_id"] != nil { + t.Errorf("application = status %v, interview_id %v; want it untouched", + stored["status"], stored["interview_id"]) + } +} + +// Cross-tenant: the application is in another organization, so it is absent +// rather than forbidden, and no interview is written for it. +func TestInterviewCannotReachAnotherOrganizationsApplication(t *testing.T) { + r := newRBAC(t) + app := applicationFor(t, r, r.activePosting, "Ours", "ours-interview@example.test") + + var before int + if err := r.h.Pool.QueryRow(context.Background(), + `SELECT count(*) FROM ai_interviews`).Scan(&before); err != nil { + t.Fatalf("count interviews: %v", err) + } + + got := r.as(r.outsider, "POST", "/api/v1/ai-interviews", + interviewBody(app, r.activePosting, 90)) + if got.code == http.StatusCreated { + t.Fatalf("an outsider wrote an interview for our application: %v", got.body) + } + + var after int + if err := r.h.Pool.QueryRow(context.Background(), + `SELECT count(*) FROM ai_interviews`).Scan(&after); err != nil { + t.Fatalf("count interviews: %v", err) + } + if after != before { + t.Errorf("ai_interviews: %d -> %d, want no row", before, after) + } + if stored := applicationByID(t, r, app); stored["interview_id"] != nil { + t.Errorf("application.interview_id = %v, want it untouched", stored["interview_id"]) + } +} diff --git a/go-api/internal/httpserver/owliver.go b/go-api/internal/httpserver/owliver.go new file mode 100644 index 0000000..6317dec --- /dev/null +++ b/go-api/internal/httpserver/owliver.go @@ -0,0 +1,61 @@ +package httpserver + +import ( + "net/http" + + "github.com/krow/krow-backend/go-api/internal/authctx" + "github.com/krow/krow-backend/go-api/internal/domain" + "github.com/krow/krow-backend/go-api/internal/owliver" +) + +// routeOwliver registers the Owliver panel's suggestion endpoint. +// +// GET, and a query string rather than a body, because the request is a read +// with no side effect and the panel issues one per keystroke: a GET is what +// makes it retryable, cancellable and cacheable by anything in front of it. +// +// It is deliberately NOT on the publicPaths allowlist in auth.go. Which +// readings exist depends on the caller's role, so an anonymous suggestion has +// no meaning — and the allowlist's failure mode is a route that refuses +// everyone, which is the direction this should fall in. +func (s *Server) routeOwliver(mux *http.ServeMux) int { + mux.HandleFunc("GET /api/v1/owliver/suggestions", s.handleOwliverSuggestions) + return 1 +} + +// suggestionsBody is the payload inside the standard data envelope. +// +// An object rather than a bare array, so the response has somewhere to grow — a +// future `truncated` or `context` field would otherwise be a breaking change to +// a client already reading `data` as a list. +type suggestionsBody struct { + Suggestions []owliver.Suggestion `json:"suggestions"` +} + +// handleOwliverSuggestions answers what the caller could usefully ask here. +// +// There is no s.authorize call and no policy lookup in this handler, and that +// is the design rather than an omission: this endpoint exposes no resource, so +// there is no operation to gate. Authorization happens per suggestion, inside +// the catalogue, against the same domain.Policy table every other endpoint +// consults — a reading the caller could not perform is never ranked, so it +// cannot be returned. Authentication is upstream, in the middleware. +func (s *Server) handleOwliverSuggestions(w http.ResponseWriter, r *http.Request) { + ident, err := authctx.MustFrom(r.Context()) + if err != nil { + // Unreachable: the middleware refuses an unauthenticated request before + // the router sees it. A missing identity here is a wiring bug. + writeError(w, s.log, domain.Internal(err)) + return + } + + params, err := s.suggestions.ParseParams(r.URL.Query()) + if err != nil { + writeError(w, s.log, err) + return + } + + writeJSON(w, http.StatusOK, envelope{ + Data: suggestionsBody{Suggestions: s.suggestions.Suggest(ident, params)}, + }) +} diff --git a/go-api/internal/httpserver/owliver_test.go b/go-api/internal/httpserver/owliver_test.go new file mode 100644 index 0000000..8a60c44 --- /dev/null +++ b/go-api/internal/httpserver/owliver_test.go @@ -0,0 +1,315 @@ +package httpserver_test + +import ( + "net/http" + "net/url" + "testing" +) + +// GET /api/v1/owliver/suggestions. +// +// The ranking itself is tested in internal/owliver, against no database and no +// server. What is tested here is only what the HTTP boundary adds: the session +// requirement, the query-string contract, the response envelope, and the fact +// that the role deciding which readings exist is the session's rather than +// anything the caller can set. + +const suggestPath = "/api/v1/owliver/suggestions" + +// suggestURL builds the endpoint's address, escaping as a browser would. +func suggestURL(page, query string) string { + v := url.Values{} + if page != "" { + v.Set("page", page) + } + if query != "" { + v.Set("query", query) + } + return suggestPath + "?" + v.Encode() +} + +// suggestions reads the list out of the data envelope, failing the test if the +// response is not shaped as the contract says. +func suggestions(t *testing.T, r response) []map[string]any { + t.Helper() + if r.code != http.StatusOK { + t.Fatalf("status %d, body %v", r.code, r.body) + } + data, ok := r.body["data"].(map[string]any) + if !ok { + t.Fatalf("data is not an object: %v", r.body) + } + raw, ok := data["suggestions"].([]any) + if !ok { + // json null decodes to nil, and an absent key to nothing at all. Both + // break a client that iterates the list without checking. + t.Fatalf("suggestions is not an array (got %#v)", data["suggestions"]) + } + out := make([]map[string]any, len(raw)) + for i, item := range raw { + entry, ok := item.(map[string]any) + if !ok { + t.Fatalf("suggestion %d is not an object: %#v", i, item) + } + out[i] = entry + } + return out +} + +/* ── Authentication ─────────────────────────────────────────────────────── */ + +// The endpoint is not on the public allowlist. Which readings exist depends on +// who is asking, so an anonymous suggestion has no meaning. +func TestOwliverSuggestionsRequireASession(t *testing.T) { + a := newAPI(t) + + got := a.doAnon("GET", suggestURL("positions", "pipeline"), nil) + if got.code != http.StatusUnauthorized { + t.Fatalf("status %d, want 401", got.code) + } + if code := got.codeOrEmpty(); code != "unauthorized" { + t.Fatalf("error code %q, want unauthorized", code) + } + // A refusal must not describe the catalogue it refused to rank. + if _, present := got.body["data"]; present { + t.Fatalf("an unauthenticated refusal carried data: %v", got.body) + } +} + +/* ── The query string ───────────────────────────────────────────────────── */ + +func TestOwliverSuggestionsValidation(t *testing.T) { + a := newAPI(t) + + cases := []struct { + name string + path string + want int + }{ + {"no page", suggestPath, http.StatusBadRequest}, + {"blank page", suggestPath + "?page=%20", http.StatusBadRequest}, + {"unknown page", suggestURL("nowhere", "pipeline"), http.StatusBadRequest}, + {"a route, not a surface", suggestURL("/admin/positions", "pipeline"), http.StatusBadRequest}, + {"unknown parameter", suggestURL("positions", "pipeline") + "&role=admin", http.StatusBadRequest}, + + // A query is optional: with nothing typed there is nothing to rank, and + // that is an empty list rather than a refusal. + {"no query", suggestURL("positions", ""), http.StatusOK}, + {"page alias", suggestURL("hired", "recent"), http.StatusOK}, + {"page spelled loosely", suggestURL("Talent Pool", "availability"), http.StatusOK}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := a.do("GET", c.path, nil) + if got.code != c.want { + t.Fatalf("status %d, want %d (body %v)", got.code, c.want, got.body) + } + if c.want == http.StatusBadRequest && got.codeOrEmpty() != "invalid_query" { + t.Fatalf("error code %q, want invalid_query", got.codeOrEmpty()) + } + }) + } +} + +// The mux answers anything but GET, so the endpoint cannot be reached with a +// body that might carry a page, a role or an identity. +func TestOwliverSuggestionsAreReadOnly(t *testing.T) { + a := newAPI(t) + for _, method := range []string{"POST", "PATCH", "DELETE", "PUT"} { + got := a.do(method, suggestURL("positions", "pipeline"), map[string]any{"page": "positions"}) + if got.code != http.StatusMethodNotAllowed { + t.Errorf("%s: status %d, want 405", method, got.code) + } + } +} + +/* ── The response ───────────────────────────────────────────────────────── */ + +func TestOwliverSuggestionsResponseShape(t *testing.T) { + a := newAPI(t) // the seeded user is an admin + + got := suggestions(t, a.do("GET", suggestURL("positions", "pipeline"), nil)) + if len(got) == 0 { + t.Fatal("pipeline on positions returned nothing") + } + if len(got) > 3 { + t.Fatalf("%d suggestions, the cap is 3", len(got)) + } + + seenIntent, seenText := map[string]bool{}, map[string]bool{} + for i, s := range got { + text, _ := s["text"].(string) + intent, _ := s["intent"].(string) + if text == "" || intent == "" { + t.Fatalf("suggestion %d is incomplete: %v", i, s) + } + if seenIntent[intent] { + t.Fatalf("duplicate intent %q", intent) + } + if seenText[text] { + t.Fatalf("duplicate text %q", text) + } + seenIntent[intent], seenText[text] = true, true + + // Nothing internal may ride along: no terms, no resource names, no + // scores, no page keys. + for key := range s { + switch key { + case "text", "intent", "capability": + default: + t.Fatalf("suggestion %d exposes %q: %v", i, key, s) + } + } + } +} + +// Asking for a rendering names it in the answer — and only where the reading +// can actually be drawn that way. +func TestOwliverSuggestionsCarryARequestedShape(t *testing.T) { + a := newAPI(t) + + got := suggestions(t, a.do("GET", suggestURL("positions", "show hiring activity as a flow"), nil)) + if len(got) != 1 { + t.Fatalf("got %d suggestions, want 1: %v", len(got), got) + } + if got[0]["intent"] != "hiring-operations" || got[0]["capability"] != "flow" { + t.Fatalf("got %v", got[0]) + } + + // With no shape asked for, the field is absent rather than empty. + plain := suggestions(t, a.do("GET", suggestURL("positions", "draft"), nil)) + if len(plain) == 0 { + t.Fatal("draft on positions returned nothing") + } + if _, present := plain[0]["capability"]; present { + t.Fatalf("capability was sent for an unshaped query: %v", plain[0]) + } +} + +// No match is an empty array, not an error and not null. +func TestOwliverSuggestionsEmptyResults(t *testing.T) { + a := newAPI(t) + + for _, c := range []struct{ name, query string }{ + {"nothing typed", ""}, + {"one character", "p"}, + {"irrelevant", "sourdough starter recipe"}, + } { + t.Run(c.name, func(t *testing.T) { + if got := suggestions(t, a.do("GET", suggestURL("positions", c.query), nil)); len(got) != 0 { + t.Fatalf("got %v, want none", got) + } + }) + } + + // A real surface the catalogue holds no readings for is the same answer. + if got := suggestions(t, a.do("GET", suggestURL("settings", "owliver"), nil)); len(got) != 0 { + t.Fatalf("settings returned %v", got) + } +} + +// The page decides the answer, so the same word must not produce the same list +// everywhere. +func TestOwliverSuggestionsAreScopedToThePage(t *testing.T) { + a := newAPI(t) + + read := func(page string) []string { + out := []string{} + for _, s := range suggestions(t, a.do("GET", suggestURL(page, "pipeline"), nil)) { + out = append(out, s["intent"].(string)) + } + return out + } + + positions, candidates := read("positions"), read("candidates") + if len(positions) == 0 || len(candidates) == 0 { + t.Fatalf("positions=%v candidates=%v", positions, candidates) + } + if len(positions) == len(candidates) { + same := true + for i := range positions { + if positions[i] != candidates[i] { + same = false + break + } + } + if same { + t.Fatalf("both pages answered pipeline with %v", positions) + } + } +} + +/* ── Authorization ──────────────────────────────────────────────────────── */ + +// Who is asking comes from the session, and it decides which readings exist. +// +// Talent may list job applications — but only their own, by a predicate in the +// repository — so the organization-wide readings the operator console offers +// are not theirs, and are absent rather than refused. +func TestOwliverSuggestionsFollowTheCallersRole(t *testing.T) { + r := newRBAC(t) + + // A query each page can actually answer, so an empty list means the role + // was filtered rather than that the words matched nothing. + operatorPages := []struct{ page, query string }{ + {"control-center", "pipeline attention"}, + {"positions", "pipeline attention"}, + {"candidates", "candidate score"}, + {"hired-history", "recent hires outcomes"}, + {"talent-pool", "talent pool availability"}, + {"activity", "audit unusual activity"}, + } + + for _, c := range operatorPages { + for _, act := range []actor{r.admin, r.empA} { + got := suggestions(t, r.as(act, "GET", suggestURL(c.page, c.query), nil)) + if len(got) == 0 { + t.Errorf("%s was offered nothing on %s for %q", act.name, c.page, c.query) + } + } + + if got := suggestions(t, r.as(r.talA, "GET", suggestURL(c.page, c.query), nil)); len(got) != 0 { + t.Errorf("talent was offered %v on %s", got, c.page) + } + } + + // Still 200 with an empty list, never 403: refusing would tell a caller + // which pages hold readings they cannot have. + refused := r.as(r.talA, "GET", suggestURL("positions", "pipeline"), nil) + if refused.code != http.StatusOK { + t.Fatalf("talent got status %d, want 200 with an empty list", refused.code) + } +} + +// The role filter is not a blanket refusal for talent: what they may genuinely +// ask — about their own account — is still offered. Without this, the test +// above would pass with the permission check stubbed out to deny everything. +func TestOwliverSuggestionsStillServeTalentTheirOwnReadings(t *testing.T) { + r := newRBAC(t) + + got := suggestions(t, r.as(r.talA, "GET", suggestURL("profile", "permission"), nil)) + if len(got) == 0 { + t.Fatal("talent was offered nothing about their own account") + } + if got[0]["intent"] != "profile-permissions" { + t.Fatalf("got %v", got[0]) + } +} + +// A permission-sensitive reading: Hired History reads the staff table, which +// policy.go grants to operators only. Nothing about the request differs — only +// the session behind it. +func TestOwliverSuggestionsHideReadingsARoleCannotPerform(t *testing.T) { + r := newRBAC(t) + const path = suggestPath + "?page=hired-history&query=recent+hires" + + for _, act := range []actor{r.admin, r.empA} { + if got := suggestions(t, r.as(act, "GET", path, nil)); len(got) == 0 { + t.Errorf("%s was offered no hiring outcomes", act.name) + } + } + if got := suggestions(t, r.as(r.talA, "GET", path, nil)); len(got) != 0 { + t.Fatalf("talent was offered readings of the staff table: %v", got) + } +} diff --git a/go-api/internal/httpserver/rbac_test.go b/go-api/internal/httpserver/rbac_test.go index e25a58d..8b81ac7 100644 --- a/go-api/internal/httpserver/rbac_test.go +++ b/go-api/internal/httpserver/rbac_test.go @@ -314,6 +314,50 @@ func mustCreate(t *testing.T, r *rbac, act actor, path string, body map[string]a /* ── 3. Mass assignment ─────────────────────────────────────────────────── */ +// The other half of a talent-only derivation: what an OPERATOR must supply. +// +// The server fills these columns from the session for a talent caller and for +// nobody else — an operator filing an application or logging evidence is +// writing about somebody who is not them. Treating the column as +// server-supplied for every role let an operator's request past validation and +// into SQL, where it came back as a not-null violation instead of the +// required-field message the contract promises. The two halves have to agree: +// what the repository will derive, and what validation stops asking for. +func TestTalentOnlyDerivedFieldsAreRequiredOfOperators(t *testing.T) { + r := newRBAC(t) + + cases := []struct { + name, path, column string + body map[string]any + }{ + {"job_applications.email", "/api/v1/job-applications", "email", + map[string]any{"job_posting_id": r.activePosting, "applicant_name": "Nameless"}}, + {"evidence.worker_email", "/api/v1/evidence", "worker_email", + map[string]any{"type": "photo_identify"}}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := r.as(r.admin, "POST", tc.path, tc.body) + if got.code != http.StatusUnprocessableEntity { + t.Fatalf("operator create without %s: got %d, want 422 (%v)", + tc.column, got.code, got.body) + } + details, _ := got.body["error"].(map[string]any)["details"].(map[string]any) + if details[tc.column] != "required" { + t.Errorf("details = %v, want %s: required", details, tc.column) + } + + // The same body from a talent caller is complete, because the + // server is about to fill the column in from their session. + if got := r.as(r.talA, "POST", tc.path, tc.body); got.code != http.StatusCreated { + t.Errorf("talent create without %s: got %d, want 201 (%v)", + tc.column, got.code, got.body) + } + }) + } +} + // Identity a caller supplies is ignored; identity the server derives wins. // // This is the test that makes the ownership predicates above mean anything. If diff --git a/go-api/internal/httpserver/server.go b/go-api/internal/httpserver/server.go index 937653b..7fd1abd 100644 --- a/go-api/internal/httpserver/server.go +++ b/go-api/internal/httpserver/server.go @@ -1,17 +1,24 @@ // Package httpserver holds the HTTP surface. // // It serves /health, the sign-in endpoints, the entity endpoints described in -// docs/api-contract.md, and the current-user endpoints. +// docs/api-contract.md, the current-user endpoints, the agent and skill +// definition endpoints, and the Owliver panel's suggestion endpoint. // -// Phase 3C replaced the development identity with real authentication. Every -// request outside the small public allowlist in auth.go must carry a session -// cookie; the middleware resolves it to a user row and puts that user, and -// their organization, on the request context. Nothing downstream changed — -// every service and repository already took the organization as a parameter, -// which is what devOrgMiddleware existed to make true. +// Authentication replaced the development identity: every request outside the +// small public allowlist in auth.go must carry a session cookie; the middleware +// resolves it to a user row and puts that user, and their organization, on the +// request context. Nothing downstream changed — every service and repository +// already took the organization as a parameter, which is what devOrgMiddleware +// existed to make true. // -// Authorization is NOT here. A signed-in user reaches every endpoint they could -// reach before; deciding which roles may do what is Phase 3D. +// Authorization is here, in Server.authorize: it reads the role off the +// authenticated identity, consults the deny-by-default policy table in +// internal/domain/policy.go, and answers 403 before any query runs. Row +// visibility — organization scope, and ownership for talent callers — is a SQL +// predicate in internal/repo instead, so an invisible row answers 404 rather +// than 403. The definition endpoints are the exception: they are not +// domain.Resource values, so their role checks are written inline in +// internal/service/definitions.go rather than in the policy table. package httpserver import ( @@ -38,6 +45,7 @@ type Server struct { api *service.Registry definitions *service.DefinitionsService workflows *service.WorkflowService + suggestions *service.SuggestionsService log *slog.Logger http *http.Server started time.Time @@ -126,6 +134,7 @@ func New(cfg *config.Config, database *db.DB, log *slog.Logger, opts ...Option) api: service.NewRegistry(database.Pool), definitions: service.NewDefinitions(database.Pool), workflows: service.NewWorkflows(database.Pool).WithClock(o.now), + suggestions: service.NewSuggestions(), started: o.now(), sessions: sessions, users: users, @@ -138,7 +147,7 @@ func New(cfg *config.Config, database *db.DB, log *slog.Logger, opts ...Option) mux := http.NewServeMux() mux.HandleFunc("GET /health", s.handleHealth) s.endpoints = s.routeAuth(mux) + s.routeResources(mux) + s.routeMe(mux) + - s.routeDefinitions(mux) + s.routeWorkflows(mux) + s.routeDefinitions(mux) + s.routeWorkflows(mux) + s.routeOwliver(mux) handler := jsonErrors(mux) // Authentication sits where devOrgMiddleware used to, so every route below diff --git a/go-api/internal/httpserver/workflows.go b/go-api/internal/httpserver/workflows.go index 340f49b..9fbe05f 100644 --- a/go-api/internal/httpserver/workflows.go +++ b/go-api/internal/httpserver/workflows.go @@ -128,6 +128,11 @@ func (s *Server) handleAssign(w http.ResponseWriter, r *http.Request) { ident, ok := s.authorizeAll(w, r, requirement{"assignments", domain.OpCreate}, requirement{"job-applications", domain.OpUpdate}, + // The workflow may now FILE an application as well as patch one, for a + // worker placed on a posting they never applied to. A write the handler + // performs has to appear in the list it is authorized against, even + // when — as here — the resulting permission set is unchanged. + requirement{"job-applications", domain.OpCreate}, requirement{"user-activity", domain.OpCreate}, ) if !ok { diff --git a/go-api/internal/httpserver/workflows_test.go b/go-api/internal/httpserver/workflows_test.go index 80d3adf..6b725d9 100644 --- a/go-api/internal/httpserver/workflows_test.go +++ b/go-api/internal/httpserver/workflows_test.go @@ -329,3 +329,326 @@ func TestWorkflowEndpointsRequireASession(t *testing.T) { } } } + +/* ── Activity vocabulary ────────────────────────────────────────────────── */ + +// activityTypes returns the event types written about one worker, newest first. +// +// Read straight from the table rather than through GET /user-activity so the +// assertion is about what was STORED. The frontend's anomaly detection reads +// these strings — PRIVILEGED_EVENTS is ['hire_candidate', 'create_position'] — +// and a value the vocabulary does not contain is not a different label, it is +// an event that silently stops counting. +func activityTypes(t *testing.T, r *rbac, workerEmail string) []string { + t.Helper() + rows, err := r.h.Pool.Query(context.Background(), + `SELECT event_type FROM user_activity + WHERE org_id = $1::uuid AND worker_email = $2::citext + ORDER BY created_date DESC, id DESC`, r.orgID, workerEmail) + if err != nil { + t.Fatalf("read user_activity: %v", err) + } + defer rows.Close() + var out []string + for rows.Next() { + var s string + if err := rows.Scan(&s); err != nil { + t.Fatalf("scan user_activity: %v", err) + } + out = append(out, s) + } + if err := rows.Err(); err != nil { + t.Fatalf("read user_activity: %v", err) + } + return out +} + +func TestHireWritesTheFrontendsActivityEvent(t *testing.T) { + r := newRBAC(t) + app := applicationFor(t, r, r.activePosting, "Evented", "evented@example.test") + + if got := r.as(r.admin, "POST", "/api/v1/job-applications/"+app+"/hire", + map[string]any{}); got.code != http.StatusCreated { + t.Fatalf("hire: got %d, want 201 (%v)", got.code, got.body) + } + + events := activityTypes(t, r, "evented@example.test") + if len(events) != 1 || events[0] != "hire_candidate" { + t.Errorf("activity = %v, want exactly [hire_candidate] — the vocabulary "+ + "activitySignals.js reads, and the one the seed fixture uses", events) + } +} + +func TestAssignWritesTheFrontendsActivityEvent(t *testing.T) { + r := newRBAC(t) + if got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", + map[string]any{"workers": []map[string]any{ + {"worker_email": "evented-assign@example.test", "worker_name": "Evented", + "starts_at": "2026-09-01T09:00:00Z"}, + }}); got.code != http.StatusCreated { + t.Fatalf("assign: got %d, want 201 (%v)", got.code, got.body) + } + + events := activityTypes(t, r, "evented-assign@example.test") + if len(events) != 1 || events[0] != "assign_employee" { + t.Errorf("activity = %v, want exactly [assign_employee]", events) + } +} + +/* ── Assign: source ─────────────────────────────────────────────────────── */ + +// An unspecified source must mean what the column says it means. The default in +// 000001 is `owliver` and the frontend sends `owliver`; substituting `manual` +// made a row written through this endpoint disagree with a row written through +// POST /assignments about where the same action came from. +func TestAssignDefaultsSourceToTheColumnDefault(t *testing.T) { + r := newRBAC(t) + got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", + map[string]any{"workers": []map[string]any{ + {"worker_email": "default-source@example.test", "starts_at": "2026-09-01T09:00:00Z"}, + {"worker_email": "explicit-source@example.test", "starts_at": "2026-09-01T09:00:00Z", + "source": "manual"}, + }}) + if got.code != http.StatusCreated { + t.Fatalf("assign: got %d, want 201 (%v)", got.code, got.body) + } + + created := assignmentsOf(t, got) + if created[0]["source"] != "owliver" { + t.Errorf("source = %v, want owliver", created[0]["source"]) + } + // An explicit value is still the caller's. + if created[1]["source"] != "manual" { + t.Errorf("source = %v, want the supplied manual", created[1]["source"]) + } +} + +// assignmentsOf reads the assignment records out of an assign response. +func assignmentsOf(t *testing.T, got response) []map[string]any { + t.Helper() + data, _ := got.body["data"].(map[string]any) + raw, _ := data["assignments"].([]any) + if raw == nil { + t.Fatalf("response carries no assignments: %v", got.body) + } + out := make([]map[string]any, 0, len(raw)) + for _, rec := range raw { + out = append(out, rec.(map[string]any)) + } + return out +} + +// applicationByID reads one application as an operator, or fails. +func applicationByID(t *testing.T, r *rbac, id string) map[string]any { + t.Helper() + list := r.as(r.admin, "GET", "/api/v1/job-applications?limit=500", nil) + if list.code != http.StatusOK { + t.Fatalf("list applications: %d (%v)", list.code, list.body) + } + for _, raw := range list.body["data"].([]any) { + rec := raw.(map[string]any) + if rec["id"] == id { + return rec + } + } + t.Fatalf("application %s not found", id) + return nil +} + +/* ── Assign: the application a worker does not have yet ─────────────────── */ + +// The behaviour the frontend had and the endpoint did not. +// +// An application is what puts a person in the pipeline for a role: the +// candidate record is addressed by it and an interview takes one as its +// subject. A worker assigned from the talent pool has none, so the endpoint has +// to file one — in the same transaction as the assignment, which is the half +// the frontend could not do. +func TestAssignCreatesTheApplicationItNeeds(t *testing.T) { + r := newRBAC(t) + appsBefore := countRows(t, r, "job_applications") + + got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", + map[string]any{"workers": []map[string]any{{ + "worker_email": "from-pool@example.test", + "worker_name": "Pool Worker", + "starts_at": "2026-09-01T09:00:00Z", + "match_score": 88, + "application": map[string]any{ + "job_title": "Open Role", + "phone": "555-0100", + "years_experience": 4, + "skills": []string{"service", "bar"}, + "professional_summary": "Placed from the talent pool.", + "ai_score": 88, + }, + }}}) + if got.code != http.StatusCreated { + t.Fatalf("assign: got %d, want 201 (%v)", got.code, got.body) + } + + if after := countRows(t, r, "job_applications"); after != appsBefore+1 { + t.Fatalf("job_applications: %d -> %d, want exactly one more", appsBefore, after) + } + + assignment := assignmentsOf(t, got)[0] + linked, _ := assignment["application_id"].(string) + if linked == "" { + t.Fatal("the assignment was not linked to the application that was created for it") + } + + app := applicationByID(t, r, linked) + if app["status"] != "assigned" { + t.Errorf("application.status = %v, want assigned", app["status"]) + } + if app["email"] != "from-pool@example.test" { + t.Errorf("application.email = %v, want the worker's email", app["email"]) + } + if app["job_posting_id"] != r.activePosting { + t.Errorf("application.job_posting_id = %v, want the posting being assigned to", app["job_posting_id"]) + } + // applicant_name falls back to the worker's name rather than being blank — + // the column has a not-blank check. + if app["applicant_name"] != "Pool Worker" { + t.Errorf("application.applicant_name = %v, want the worker's name", app["applicant_name"]) + } + if app["phone"] != "555-0100" { + t.Errorf("application.phone = %v, want the supplied phone", app["phone"]) + } + if score, ok := app["ai_score"].(float64); !ok || int(score) != 88 { + t.Errorf("application.ai_score = %v, want 88", app["ai_score"]) + } + + // The audit entry names the application, so the feed can open it. + var activityApp *string + if err := r.h.Pool.QueryRow(context.Background(), + `SELECT application_id::text FROM user_activity + WHERE org_id = $1::uuid AND worker_email = $2::citext`, + r.orgID, "from-pool@example.test").Scan(&activityApp); err != nil { + t.Fatalf("read the activity entry: %v", err) + } + if activityApp == nil || *activityApp != linked { + t.Errorf("activity.application_id = %v, want %s", activityApp, linked) + } +} + +// (job_posting_id, email) is UNIQUE, so the second assign of the same person to +// the same posting must find the application rather than try to file another — +// and the comparison is case-insensitive, because the column is citext and the +// frontend's own lookup lowercased both sides. +func TestAssignLinksAnExistingApplicationInsteadOfDuplicating(t *testing.T) { + r := newRBAC(t) + existing := applicationFor(t, r, r.activePosting, "Already Applied", "Already.Applied@example.test") + + appsBefore := countRows(t, r, "job_applications") + got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", + map[string]any{"workers": []map[string]any{{ + "worker_email": "already.applied@example.test", + "worker_name": "Already Applied", + "starts_at": "2026-09-01T09:00:00Z", + "application": map[string]any{"job_title": "Open Role"}, + }}}) + if got.code != http.StatusCreated { + t.Fatalf("assign: got %d, want 201 (%v)", got.code, got.body) + } + + if after := countRows(t, r, "job_applications"); after != appsBefore { + t.Errorf("job_applications: %d -> %d, want no new row for a person who already applied", + appsBefore, after) + } + if linked := assignmentsOf(t, got)[0]["application_id"]; linked != existing { + t.Errorf("assignment.application_id = %v, want the existing application %s", linked, existing) + } + if app := applicationByID(t, r, existing); app["status"] != "assigned" { + t.Errorf("application.status = %v, want assigned", app["status"]) + } +} + +// An id the caller already has still wins over the payload: it is a decision +// they have made, and honouring the payload instead could file a second +// application for the same placement. +func TestAssignPrefersTheSuppliedApplicationID(t *testing.T) { + r := newRBAC(t) + app := applicationFor(t, r, r.activePosting, "Named", "named@example.test") + appsBefore := countRows(t, r, "job_applications") + + got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", + map[string]any{"workers": []map[string]any{{ + "worker_email": "named@example.test", + "worker_name": "Named", + "starts_at": "2026-09-01T09:00:00Z", + "application_id": app, + "application": map[string]any{"applicant_name": "Ignored"}, + }}}) + if got.code != http.StatusCreated { + t.Fatalf("assign: got %d, want 201 (%v)", got.code, got.body) + } + if after := countRows(t, r, "job_applications"); after != appsBefore { + t.Errorf("job_applications: %d -> %d, want no new row", appsBefore, after) + } + if linked := assignmentsOf(t, got)[0]["application_id"]; linked != app { + t.Errorf("assignment.application_id = %v, want %s", linked, app) + } + if stored := applicationByID(t, r, app); stored["applicant_name"] != "Named" { + t.Errorf("applicant_name = %v — the payload overwrote a named application", + stored["applicant_name"]) + } +} + +// No payload, no application. A worker placed straight from the workforce is +// legitimate, and one must not be invented for them. +func TestAssignWithoutAnApplicationPayloadLinksNothing(t *testing.T) { + r := newRBAC(t) + appsBefore := countRows(t, r, "job_applications") + + got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", + map[string]any{"workers": []map[string]any{ + {"worker_email": "unattached@example.test", "worker_name": "Unattached", + "starts_at": "2026-09-01T09:00:00Z"}, + }}) + if got.code != http.StatusCreated { + t.Fatalf("assign: got %d, want 201 (%v)", got.code, got.body) + } + if after := countRows(t, r, "job_applications"); after != appsBefore { + t.Errorf("job_applications: %d -> %d, want no application invented", appsBefore, after) + } + if linked := assignmentsOf(t, got)[0]["application_id"]; linked != nil { + t.Errorf("assignment.application_id = %v, want null", linked) + } +} + +// The application payload is validated exactly as POST /job-applications would +// validate it, and the batch element that produced the complaint is named. +func TestAssignValidatesTheApplicationPayload(t *testing.T) { + r := newRBAC(t) + assignBefore := countRows(t, r, "assignments") + appsBefore := countRows(t, r, "job_applications") + + got := r.as(r.admin, "POST", "/api/v1/job-postings/"+r.activePosting+"/assignments", + map[string]any{"workers": []map[string]any{ + {"worker_email": "good@example.test", "worker_name": "Good", + "starts_at": "2026-09-01T09:00:00Z", + "application": map[string]any{"job_title": "Open Role"}}, + {"worker_email": "bad@example.test", "worker_name": "Bad", + "starts_at": "2026-09-01T09:00:00Z", + "application": map[string]any{"english_level": "telepathic"}}, + }}) + if got.code != http.StatusUnprocessableEntity { + t.Fatalf("assign with an invalid application: got %d, want 422 (%v)", got.code, got.body) + } + details, _ := got.body["error"].(map[string]any)["details"].(map[string]any) + if details["workers[1].english_level"] == nil { + t.Errorf("details = %v, want the failure attributed to workers[1]", details) + } + + // And the first worker — whose application WAS filed before the second + // failed — must be gone with it. + if after := countRows(t, r, "job_applications"); after != appsBefore { + t.Errorf("job_applications: %d -> %d — a rejected batch left an application behind", + appsBefore, after) + } + if after := countRows(t, r, "assignments"); after != assignBefore { + t.Errorf("assignments: %d -> %d — a rejected batch left an assignment behind", + assignBefore, after) + } +} diff --git a/go-api/internal/orgctx/orgctx.go b/go-api/internal/orgctx/orgctx.go index 067ea67..61362ee 100644 --- a/go-api/internal/orgctx/orgctx.go +++ b/go-api/internal/orgctx/orgctx.go @@ -1,14 +1,17 @@ // Package orgctx carries the organization a request operates on. // -// Phase 2C has no authentication, so there is nothing to derive a tenant from. -// Rather than defaulting org_id deep inside the SQL — where it would have to be -// unpicked from fourteen repositories once auth arrives — the value is put on -// the request context by one middleware and threaded explicitly through the -// service and repository boundaries. +// It predates authentication. Before there was a session to derive a tenant +// from, org_id was put on the request context by one middleware rather than +// defaulted deep inside the SQL, so that it would not have to be unpicked from +// fourteen repositories later. That handover has since happened: the +// authenticate middleware in httpserver reads the organization off the user's +// row, and authctx.Identity.OrgID is what the service and repository layers +// actually read. // -// When authentication lands, DevMiddleware is replaced by one that reads the -// organization off the authenticated session. Nothing below this package -// changes: every caller already takes an org id as a parameter. +// What is still load-bearing here is DevOrgSlug and DevOrgName, which name the +// organization the seeder creates. With and From remain the read/write pair for +// the context key the middleware still sets; retiring them in favour of +// authctx.Identity.OrgID alone is a deliberate change, not a tidy-up. package orgctx import ( diff --git a/go-api/internal/owliver/catalog.go b/go-api/internal/owliver/catalog.go new file mode 100644 index 0000000..13c8496 --- /dev/null +++ b/go-api/internal/owliver/catalog.go @@ -0,0 +1,625 @@ +// Package owliver answers "what could I usefully ask here?". +// +// It is the suggestion side of the Owliver panel and nothing else. It does not +// answer questions, does not reach a database, does not call a model, and holds +// no state: a request names a page and what the user has typed so far, and this +// package ranks a static catalogue against it. That is deliberate — the panel +// calls the endpoint on every keystroke, so the work per call has to be a few +// string comparisons over a table built once at process start. +// +// WHERE THE CATALOGUE COMES FROM. Owliver's capabilities are declared in the +// frontend, one manifest per page context, in +// src/components/ai-assistant/capabilities/. Each entry there is an id, a label +// and the function that answers it. This file transcribes the id and the page +// it is offered on, and adds the two things a manifest does not carry because +// the browser never needed them: the words that mean a user is reaching for +// that reading, and the records it reads. +// +// Transcription rather than a second registry, on the pattern already set by +// internal/definition/vocabulary.go: the frontend owns the vocabulary, this +// side names keys from it, and TestIntentIDsAreFrontendCapabilities keeps the +// two honest. An Intent.ID that no manifest declares is a bug here, not a new +// capability — the id in a response is the id the panel dispatches on. +// +// WHAT MAKES A SUGGESTION SAFE TO SHOW. Offering someone a question is a claim +// that they could ask it. Reads is that claim written down: the resources the +// capability actually reads, checked against the one authorization table in +// internal/domain/policy.go. No role list is repeated here, and no rule is +// re-derived — a capability that reads staff is unavailable to talent because +// policy.go says talent may not list staff, and for no other reason. +package owliver + +import "github.com/krow/krow-backend/go-api/internal/domain" + +// Need is one record set a capability reads, and how much of it. +// +// OrgWide is the half a role check alone cannot express. Every role may list +// job applications; a talent caller sees only their own, because the policy +// attaches a row predicate rather than a refusal. So "which position has the +// strongest pipeline?" is not forbidden to talent — it is unanswerable for +// them, and offering it would promise a reading across records they will never +// be shown. OrgWide asks policy.ScopeFor for exactly that: is this caller's +// view of this resource narrowed, or is it the whole organization? +type Need struct { + Resource string // domain resource path, e.g. "job-applications" + Op domain.Op + OrgWide bool +} + +// permitted reports whether a role may perform this reading. +// +// Three gates, all sourced from the descriptors: the API must expose the +// operation at all, the policy must allow the role, and — for an org-wide +// reading — the policy must not narrow the role's rows. +func (n Need) permitted(role domain.Role) bool { + res, ok := domain.ResourceByPath[n.Resource] + if !ok || !res.Supports(n.Op) { + return false + } + if !res.Policy.Allows(n.Op, role) { + return false + } + if n.OrgWide && res.Policy.ScopeFor(role).Kind != domain.ScopeNone { + return false + } + return true +} + +// Intent is one reading Owliver can offer on one page. +type Intent struct { + // ID is the frontend capability id, verbatim. It is what the panel + // dispatches on, which is why it is not invented here. + ID string + + // Text is the suggestion as the user reads it: a question in their words, + // not a label. Unique within a page. + Text string + + // Subject names what the reading is about — "hiring activity", "the + // candidate pipeline" — and exists so a shaped variant can be phrased + // ("Summarize hiring activity", "Show hiring activity as a flow") without + // writing every combination out. An intent with no Subject is offered only + // as its Text. + Subject string + + // Shapes are the section types this reading can be drawn as, named from the + // closed OWLIVER_CAPABILITIES vocabulary in src/lib/skills/surfaces.js. + // Empty means prose only. `summary` is never listed: it has no component, + // so it applies to anything with a Subject. + Shapes []string + + // Terms are the words that mean a user is reaching for this reading. About + // the subject, never about the shape — "as a flow" belongs to the shape + // table, and putting it here would offer every page's intents to anyone who + // typed "chart". + Terms []string + + // Reads is what the capability actually reads. Empty means it reads nothing + // but the caller's own account, and is therefore available to anyone signed + // in. + Reads []Need +} + +// permitted reports whether a role may be offered this intent. Every reading +// must be permitted: a suggestion that is half-answerable is not answerable. +func (i Intent) permitted(role domain.Role) bool { + for _, n := range i.Reads { + if !n.permitted(role) { + return false + } + } + return true +} + +/* ── The readings each resource stands for ──────────────────────────────── */ +// +// Named rather than repeated so that "this capability reads the organization's +// applications" is written once and reads the same everywhere it appears. + +var ( + postings = Need{Resource: "job-postings", Op: domain.OpList, OrgWide: true} + applications = Need{Resource: "job-applications", Op: domain.OpList, OrgWide: true} + interviews = Need{Resource: "ai-interviews", Op: domain.OpList, OrgWide: true} + profiles = Need{Resource: "worker-profiles", Op: domain.OpList, OrgWide: true} + staff = Need{Resource: "staff", Op: domain.OpList, OrgWide: true} + orgActivity = Need{Resource: "user-activity", Op: domain.OpList, OrgWide: true} + courses = Need{Resource: "courses", Op: domain.OpList, OrgWide: true} + certs = Need{Resource: "certifications", Op: domain.OpList, OrgWide: true} + evidence = Need{Resource: "evidence", Op: domain.OpList, OrgWide: true} + + // The caller's own audit trail rather than the organization's — the Profile + // page reads what *you* did, which every role may do for themselves. + ownActivity = Need{Resource: "user-activity", Op: domain.OpList} +) + +/* ── The catalogue ──────────────────────────────────────────────────────── */ + +// catalogue is every intent, keyed by canonical page id. +// +// Keys are canonical SKILL_SURFACES ids — the same vocabulary +// internal/definition validates a definition's `pages:` against. A page that is +// a real surface but has no entry here is not an error: it answers with an +// empty list, which is the honest reply for a screen holding no readings. +// +// The seven workspace and configuration surfaces are deliberately absent. +// Their manifests explain the screen rather than read workforce records — they +// have no data behind them to rank a typed query against, and the panel there +// answers from the registries directly. +// +// Declaration order is the tie-break when two intents score equally, so the +// order within a page is the order the frontend manifest lists them in. +var catalogue = map[string][]Intent{ + + /* ── Control Center — the platform read as a whole ─────────────────── */ + + "control-center": { + { + ID: "platform-health", Text: "How healthy is the platform right now?", + Subject: "platform health", Shapes: []string{"stats", "card", "insight"}, + Terms: []string{"health", "healthy", "platform", "integrity", "data quality", + "status", "wrong", "broken", "degraded"}, + Reads: []Need{postings, applications, interviews, profiles}, + }, + { + ID: "workforce-summary", Text: "Summarize the workforce across the platform", + Subject: "the workforce", Shapes: []string{"stats", "table", "progress", "card"}, + Terms: []string{"workforce", "scale", "how big", "composition", "headcount", + "people", "how many"}, + Reads: []Need{postings, profiles, staff}, + }, + { + ID: "hiring-operations", Text: "How is hiring operating overall?", + Subject: "hiring activity", Shapes: []string{"flow", "stats", "timeline", "table", "card"}, + Terms: []string{"hiring", "operations", "activity", "velocity", "speed", + "throughput", "how fast", "time to hire"}, + Reads: []Need{applications, postings}, + }, + { + ID: "pipeline-health", Text: "Where is the hiring pipeline getting stuck?", + Subject: "the hiring pipeline", Shapes: []string{"flow", "stats", "progress", "table", "card"}, + Terms: []string{"pipeline", "funnel", "bottleneck", "stuck", "blocked", + "conversion", "stage", "stages", "drop off", "dropoff"}, + Reads: []Need{applications}, + }, + { + ID: "attention-required", Text: "What needs attention right now?", + Subject: "what needs attention", Shapes: []string{"list", "table", "insight"}, + Terms: []string{"attention", "urgent", "priority", "action", "unusual", + "anomaly", "anomalous", "risk", "risks", "problem", "problems", "issue"}, + Reads: []Need{applications, postings}, + }, + { + ID: "recommendations", Text: "What should I do next?", + Subject: "the recommendations", Shapes: []string{"list", "insight"}, + Terms: []string{"recommend", "recommendation", "recommendations", "suggest", + "should", "advice", "next step", "next"}, + Reads: []Need{applications, postings}, + }, + }, + + /* ── Positions — the roles being filled ────────────────────────────── */ + + "positions": { + { + ID: "position-drafts", Text: "Which positions are still unfinished drafts?", + Subject: "the unfinished drafts", Shapes: []string{"list", "table"}, + Terms: []string{"draft", "drafts", "unfinished", "incomplete", "unpublished", + "not posted", "half", "position", "positions", "role", "roles"}, + Reads: []Need{postings}, + }, + { + ID: "position-strength", Text: "Which position has the strongest pipeline?", + Subject: "pipeline strength by position", Shapes: []string{"table", "stats", "list", "card"}, + Terms: []string{"pipeline", "strength", "strongest", "healthiest", "best", + "conversion", "which position", "compare", "position", "positions", + "role", "roles"}, + Reads: []Need{postings, applications}, + }, + { + ID: "positions-attention", Text: "Which positions need attention?", + Subject: "positions needing attention", Shapes: []string{"list", "table", "insight"}, + Terms: []string{"attention", "risk", "risks", "at risk", "stalled", "stale", + "ageing", "aging", "neglected", "urgent", "slipping", "behind", + "position", "positions", "role", "roles"}, + Reads: []Need{postings, applications}, + }, + { + ID: "hiring-priority", Text: "Which position should I fill first?", + Subject: "what to fill first", Shapes: []string{"list", "table"}, + Terms: []string{"priority", "prioritise", "prioritize", "fill", "fill first", + "first", "most important", "which position", "vacancy", "vacancies", + "position", "positions", "role", "roles"}, + Reads: []Need{postings, applications}, + }, + { + ID: "pipeline-health", Text: "Where are the hiring bottlenecks?", + Subject: "the hiring bottlenecks", Shapes: []string{"flow", "stats", "progress", "table"}, + Terms: []string{"bottleneck", "bottlenecks", "funnel", "stuck", "blocked", + "stage", "stages", "waiting", "backlog", "pipeline"}, + Reads: []Need{applications}, + }, + { + ID: "hiring-operations", Text: "Summarize hiring activity across all positions", + Subject: "hiring activity", Shapes: []string{"flow", "stats", "timeline", "table", "card"}, + Terms: []string{"hiring", "activity", "operations", "throughput", "velocity", + "how many", "posting", "postings", "role", "roles", "position", "positions"}, + Reads: []Need{applications, postings}, + }, + { + /* Deliberately NOT carrying "pipeline". This is the Positions page's + reading of who is waiting on a decision, and it belongs here — the + frontend manifest declares it on this page. But "pipeline" on + Positions is a question about how the ROLES are converting, and + answering it with a list of people is the page context being + right and the ranking being wrong. Its own words are what it + answers to. */ + ID: "candidates-waiting", Text: "Which candidates are waiting on a decision?", + Subject: "the candidates waiting", Shapes: []string{"list", "table", "stats"}, + Terms: []string{"waiting", "candidate", "candidates", "applicant", "applicants", + "review", "screen", "screening", "decision", "queue"}, + Reads: []Need{applications}, + }, + }, + + /* ── Candidates — the people in the funnel, as a triage queue ──────── */ + + "candidates": { + { + ID: "candidates-attention", Text: "Which candidates need attention?", + Subject: "the candidates needing attention", Shapes: []string{"list", "table", "insight"}, + Terms: []string{"attention", "waiting", "stalled", "overdue", "action", + "decision", "decide", "urgent", "candidate", "candidates", "applicant", + "applicants"}, + Reads: []Need{applications}, + }, + { + ID: "top-candidates", Text: "Who are the strongest candidates?", + Subject: "the strongest candidates", Shapes: []string{"list", "table", "stats", "card"}, + Terms: []string{"strongest", "top", "best", "compare", "shortlist", "rank", + "ranking", "highest", "score", "scores", "scored", "scoring", "who", + "candidate", "candidates", "applicant", "applicants"}, + Reads: []Need{applications}, + }, + { + ID: "interview-ready", Text: "Who is ready to interview?", + Subject: "the interview-ready candidates", Shapes: []string{"list", "table", "stats"}, + Terms: []string{"interview", "interviews", "interviewed", "ready", "schedule", + "next round", "shortlist", "candidate", "candidates", "applicant", + "applicants"}, + Reads: []Need{applications, interviews}, + }, + { + ID: "screening-gaps", Text: "Which candidates have not been scored yet?", + Subject: "the screening gaps", Shapes: []string{"list", "table", "stats", "progress"}, + Terms: []string{"unscored", "score", "scores", "scoring", "screening", "screen", + "gap", "gaps", "missing", "incomplete", "coverage", "candidate", + "candidates", "applicant", "applicants"}, + Reads: []Need{applications}, + }, + { + ID: "pipeline-summary", Text: "Summarize the candidate pipeline", + Subject: "the candidate pipeline", Shapes: []string{"flow", "stats", "progress", "table", "card"}, + Terms: []string{"pipeline", "funnel", "stage", "stages", "breakdown", + "how many", "where are", "conversion"}, + Reads: []Need{applications}, + }, + { + ID: "candidate-risk", Text: "Which candidates carry risk flags?", + Subject: "the candidate risks", Shapes: []string{"list", "table", "insight"}, + Terms: []string{"risk", "risks", "risky", "flag", "flags", "flagged", "concern", + "concerns", "integrity", "doubt", "decision", "candidate", "candidates", + "applicant", "applicants"}, + Reads: []Need{applications, interviews}, + }, + }, + + /* ── Candidates Analysis — the same records read as a pool ─────────── */ + + "candidates-analysis": { + { + ID: "candidate-risk", Text: "Where is candidate risk concentrated?", + Subject: "candidate risk", Shapes: []string{"table", "stats", "insight"}, + Terms: []string{"risk", "risks", "flag", "flags", "flagged", "concern", + "integrity", "concentrated"}, + Reads: []Need{applications, interviews}, + }, + { + ID: "recruitment-insights", Text: "What do the recruitment numbers show?", + Subject: "the recruitment insights", Shapes: []string{"stats", "table", "insight", "card"}, + Terms: []string{"insight", "insights", "recruitment", "quality", "pool", + "stands out", "numbers", "trend", "trends"}, + Reads: []Need{applications, profiles}, + }, + { + ID: "screening-gaps", Text: "Where are the screening gaps?", + Subject: "the screening gaps", Shapes: []string{"table", "stats", "progress"}, + Terms: []string{"screening", "screen", "gap", "gaps", "unscored", "coverage", + "missing", "incomplete", "score", "scores"}, + Reads: []Need{applications}, + }, + { + ID: "hiring-recommendations", Text: "Who should we hire?", + Subject: "the hiring recommendations", Shapes: []string{"list", "table", "insight"}, + Terms: []string{"recommend", "recommendation", "recommendations", "hire", + "hiring", "should", "advice", "decision", "who"}, + Reads: []Need{applications, profiles}, + }, + { + ID: "top-candidates", Text: "Compare the top candidates", + Subject: "the top candidates", Shapes: []string{"table", "list", "stats"}, + Terms: []string{"compare", "comparison", "top", "best", "strongest", + "shortlist", "rank", "ranking", "side by side"}, + Reads: []Need{applications}, + }, + }, + + /* ── Analytics — performance over time ─────────────────────────────── */ + + "analytics": { + { + ID: "hiring-trend", Text: "How has hiring trended over time?", + Subject: "the hiring trend", Shapes: []string{"timeline", "flow", "stats", "table"}, + Terms: []string{"trend", "trends", "trending", "over time", "month", "monthly", + "week", "weekly", "history", "growth", "change"}, + Reads: []Need{applications, postings}, + }, + { + ID: "department-performance", Text: "How is each department performing?", + Subject: "department performance", Shapes: []string{"table", "stats", "progress"}, + Terms: []string{"department", "departments", "team", "teams", "category", + "categories", "performance", "performing", "breakdown", "compare"}, + Reads: []Need{applications, postings}, + }, + { + ID: "pipeline-health", Text: "Where are the hiring bottlenecks?", + Subject: "the hiring bottlenecks", Shapes: []string{"flow", "stats", "progress", "table"}, + Terms: []string{"bottleneck", "bottlenecks", "funnel", "pipeline", "conversion", + "stuck", "stage", "stages"}, + Reads: []Need{applications}, + }, + { + ID: "position-conversion", Text: "Which positions convert best?", + Subject: "position conversion", Shapes: []string{"table", "stats", "list"}, + Terms: []string{"conversion", "convert", "converts", "position", "positions", + "role", "roles", "rate", "rates", "ratio", "yield"}, + Reads: []Need{postings, applications}, + }, + { + ID: "hiring-operations", Text: "How is hiring performing overall?", + Subject: "hiring performance", Shapes: []string{"stats", "flow", "timeline", "card"}, + Terms: []string{"hiring", "performance", "operations", "velocity", "speed", + "average", "averages", "time to hire", "throughput"}, + Reads: []Need{applications, postings}, + }, + { + ID: "attention-required", Text: "What needs attention in the numbers?", + Subject: "what needs attention", Shapes: []string{"list", "insight", "table"}, + Terms: []string{"attention", "outlier", "outliers", "anomaly", "unusual", + "risk", "risks", "worst", "falling"}, + Reads: []Need{applications, postings}, + }, + }, + + /* ── Activity — the audit log ──────────────────────────────────────── */ + + "activity": { + { + ID: "audit-summary", Text: "Summarize the audit log", + Subject: "the audit log", Shapes: []string{"stats", "table", "timeline", "card"}, + Terms: []string{"audit", "log", "logs", "event", "events", "activity", + "trail", "record", "records", "how many"}, + Reads: []Need{orgActivity}, + }, + { + ID: "user-activity", Text: "Who has been most active?", + Subject: "activity by user", Shapes: []string{"table", "list", "stats"}, + Terms: []string{"user", "users", "who", "account", "accounts", "busiest", + "most active", "behaviour", "behavior", "person"}, + Reads: []Need{orgActivity}, + }, + { + ID: "unusual-activity", Text: "Has anything unusual happened?", + Subject: "the unusual activity", Shapes: []string{"list", "timeline", "insight"}, + Terms: []string{"unusual", "anomaly", "anomalies", "anomalous", "suspicious", + "spike", "spikes", "burst", "odd", "strange", "out of hours"}, + Reads: []Need{orgActivity}, + }, + { + ID: "security-insights", Text: "Are there any security concerns?", + Subject: "the security findings", Shapes: []string{"list", "insight", "table"}, + Terms: []string{"security", "secure", "breach", "compliance", "compliant", + "integrity", "trace", "traceable", "attributable", "risk", "risks"}, + Reads: []Need{orgActivity}, + }, + }, + + /* ── Talent Pool — supply, before anyone applies ───────────────────── */ + + "talent-pool": { + { + ID: "talent-priorities", Text: "Who should I prioritize in the talent pool?", + Subject: "the talent priorities", Shapes: []string{"list", "table", "stats"}, + Terms: []string{"prioritise", "prioritize", "priority", "priorities", "who", + "best", "top", "strongest", "elite", "star", "shortlist", "score", "scores"}, + Reads: []Need{profiles}, + }, + { + ID: "talent-summary", Text: "Summarize the talent pool", + Subject: "the talent pool", Shapes: []string{"stats", "table", "progress", "card"}, + Terms: []string{"pool", "talent", "worker", "workers", "profile", "profiles", + "segment", "segments", "composition", "supply", "how many"}, + Reads: []Need{profiles}, + }, + { + ID: "talent-verification", Text: "Which profiles are missing verification?", + Subject: "the verification gaps", Shapes: []string{"list", "table", "progress", "stats"}, + Terms: []string{"verification", "verify", "verified", "unverified", "gap", + "gaps", "missing", "credential", "credentials", "proof", "evidence"}, + Reads: []Need{profiles, evidence}, + }, + { + ID: "talent-availability", Text: "Who is available to start?", + Subject: "availability", Shapes: []string{"list", "table", "stats"}, + Terms: []string{"available", "availability", "unavailable", "free", "start", + "capacity", "when", "now", "notice"}, + Reads: []Need{profiles}, + }, + }, + + /* ── Hired History — what happened after the hire ───────────────────── */ + + "hired-history": { + { + ID: "hiring-outcomes", Text: "How have our hires worked out?", + Subject: "the hiring outcomes", Shapes: []string{"stats", "table", "progress", "card"}, + Terms: []string{"outcome", "outcomes", "result", "results", "quality", + "retention", "worked out", "performance", "department", "departments"}, + Reads: []Need{staff, applications}, + }, + { + ID: "hiring-strongest", Text: "Who are our strongest hires?", + Subject: "the strongest hires", Shapes: []string{"list", "table", "stats"}, + Terms: []string{"strongest", "best", "top", "star", "highest", "score", + "scores", "standout"}, + Reads: []Need{staff, applications}, + }, + { + ID: "hiring-patterns", Text: "What stands out about who we hire?", + Subject: "the hiring patterns", Shapes: []string{"table", "stats", "insight"}, + Terms: []string{"pattern", "patterns", "stands out", "trend", "trends", + "common", "typical", "breakdown", "profile"}, + Reads: []Need{staff, applications}, + }, + { + ID: "hiring-recent", Text: "Who did we hire recently?", + Subject: "the recent hires", Shapes: []string{"list", "timeline", "table"}, + Terms: []string{"recent", "recently", "latest", "last", "new hire", "new hires", + "who did we hire", "this month", "hired"}, + Reads: []Need{staff}, + }, + }, + + /* ── KROW Forge — the skill library and the proof behind it ────────── */ + + "krow-forge": { + { + ID: "forge-library", Text: "What is in the skill library?", + Subject: "the skill library", Shapes: []string{"stats", "table", "list", "card"}, + Terms: []string{"library", "skill", "skills", "catalogue", "catalog", "course", + "courses", "challenge", "challenges", "what do we have", "how many"}, + Reads: []Need{courses}, + }, + { + ID: "forge-published", Text: "Which skills are published?", + Subject: "the published skills", Shapes: []string{"list", "table", "stats"}, + Terms: []string{"published", "publish", "live", "draft", "drafts", "archived", + "archive", "status", "in service"}, + Reads: []Need{courses}, + }, + { + ID: "forge-evaluation", Text: "How is submitted proof evaluated?", + Subject: "the evaluation criteria", Shapes: []string{"list", "table", "insight"}, + Terms: []string{"evaluate", "evaluated", "evaluation", "rubric", "rubrics", + "criteria", "criterion", "grading", "graded", "proof", "verify", + "verification", "assess"}, + Reads: []Need{courses, evidence}, + }, + { + ID: "forge-workforce", Text: "How is the workforce using the Forge?", + Subject: "workforce usage", Shapes: []string{"stats", "progress", "table"}, + Terms: []string{"workforce", "usage", "using", "uptake", "adoption", "progress", + "completion", "completed", "training", "learning"}, + Reads: []Need{courses, evidence, profiles}, + }, + { + ID: "forge-gaps", Text: "Which skill gaps are still open?", + Subject: "the skill gaps", Shapes: []string{"list", "table", "progress"}, + Terms: []string{"gap", "gaps", "missing", "uncovered", "close", "coverage", + "needed", "shortfall", "certification", "certifications"}, + Reads: []Need{courses, certs, profiles}, + }, + }, + + /* ── Create Position — the authoring form ──────────────────────────── */ + + "create-position": { + { + ID: "vetting-weights", Text: "What do the vetting weights score?", + Subject: "the vetting weights", Shapes: []string{"table", "insight"}, + Terms: []string{"weight", "weights", "weighting", "weightings", "vetting", + "criteria", "criterion", "scoring", "score", "balance", "importance"}, + Reads: []Need{postings}, + }, + { + ID: "position-benchmarks", Text: "How does this compare with similar roles?", + Subject: "the benchmarks", Shapes: []string{"table", "stats", "insight"}, + Terms: []string{"compare", "comparison", "benchmark", "benchmarks", "similar", + "typical", "average", "pay", "rate", "rates", "salary", "experience", + "market"}, + Reads: []Need{postings}, + }, + { + ID: "position-requirements", Text: "Which credentials are already in use?", + Subject: "the credentials in use", Shapes: []string{"list", "table", "stats"}, + Terms: []string{"credential", "credentials", "certification", "certifications", + "requirement", "requirements", "qualification", "qualifications", + "licence", "license", "skill", "skills"}, + Reads: []Need{postings, certs}, + }, + { + ID: "position-spec-steps", Text: "How does this form work?", + Terms: []string{"form", "field", "fields", "step", "steps", "section", + "sections", "how do i", "what do i", "explain", "fill in", "required"}, + }, + }, + + /* ── Profile — the account, not the workforce ──────────────────────── */ + + "profile": { + { + ID: "profile-permissions", Text: "What am I permitted to do?", + Terms: []string{"permission", "permissions", "permitted", "allowed", "can i", + "scope", "access", "role", "rights", "privilege", "privileges"}, + }, + { + ID: "profile-identity", Text: "What are my account details?", + Terms: []string{"account", "details", "name", "email", "identity", "profile", + "who am i", "my details"}, + }, + { + ID: "profile-security", Text: "How is my account secured?", + Terms: []string{"security", "secure", "secured", "password", "two-factor", + "two factor", "2fa", "session", "sessions", "sign out", "log out", + "protect", "protected"}, + }, + { + ID: "profile-preferences", Text: "Which preferences are set?", + Terms: []string{"preference", "preferences", "setting", "settings", "digest", + "density", "owliver", "default", "defaults", "toggle"}, + }, + { + ID: "profile-activity", Text: "What have I done recently?", + Subject: "my recent activity", Shapes: []string{"timeline", "list", "table"}, + Terms: []string{"activity", "recent", "recently", "history", "what have i", + "my actions", "audit", "did i"}, + Reads: []Need{ownActivity}, + }, + { + ID: "profile-actions", Text: "What can I do on this page?", + Terms: []string{"do here", "what can i", "action", "actions", "edit", "change", + "change my", "update", "manage"}, + }, + }, +} + +// Pages is every page the catalogue holds intents for, for tests and +// diagnostics. It is not the set of valid pages — that is +// definition.SupportedPages, and a valid page absent from here answers with an +// empty list rather than a validation error. +func Pages() []string { + out := make([]string, 0, len(catalogue)) + for page := range catalogue { + out = append(out, page) + } + return out +} diff --git a/go-api/internal/owliver/suggest.go b/go-api/internal/owliver/suggest.go new file mode 100644 index 0000000..f645d23 --- /dev/null +++ b/go-api/internal/owliver/suggest.go @@ -0,0 +1,359 @@ +package owliver + +import ( + "sort" + "strings" + "unicode" + + "github.com/krow/krow-backend/go-api/internal/domain" +) + +// MaxSuggestions is the most a response may carry. +// +// Three, because the panel shows them under a composer the user is still typing +// into. A fourth line pushes the input off a phone screen, and a ranked list +// nobody reads to the bottom is a longer list, not a better one. +const MaxSuggestions = 3 + +// MinQueryChars is the shortest query that is worth ranking. +// +// Counted in letters and digits after normalization, so " a " and "?!" are +// both too short. One character matches a prefix of almost every term in the +// catalogue, which would make the first keystroke return three arbitrary +// readings and the second replace all three — noise that reads as a bug. +const MinQueryChars = 2 + +// MaxQueryChars bounds the work one request can ask for. The panel sends what +// is in the composer, and nothing about a suggestion improves past a couple of +// sentences. Beyond this the query is truncated, never rejected: a long paste +// should rank on its opening words, not fail. +const MaxQueryChars = 200 + +// Suggestion is one offered question. +// +// Text is what the user reads. Intent is the frontend capability id the panel +// dispatches on. Capability is the section type the answer should be drawn as, +// present only when the query asked for one — nothing internal is exposed here: +// no terms, no resource names, no policy detail, no scores. +type Suggestion struct { + Text string `json:"text"` + Intent string `json:"intent"` + Capability string `json:"capability,omitempty"` +} + +/* ── Shapes ─────────────────────────────────────────────────────────────── */ + +// shape is one section type an answer can be drawn as. +// +// Transcribed from OWLIVER_CAPABILITIES in src/lib/skills/surfaces.js: the ids +// and the terms are that table's, and `phrase` is how the id reads inside a +// sentence. `summary` is first and has no component — it is prose, so it +// applies to any intent with a subject. +// +// Terms here describe the SHAPE and never a subject, which is what keeps a +// shape from dragging in another page's readings: "as a flow" belongs here, +// "hiring activity" belongs to an intent. +type shape struct { + id string + phrase string + terms []string +} + +var shapes = []shape{ + {id: "summary", phrase: "", terms: []string{ + "summary", "summarise", "summarize", "summarised", "summarized", + "summarising", "summarizing", "sum up", "recap", "overview", "brief me", + "in short", "tell me about"}}, + {id: "flow", phrase: "as a flow", terms: []string{ + "flow", "as a flow", "chart", "graph", "diagram", "funnel", "visual", + "visualise", "visualize", "step by step"}}, + {id: "stats", phrase: "as stats", terms: []string{ + "stats", "statistics", "figures", "numbers", "counts"}}, + {id: "list", phrase: "as a list", terms: []string{ + "list", "which ones", "show me the records"}}, + {id: "table", phrase: "as a table", terms: []string{ + "table", "as a table", "rows", "grid", "spreadsheet"}}, + {id: "timeline", phrase: "as a timeline", terms: []string{ + "timeline", "chronology", "over time", "what happened"}}, + {id: "progress", phrase: "as progress bars", terms: []string{ + "progress", "bars", "completion", "how far"}}, + {id: "weights", phrase: "as weights", terms: []string{ + "weighting", "weightings", "set the weights", "adjust the weights", + "screening weight", "vetting weight"}}, + {id: "insight", phrase: "as an insight", terms: []string{ + "insight", "finding", "takeaway", "headline"}}, + {id: "card", phrase: "as a card", terms: []string{ + "card", "panel", "at a glance"}}, +} + +/* ── Normalization ──────────────────────────────────────────────────────── */ + +// normalize reduces a raw query to the one form everything downstream matches +// against: lower case, letters and digits only, single-spaced. +// +// Every other character — punctuation, quotes, brackets, control characters, +// emoji, an SQL fragment, a script tag — becomes a space rather than being +// stripped, so nothing can be glued into a token that was not typed as one. +// The result is compared against a fixed table of literals and never reaches a +// query, a template or a log message, so there is no construction to inject +// into; this is about matching sanely, not about escaping. +func normalize(raw string) (phrase string, tokens []string, meaningful int) { + runes := []rune(raw) + if len(runes) > MaxQueryChars { + runes = runes[:MaxQueryChars] + } + + var b strings.Builder + b.Grow(len(runes)) + for _, r := range runes { + switch { + case unicode.IsLetter(r) || unicode.IsDigit(r): + b.WriteRune(unicode.ToLower(r)) + meaningful++ + default: + b.WriteByte(' ') + } + } + + tokens = strings.Fields(b.String()) + return strings.Join(tokens, " "), tokens, meaningful +} + +/* ── Scoring ────────────────────────────────────────────────────────────── */ + +// Scores are small integers with a deliberate order: +// +// exact a token is the term — the user typed it +// phrase a multi-word term appears in the query — the most specific hit +// prefix the term begins with a token — mid-typing: "pipel" +// extension a token begins with the term — "pipelines" +// shaped the query named a section type — weakest on its own +// +// A shape hit is worth less than any subject hit, so typing "overview" can +// surface a page's readings but can never outrank a reading the user named. +const ( + scoreExact = 10 + scorePhrase = 12 + scorePrefix = 6 + scoreExtension = 5 + scoreShaped = 4 + + // minPrefixToken keeps one- and two-letter tokens from matching a term by + // prefix. "a" begins nothing usefully; "at" would match "attention", + // "audit" and "activity" at once. + minPrefixToken = 3 + // minExtensionTerm keeps a short term from being found inside a longer + // word: without it "list" matches "listen" and "score" matches "scoreboard". + minExtensionTerm = 4 +) + +// tokenScore is how well one typed token matches one single-word term. +func tokenScore(token, term string) int { + switch { + case token == term: + return scoreExact + case len(token) >= minPrefixToken && strings.HasPrefix(term, token): + return scorePrefix + case len(term) >= minExtensionTerm && strings.HasPrefix(token, term): + return scoreExtension + default: + return 0 + } +} + +// termsScore ranks a whole term list against the query. +// +// Multi-word terms are matched against the phrase, because "how many" means +// something its two words do not. Single-word terms are scored per TYPED TOKEN, +// taking that token's best term — so a query is rewarded for how much of what +// the user typed the intent accounts for, and an intent cannot climb the +// ranking by listing eight synonyms of one word. +func termsScore(terms []string, tokens []string, phrase string) int { + total := 0 + for _, term := range terms { + if !strings.Contains(term, " ") { + continue + } + if strings.Contains(phrase, term) { + total += scorePhrase + 2*strings.Count(term, " ") + } + } + for _, token := range tokens { + best := 0 + for _, term := range terms { + if strings.Contains(term, " ") { + continue + } + if s := tokenScore(token, term); s > best { + best = s + } + } + total += best + } + return total +} + +// matchShape is the section type the query asked for, if it asked for one. +// Highest scoring wins; ties go to declaration order, which puts `summary` +// first. +func matchShape(tokens []string, phrase string) (shape, bool) { + best, bestScore := shape{}, 0 + for _, s := range shapes { + if score := termsScore(s.terms, tokens, phrase); score > bestScore { + best, bestScore = s, score + } + } + return best, bestScore > 0 +} + +// supportsShape reports whether an intent can be drawn as a section type. +// `summary` needs only a subject, having no component of its own; every other +// shape must be one the intent declares. +func (i Intent) supportsShape(id string) bool { + if i.Subject == "" { + return false + } + if id == "summary" { + return true + } + for _, s := range i.Shapes { + if s == id { + return true + } + } + return false +} + +// shaped is the suggestion text for an intent asked for in a given shape. +func (i Intent) shaped(s shape) string { + if s.id == "summary" { + return "Summarize " + i.Subject + } + return "Show " + i.Subject + " " + s.phrase +} + +/* ── The pipeline ───────────────────────────────────────────────────────── */ + +// filterOnTopic keeps the candidates the query actually named, or nothing if +// it named none. +func filterOnTopic(candidates []scored) []scored { + out := make([]scored, 0, len(candidates)) + for _, c := range candidates { + if c.onTopic { + out = append(out, c) + } + } + return out +} + +// scored is one candidate on its way through ranking. +type scored struct { + suggestion Suggestion + score int + order int // declaration index, the tie-break + + // onTopic records that the query matched this reading's own terms, rather + // than only naming a section type it happens to support. See Suggest. + onTopic bool +} + +// Suggest ranks the page's catalogue against what the user has typed. +// +// The order is fixed and each stage only ever removes: page context, then +// permission, then relevance, then duplicates, then the cap. Permission comes +// before relevance so a reading the caller cannot perform is never scored, and +// therefore cannot be leaked by an ordering bug later. +// +// It returns an empty slice, never nil and never a filler suggestion: a query +// that matches nothing on this page has no answer here, and saying so is more +// useful than three questions the user did not ask. +func Suggest(page, query string, role domain.Role) []Suggestion { + out := []Suggestion{} + + // Deny by default, as policy.go does. The service resolves the role before + // calling, so an unrecognised one should be unreachable — but an intent + // that reads nothing is permitted by every role it is asked about, so + // without this line a caller whose role failed to parse would be offered + // the account readings. The check belongs where the answer is decided. + if _, known := domain.ParseRole(string(role)); !known { + return out + } + + intents, ok := catalogue[page] + if !ok { + return out + } + + phrase, tokens, meaningful := normalize(query) + if meaningful < MinQueryChars { + return out + } + + requested, wantsShape := matchShape(tokens, phrase) + + candidates := make([]scored, 0, len(intents)) + for order, intent := range intents { + if !intent.permitted(role) { + continue + } + + score := termsScore(intent.Terms, tokens, phrase) + onTopic := score > 0 + + suggestion := Suggestion{Text: intent.Text, Intent: intent.ID} + if wantsShape && intent.supportsShape(requested.id) { + score += scoreShaped + suggestion.Text = intent.shaped(requested) + suggestion.Capability = requested.id + } + + if score == 0 { + continue + } + candidates = append(candidates, scored{ + suggestion: suggestion, score: score, order: order, onTopic: onTopic, + }) + } + + // A shape on its own is a weak signal, and what it means depends on what + // else matched. "as a table" typed alone is a real request — draw this + // page's readings that way — but the same words after "hiring activity" are + // how the user asked for ONE reading, and offering two more that merely + // support tables is the padding this endpoint is supposed to refuse. + // + // So the two are kept in separate tiers: if anything matched the query's + // subject, only those compete. Shape-only matches answer for the whole page + // or not at all. + if onTopic := filterOnTopic(candidates); len(onTopic) > 0 { + candidates = onTopic + } + + // Highest score first; declaration order breaks every tie, so the same + // request always produces the same three in the same sequence. + sort.SliceStable(candidates, func(a, b int) bool { + if candidates[a].score != candidates[b].score { + return candidates[a].score > candidates[b].score + } + return candidates[a].order < candidates[b].order + }) + + // One suggestion per intent, and no two reading the same. The catalogue is + // unique per page by construction — TestCatalogueIsWellFormed holds it that + // way — so this guards the shaped rewrite, which can phrase two intents + // identically only if two subjects ever collide. + seenIntent := make(map[string]bool, MaxSuggestions) + seenText := make(map[string]bool, MaxSuggestions) + for _, c := range candidates { + if len(out) == MaxSuggestions { + break + } + key := strings.ToLower(c.suggestion.Text) + if seenIntent[c.suggestion.Intent] || seenText[key] { + continue + } + seenIntent[c.suggestion.Intent] = true + seenText[key] = true + out = append(out, c.suggestion) + } + return out +} diff --git a/go-api/internal/owliver/suggest_test.go b/go-api/internal/owliver/suggest_test.go new file mode 100644 index 0000000..8d706db --- /dev/null +++ b/go-api/internal/owliver/suggest_test.go @@ -0,0 +1,695 @@ +package owliver + +import ( + "fmt" + "reflect" + "strings" + "testing" + + "github.com/krow/krow-backend/go-api/internal/definition" + "github.com/krow/krow-backend/go-api/internal/domain" +) + +// These tests need no database and no server: the catalogue is static and the +// ranking is a pure function of (page, query, role). That is the property worth +// protecting — an endpoint the panel calls on every keystroke should be +// testable at the speed of a string comparison. + +// intents is the ids Suggest returned, in order. +func intents(got []Suggestion) []string { + out := make([]string, len(got)) + for i, s := range got { + out[i] = s.Intent + } + return out +} + +// ask is Suggest for an admin, the role the Owliver panel is placed for. +func ask(page, query string) []Suggestion { + return Suggest(page, query, domain.RoleAdmin) +} + +/* ── Control Center ─────────────────────────────────────────────────────── */ + +func TestControlCenterSuggestions(t *testing.T) { + cases := []struct { + name string + query string + want []string + }{ + {"attention", "attention", []string{"attention-required"}}, + {"pipeline", "pipeline", []string{"pipeline-health"}}, + {"health", "health", []string{"platform-health"}}, + {"recommendation", "what should i do", []string{"recommendations"}}, + + // A question about a subject this page does not hold. The Control + // Center reads the platform, not the training library. + {"irrelevant", "forklift certification renewal", nil}, + {"empty", "", nil}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := intents(ask("control-center", c.query)) + if len(c.want) == 0 { + if len(got) != 0 { + t.Fatalf("query %q: want no suggestions, got %v", c.query, got) + } + return + } + if !reflect.DeepEqual(got, c.want) { + t.Fatalf("query %q: got %v, want %v", c.query, got, c.want) + } + }) + } +} + +/* ── Positions ──────────────────────────────────────────────────────────── */ + +func TestPositionsSuggestions(t *testing.T) { + cases := []struct { + name string + query string + want []string + }{ + // The page's own noun offers the page's readings, in declaration order. + {"position", "position", []string{"position-drafts", "position-strength", "positions-attention"}}, + + // Pipeline on Positions is a question about the ROLES: which one is + // converting, and where it is stuck. Two answers, not three — the third + // slot is left empty rather than filled with the page's list of people + // waiting, which is a different question wearing a nearby word. + {"pipeline", "pipeline", []string{"position-strength", "pipeline-health"}}, + + {"attention", "attention", []string{"positions-attention"}}, + {"risk", "risk", []string{"positions-attention"}}, + {"drafts", "draft", []string{"position-drafts"}}, + {"fill first", "what should i fill first", []string{"hiring-priority"}}, + + // Attendance is a workforce reading and belongs to another surface. It + // must not fall through to this page's default report. + {"irrelevant", "attendance last week", nil}, + {"empty", "", nil}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := intents(ask("positions", c.query)) + if len(c.want) == 0 { + if len(got) != 0 { + t.Fatalf("query %q: want no suggestions, got %v", c.query, got) + } + return + } + if !reflect.DeepEqual(got, c.want) { + t.Fatalf("query %q: got %v, want %v", c.query, got, c.want) + } + }) + } +} + +/* ── Candidates ─────────────────────────────────────────────────────────── */ + +func TestCandidatesSuggestions(t *testing.T) { + cases := []struct { + name string + query string + want []string + }{ + {"candidate", "candidate", []string{"candidates-attention", "top-candidates", "interview-ready"}}, + {"score", "score", []string{"top-candidates", "screening-gaps"}}, + {"interview", "interview", []string{"interview-ready"}}, + {"decision", "decision", []string{"candidates-attention", "candidate-risk"}}, + {"pipeline", "pipeline", []string{"pipeline-summary"}}, + {"risk", "risk", []string{"candidate-risk"}}, + + {"irrelevant", "payroll export", nil}, + {"empty", "", nil}, + } + + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + got := intents(ask("candidates", c.query)) + if len(c.want) == 0 { + if len(got) != 0 { + t.Fatalf("query %q: want no suggestions, got %v", c.query, got) + } + return + } + if !reflect.DeepEqual(got, c.want) { + t.Fatalf("query %q: got %v, want %v", c.query, got, c.want) + } + }) + } +} + +/* ── Page context is the first filter ───────────────────────────────────── */ + +// One keyword, every page: the answers must differ, and none may name a +// reading belonging to another surface. +func TestSameKeywordDiffersByPage(t *testing.T) { + const query = "pipeline" + + seen := map[string][]string{} + for _, page := range Pages() { + got := intents(ask(page, query)) + if len(got) == 0 { + continue + } + seen[page] = got + + for _, id := range got { + if !declaredOn(page, id) { + t.Fatalf("page %q returned %q, which it does not declare", page, id) + } + } + } + + if len(seen) < 2 { + t.Fatalf("%q matched on %d pages; the comparison needs at least two", query, len(seen)) + } + if reflect.DeepEqual(seen["positions"], seen["candidates"]) { + t.Fatalf("positions and candidates both answered %q with %v", query, seen["positions"]) + } + // The pipeline reading on Candidates is the candidates' own. + if !reflect.DeepEqual(seen["candidates"], []string{"pipeline-summary"}) { + t.Fatalf("candidates answered %q with %v", query, seen["candidates"]) + } +} + +// An unknown page is not this package's error to raise — the service refuses it +// during parsing. Reached directly it answers empty rather than borrowing +// another page's readings. +func TestUnknownPageIsEmpty(t *testing.T) { + for _, page := range []string{"", "nowhere", "POSITIONS", "settings"} { + if got := ask(page, "pipeline"); len(got) != 0 { + t.Fatalf("page %q: got %v, want none", page, got) + } + } +} + +func declaredOn(page, id string) bool { + for _, i := range catalogue[page] { + if i.ID == id { + return true + } + } + return false +} + +/* ── Query handling ─────────────────────────────────────────────────────── */ + +func TestQueryNormalization(t *testing.T) { + want := intents(ask("positions", "pipeline")) + if len(want) == 0 { + t.Fatal("the baseline query matched nothing") + } + + // Every one of these is the same question typed differently: case, + // surrounding whitespace, punctuation and control characters carry no + // meaning, so all of them must rank identically. + for _, query := range []string{ + " pipeline ", "PIPELINE", "PiPeLiNe", "\tpipeline\n", + "pipeline?", "\"pipeline\"", "pipeline!!!", "…pipeline…", + "(pipeline)", "**pipeline**", "pipeline\x00\x01", "\u200bpipeline", + } { + if got := intents(ask("positions", query)); !reflect.DeepEqual(got, want) { + t.Errorf("query %q: got %v, want %v", query, got, want) + } + } +} + +// Hostile input is data like any other. There is no query to inject into — the +// normalized text is compared against a fixed table of literals and never +// reaches SQL, a template or a shell — so the property under test is that such +// a query is ranked rather than refused, and that it can only ever produce +// entries this page declares. +func TestHostileInputIsJustText(t *testing.T) { + for _, query := range []string{ + "pipeline'; DROP TABLE job_postings; --", + "pipeline\" OR 1=1 --", + "", + "{{7*7}} pipeline ${jndi:ldap://x/y}", + "../../etc/passwd pipeline", + "pipeline%00%0d%0aSet-Cookie:+x=1", + strings.Repeat("' OR ''='", 40), + } { + for _, s := range ask("positions", query) { + if !declaredOn("positions", s.Intent) { + t.Errorf("query %q produced %q, which positions does not declare", query, s.Intent) + } + if !declaredText("positions", s) { + t.Errorf("query %q produced unrecognised text %q", query, s.Text) + } + } + } +} + +// declaredText reports whether a suggestion's wording came from the catalogue — +// either an intent's own Text, or its subject phrased in a shape it declares. +// Nothing the caller typed may appear in a response. +func declaredText(page string, got Suggestion) bool { + for _, i := range catalogue[page] { + if i.ID != got.Intent { + continue + } + if got.Capability == "" { + return got.Text == i.Text + } + for _, s := range shapes { + if s.id == got.Capability { + return got.Text == i.shaped(s) + } + } + } + return false +} + +// Fewer than two meaningful characters is not a question yet. Punctuation and +// whitespace are not meaningful. +func TestShortQueriesAreEmpty(t *testing.T) { + for _, query := range []string{"", " ", "\n\t ", "p", " p ", "?", "!!!", "-", "€", " , "} { + if got := ask("positions", query); len(got) != 0 { + t.Errorf("query %q: got %v, want none", query, got) + } + } +} + +// The panel calls this on every keystroke, so a half-typed word has to match. +func TestPrefixMatchingWhileTyping(t *testing.T) { + full := intents(ask("positions", "pipeline")) + for _, query := range []string{"pip", "pipe", "pipel", "pipelin", "pipeline", "pipelines"} { + got := intents(ask("positions", query)) + if len(got) == 0 { + t.Fatalf("query %q matched nothing; the panel would blank mid-word", query) + } + if query != "pipelines" && !reflect.DeepEqual(got, full) { + t.Errorf("query %q: got %v, want %v", query, got, full) + } + } +} + +// A long paste ranks on its opening words rather than being refused. +func TestOverlongQueryIsTruncatedNotRejected(t *testing.T) { + query := "pipeline " + strings.Repeat("x", 5000) + got := intents(ask("positions", query)) + if len(got) == 0 { + t.Fatal("an overlong query was refused instead of truncated") + } + if !reflect.DeepEqual(got, intents(ask("positions", "pipeline"))) { + t.Fatalf("an overlong query ranked differently: %v", got) + } +} + +/* ── Shapes ─────────────────────────────────────────────────────────────── */ + +// Asking for a section type names it in the answer and phrases the suggestion +// in those terms — the brief's "Show hiring activity as a flow". +func TestShapedSuggestions(t *testing.T) { + got := ask("positions", "show hiring activity as a flow") + if len(got) != 1 { + t.Fatalf("got %d suggestions, want 1: %+v", len(got), got) + } + want := Suggestion{ + Text: "Show hiring activity as a flow", + Intent: "hiring-operations", + Capability: "flow", + } + if got[0] != want { + t.Fatalf("got %+v, want %+v", got[0], want) + } + + summarized := ask("positions", "summarize hiring activity") + if len(summarized) != 1 || summarized[0].Capability != "summary" || + summarized[0].Text != "Summarize hiring activity" { + t.Fatalf("got %+v", summarized) + } +} + +// A shape alone is a question about the page. A shape after a subject is a +// question about that subject, and the other readings that merely support the +// shape are padding — which this endpoint does not do. +func TestShapeAloneAnswersThePageButNeverPads(t *testing.T) { + alone := ask("control-center", "summarize") + if len(alone) != MaxSuggestions { + t.Fatalf("a bare shape returned %d suggestions, want %d: %+v", + len(alone), MaxSuggestions, alone) + } + for _, s := range alone { + if s.Capability != "summary" { + t.Fatalf("got capability %q, want summary: %+v", s.Capability, s) + } + } + + // "attention" names one reading; nothing else may ride along on the shape. + withSubject := ask("control-center", "summarize what needs attention") + if len(withSubject) != 1 || withSubject[0].Intent != "attention-required" { + t.Fatalf("got %+v, want only attention-required", withSubject) + } +} + +// A shape an intent cannot be drawn as leaves its wording alone. +func TestUnsupportedShapeIsNotClaimed(t *testing.T) { + for _, s := range ask("create-position", "adjust the weights") { + if s.Capability == "weights" { + t.Fatalf("offered a weights rendering nothing declares: %+v", s) + } + } +} + +/* ── Response limits ────────────────────────────────────────────────────── */ + +func TestNeverMoreThanThreeAndNeverDuplicated(t *testing.T) { + // A query broad enough to match everything the page has. + queries := []string{ + "position pipeline attention risk draft hiring activity waiting candidates", + "candidate score interview decision risk pipeline screening", + "summarize", "attention risk", "who what how many", + } + + for _, page := range Pages() { + for _, query := range queries { + got := Suggest(page, query, domain.RoleAdmin) + if len(got) > MaxSuggestions { + t.Fatalf("page %q query %q: %d suggestions, cap is %d", + page, query, len(got), MaxSuggestions) + } + + seenIntent, seenText := map[string]bool{}, map[string]bool{} + for _, s := range got { + if s.Text == "" || s.Intent == "" { + t.Fatalf("page %q query %q: incomplete suggestion %+v", page, query, s) + } + if seenIntent[s.Intent] { + t.Fatalf("page %q query %q: duplicate intent %q", page, query, s.Intent) + } + if seenText[strings.ToLower(s.Text)] { + t.Fatalf("page %q query %q: duplicate text %q", page, query, s.Text) + } + seenIntent[s.Intent], seenText[strings.ToLower(s.Text)] = true, true + } + } + } +} + +// The same request must answer the same way every time — the panel re-issues it +// on every keystroke, and a list that reshuffles under the cursor is unusable. +func TestSuggestIsDeterministic(t *testing.T) { + for _, page := range Pages() { + first := Suggest(page, "attention risk pipeline summary", domain.RoleAdmin) + for i := 0; i < 20; i++ { + again := Suggest(page, "attention risk pipeline summary", domain.RoleAdmin) + if !reflect.DeepEqual(first, again) { + t.Fatalf("page %q: run %d differed\n first: %+v\n again: %+v", + page, i, first, again) + } + } + } +} + +// Empty, never nil: `{"suggestions": []}` and not `{"suggestions": null}`. +func TestNoMatchIsAnEmptySliceNotNil(t *testing.T) { + for _, c := range []struct{ page, query string }{ + {"positions", "sourdough"}, {"positions", ""}, {"nowhere", "pipeline"}, + } { + got := ask(c.page, c.query) + if got == nil { + t.Fatalf("page %q query %q: got nil, want an empty slice", c.page, c.query) + } + if len(got) != 0 { + t.Fatalf("page %q query %q: got %v", c.page, c.query, got) + } + } +} + +/* ── Authorization ──────────────────────────────────────────────────────── */ + +// Talent may list job applications, but only their own — so a reading across +// the organization's pipeline is not theirs to be offered, even though the +// operation itself is permitted. The same holds for postings, profiles, staff, +// evidence and the audit log. +// +// The exception is stated rather than hidden: `courses` is the one resource in +// the policy table that talent lists unscoped, because the training library is +// shared platform-wide and everybody learns from it. So the two Forge readings +// that ask only what the library holds survive, and every other Forge reading — +// evaluation, workforce usage, gaps, all of which read evidence or profiles — +// does not. If that ever widens, this test says exactly what widened. +func TestTalentIsOfferedOnlyUnscopedReadings(t *testing.T) { + pages := []string{ + "control-center", "positions", "candidates", "candidates-analysis", + "analytics", "activity", "talent-pool", "hired-history", "krow-forge", + "create-position", + } + queries := []string{ + "pipeline", "attention", "candidate", "position", "risk", "summarize", + "hiring", "score", "activity", "who", "how many", "library", "skill", + "published", "gaps", "evaluation", "weights", "credential", + } + + allowed := map[string]bool{"krow-forge/forge-library": true, "krow-forge/forge-published": true} + + for _, page := range pages { + for _, query := range queries { + for _, s := range Suggest(page, query, domain.RoleTalent) { + if !allowed[page+"/"+s.Intent] { + t.Errorf("talent was offered %q on %q for %q", s.Intent, page, query) + } + } + } + } + + // And the operator's own Forge readings stay the operator's. + for _, id := range []string{"forge-evaluation", "forge-workforce", "forge-gaps"} { + for _, s := range Suggest("krow-forge", "evaluation workforce gaps", domain.RoleTalent) { + if s.Intent == id { + t.Errorf("talent was offered the operator reading %q", id) + } + } + } + if len(Suggest("krow-forge", "evaluation workforce gaps", domain.RoleAdmin)) == 0 { + t.Error("admin was offered none of them either; the query no longer matches") + } +} + +// The filter is not a blanket refusal: what a talent caller may genuinely ask — +// about their own account — is still offered. Otherwise the test above would +// pass with the role check stubbed out to "deny". +func TestTalentIsStillOfferedTheirOwnReadings(t *testing.T) { + for _, query := range []string{"permission", "password", "my recent activity"} { + if got := Suggest("profile", query, domain.RoleTalent); len(got) == 0 { + t.Fatalf("talent was offered nothing on profile for %q", query) + } + } +} + +// A role the API does not recognise authorizes nothing, matching policy.go. +func TestUnknownRoleIsOfferedNothing(t *testing.T) { + for _, role := range []domain.Role{"", "root", "superuser", "Admin"} { + for _, page := range Pages() { + if got := Suggest(page, "attention pipeline permission", role); len(got) != 0 { + t.Fatalf("role %q was offered %v on %q", role, intents(got), page) + } + } + } +} + +// Every permission decision must come from the policy table, not from a list +// kept here. This asserts the mechanism rather than a particular outcome: an +// intent is offered exactly when policy.go allows every reading it declares. +func TestPermissionsComeFromThePolicyTable(t *testing.T) { + for _, role := range []domain.Role{domain.RoleAdmin, domain.RoleEmployer, domain.RoleTalent} { + for page, list := range catalogue { + for _, intent := range list { + want := true + for _, need := range intent.Reads { + res, ok := domain.ResourceByPath[need.Resource] + if !ok || !res.Supports(need.Op) || !res.Policy.Allows(need.Op, role) { + want = false + break + } + if need.OrgWide && res.Policy.ScopeFor(role).Kind != domain.ScopeNone { + want = false + break + } + } + if got := intent.permitted(role); got != want { + t.Errorf("%s/%s for %s: permitted=%v, policy says %v", + page, intent.ID, role, got, want) + } + } + } + } +} + +/* ── The catalogue itself ───────────────────────────────────────────────── */ + +func TestCatalogueIsWellFormed(t *testing.T) { + for page, list := range catalogue { + if definition.CanonicalPage(page) != page { + t.Errorf("page key %q is not a canonical surface", page) + } + if len(list) == 0 { + t.Errorf("page %q has no intents; omit the key instead", page) + } + + ids, texts := map[string]bool{}, map[string]bool{} + for _, intent := range list { + where := fmt.Sprintf("%s/%s", page, intent.ID) + + if intent.ID == "" || intent.Text == "" { + t.Errorf("%s: an intent needs both an id and a text", where) + } + if ids[intent.ID] { + t.Errorf("%s: duplicate intent id on this page", where) + } + if texts[strings.ToLower(intent.Text)] { + t.Errorf("%s: duplicate suggestion text on this page", where) + } + ids[intent.ID], texts[strings.ToLower(intent.Text)] = true, true + + if len(intent.Terms) == 0 { + t.Errorf("%s: no terms, so it can never be suggested", where) + } + for _, term := range intent.Terms { + if term != strings.ToLower(strings.TrimSpace(term)) || term == "" { + t.Errorf("%s: term %q must be lower case and trimmed", where, term) + } + if _, _, meaningful := normalize(term); meaningful < MinQueryChars { + t.Errorf("%s: term %q is shorter than the shortest query", where, term) + } + } + + if len(intent.Shapes) > 0 && intent.Subject == "" { + t.Errorf("%s: declares shapes but no subject to phrase them with", where) + } + for _, id := range intent.Shapes { + if id == "summary" { + t.Errorf("%s: `summary` applies to every subject and is never declared", where) + } + if !knownShape(id) { + t.Errorf("%s: shape %q is not in the Owliver vocabulary", where, id) + } + } + + for _, need := range intent.Reads { + res, ok := domain.ResourceByPath[need.Resource] + if !ok { + t.Errorf("%s: reads %q, which is not a resource", where, need.Resource) + continue + } + if !res.Supports(need.Op) { + t.Errorf("%s: reads %q with an operation it does not serve", where, need.Resource) + } + // An intent nobody can be offered is dead weight, and usually a + // typo in the resource path rather than a deliberate lockout. + if !res.Policy.Allows(need.Op, domain.RoleAdmin) { + t.Errorf("%s: reads %q, which not even admin may list", where, need.Resource) + } + } + } + } +} + +// Every id in the catalogue must be a capability the frontend actually +// declares, because the id in a response is what the panel dispatches on. The +// list is the union of the manifests in +// src/components/ai-assistant/capabilities/, transcribed alongside the +// catalogue; an id here that is absent there would be a suggestion the panel +// cannot run. +func TestIntentIDsAreFrontendCapabilities(t *testing.T) { + frontend := map[string]bool{} + for _, id := range []string{ + // CONTROL_CENTER_CAPABILITIES + "platform-health", "workforce-summary", "hiring-operations", "pipeline-health", + "attention-required", "recommendations", + // POSITIONS_CAPABILITIES + "position-drafts", "position-strength", "positions-attention", "hiring-priority", + "candidates-waiting", + // CANDIDATE_LIST_CAPABILITIES + "candidates-attention", "top-candidates", "interview-ready", "screening-gaps", + "pipeline-summary", "candidate-risk", + // ADMIN_CANDIDATE_CAPABILITIES + "recruitment-insights", "hiring-recommendations", + // ADMIN_ANALYTICS_CAPABILITIES + "hiring-trend", "department-performance", "position-conversion", + // ACTIVITY_CAPABILITIES + "audit-summary", "user-activity", "unusual-activity", "security-insights", + // TALENT_POOL_CAPABILITIES + "talent-priorities", "talent-summary", "talent-verification", "talent-availability", + // HIRED_HISTORY_CAPABILITIES + "hiring-outcomes", "hiring-strongest", "hiring-patterns", "hiring-recent", + // FORGE_CAPABILITIES + "forge-library", "forge-published", "forge-evaluation", "forge-workforce", "forge-gaps", + // CREATE_POSITION_CAPABILITIES + "vetting-weights", "position-benchmarks", "position-requirements", "position-spec-steps", + // PROFILE_CAPABILITIES + "profile-permissions", "profile-identity", "profile-security", "profile-preferences", + "profile-activity", "profile-actions", + } { + frontend[id] = true + } + + used := map[string]bool{} + for page, list := range catalogue { + for _, intent := range list { + used[intent.ID] = true + if !frontend[intent.ID] { + t.Errorf("%s/%s names no frontend capability", page, intent.ID) + } + } + } + for id := range frontend { + if !used[id] { + t.Errorf("capability %q is declared here but suggested on no page", id) + } + } +} + +func knownShape(id string) bool { + for _, s := range shapes { + if s.id == id { + return true + } + } + return false +} + +// The shape vocabulary is closed and mirrors OWLIVER_CAPABILITIES. +func TestShapeVocabularyMatchesTheFrontend(t *testing.T) { + want := []string{"summary", "flow", "stats", "list", "table", "timeline", + "progress", "weights", "insight", "card"} + + got := make([]string, len(shapes)) + for i, s := range shapes { + got[i] = s.id + if len(s.terms) == 0 { + t.Errorf("shape %q has no terms", s.id) + } + if s.id != "summary" && s.phrase == "" { + t.Errorf("shape %q has no phrase to read inside a sentence", s.id) + } + } + if !reflect.DeepEqual(got, want) { + t.Fatalf("shapes are %v, want %v", got, want) + } +} + +// Nothing internal may reach a response: no terms, no resource names, no +// scores. The struct is the whole contract, so this asserts its shape. +func TestSuggestionExposesNothingInternal(t *testing.T) { + fields := reflect.VisibleFields(reflect.TypeOf(Suggestion{})) + if len(fields) != 3 { + t.Fatalf("Suggestion has %d fields; the response contract is text, intent, capability", len(fields)) + } + want := map[string]string{ + "Text": `json:"text"`, + "Intent": `json:"intent"`, + "Capability": `json:"capability,omitempty"`, + } + for _, f := range fields { + if string(f.Tag) != want[f.Name] { + t.Errorf("field %s has tag %q, want %q", f.Name, f.Tag, want[f.Name]) + } + } +} diff --git a/go-api/internal/service/interviews.go b/go-api/internal/service/interviews.go new file mode 100644 index 0000000..e16cfd3 --- /dev/null +++ b/go-api/internal/service/interviews.go @@ -0,0 +1,119 @@ +package service + +// Completing an AI interview, in one transaction. +// +// THE PROBLEM THIS SOLVES +// +// Finishing an interview is two writes: the interview record, and the +// application it was for — which has to carry the verdict forward as +// `status: interview`, `interview_id` and the score, because that is what the +// funnel and the analytics read. The frontend performed them as two independent +// requests (AIInterviewModal.finishInterview), and that had two consequences. +// +// The first is authorization. `ai-interviews:Create` is open to everyone and +// `job-applications:Update` is operators only, so a talent user sitting their +// own interview — which is the whole talent flow — got a 201 for the interview +// and a 403 for the link. The interview existed, the application still said +// `applied`, `interview_id` was never set, and every consumer that counts +// `status === 'interview' || interview_id` could not see it. +// +// The second is atomicity: even for an operator, a failure between the two left +// an interview attached to an application that did not know about it. +// +// WHY THE SERVER MAY WRITE WHAT THE CALLER MAY NOT +// +// The link is not a widening of `job-applications:Update`. A talent caller +// still cannot PATCH an application — the policy table is unchanged, and the +// role gate on that route still refuses them. What happens here is that the +// server updates the one row the interview it just wrote already names, and +// only after repo.guardInsert has proved that row belongs to the caller: a +// talent caller creating an interview for an application that is not theirs is +// answered 404 before anything is written. That guard is exactly the ownership +// proof this update needs. +// +// The alternative — adding `talent` to `job-applications:Update` with a +// per-column allowlist — was rejected in the plan for the reason the workflows +// file header gives: it would put a second authorization mechanism beside the +// per-operation one, and the two would eventually disagree. + +import ( + "context" + + "github.com/jackc/pgx/v5" + + "github.com/krow/krow-backend/go-api/internal/authctx" + "github.com/krow/krow-backend/go-api/internal/domain" + "github.com/krow/krow-backend/go-api/internal/repo" +) + +// InterviewsPath is the resource whose Create is routed through here. +// Exported so the HTTP layer names the same resource this file special-cases, +// rather than repeating a string literal that could drift. +const InterviewsPath = "ai-interviews" + +// CreateInterview inserts an interview and links its application, atomically. +// +// The response is the interview record, unchanged: POST /api/v1/ai-interviews +// answered 201 with the created interview before this existed and answers 201 +// with the created interview now. The application update is a consequence of +// the request, not a second thing in it. +func (s *WorkflowService) CreateInterview(ctx context.Context, ident authctx.Identity, + body domain.Record) (domain.Record, error) { + + interviews, err := resourceByPath(InterviewsPath) + if err != nil { + return nil, err + } + apps, err := resourceByPath("job-applications") + if err != nil { + return nil, err + } + + var out domain.Record + err = s.inTx(ctx, func(tx pgx.Tx) error { + // Through the resource's own service over the transaction, so the body + // is validated and the ownership guard runs exactly as they do on the + // plain create path. Nothing about the interview itself changes here. + created, err := New(interviews, tx).Create(ctx, ident, body) + if err != nil { + return err + } + out = created + + applicationID, _ := created["application_id"].(string) + if applicationID == "" { + // Unreachable: application_id is NOT NULL and Required, so the + // create above would have refused. Checked rather than assumed + // because the alternative is an Update against an empty id. + return nil + } + + patch := domain.Record{ + "status": "interview", + "interview_id": created["id"], + } + // The score moves onto the application only when the caller said + // something about it. Copying the column unconditionally would write + // the interview's default 0 over a real screening score, which is a + // loss caused by a field the request never mentioned. + if _, said := body["overall_interview_score"]; said { + patch["ai_score"] = created["overall_interview_score"] + } + + updated, err := repo.New(apps, tx).Update(ctx, ident, applicationID, patch) + if err != nil { + return err + } + if updated == nil { + // Unreachable for the same reason the guard above passed: the row + // is in this organization and, for a talent caller, theirs. Kept so + // a silent no-op cannot pass for a completed interview. + return domain.NotFound(apps.Name, applicationID) + } + return nil + }) + if err != nil { + return nil, err + } + return out, nil +} diff --git a/go-api/internal/service/owliver.go b/go-api/internal/service/owliver.go new file mode 100644 index 0000000..0be1bb2 --- /dev/null +++ b/go-api/internal/service/owliver.go @@ -0,0 +1,103 @@ +package service + +import ( + "fmt" + "net/url" + "strings" + + "github.com/krow/krow-backend/go-api/internal/authctx" + "github.com/krow/krow-backend/go-api/internal/definition" + "github.com/krow/krow-backend/go-api/internal/domain" + "github.com/krow/krow-backend/go-api/internal/owliver" +) + +// allowedSuggestionParams names the accepted query parameters, on the pattern +// of allowedDefinitionFilters: anything else is a caller mistake worth saying +// out loud rather than a filter to ignore. It also keeps the endpoint from +// quietly accepting a `role`, `org` or `user` parameter should one ever be +// added by a client — who is asking is read from the session and nowhere else. +var allowedSuggestionParams = map[string]bool{ + "page": true, + "query": true, +} + +// maxEchoedPage bounds how much of a rejected page value is quoted back. Long +// enough to name every real surface, short enough that the error cannot be used +// to reflect a payload. +const maxEchoedPage = 64 + +// SuggestionQuery is a validated suggestion request. +// +// Page is canonical: aliases are resolved here so nothing downstream has to +// know that `hired` and `hired-history` are the same surface. +type SuggestionQuery struct { + Page string + Query string +} + +// SuggestionsService answers "what could I usefully ask on this page?". +// +// It holds no pool, opens no transaction and reads no table. That is not an +// omission — the panel calls it while the user types, and everything it needs +// is the static catalogue in internal/owliver plus the caller's role. It is a +// service rather than a function in the handler so that validation and +// authorization sit where every other endpoint's do. +type SuggestionsService struct{} + +// NewSuggestions builds the suggestion service. +func NewSuggestions() *SuggestionsService { return &SuggestionsService{} } + +// ParseParams validates the query string. +// +// `page` is required and must name a real surface — the same closed vocabulary +// internal/definition validates a definition's `pages:` against, so there is +// one answer to "is that a page" in this process. `query` is optional: an +// absent or too-short one is not an error, it is a request that has nothing to +// rank yet, and Suggest answers it with an empty list. +func (s *SuggestionsService) ParseParams(q url.Values) (SuggestionQuery, error) { + var out SuggestionQuery + + for name := range q { + if !allowedSuggestionParams[name] { + return out, domain.Invalid(fmt.Sprintf("unknown parameter %q", name)) + } + } + + raw := strings.TrimSpace(q.Get("page")) + if raw == "" { + return out, domain.Invalid("page is required") + } + page := definition.CanonicalPage(raw) + if page == "" { + echoed := raw + if len(echoed) > maxEchoedPage { + echoed = echoed[:maxEchoedPage] + } + return out, domain.Invalid(fmt.Sprintf( + "Unsupported page: %s. Supported pages: %s.", + echoed, strings.Join(definition.SupportedPages, ", "))) + } + + out.Page = page + out.Query = q.Get("query") + return out, nil +} + +// Suggest ranks the page's readings for this caller. +// +// The role comes off the session-resolved identity, exactly as Server.authorize +// reads it, and an unrecognised role is offered nothing — the same deny-by- +// default the policy table applies. Nothing else about the caller is consulted: +// there is no branch here on organization, account type or anything a request +// could set. +// +// No error case beyond parsing. A page with no readings for this caller, and a +// query that matches none of them, both answer with an empty list — an empty +// result is an answer, not a failure. +func (s *SuggestionsService) Suggest(ident authctx.Identity, q SuggestionQuery) []owliver.Suggestion { + role, known := domain.ParseRole(ident.Role) + if !known { + return []owliver.Suggestion{} + } + return owliver.Suggest(q.Page, q.Query, role) +} diff --git a/go-api/internal/service/service.go b/go-api/internal/service/service.go index b83945c..2bd4ce1 100644 --- a/go-api/internal/service/service.go +++ b/go-api/internal/service/service.go @@ -150,7 +150,7 @@ func (s *Service) Get(ctx context.Context, ident authctx.Identity, id string) (d // Create validates and inserts, returning the complete stored record. func (s *Service) Create(ctx context.Context, ident authctx.Identity, body domain.Record) (domain.Record, error) { - clean, err := s.validate(body, true) + clean, err := s.validate(ident, body, true) if err != nil { return nil, err } @@ -162,7 +162,7 @@ func (s *Service) Update(ctx context.Context, ident authctx.Identity, id string, if !isUUID(id) { return nil, domain.NotFound(s.res.Name, id) } - clean, err := s.validate(patch, false) + clean, err := s.validate(ident, patch, false) if err != nil { return nil, err } @@ -201,7 +201,11 @@ func (s *Service) Delete(ctx context.Context, ident authctx.Identity, id string) // exactly how `interview_id`, `training_outline` and `score_breakdown` would // have been lost: the frontend would have written them, the API would have // accepted the request, and the data would never have arrived. -func (s *Service) validate(in domain.Record, isCreate bool) (domain.Record, error) { +// +// The identity is a parameter because "the server will supply this column" +// is not a property of the column alone: a talent-only derivation supplies it +// for a talent caller and for nobody else. See serverSupplies. +func (s *Service) validate(ident authctx.Identity, in domain.Record, isCreate bool) (domain.Record, error) { details := map[string]string{} out := make(domain.Record, len(in)) @@ -237,7 +241,7 @@ func (s *Service) validate(in domain.Record, isCreate bool) (domain.Record, erro if !col.Required { continue } - if s.serverSupplies(col.Name) { + if s.serverSupplies(col.Name, ident) { // The repository fills this from the session, so demanding it // from the caller would reject a request the server is about to // complete correctly. evidence.worker_email is the live case. @@ -262,13 +266,24 @@ func (s *Service) validate(in domain.Record, isCreate bool) (domain.Record, erro } // serverSupplies reports whether a column is filled in from the authenticated -// session rather than from the request body. -func (s *Service) serverSupplies(name string) bool { +// session rather than from the request body, FOR THIS CALLER. +// +// The caller matters. A TalentOnly derivation records who the row is ABOUT, and +// the repository fills it for a talent caller only — when an operator files an +// application or logs evidence on somebody else's behalf, the subject is not +// the operator, so nothing is derived and the value has to come from the body. +// Treating those columns as server-supplied for every role was how an operator +// creating a job application without an email got as far as SQL and came back +// with a not-null violation instead of the required-field message the contract +// promises. It must agree with repo.derivedValues, which decides the same thing +// on the write path. +func (s *Service) serverSupplies(name string, ident authctx.Identity) bool { if s.res.Policy == nil { return false } + isTalent := ident.Role == string(domain.RoleTalent) for _, d := range s.res.Policy.Derived { - if d.Column == name { + if d.Column == name && (!d.TalentOnly || isTalent) { return true } } diff --git a/go-api/internal/service/service_test.go b/go-api/internal/service/service_test.go index c2c816e..81287ab 100644 --- a/go-api/internal/service/service_test.go +++ b/go-api/internal/service/service_test.go @@ -1,12 +1,20 @@ package service import ( + "errors" "net/url" "testing" + "github.com/krow/krow-backend/go-api/internal/authctx" "github.com/krow/krow-backend/go-api/internal/domain" ) +// The two callers validation distinguishes. Only the role is read. +var ( + asOperator = authctx.Identity{Role: string(domain.RoleAdmin)} + asTalent = authctx.Identity{Role: string(domain.RoleTalent)} +) + // These exercise query parsing and validation without a database, so the // contract's defaults are pinned even when PostgreSQL is not available. @@ -139,24 +147,24 @@ func TestReservedParametersAreNotFilters(t *testing.T) { func TestValidateRequiredAndUnknownAndEnum(t *testing.T) { svc := New(resource(t, "job-postings"), nil) - if _, err := svc.validate(domain.Record{}, true); err == nil { + if _, err := svc.validate(asOperator, domain.Record{}, true); err == nil { t.Error("a create with no title was accepted") } - if _, err := svc.validate(domain.Record{"title": " "}, true); err == nil { + if _, err := svc.validate(asOperator, domain.Record{"title": " "}, true); err == nil { t.Error("a blank title was accepted") } - if _, err := svc.validate(domain.Record{"title": "X", "bogus": 1}, true); err == nil { + if _, err := svc.validate(asOperator, domain.Record{"title": "X", "bogus": 1}, true); err == nil { t.Error("an unknown field was accepted") } - if _, err := svc.validate(domain.Record{"title": "X", "status": "archived"}, true); err == nil { + if _, err := svc.validate(asOperator, domain.Record{"title": "X", "status": "archived"}, true); err == nil { t.Error("an invalid enum value was accepted") } - if _, err := svc.validate(domain.Record{"title": "X", "status": "active"}, true); err != nil { + if _, err := svc.validate(asOperator, domain.Record{"title": "X", "status": "active"}, true); err != nil { t.Errorf("a valid payload was rejected: %v", err) } // Server-owned fields are stripped, not rejected. - out, err := svc.validate(domain.Record{"title": "X", "id": "abc", "org_id": "def"}, true) + out, err := svc.validate(asOperator, domain.Record{"title": "X", "id": "abc", "org_id": "def"}, true) if err != nil { t.Fatalf("server-owned fields caused a rejection: %v", err) } @@ -168,11 +176,62 @@ func TestValidateRequiredAndUnknownAndEnum(t *testing.T) { } // An update needs no required fields — it is a partial by definition. - if _, err := svc.validate(domain.Record{"location": "Here"}, false); err != nil { + if _, err := svc.validate(asOperator, domain.Record{"location": "Here"}, false); err != nil { t.Errorf("a partial update was rejected: %v", err) } } +// A talent-only derivation is only server-supplied for a talent caller. +// +// The column records who the row is ABOUT, and the repository fills it from the +// session for talent and for nobody else (repo.derivedValues). Validation has +// to agree: an operator filing an application for somebody else must be told +// `email` is required, rather than being let through to a not-null violation +// from SQL — and a talent caller must not be asked for the value the server is +// about to override anyway. +func TestValidateHonoursTalentOnlyDerivation(t *testing.T) { + apps := New(resource(t, "job-applications"), nil) + body := domain.Record{ + "job_posting_id": "00000000-0000-0000-0000-000000000000", + "applicant_name": "Someone", + } + + if _, err := apps.validate(asTalent, body, true); err != nil { + t.Errorf("a talent create without email was rejected: %v", err) + } + + _, err := apps.validate(asOperator, body, true) + if err == nil { + t.Fatal("an operator create without email was accepted") + } + var apiErr *domain.Error + if !errors.As(err, &apiErr) { + t.Fatalf("error is not an API error: %v", err) + } + if apiErr.Details["email"] != "required" { + t.Errorf("details = %v, want email: required", apiErr.Details) + } + + // evidence.worker_email is the same shape, and the case the original + // comment in serverSupplies was written for. + ev := New(resource(t, "evidence"), nil) + if _, err := ev.validate(asTalent, domain.Record{"type": "photo_identify"}, true); err != nil { + t.Errorf("a talent evidence create without worker_email was rejected: %v", err) + } + if _, err := ev.validate(asOperator, domain.Record{"type": "photo_identify"}, true); err == nil { + t.Error("an operator evidence create without worker_email was accepted") + } + + // A derivation that is NOT talent-only stays server-supplied for everyone: + // user_activity records who acted, whoever that is. + act := New(resource(t, "user-activity"), nil) + for name, ident := range map[string]authctx.Identity{"operator": asOperator, "talent": asTalent} { + if _, err := act.validate(ident, domain.Record{"event_type": "x"}, true); err != nil { + t.Errorf("%s: an activity create was rejected: %v", name, err) + } + } +} + func TestIsUUID(t *testing.T) { valid := []string{ "00000000-0000-0000-0000-000000000000", diff --git a/go-api/internal/service/workflows.go b/go-api/internal/service/workflows.go index 2076aa2..b0d5f08 100644 --- a/go-api/internal/service/workflows.go +++ b/go-api/internal/service/workflows.go @@ -209,7 +209,7 @@ func (s *WorkflowService) Hire(ctx context.Context, ident authctx.Identity, // happened without a log entry, or a log entry for a hire that rolled // back, are both worse than neither. if _, err := repo.New(activity, tx).Insert(ctx, ident, domain.Record{ - "event_type": "candidate_hired", + "event_type": "hire_candidate", "details": fmt.Sprintf("%s was hired", name), "application_id": applicationID, "position_id": app["job_posting_id"], @@ -230,6 +230,11 @@ func (s *WorkflowService) Hire(ctx context.Context, ident authctx.Identity, /* ── Assign ─────────────────────────────────────────────────────────────── */ // AssignWorker is one worker in an assign request. +// +// Three ways of naming the application this placement belongs to, in order of +// precedence: an id the caller already has, an `application` payload to find or +// file one from, and neither — a worker placed straight from the talent pool, +// who has no application and is not given an invented one. type AssignWorker struct { WorkerEmail string `json:"worker_email"` WorkerName string `json:"worker_name"` @@ -239,6 +244,17 @@ type AssignWorker struct { WorkerProfileID *string `json:"worker_profile_id"` MatchScore *int `json:"match_score"` Source string `json:"source"` + + // Application is the application to attach this worker to when no + // ApplicationID is supplied: the same fields POST /job-applications takes. + // + // It exists because an application is what puts a person in the pipeline + // for a role, and every downstream step keys on it — the candidate record + // is addressed by it, and an AI interview takes one as its subject. A + // worker assigned without one is unreachable: nothing to open, and nobody + // to interview. The frontend was creating it in a second, untransacted + // request; this carries it into the same transaction as the assignment. + Application *domain.Record `json:"application"` } // AssignRequest is the body of POST /job-postings/{id}/assignments. @@ -258,6 +274,11 @@ type AssignResult struct { // request and one transaction. All-or-nothing across the whole batch: assigning // six workers and having the fourth fail should not leave three assigned, three // not, and the caller unsure which. +// +// Per worker the flow is: settle the application (see settleApplication — +// patch the one named, find-or-file the one described, or neither), insert the +// assignment linked to whatever that produced, and write the audit entry. The +// application comes first because the assignment references it. func (s *WorkflowService) Assign(ctx context.Context, ident authctx.Identity, postingID string, req AssignRequest) (*AssignResult, error) { @@ -324,22 +345,43 @@ func (s *WorkflowService) Assign(ctx context.Context, ident authctx.Identity, assignRepo := repo.New(assignments, tx) appRepo := repo.New(apps, tx) + // The application is filed through the service rather than the + // repository so that a payload the caller sent is validated exactly as + // POST /job-applications would validate it — unknown fields rejected, + // enums checked, required fields demanded — instead of reaching SQL and + // coming back as a constraint violation. + appSvc := New(apps, tx) activityRepo := repo.New(activity, tx) for i, w := range req.Workers { + // The application is settled BEFORE the assignment is written, + // because the assignment carries the reference to it and a row + // cannot point at one that does not exist yet. An empty id means + // this worker legitimately has no application. + applicationID, err := s.settleApplication(ctx, ident, appRepo, appSvc, postingID, w) + if err != nil { + return annotate(err, i) + } + record := domain.Record{ "job_posting_id": postingID, "worker_email": w.WorkerEmail, "worker_name": w.WorkerName, "starts_at": w.StartsAt, "status": "active", - "source": orDefault(w.Source, "manual"), + // `owliver` is the column's own default (000001:511) and what + // the frontend sends. Substituting `manual` here made the API + // and the schema disagree about what an unspecified source + // means, so a row written through this endpoint and a row + // written through POST /assignments recorded different origins + // for the same action. + "source": orDefault(w.Source, "owliver"), } if w.EndsAt != nil { record["ends_at"] = *w.EndsAt } - if w.ApplicationID != nil { - record["application_id"] = *w.ApplicationID + if applicationID != "" { + record["application_id"] = applicationID } if w.WorkerProfileID != nil { record["worker_profile_id"] = *w.WorkerProfileID @@ -354,29 +396,14 @@ func (s *WorkflowService) Assign(ctx context.Context, ident authctx.Identity, } out.Assignments = append(out.Assignments, created) - // The application moves to `assigned` only when one was named. A - // worker can be placed without having applied — that is what the - // talent pool is for — and inventing an application for them would - // be worse than leaving the link absent. - if w.ApplicationID != nil { - updated, err := appRepo.Update(ctx, ident, *w.ApplicationID, - domain.Record{"status": "assigned"}) - if err != nil { - return annotate(err, i) - } - if updated == nil { - return domain.NotFound("JobApplication", *w.ApplicationID) - } - } - entry := domain.Record{ - "event_type": "worker_assigned", + "event_type": "assign_employee", "details": fmt.Sprintf("%s was assigned", orDefault(w.WorkerName, w.WorkerEmail)), "position_id": postingID, "worker_email": w.WorkerEmail, } - if w.ApplicationID != nil { - entry["application_id"] = *w.ApplicationID + if applicationID != "" { + entry["application_id"] = applicationID } if _, err := activityRepo.Insert(ctx, ident, entry); err != nil { return annotate(err, i) @@ -391,6 +418,122 @@ func (s *WorkflowService) Assign(ctx context.Context, ident authctx.Identity, return out, nil } +// settleApplication resolves the application an assignment belongs to and +// returns its id, or "" when this worker has none. +// +// Three cases, and the middle one is why this exists: +// +// - an id was supplied — move that application to `assigned`. Unchanged +// behaviour, and it still wins over any payload, because an id is a +// decision the caller has already made. +// - an `application` payload was supplied — find the application on this +// posting for this email, and move it to `assigned` if there is one or file +// it if there is not. (job_posting_id, email) is UNIQUE +// (job_applications_posting_email_key, 000001), so there is at most one to +// find and the insert cannot produce a second. +// - neither — nothing. A worker placed straight from the talent pool has no +// application, and inventing one for them would be worse than leaving the +// link absent. +// +// Everything here runs inside the caller's transaction, which is what makes the +// find-or-create safe: a concurrent assign of the same person to the same +// posting blocks on the unique index and is then reported as a conflict, rather +// than racing past the lookup and writing a duplicate. +func (s *WorkflowService) settleApplication(ctx context.Context, ident authctx.Identity, + appRepo *repo.Repo, appSvc *Service, postingID string, w AssignWorker) (string, error) { + + if w.ApplicationID != nil { + updated, err := appRepo.Update(ctx, ident, *w.ApplicationID, + domain.Record{"status": "assigned"}) + if err != nil { + return "", err + } + if updated == nil { + return "", domain.NotFound("JobApplication", *w.ApplicationID) + } + return *w.ApplicationID, nil + } + if w.Application == nil { + return "", nil + } + + existing, err := findApplication(ctx, ident, appRepo, postingID, w.WorkerEmail) + if err != nil { + return "", err + } + if existing != nil { + id, _ := existing["id"].(string) + updated, err := appRepo.Update(ctx, ident, id, domain.Record{"status": "assigned"}) + if err != nil { + return "", err + } + if updated == nil { + return "", domain.NotFound("JobApplication", id) + } + return id, nil + } + + // The posting and the person come from the assignment, not from the + // payload. A body naming a different posting or a different email would + // file an application about somebody other than the worker being placed, + // and the lookup above would never find it again. + record := domain.Record{} + for k, v := range *w.Application { + record[k] = v + } + record["job_posting_id"] = postingID + record["email"] = w.WorkerEmail + if _, ok := record["applicant_name"]; !ok { + record["applicant_name"] = w.WorkerName + } + if _, ok := record["status"]; !ok { + record["status"] = "assigned" + } + + created, err := appSvc.Create(ctx, ident, record) + if err != nil { + return "", err + } + id, _ := created["id"].(string) + return id, nil +} + +// findApplication returns this posting's application for this email, or nil. +// +// The read goes through the repository so it carries the same organization +// scope and ownership predicate every other read does — an application in +// another tenant is not found, rather than found and then refused. The email +// column is citext, so the comparison is case-insensitive: the same equality +// the frontend performed with toLowerCase before it had this endpoint. +func findApplication(ctx context.Context, ident authctx.Identity, appRepo *repo.Repo, + postingID, email string) (domain.Record, error) { + + res := appRepo.Resource() + postingCol, ok := res.Column("job_posting_id") + if !ok { + return nil, domain.Internal(errors.New("service: job_applications has no job_posting_id column")) + } + emailCol, ok := res.Column("email") + if !ok { + return nil, domain.Internal(errors.New("service: job_applications has no email column")) + } + + page, err := appRepo.List(ctx, ident, domain.ListParams{ + Limit: 1, + Filters: []domain.Filter{ + {Column: postingCol, Values: []string{postingID}}, + {Column: emailCol, Values: []string{email}}, + }, + }) + if err != nil { + return nil, err + } + if len(page.Records) == 0 { + return nil, nil + } + return page.Records[0], nil +} + /* ── Helpers ────────────────────────────────────────────────────────────── */ // pick takes the caller's value for a key when they supplied a usable one, and diff --git a/infrastructure/README.md b/infrastructure/README.md index 155bfff..6cf4f73 100644 --- a/infrastructure/README.md +++ b/infrastructure/README.md @@ -12,10 +12,13 @@ What lands here in later phases, once each is actually approved: | File | Phase | Contents | | --- | --- | --- | | `docker-compose.dev.yml` | 2 | PostgreSQL + pgvector, so the dev database stops being a machine-local install | -| `docker-compose.dev.yml` (extended) | later | Redis, NATS, MinIO — each only when the phase that needs it starts | +| `docker-compose.dev.yml` (extended) | later | Redis, MinIO — each only when the phase that needs it starts | | `Dockerfile.api` | later | Multi-stage build for `go-api` | | `Dockerfile.owliver` | later | The Python service | | `otel-collector.yaml` | later | OpenTelemetry collector config | +NATS is deliberately absent from that table: it is not part of the target +architecture, and nothing here should reintroduce it. + Adding any of these before its phase would be speculative, so the directory holds only this note for now. diff --git a/scripts/oracle.mjs b/scripts/oracle.mjs index 950fbae..a2b4935 100644 --- a/scripts/oracle.mjs +++ b/scripts/oracle.mjs @@ -3,8 +3,9 @@ * * Runs the REAL frontend module graph through Vite — `import.meta.glob`, the * `@/` alias and raw Markdown loading behave exactly as they do in the app, the - * same technique `scripts/skill-check.mjs` uses. A mock of the registry would - * reproduce none of the behaviour this file exists to capture. + * same technique the frontend's own `scripts/skill-check.mjs` uses (that file + * lives in krow-demo, not here). A mock of the registry would reproduce none of + * the behaviour this file exists to capture. * * Emits one JSON document: for every shipped definition and every adversarial * case, what the JS parser did with it. That document is the fixture the Go