Skip to content

Commit 60e1d53

Browse files
committed
docs(project): add junior-executable follow-up roadmap for manage assistant
W1 live judge run + nightly secrets, W2 X3 tracer (live_quiz_running_list via the persisted-GraphQL path), gated T1/T2 items behind the A1/A2 rulings, small cleanups, and the PR finish. Carries the environment gotchas, non-negotiable rules (hard gates, hygiene, push refspec), the decision-gate table, and the per-tool rollout checklist so a new engineer can execute without prior session context.
1 parent c42fa4c commit 60e1d53

2 files changed

Lines changed: 279 additions & 0 deletions

File tree

project/2026-07-26-pr-5109-verification-and-extension-plan.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,8 @@ Any T3 item pursued is its own project with its own threat model, not a tool add
229229

230230
Open decisions to surface to Roland, in order: **A1** (answer-first vs act-first value), **A2** (T2 preview/confirm model), **A3** (consolidate vs multiply), then **A4/A5** only if T3/external ever come up.
231231

232+
> **Follow-up execution:** remaining work (W1 live judge run + nightly secrets, W2 X3 tracer/T1 slice, gated T1/T2 items, cleanups) is broken down junior-executable in [2026-07-27-manage-assistant-followup-roadmap.md](./2026-07-27-manage-assistant-followup-roadmap.md). This plan stays the architectural record; that roadmap owns execution from here.
233+
232234
## Progress (implementation phase, from 2026-07-26)
233235

