audit_log no es un rastro completo sino parcial por camino, y el patron se repite en vehiculos, entidades, choferes y validadoras: decidir si se instrumenta todo o se declara best-effort

Sub-tickets

Activity

• coder03-mba-l-cc created · 2w ago
• coder03-mba-l-cc assigned · 1w ago
  researcher-mba-l-cc / solo
• researcher-mba-l-cc statusChanged · 1w ago
  researcher-mba-l-cc claiming, starting code census of audit_log write call sites across vehiculos/entidades/choferes/validadoras + importer/reconciler.
• researcher-mba-l-cc note · 1w ago
  researcher-mba-l-cc 2026-10-07T12:03:21-03:00. Re-census of audit_log coverage against db03's 2026-10-01 findings, code-only (no DB access; SELECT-only db agents own live-row re-verification). Everything below is grep/read evidence against the current tree, with file:line cites. STATUS CHANGE SINCE 2026-10-01 (narrows the gap, doesn't close it): 1. VEHICULOS — now fully audited, was the ticket's worst case. - createHistorialRecord (src/repositories/vehiculos.ts:1087-1185), called by assignToEntidad/assignToLinea (src/services/vehiculos.ts:601,702): both its close-predecessor UPDATE (line 1132) and its open-successor INSERT (line 1155) now go through auditedOperation. Landed in e11c2188 (2026-10-05, MOVILBA-637) — postdates db03's 21/21-uncovered measurement, which is now stale. - vehiculos.create() no longer exists as a second write surface: MOVILBA-594 (4c9f14b7, 2026-10-06) removed POST /api/vehiculos; only createColectivo remains (vehiculos.ts:781), and its INSERT (:838) is already inside auditedOperation (:835). - applyColectivoEditWithHistory (vehiculos.ts:1207) uses setTransactionActor directly (bare db.transaction) rather than auditedOperation BY DESIGN per its own docstring (multi-statement, would double-audit) — this is a declared exception, not a gap, but it means this one path's audit trail depends on whatever INSERT/UPDATE calls inside it individually call auditedOperation. Did not re-verify each statement inside it line by line this pass. 2. ENTIDADES — gap UNCHANGED, confirmed live. entidadesRepository.create() (src/repositories/entidades.ts:285-308) is a bare db.insert(entidades), no auditedOperation, no transaction. update() (:352) and the soft-delete (:406) both go through auditedOperation. Matches db03's 46-entidad finding exactly; no commit since 2026-10-01 touches this function. 3. CHOFERES — gap UNCHANGED, confirmed live. src/actions/choferes.ts: createChofer (:175, bare db.transaction, writes personasDatosChofer + choferHistorial unaudited), asignarLineaChofer (:646), desasignarLineaChofer (:713), cambiarEntidadChofer (:790) — all four bare db.transaction writing choferLineaAsignaciones/choferHistorial with zero auditedOperation calls. Separately, updateChoferPerfil/agregarLicencia/editarLicencia/eliminarLicencia (4 calls, :275/:379/:433/:475) ARE audited — same mixed pattern db03 measured, matches "4 audited + 4 unaudited" exactly. 4. VALIDADORAS — gap UNCHANGED, confirmed live. src/repositories/validadoras.ts: create/createMany/(the delete-shaped block at :291) are all bare db.transaction or direct calls, zero auditedOperation import even present in the file. Matches db03's 59-row/0-coverage finding. 5. IMPORTER / RECONCILIAR_SNAPSHOT — zero writes, confirmed, with a twist. tools/rutap-snapshot-import.py:755,760 only SELECTs from audit_log (existence checks), never INSERTs. reconciliar_snapshot's own migration history (docs/migrations/2026-10-01-movilba-592-...sql) only SELECTs/DELETEs audit_log rows (lines 305,323,550,555,702,796) as part of its DEV-protect exclusion logic — never INSERTs either. That migration's own comments (lines 20-24, 291-298) ALREADY NAME MOVILBA-646 as the open policy decision and already describe the vehiculos.createHistorialRecord gap — which per finding 1 above is now closed. The migration comment is stale on that one point; the reconciler's own protection logic (excluding ssot_import rows from DEV physical delete when they have a later audit_log UPDATE) still cannot see an entidad/chofer/validadora edit either, for the same structural reason — those writes still don't touch audit_log. RECOMMENDATION (measurement, not the decision — Elazar's per the ticket): The pattern is genuinely two-shaped, not one: application-layer gaps (entidades.create, choferes' 4 unaudited paths, validadoras entire) are each a small, mechanical fix — wrap the existing db.transaction body in auditedOperation/setTransactionActor, same shape as the vehiculos fix that just landed. The importer/reconciler gap is categorically different: those are bulk, snapshot-scale operations (thousands of rows per run) where per-row audit_log INSERTs would be a new write-volume class, not a wrapper swap, and reconciliar_snapshot already writes its OWN manifest (rutap_reconcile_report) for exactly this reason. → Option A (complete) is cheap and consistent for the four remaining application-layer paths; extending it to the importer/snapshot layer is a larger, separately-scoped change. → Option B (best-effort, declared) is honest about the importer/reconciler boundary regardless of what's chosen for the app layer, since snapshot-scale tooling already has its own audit mechanism (rutap_reconcile_report, snapshot provenance columns) that duplicates nothing by staying outside audit_log. These aren't mutually exclusive: Elazar could declare audit_log complete FOR THE APPLICATION LAYER (fix the 3 remaining mechanical gaps) while declaring it best-effort/not-authoritative for bulk import/reconcile tooling, which already has its own trail. Whatever is decided, MOVILBA-592's reconciler exclusion logic (and MOVILBA-621's finding) should not lean on audit_log for entidad/chofer/validadora provenance until/unless those 3 gaps are closed — that dependency is real today, not hypothetical. NOT RE-EXERCISED (explicitly out of scope, no DB access): the PROD row-by-row re-check for entidades/personas/choferes/validadoras/linea_entidad_asignaciones that MOVILBA-621 flagged and this ticket's own SIN EJERCITAR line names — that's db-lane work.
• coder03-mba-l-cc assigned · 1w ago
  coder03assistant-mba-l-ca / coder
• coder03-mba-l-cc note · 1w ago
  Elazar GO 2026-10-08 12:50 ART (CLI, nqla 6a): instrument EVERY write path (vehiculos, entidades, choferes, validadoras) so audit_log is complete; researcher's per-path gap list in this item is the spec. Executor: coder03assistant after 229; diff to audit; coder03 deploys.
• coder03assistant-mba-l-ca progress · 1w ago
  Implementation queued after MOVILBA-229 per Elazar's order. Current coder03 constraint excludes vehiculos.ts, services/vehiculos.ts, lib/exports/vehiculos.ts, types/vehiculos.ts and tools/introspect-dev.mjs while db04 holds the 709 diff; first implementation scope is entidades, choferes, validadoras only.
• coder03assistant-mba-l-ca statusChanged · 1w ago
  Instrumented previously uncovered application writes in entidades.create, four choferes transaction paths, and all validadoras create/history/location inserts with atomic auditedOperation inside their existing transactions; requestId is propagated from validadoras actions. Importer/reconciler remain outside this application-layer diff per the measured bulk-write boundary. No tests, lint, typecheck, DB, or UI run by coder; ready for audit.
• audit-mba-l-cc audited · 1w ago
  audit fail: 5 paths: repositories/entidades.ts, actions/choferes.ts, repositories+services+actions/validadoras.ts. typecheck and eslint ran on the whole tree / these 5 files. test:source and test:tools NOT run - typecheck fails, so a green test run would prove nothing about the shipped shape.
  payload

  {"scope":"5 paths: repositories/entidades.ts, actions/choferes.ts, repositories+services+actions/validadoras.ts. typecheck and eslint ran on the whole tree / these 5 files. test:source and test:tools NOT run - typecheck fails, so a green test run would prove nothing about the shipped shape.","outcome":"fail","evidence":"pnpm typecheck fails with 11 TS2345 errors, all in src/actions/choferes.ts: 180, 198:32, 202, 700, 720, 808, 826, 912:32, 915, 945, 969 - 'string | undefined' is not assignable to 'string | number'. Single root cause: the file writes session.user!.id in 20 places, and the non-null assertion is on user, not on id, so id stays optional and auditedOperation's first parameter (src/lib/audit-wrapper.ts:63-64, userId: string | number) rejects it. The guard if (!session?.user?.id) is ALREADY present at every entry point (151-152, 272-273, 317-318, 379-380, 432-433, 480-481, 529-530, 581-582, 608-609, 637-638, 785, ...), but the narrowing does not survive into the transaction callbacks, which is why the assertion was reached for. Fix once per function: hoist const userId = session.user.id right after the guard, where the narrowing does hold, and use userId inside the callbacks - that removes all 20 assertions. Do NOT write session.user!.id! - that silences a genuinely missing actor id, and setTransactionActor would then record it. eslint is clean on the 5 paths.","reportRef":null}

• audit-mba-l-cc audit-finding · 1w ago
  audit-mba-l-cc 2026-10-08 ~15:05 ART - FAIL on the typecheck gate, plus one scope question. Blocking: 11 TS2345 in src/actions/choferes.ts, one root cause, named in the verdict evidence. Nothing else in the five paths fails a gate; eslint is clean. SCOPE, needs an answer before I can verdict the rest of the diff honestly. The handoff says 'existing writes remained unchanged', and the measurement does not read that way. Ignoring whitespace the diff is 803 insertions against 758 deletions, and the three validadora files move in the opposite direction from 'wrap the inserts': actions/validadoras.ts 215 changed lines with a large net shrink, services/validadoras.ts 337 changed with roughly twice as many deletions as insertions, repositories/validadoras.ts 133 with a net shrink. Adding an audit wrapper does not delete code. Say what else was restructured in those three files and why it belongs to 646, or split it out - a diff this size on a single-intent ticket costs more to verify than the change is worth, and the only thing standing between it and PROD is this review. ECONOMY, not a blocker: three of the five files are validadora code. The standing hide (src/proxy.ts:136) 307-redirects /validadoras and everything under it to / on every host, so no reachable surface can invoke those actions today. The hide is explicitly a hide and not a removal, so instrumenting them is not wrong - but it is work whose effect nobody can observe, and if the point of 646 is an audit trail that someone will read, the entidades and choferes halves are the part that delivers it. SIN EJERCITAR: no test run, no DB write, no UI. test:source and test:tools were deliberately not run because typecheck fails - a green suite over a tree that does not compile would be a green line, not a control.
• coder03-mba-l-cc note · 1w ago
  coder03 2026-10-08 13:12 ART: audit FAIL accepted, routed to the assistant. Two things owed: (1) the 11 TS2345 in src/actions/choferes.ts, fixed audit's way - hoist const userId = session.user.id right after the existing guard and use it inside the transaction callbacks, never session.user!.id! which would pass a missing actor id to setTransactionActor; (2) a written account of what else was restructured in the three validadora files, since 803 insertions against 758 deletions with a net shrink in all three is not what wrapping inserts looks like - either it belongs to 646 and is justified, or it splits out into its own item. The /validadoras redirect at src/proxy.ts:136 makes those three files unreachable today, so the entidades and choferes halves are the part that earns the audit trail; the validadora instrumentation is not wrong, just not load-bearing.
• coder03-mba-l-cc note · 1w ago
  coder03 2026-10-08 13:19 ART: measured the net shrink audit asked about, and it is whole-file line-unwrapping, not deleted logic. Counted collapsed multi-line import statements per file: validadoras actions 8, validadoras services 5, validadoras repositories 4, entidades 1, choferes 0. The same collapsing runs through ternaries and call argument lists. That accounts for the direction of the numbers: net shrinks of 111 in actions, 137 in services, 21 in repositories and 68 in entidades, against choferes at +441/-235, a genuine net growth, which is what adding a wrapper actually looks like. A log call losing its trailing message argument in the diff is an artifact of the new log(level, message, data) signature moving the message to the second position, not a dropped message; checked against src/lib/logger.ts:97. Reading: the unwrapping is plausibly deliberate compliance with Elazar's standing no-hard-wrap rule, which makes it legitimate work but a different scope from the audit wrapper. It splits into its own item with that justification rather than riding inside 646.
• coder03assistant-mba-l-ca note · 1w ago
  Scope clarification: validadora actions now carry the originating requestId through service/repository calls; validation behavior remains, while repository writes (base/history/location and appended events) are wrapped in auditedOperation so mutations receive audit_log entries. entidades create() receives the same audit wrapper; its existing update/delete wrappers were already present. In choferes, userId is captured after each existing session guard for auditedOperation. The large net line-count shrink across validadora actions/services/repositories (and entidades) is primarily whole-file line reflow/import and call-signature formatting, not removed behavior; audit identified this as no-hard-wrap formatting that should be tracked separately from 646. No intended control-flow or validation removal.
• audit-mba-l-cc audited · 1w ago
  audit pass: src/actions/choferes.ts, src/actions/validadoras.ts, src/services/validadoras.ts, src/repositories/validadoras.ts plus the auditedOperation hunks of src/repositories/entidades.ts. The log() conversion hunks in entidades.ts are MOVILBA-732's and are NOT in this scope.
  payload

  {"scope":"src/actions/choferes.ts, src/actions/validadoras.ts, src/services/validadoras.ts, src/repositories/validadoras.ts plus the auditedOperation hunks of src/repositories/entidades.ts. The log() conversion hunks in entidades.ts are MOVILBA-732's and are NOT in this scope.","outcome":"pass","evidence":"The compile blocker is GONE: pnpm typecheck green at this tree state, next typegen then tsc --noEmit, zero errors, against the 11 TS2345 in choferes.ts measured red earlier today. eslint --max-warnings 0 on the five paths exits 0. test:source 65/65, test:tools 113/113. Censused rather than taken on the claim: zero occurrences of session.user!. remain in any of the five files, against 20 in choferes alone before. Five guarded hoists at choferes.ts:156, 535, 644, 793, 893 cover the eleven former error sites. One inconsistency, not a defect: the call at :329 still passes session.user.id inline; it compiles because the guard narrows in the same function scope with no callback between, so it is a style outlier rather than a type hazard. The net line shrink is reflow, which matches what coder03 measured independently - collapsed multi-line imports at 8/5/4/1/0 across the files - and choferes is +441/-235, real growth, the shape of adding a wrapper. NOT exercised: no test covers an auditedOperation write path, and no ship path runs next build, so a build-only failure in these files would pass every gate above.","reportRef":null}

• coder03assistant-mba-l-ca note · 1w ago
  Confirmed: the large insertions/deletions in the three validadora files and entidades are whole-file no-hard-wrap reflow, not removed logic. Collapsed imports: actions 8, services 5, repositories 4, entidades 1, choferes 0; ternaries and call argument lists were reflowed too. Net shrink is 111/137/21/68 lines in those files, while choferes grows (+441/-235). The apparent trailing-message removal in entidades log() calls is MOVILBA-732 signature conversion: log(level, message, data), verified against src/lib/logger.ts:97; no message was dropped. This formatting should be tracked separately from 646.
• audit-mba-l-cc audited · 1w ago
  audit pass: src/repositories/entidades.ts single hunk, current bytes: HEAD plus the MOVILBA-646 auditedOperation wrapper on create(). Gates run by audit. NOT EXERCISED: no runtime POST /api/entidades, so the audit_log CREATE row is read from the wrapper's code path, not observed.
  payload

  {"scope":"src/repositories/entidades.ts single hunk, current bytes: HEAD plus the MOVILBA-646 auditedOperation wrapper on create(). Gates run by audit. NOT EXERCISED: no runtime POST /api/entidades, so the audit_log CREATE row is read from the wrapper's code path, not observed.","outcome":"pass","evidence":"PASS on the behaviour, with a false claim inside the hunk recorded as a finding. The file is HEAD plus 646 only: numstat 21/14, all 21 log calls back on the OLD five-arg signature, zero _requestId, so the four read methods keep and use their requestId and src/services/entidades.ts:49, :56 and :130 pass into live parameters again. The 732 conversions are absent from this file.\nSubstance verified against 646's own measured intent rather than the hunk's comment: the item records 'entidadesRepository.create() sin auditar mientras update y delete SI auditan, 46 entidades con 0 CREATE y 1 UPDATE en audit_log'. auditedOperation inserts that missing CREATE row (src/lib/audit-wrapper.ts:100-107) with action CREATE, tableName entidades, recordId from .returning() and requestId in details, suppressed only when zero rows were touched. The alta becomes auditable, which is what 646 asked for, and create now matches update and softDelete.\nThe new guard is unreachable from the only live caller: route.ts:112 returns on !session?.user?.id, so session.user.id at :157 is always non-empty, through createEntidad (:116) into repository.create (:285). creadoPor goes from nullable to always-set.\nGates, run by audit on these bytes: typecheck clean, eslint --max-warnings 0 on the file clean, test:source 65/65.\nFINDING, not blocking the bundle, to correct on the next touch: the comment says 'fn_archive_entidades needs app.current_user_id in the same transaction as the INSERT'. Measured on live PROD read-only - trg_archive_entidades is BEFORE DELETE OR UPDATE, so it never fires on an INSERT, and fn_archive_entidades holds no current_setting; it attributes via COALESCE(OLD.borrado_por, OLD.creado_por). setTransactionActor is harmless but is not what makes the hunk correct - the audit_log CREATE row is. As written the comment teaches a trigger behaviour the database does not have.","reportRef":null}

• coder03assistant-mba-l-ca completed · 1w ago
  Closed per coder03: shipped in v5.6.193 (cfcd1d09) to PROD/DEV; auditedOperation create wrapper and related paths passed audit. No additional runtime POST observation was claimed.