Teachers can claim out-of-scope students as theirs (JTP/ayudante takeover)

Sub-tickets

Activity

• wi-cli-venus created · 2026-07-23
• wi-cli-venus assigned · 5w ago
  coder02-mars-cc
• wi-cli-venus note · 5w ago
  VERIFIED STILL OPEN 2026-09-11, against the deployed code. The MVP this row was deferred out of DID ship (ea68430, /usuarios/[id] + updateAlumno widened to viewStudents) and MARS-108 is closed on it. This row's remainder - a teacher claiming a student who is NOT currently theirs - is still absent, and the blocking mechanism is explicit: decorateAssignmentsForTeacherScope (src/lib/usuarios.ts, called at usuarios/[id]/page.tsx:71-75) gives a non-roster teacher per-row editability only where they already have standing, and updateAlumno (administrar-actions.ts:1331-1340) scope-checks non-roster actors IN-TX against the locked assignment and the target comisión. So takeover fails at the server, not just the UI - any implementation has to add a deliberate path, not relax a gate. No commit anywhere in history names MARS-553.
• wi-cli-venus audit-finding · 5w ago
  INVESTIGATED 2026-09-11 (coder02-mars-cc). The directive stands, but the row's assumption that takeover has to be built from nothing is wrong, and the reason is instructive. Nothing built; no code or data changed. A PERMISSIVE TAKEOVER PATH ALREADY EXISTS IN THE TREE, WITH A WEAKER GATE THAN THE SURFACE THAT REPLACED IT. reassignTeacher in src/app/(protected)/alumnos/actions.ts:39 gates on exactly three things: hasCap(manageStudents) OR fullAccess (:65 - manageStudents is held by titular, adjunto, jtp AND ayudante, so every teacher role); isTeacherForComision(session.id, comisionId) (:92 - COMISION-level); and isTeacherForComision(newTeacherId, comisionId) for the incoming teacher (:99). THERE IS NO CHECK THAT THE ACTOR CURRENTLY HOLDS THE STUDENT, AND NO CHECK THAT THE NEW TEACHER IS NOT THE ACTOR. That is precisely the takeover this row asks for. Contrast the note above: /usuarios' updateAlumno scope-checks a non-roster actor IN-TX against the locked assignment, and decorateAssignmentsForTeacherScope withholds per-row editability - so the two surfaces disagree about the same operation, which is the real defect here. IT WAS EXERCISED WHILE ITS PICKER WAS RENDERED. studentAssignmentsArchive, reason admin:reassign-jtp: 106 rows, 21 distinct actors, 2026-04-29 to 2026-06-10. Role sets of those 21 actors: jtp 13, ayudante 5, ayudante+bedel 1, adjunto 1, adjunto+jtp 1 - ZERO admins, so the admin:* reason is mislabelled and that path was always teacher-facing. Reconstructing each change's NEW value exactly (LEAD over archiveSeq per assignment id, falling back to the live row for the newest) rather than comparing against the current value: 68 of the 106 record no change to the teacher at all, leaving 38 real teacher changes - 11 SELF-TAKEOVERS by 4 distinct actors, 10 give-aways by the outgoing teacher, 17 third-party moves. So demand is real but small: 11 takeovers by 4 teachers across the six weeks the control was reachable. (My first cut used the CURRENT assignedTeacherUserId as the new value and reported 32 takeovers by 10 actors; that over-counts because later writes by other reasons touched the same rows. The reconstruction above supersedes it.) REACHABILITY TODAY IS UNVERIFIED AND I AM NOT GOING TO OVERSTATE IT. The only caller of reassignTeacher is src/components/teacher-assign-select.tsx (a \"use client\" component), and NOTHING imports that component - its render site went away in c350f3d/7cae492's /alumnos rework, which matches the archive going quiet after 2026-06-10. Exporting from a \"use server\" module normally makes a function a directly-invokable RPC, but Next compiles from route entries, so a module no entry reaches plausibly never gets a server-action id. I have not verified which is true here and the check that would settle it is the deployed build's server-reference-manifest, which I have no read path to. Either way the remediation is the same and is correct under both answers: delete teacher-assign-select.tsx and reassignTeacher, or give reassignTeacher updateAlumno's in-tx standing check. Deleting also removes the question permanently. WHAT THIS MEANS FOR THE ROW. The feature is still wanted (Elazar directive, and 11 real uses argue it was used as intended). What changes is the shape: build the deliberate path on the LIVE surface with an explicit takeover affordance and an audit trail, and retire the divergent orphan in the same change - do not relax /usuarios' gate to match the orphan, which would be the narrow-a-permission-to-match-a-list move commons forbids. TWO QUESTIONS THAT ARE ELAZAR'S, NOT MINE AND NOT THE PM'S, because they are consent semantics between colleagues rather than implementation: (1) may a teacher take a student from another teacher WITHOUT that teacher's agreement, or does takeover need the displaced teacher's approval (the MARS-147 request/approve shape) - the 11 historical uses happened with no consent step, so 'no approval' is the status quo rather than a new liberty; (2) is the displaced teacher NOTIFIED. Today nothing tells them: the action writes an appEvents row and archive attribution, and no notification or email at all. Silence is what makes an unconsented takeover invisible rather than merely permitted. Coverage: read alumnos/actions.ts in full, teacher-assign-select.tsx, the import graph around both, the MARS-553 note's named gates; queried studentAssignmentsArchive (all reasons) and reconstructed the reassign-jtp sequence. NOT exercised: no browser walkthrough, no build inspection, no attempt to invoke the action. Aggregates and role sets only, no PII.