234236
- 2026-07-26: **Implementation authority granted** (Roland: "work through the plan", Opus subagents for investigation/research/implementation). Rulings A1-A5 + sequencing as proposed. Plan committed first (`dbf2147b0`, pre-commit 25/25 green). No push/merge authority.
Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
# Manage Assistant — Follow-Up Roadmap (junior-executable)
2+
3+
Status: ACTIVE. Written 2026-07-27, after the X2b eval-extension slice landed
4+
(`c42fa4c54`) and PR #5109 CI went green (except the inherited GitGuardian
5+
finding, see W6).
6+
7+
**Who this is for:** an engineer new to this feature area, picking up the
8+
remaining work without the prior sessions' context. Every work item below is
9+
self-contained: context, exact steps, verification, and done-criteria. Read
10+
the two "before you start" sections first; they prevent the mistakes that
11+
cost the most time so far.
12+
13+
**Parent artifacts (read on demand, not up front):**
14+
15+
- [2026-07-26-pr-5109-verification-and-extension-plan.md](./2026-07-26-pr-5109-verification-and-extension-plan.md)
16+
— the architect-level plan this roadmap executes. §5.1a explains the two
17+
substrate constraints behind every authZ decision here; §5.5 is the
18+
rollout gate every new tool must pass; the Progress sections record what
19+
already landed and why.
20+
- [docs/auth-model.md](../docs/auth-model.md) — lecturer MCP trust chain,
21+
injection defense (fencing + sentinel no-disclosure), proposal signing.
22+
- [evaluation/manage-assistant/README.md](../evaluation/manage-assistant/README.md)
23+
— how to run the eval harness, env vars, seeding, summary semantics.
24+
- [docs/solutions/best-practice/dev-seed-is-not-idempotent-reset-first.md](../docs/solutions/best-practice/dev-seed-is-not-idempotent-reset-first.md)
25+
— read BEFORE touching the dev database.
26+
27+
## Current state (what is already done — do not redo)
28+
29+
| Item | State | Evidence |
30+
| --- | --- | --- |
31+
| X1 CI MCP integration test | DONE | `test-mcp-lecturer` workflow: unit tests + happy smoke + 11 negative cases |
32+
| X2a eval harness E1/E5/E6 | DONE + live-verified | Full-strength run: E1 1.000 (12/12), E5 1.000 (24/24 trials), E6 1.000 (30/30 trials) |
33+
| X2b judge dims E3/E4/E7 + nightly workflow | DONE (code); live judge run NOT yet executed | `c42fa4c54`; see W1 |
34+
| X3 read-authZ convergence | DECIDED (Option C), not implemented | Plan §5.1 X3 row; tracer = W2 |
35+
| X4 injection defense (fencing + prompt rules) | DONE | incl. sentinel no-disclosure fix `5bace13c6`, measured 0/3 leaks post-fix |
36+
| X5 audit record on confirmed persistence | DONE | plan Progress 2026-07-26 |
37+
| X6 mint-chain least-privilege | DONE | plan Progress 2026-07-26 |
38+
39+
PR **#5109** (head branch `codex/manage-assistant-mcp-v3-ai`) carries all of
40+
it. CI: all green except **GitGuardian**, which is an inherited finding (a
41+
staging JWT in an old `k6.js` commit; the secret is already rotated — it is
42+
NOT caused by new pushes and does not block work, see W6).
43+
44+
## Before you start: environment
45+
46+
Everything runs against the self-contained devcontainer stack. From the
47+
checkout root:
48+
49+
```bash
50+
devrouter ensure .
51+
```
52+
53+
- Run pnpm / prisma / tests **inside the container**
54+
(`devrouter exec . -- <cmd>`), never on the host. Never `pnpm install` on
55+
the host in a linked worktree — it breaks `turbo dev` with 502s.
56+
- The eval harness itself (Python/uv) runs **on the host** against the
57+
stack's routed URLs; its README documents the exact env vars.
58+
- Local login: delegated `lecturer` / `abcd` — never Edu-ID locally.
59+
60+
**Database seeding — order is load-bearing:**
61+
62+
1. Base seed only into a FRESH database (reset first). The base seed is NOT
63+
idempotent and DELETES `Element` rows (see the solutions doc above).
64+
2. Eval fixtures AFTER the base seed:
65+
`PYTHONPATH=src uv run python -m manage_assistant_eval.seed --seed`
66+
(from `evaluation/manage-assistant/`).
67+
3. The harness refuses to run if either layer is missing — trust that error
68+
message; it tells you the exact command to run.
69+
70+
**Known traps (each cost real debugging time):**
71+
72+
- Running `pnpm run check` (typegen) while the dev stack is up de-registers
73+
Next.js API routes (chat's `/api/manage/*` starts 404ing). Remedy: touch
74+
the affected `route.ts` files in-container; do NOT restart the stack.
75+
- `pnpm run check:all` can fail flakily under full parallelism with the dev
76+
stack running (resource contention). If the composite fails, re-run the
77+
six sub-checks sequentially (`check`, `check:format`, `check:lint`,
78+
`check:syncpack`, `check:agents-md`, `check:prisma-sync`) — if each passes
79+
alone, the code is fine.
80+
- This worktree can silently end up on a **detached HEAD**. Before every
81+
commit: `git status | head -2`. If detached and HEAD is a descendant of
82+
the branch, reattach with `git checkout -B <branch>`.
83+
- Pushing: the PR head branch is `codex/manage-assistant-mcp-v3-ai`, which
84+
does NOT match the local branch name. Always push with an explicit
85+
refspec: `git push origin HEAD:codex/manage-assistant-mcp-v3-ai`. A bare
86+
push creates a stray branch that CI never sees. If SSH fails with "agent
87+
refused operation", push HTTPS with
88+
`git -c credential.helper='!gh auth git-credential' push https://github.com/uzh-bf/klicker-uzh.git HEAD:codex/manage-assistant-mcp-v3-ai`.
89+
90+
## Before you start: non-negotiable rules
91+
92+
1. **Never weaken an eval gate to make it pass.** E5 and E6 are hard gates
93+
(0 failures allowed). If a case seems wrong, first verify the environment
94+
(the harness's readiness probes exist because 7 "model failures" were
95+
once a half-seeded DB), then raise the case-design question — do not
96+
lower a threshold, delete a case, or add trials-capping to CI.
97+
2. **Public repo.** Data-hygiene check before every commit
98+
(`git diff --cached` + open any staged data file): no secrets, no real
99+
personal data. Secret values never in chat, code, logs, or plan files.
100+
3. **No merge of PR #5109** without explicit authority from Roland.
101+
4. **Parked:** `packages/i18n/messages/de.ts` has an unresolved review
102+
thread — do not touch that file.
103+
5. Every new MCP tool must pass the plan's §5.5 rollout gate (checklist
104+
below) before its slice is called done.
105+
106+
---
107+
108+
## Work items
109+
110+
Recommended order: **W1 → W2 → (A1 ruling) → W3 → (A2 ruling) → W4**, with
111+
W5/W6 as fill-in work anytime. W2 is the only item that needs no ruling and
112+
no secrets — it is the best starting point if W1's secrets are not yet
113+
available.
114+
115+
### W1 — First live judge run + nightly secrets (S effort, needs Roland for secrets)
116+
117+
**Goal:** the judge-based dimensions (E3 grounding, E4 proposal-quality
118+
judge, E7 graceful-message) have never scored a live turn. Execute one full
119+
live judged run locally, fix whatever it surfaces, then wire the nightly.
120+
121+
**Context:** `evaluation/manage-assistant/src/manage_assistant_eval/judge.py`
122+
builds a DeepEval `GPTModel` pointed at any OpenAI-compatible base URL. The
123+
devcontainer's litellm gateway serves `gpt-4.1-mini` (which is also in
124+
DeepEval's `valid_gpt_models` list) — but litellm has **no published host
125+
port**, so a host-run harness cannot reach it directly. Known residual risk
126+
(documented in judge.py's docstring): GEval requests logprobs; a gateway
127+
that does not proxy logprobs faithfully will error on the first live call.
128+
129+
**Steps:**
130+
131+
1. Pick the judge route. Two options, in order of preference:
132+
a. Publish the litellm port to the host for the run (compose override or
133+
`docker compose ... port` mapping) and set
134+
`MANAGE_ASSISTANT_EVAL_JUDGE_API_BASE` to it, model `gpt-4.1-mini`.
135+
b. Use a direct OpenAI-compatible key via the `rs-infisical-operator`
136+
flow (ask Roland; never paste key values anywhere).
137+
2. Run the full suite with the judge configured (README "Environment
138+
variables"). Expect the 18 previously-skipped judge cases to now run.
139+
3. If GEval errors on logprobs: try a model the gateway proxies faithfully,
140+
or record the incompatibility in judge.py's docstring and the README and
141+
raise it — do NOT silently disable the metric.
142+
4. Review judge scores case-by-case on first run (they are soft gates at
143+
0.85/0.90): a systematically low score usually means the rubric text in
144+
the case files needs tightening, not that the assistant regressed.
145+
5. For nightly CI: ask Roland to configure the repository secrets the
146+
workflow reads (`MANAGE_ASSISTANT_EVAL_JUDGE_MODEL`, `_JUDGE_API_KEY`,
147+
`_JUDGE_API_BASE`, `_CHAT_BASE_URL`, `_APP_SECRET`, `_DATABASE_URL`,
148+
`_LECTURER_SUB`) pointing at a reachable deployment. Without them the
149+
workflow self-skips cleanly (by design).
150+
6. Trigger `workflow_dispatch` once; verify the job summary and that the
151+
printed verdict is `OVERALL: PASS` (three-state: PASS requires all eight
152+
dimensions measured — `INCOMPLETE` means something skipped).
153+
154+
**Done when:** one live judged run has completed with all eight dimensions
155+
measured; any surfaced defects are fixed or filed; nightly ran once green
156+
via dispatch.
157+
158+
### W2 — X3 tracer: `live_quiz_running_list` T1 read tool (M effort, no ruling needed)
159+
160+
**Goal:** implement the decided (Option C) read-authZ convergence pattern
161+
with its tracer tool: a lecturer MCP tool listing the lecturer's currently
162+
running live quizzes.
163+
164+
**Context (read plan §5.1 X3 row + §5.1a first):** most GraphQL service
165+
functions have NO internal permission check — authZ lives in the resolver
166+
layer (`withPermission`). Calling such a function directly from an MCP tool
167+
bypasses authorization entirely. Option C: new read tools go through the
168+
**persisted-GraphQL client** path (as mcp-student already does), which IS
169+
the resolver path. The tracer op `QGetUserRunningLiveQuizzes` is already
170+
persisted, role-only gated, and its backing service function is
171+
self-contained (scoped to `ctx.user.sub`) — the safest possible first case.
172+
173+
**Steps:**
174+
175+
1. Study the mcp-student `PersistedGraphQLClient` usage (that app is the
176+
working precedent) and mcp-lecturer's existing tool registration
177+
(`apps/mcp-lecturer/src/`).
178+
2. Add the tool: name/description written for model selectability (plan
179+
§5.5 item 3 — the E1 eval measures this); payload limited to `id` +
180+
`name` per the X3 decision. NO pinCode-style fields ever (plan §5.1 X3
181+
caveats).
182+
3. Extend the X1 CI smoke: happy path + the negative matrix
183+
(owned/foreign/missing/malformed) for the new tool.
184+
4. Add eval cases: one E1 selection case ("what's currently running?"
185+
should select the new tool), one E3 grounding case. Run the harness
186+
locally — E1 must stay ≥ 0.95 with the enlarged tool surface; if it
187+
drops, the tool description needs work (plan A3: selection reliability
188+
caps value).
189+
5. Update `docs/auth-model.md` (tool inventory + the persisted-GraphQL read
190+
path) and `docs/log.md` in the same PR.
191+
6. Walk the §5.5 rollout-gate checklist (below) before calling it done.
192+
193+
**Done when:** tool live-verified via the chat assistant on the dev stack
194+
(ask "which of my live quizzes are running?"), CI smoke covers it, eval
195+
passes at full thresholds, wiki updated, gate checklist all ticked.
196+
197+
### W3 — Remaining T1 reads (M-L effort, GATED on Roland's A1 ruling)
198+
199+
**Do not start until Roland rules A1** (answer-first vs act-first — plan
200+
§5.0). If A1 confirms answer-quality investment, implement in this order,
201+
each as its own slice repeating the W2 recipe (persisted-GraphQL path, CI
202+
negative matrix, E1+E3 cases, wiki, §5.5 gate):
203+
204+
1. `course_summary` (op exists: `getCourseSummary`, resolver-wrapped
205+
`withPermission(READ)` — MUST ride the persisted-op path, never a direct
206+
service call).
207+
2. `activity_summary` — one tool with a `type` arg covering live quiz /
208+
microlearning / practice quiz / group activity summaries (all four
209+
backing fns are resolver-wrapped `withPermission(READ)`).
210+
211+
Explicitly rejected (do not implement): `course_performance_analytics`
212+
(carries per-participant `username`+`email` — PII; plan §5.2) and
213+
`element_usage_get` (low chat value, ADMIN-gated).
214+
215+
### W4 — T2 proposal-writes on existing objects (L effort, GATED on Roland's A2 ruling)
216+
217+
**Do not start until Roland rules A2** (diff-preview requirement — plan
218+
§5.0/§5.3). This is design-first work: mutations of existing state need a
219+
before/after diff preview in the proposal card and a staleness guard (the
220+
confirm route re-reads current state and rejects if it changed since the
221+
proposal was minted). The signing/confirm substrate already exists
222+
(`signProposalToken`, jti claim, confirm under the lecturer's own session —
223+
plan §5.1a correction). Candidate order when unblocked:
224+
`element_batch_op_proposal` (best-designed backing fn; MUST preview the
225+
exact matched id list), `tag_rename_proposal`, then
226+
`element_update_proposal` (highest hazard — read its §5.3 row carefully).
227+
Every T2 slice needs E4 + E5 + E7 eval cases, an X5 audit record, a feature
228+
flag, and the full §5.5 gate.
229+
230+
### W5 — Small cleanups (S each, no ruling needed, good first tasks)
231+
232+
| # | Task | Detail |
233+
| --- | --- | --- |
234+
| W5.1 | httpx `verify=<str>` DeprecationWarning | `sse_client.py`/`conftest.py` pass a CA-bundle path string; switch to `ssl.create_default_context(cafile=...)`. Offline tests + one live smoke must stay green. |
235+
| W5.2 | Langfuse MCP tool spans | Plan §3.5 / §5.5 item 8: MCP tool calls are currently inferred, not first-class spans. Add explicit spans so each tool call is visible in a trace. |
236+
| W5.3 | P9 i18n remainder | The one open item from the prior hardening plan. NOTE: `packages/i18n/messages/de.ts` is parked under an unresolved review thread — coordinate with Roland before touching anything in that file. |
237+
| W5.4 | Judge-model qualification note | After W1: record which judge model/gateway combo actually worked (logprobs behavior included) in the harness README so nightly configuration is copy-paste. |
238+
239+
### W6 — PR #5109 finish (S effort, mostly Roland)
240+
241+
- The GitGuardian red is inherited (rotated staging JWT in an old k6 script
242+
commit; the `__ENV` fix ships in a separate PR). It will stay red on every
243+
push until that PR merges — do not chase it here.
244+
- Keep the PR description in sync with what the branch now contains
245+
(eval harness + X2b + fence fix) when it next changes.
246+
- Merge decision, timing, and any squash strategy: Roland's call, not
247+
yours. Your job is keeping CI green and review threads addressed.
248+
249+
---
250+
251+
## Decision gates (Roland only — do not pre-empt)
252+
253+
| Gate | Question | Blocks | Current recommendation on file (plan §5.0) |
254+
| --- | --- | --- | --- |
255+
| A1 | Answer-first vs act-first product direction | W3 | answer-first is under-served → build T1 reads |
256+
| A2 | Is create-card confirm enough for mutations of existing state? | W4 | no — diff preview + staleness guard required |
257+
| A3 | Consolidate vs multiply tools | tool naming/shape in W2/W3 | consolidate; fewer well-named tools |
258+
| A4 | Does T3 (session control/grading/roster) belong in chat at all? | any T3 idea | no — stays in direct UI |
259+
| A5 | External MCP exposure / OAuth | any external-facing work | stay internal until a concrete consumer exists |
260+
261+
## Rollout-gate checklist (copy into every new-tool slice; from plan §5.5)
262+
263+
1. Non-goals stated.
264+
2. AuthZ rides the shared path (X3 pattern); CI negative matrix
265+
(owned/shared/foreign/missing/malformed) passes.
266+
3. Tool name/description reviewed for selectability; E1 case added, E1 ≥ 0.95.
267+
4. E3 (reads) or E4 (writes) case added and passing.
268+
5. E5 + E6 cases pass (hard gates, full trial counts).
269+
6. Writes only: diff preview, staleness guard, X5 audit record, jti replay
270+
guard, rate limit.
271+
7. Feature-flagged, internal-only; rollback = flag off.
272+
8. Langfuse span visible for the tool call.
273+
274+
## Progress
275+
276+
- 2026-07-27: Roadmap created. W1-W6 scoped; W2 identified as the
277+
no-dependency starting point. Nothing below this line has started.

0 commit comments

Comments
 (0)