• wi-cli-venus decision · 5w ago
  PM DECISION 2026-09-11, coder-mars-cc in the PM seat. Answering coder02-mars-cc's kind=blocked. Q1 is mine: GO - delete teacher-assign-select.tsx and reassignTeacher. Q2 and Q3 are Elazar's and are now in his terminal; they do NOT gate the deletion. I VERIFIED THE GATING CLAIM MYSELF RATHER THAN TAKING IT, because it is the one that authorises the action. grep over src/: teacher-assign-select.tsx has ZERO importers, and reassignTeacher is imported by exactly one file - that orphan component. Confirmed independently, not relayed. WHY THE DELETION IS CORRECT UNDER BOTH REACHABILITY ANSWERS, which is what makes it decidable without resolving the manifest question: if the module never gets a server-action id, this is dead code and removing it is a no-op. If it DOES, then a live permissive path exists with no UI surface at all - reachable only by hand-crafting a POST to an action id - while diverging from /usuarios' in-transaction scope check on the SAME operation. Neither branch argues for keeping it. The reachability question stays genuinely unresolved and you were right not to claim an answer; it just stops being load-bearing. TWO CONSTRAINTS ON THE CHANGE: A. THE COMMENTS THAT NAME reassignTeacher GO IN THE SAME PUSH. src/lib/self-service-assignment.ts:56 and src/lib/practica-jtp-sync.ts:26 both cite it by name as a live thing. A comment citing a deleted function is the stale-citation class commons names explicitly - a citation is a claim that its target is still there - and it is worse here than ordinary rot, because the next reader looking for the permissive path will find the citation, fail to find the function, and not know whether it was deleted or renamed. Rewrite them to describe the behaviour without the dead name, or to say it was removed and why. B. DO NOT TOUCH THE admin:reassign-jtp REASON ROW. practica-jtp-sync.ts names data-health-actions fixAssignmentJtp/fixAssignmentAyudante as other writers on that path, so the vocabulary entry is not yours to retire on this WI. Your separate finding that the reason is MISLABELLED - 106 rows, 21 actors, zero of them admins, so an admin: prefix on a teacher-facing path - is REAL and stays on this row as a named residual. It is not fixed by this deletion and must not be folded into it: renaming an archive reason rewrites how historical rows read, which is its own decision with its own blast radius. NOT DONE, and you were right to refuse it: relaxing /usuarios' gate to match the orphan. That is narrowing a permission to make a surface match another surface, which commons forbids outright. The divergence is resolved by deleting the permissive side, which is what this GO does. Class-A on the deletion (permission-gate surface), so: build AND runtime log reads on the push, and I review the diff. This event is the design ping; do not wait for a second one.
• wi-cli-venus progress · 5w ago
  GO received from the PM on the deletion half (Q1); Q2/Q3 are with Elazar and do not gate it. DONE LOCALLY, awaiting the push gate: deleted src/app/(protected)/alumnos/actions.ts (the whole file - it held only reassignTeacher and its private logReassignReject) and src/components/teacher-assign-select.tsx (its sole importer, itself imported by nothing). Per the GO's first constraint, both surviving comments that NAME reassignTeacher were corrected in the same change rather than left as citations to a deleted target: practica-jtp-sync.ts:26 now lists two admin-reassign sites instead of three and records that the third was deleted here, and self-service-assignment.ts:56 now says 'the since-deleted alumnos/actions reassignTeacher'. Per the second constraint the admin:reassign-jtp reason row in mars-archive-reasons.md is UNTOUCHED - other writers use it, and the mislabelled-reason finding (21 actors on that reason, zero of them admins) stays a named residual on this row rather than being folded into a deletion push. Post-deletion grep over src/ returns only those two intentional historical mentions. What this does NOT do: it removes the divergence, it does not build the takeover feature. The feature remains open on this row pending Elazar's two consent questions.
• wi-cli-venus progress · 5w ago
  SHIPPED e9581a5, v2.79.62 (coder02-mars-cc) - the DELETION half only. This row is NOT closed: the takeover feature itself is still unbuilt and still waits on two Elazar consent questions recorded here (may a teacher take a student without the colleague's agreement; is the displaced teacher notified - today nothing tells them). DELETED, not disabled: src/app/(protected)/alumnos/actions.ts in full (reassignTeacher plus its private logReassignReject) and src/components/teacher-assign-select.tsx, its sole importer, imported by nothing. A disabled control over a live permission produces the inverse false belief ("I am not allowed"), which is quieter than the original because an unexercised permission is never reported. WHY: reassignTeacher gated at COMISION level with no standing check while updateAlumno gates on standing inside the transaction - two paths onto the same column, the permissive one being the one on the page the teacher was already looking at. Measured by exact reconstruction (LEAD over studentAssignmentsArchive by archiveSeq, NOT a comparison against the current assignedTeacherUserId - that is what made my first count 32/10 and it was wrong): 68 no-ops, 38 real changes, of which 11 SELF-TAKEOVERS by 4 actors, 10 give-aways, 17 third-party. The path was used while its picker was rendered. Also corrected two comments that cited reassignTeacher as a live call site (practica-jtp-sync.ts, self-service-assignment.ts) - a citation is a claim its target still exists. PTD, review-depth pilot: DEPTH=deep CHECK=2b REASON="gate vocabulary in diff: 8:-import { getSession } from \"@/lib/auth\";" SAMPLED=no. Cold brief sent to coder-mars-cc; verdict pending. LIVE-VERSION CONFIRM, clean for this SHA: live /api/health version 2.79.62, deploymentId dpl_L5RoWYWuutdf5ZRs922iNVjuitY7; REMOTE package.json on main 2.79.62 (gh api, not the local checkout); e9581a5 is the remote tip and an ancestor of origin/main. CLASS-A evidence (the PM called this Class-A explicitly): Vercel build completed in 30s with zero error/stderr/fatal lines; get_runtime_errors over 1h returns no clusters. COVERAGE BOUND: no error observed, and the meaningful negative check - that no surface still tries to import the deleted module - is carried by the whole-tree typecheck and the 807-test suite passing with both files removed, not by the log window.