core/cli: file lister freshness, repo-scoped agent keys, clean_exports guardrail re-armed - #168
Open
cagodoy wants to merge 23 commits into
Open
core/cli: file lister freshness, repo-scoped agent keys, clean_exports guardrail re-armed#168cagodoy wants to merge 23 commits into
cagodoy wants to merge 23 commits into
Conversation
…mero y tipo derivado
Una union pelada ("a" | "b") no tiene representacion en runtime, asi que todo
consumidor que necesite los VALORES los vuelve a enumerar a mano y ningun
compilador vigila esas copias. Medido: FindingStatus estaba escrito a mano en
tres sitios de codigo mas un comentario, y de las cuatro repeticiones una sola
fallaba ruidosamente al desfasarse.
- FINDING_SEVERITIES y FINDING_STATUSES como tuplas readonly con 'as const', y
los tipos derivados de ellas. El 'as const' es load-bearing, no estilo:
produce una tupla readonly no vacia, que es la forma que z.enum() acepta sin
cast. Verificado sobre el .d.ts compilado — la tupla literal sobrevive el
borde del paquete.
- Ambas se exportan como VALOR desde los dos barrels. AUDIT-A6 declara
"re-export all types", asi que una constante que viviera solo en el barrel de
tipos seria invisible desde '@gitgov/core' sin que ningun test lo notara: por
eso los tests las importan del barrel principal.
- AUDIT-J1 lleva test de arquitectura con control de anti-vacuidad — exige ver
las cinco uniones preexistentes que sabe que existen antes de creerle a su
cero. Sin eso, un refactor de formato dejaria el guard vacuo.
- El alcance de J1 esta acotado: dominio CERRADO, y en el tiempo. No aplica a
FindingCategory, que es extensible por diseno, ni obliga a refactor
retroactivo de las cinco preexistentes (DetectorName, WaiverStatus,
ScanDisplayStatus, ScanScope, PolicyStatus), que quedan registradas con su
disparador. Una regla que el codigo viola el dia que nace se aprende a ignorar.
Origen: EARS-PJ5 de scan_pipeline asserto 3 de los 4 FindingStatus durante
meses. El test no invento la lista: la copio del spec, que la enumeraba dentro
del cuerpo del EARS.
Gates: tsc 0, 126 suites / 3066 tests verdes.
…concurrente Dos escritores commitean la misma ref de estado con dos identidades de git distintas, y uno de ellos no puede reconciliar. Medido con observador externo el 2026-08-13, con anti-vacuidad: la ref se vio en 119 de 140 polls, asi que el resultado es dato y no ceguera del instrumento. El CLI tiene worktree local y hace pull --rebase; el SaaS escribe por la API de GitHub SIN worktree, asi que no participa del rebase. No hay negociacion posible entre ambos, solo carrera: el CLI reconcilia contra el estado remoto de hace un instante y empuja contra el de ahora. Es un TOCTOU de libro — el pull --rebase es el check, el push es el use — y el sintoma se movia de fase en fase segun quien estuviera empujando cuando el servidor commiteaba. Por eso durante meses se leyo como intermitencia sin causa. B18 reintenta el ciclo hasta 3 veces. Un conflicto durante un reintento ABORTA y retorna conflictDetected: un conflicto es decision del usuario, no algo que reintentar. Y un fallo de rebase NO es prueba de conflicto — solo aborta si getConflictedFiles() devuelve archivos; si no, relanza el error real. Asumir conflicto sin comprobarlo seria afirmar un estado nunca medido, que es el defecto que este bloque existe para eliminar. Agotados los intentos, el error nombra el rechazo Y la cantidad: un fallo generico aca es lo que mantuvo esto indiagnosticable. B19 expone retryCount y retryReasons EN EL RESULTADO, no solo en log, porque el logger de core es silencioso bajo NODE_ENV=test y un requisito cuya unica evidencia es un log apagado no se puede auditar. Un retry silencioso arregla el sintoma y borra la evidencia de que la carrera existe. Detalles que son contrato y no implementacion: - La deteccion del rechazo no puede keyear solo en "! [rejected] ... (fetch first)". Medido sobre 392 fallos de push reales: esa frase aparecio CERO veces y "failed to push some refs" aparecio en el 100%. Con el predicado angosto el retry no se aplicaba y no lo decia. - El rebase del reintento usa --autostash y el primer reconcile no: al llegar al reintento el worktree puede cargar sobrantes que el commit filtro, y git rechaza rebasar con unstaged changes. El camino normal nunca rebasa dos veces. - Rebasar no rompe firmas. Verificado contra 02_protocol/01_embedded, no asumido: la firma se computa sobre el CONTENIDO del record (payloadChecksum + keyId + role + notes + timestamp) y el SHA del commit no entra. Si entrara, cada reintento habria invalidado las firmas de todo lo rebaseado y el remedio habria cambiado un rechazo visible por corrupcion silenciosa. - El techo de 3 sale de la medicion (un commit ajeno cada ~22s contra un ciclo de 2-5s), y queda declarado como provisional CON su base y con el dato que lo valida: la distribucion real de reintentos, que B19 hace medible. LIMITE, declarado dentro del propio EARS: hace que el CLI SOBREVIVA a las escrituras del servidor. NO coordina a los dos. Si dos escrituras del servidor colisionan entre si, el retry no las cubre — eso necesita fencing sobre la ref y es otro tramo. 5 tests contra git real (remoto bare + hook pre-push determinista). Bump del submodulo private con los specs que acompañan.
…ga la causa del 5xx PROJ-H3b — un null de una fuente eventualmente consistente NO es evidencia de ausencia. addActor lanzaba GIT_WRITE_FAILED sobre actores que SI estaban en la branch. Medido en corrida real con los dos relojes en la misma captura: el commit del actor aterrizo a las 02:36:05 y el error salio a las 02:36:06.369 — 1.4 segundos DESPUES. Cada paso de la secuencia es correcto salvo el ultimo. createActor() escribe el actor: en el backend GitHub el store commitea dentro del put(), y ese es el commit. finalize() lanza "Nothing to commit: staging buffer is empty", que es correcto y esperado — el buffer esta vacio justo porque el store ya commiteo, y esa asimetria entre FsRecordStore (stage-then-commit) y GitHubRecordStore (commit-on-write) es lo que PROJ-H3b existe para cubrir. Pero el guard resolvia con UNA lectura: getActor() -> stores.actors.get() -> un GET a la API de GitHub, 1.4s despues del commit, que devuelve null. El defecto es de diseño, no de latencia: el guard le pregunta a una fuente eventualmente consistente si ocurrio algo que la linea anterior ya establecio, y trata la primera respuesta como verdad. Es el MISMO defecto que se corrigio el mismo dia en cuatro tests E2E del flow de collaborator — el harness y el producto tenian la misma falla, lo que confirma que el remedio es el patron y no un parche. EL GUARD NO SE ELIMINA. Sigue distinguiendo "el store commiteo directo" de "no se escribio nada", que son casos reales y distintos. Lo unico que cambia es que la verificacion espera dentro de un deadline acotado: el caso legitimo converge y devuelve exito, el caso que el guard protege agota el deadline y lanza igual. Los tres tests previos lo demuestran sin necesidad de afirmarlo — "store ya commiteo" pasa en 1ms, "actor ausente de verdad" tarda 5025ms y SIGUE LANZANDO. 5s sobre una ventana medida de 1.4s son ~3.5x de margen. El intervalo es corto porque el camino feliz sale en la primera lectura y no paga nada. Sospechosos descartados por medicion, no por argumento — los dos bugs que el proyecto ya documento con esta firma: el store se construye con ref: stateBranch correcto y con idEncoder: DEFAULT_ID_ENCODER. Alcance acotado y verificable: el camino CLI usa FsProjectInitializer, cuyo finalize() es no-op y por lo tanto nunca lanza "Nothing to commit" — el guard solo se ejecuta en el backend GitHub. EARS-C1 — un 5xx sin causa ni request-id es un callejon sin salida. El error se construia con el status y nada mas, descartando el message de GitHub y el x-github-request-id, que es el unico identificador que su soporte acepta para investigar. La causa se descartaba en el mismo punto donde se detecta. Es la misma forma exacta que IDS-H1, corregida hoy en identity_service: un error que captura el codigo y tira el mensaje del origen. Dos modulos independientes con el mismo defecto es patron, no descuido puntual. Era HUECO DE CONTRATO y no incumplimiento — el EARS solo pedia el status code, asi que el codigo cumplia. Por eso se enmendo el EARS primero. Nuevo helper getOctokitRequestId con la misma forma defensiva que getOctokitRateLimitReset. Los dos tests se escribieron en rojo observado. El de PROJ-H3b fallo en project_module.ts:257, la linea exacta del guard, y lleva anti-vacuidad: asserta que getActor fue llamado mas de una vez, porque un verde donde el actor aparece a la primera no ejercitaria el poll y no probaria nada. Gates: tsc --noEmit 0 · suite de core 3073 passed / 2 skipped / 3075 total (126 suites), contra 3071 al empezar — los dos tests nuevos, cero regresiones. Verificado end-to-end: racha de 5 corridas verdes del flow de collaborator, 16/16 cada una. Bump del submodulo private con las enmiendas de spec.
…ache en la interfaz EARS-FL05 en la interfaz: toda implementacion expone invalidateCache(), no-op en las que no cachean, y el contrato declara que una instancia NO garantiza observar cambios posteriores a su primera lectura. Va en la abstraccion y no en GitHubFileLister porque hay consumidores tipados contra FileLister (scope_selector, source_auditor): con el metodo solo en la clase concreta, la unica salida era instanceof — el acoplamiento que la interfaz existe para evitar. Y el proximo backend (GitlabFileLister, ya previsto) nacia sin la obligacion. El compilador lo confirmo: TS2420 cazo una cuarta implementacion que no estaba en el radar (MockFileLister). Eso no pasa si el contrato vive en una clase. Test de contrato con escaneo POR CLASE (implements FileLister), no por directorio: la primera version escaneaba directorios y era ciega a MockFileLister, que vive junto a MemoryFileLister en memory/ — reportaba cobertura completa mientras se le escapaba una implementacion entera. Control positivo corrido: una clase dummy dentro de memory/ hace fallar el guard. Gates: tsc 0 en los 5 paquetes, core 127/127 suites, 3078 passed.
… file_lister, sync_state and login Rule 16: code carries the knowledge, not the process trail — session ids and audit dates leave the comments while the mechanism they document stays (the retry TOCTOU, the frozen array in the poll, the severity enumeration with its spec vertex).
…with throw Change dated 2026-08-17 (earlier session, committed with its date): the two it.skip guarding entrypoint imports are enabled, and fail() — nonexistent in this runner — becomes throw new Error, so the guardrail can report what it detects. Once on, it found the entrypoint's prohibited imports (EARS-CI03, registered as a product finding in mvp_blockers input 28).
Batch 432 passed / 0 red / exit 0. Sweep-race fix in scan_pipeline and audit_webapp_flow, HP-A9..A12 helpers, stale-state sweep across specs, AP-A5b, mvp_blockers checkpoint and the protocol_integrity split.
…tations
@gitgov/core is published to npm and its package.json promises a runtime-agnostic
root entrypoint, with Node-only code behind ./fs. Three barrels broke that promise
and dragged node:fs into the published bundle.
EARS-CI02 caught it once the guardrail was re-armed. It had been it.skip'd for
seven months, and its report was dead on top of that: the catch called fail(), a
Jasmine global Jest does not expose, so it raised ReferenceError before printing.
It could detect without being able to say what it found.
key_provider/index.ts dropped `export * from './fs'` and './memory'
file_lister/index.ts same. github and prisma already complied
agent_discovery split: the two pure functions stay, discoverInstalledAgents
moves to fs/fs_agent_discovery.ts and its three tests
move with it, keeping ids DISC-A1..A3
The notes left in each barrel list every implementation of that module and why
they are not re-exported, following record_store/index.ts.
The defect was real but latent: every consumer that runs is Node, and saas-web —
the only browser consumer — imports seven symbols from @gitgov/core and all seven
are `import type`, so no value import reaches the client bundle. Only static
analysis of the built bundle sees it, and that analysis was switched off.
tsc caught every break: six KeyProvider.FsKeyProvider call sites in the CLI and two
source_auditor tests importing FsFileLister from the barrel.
Guardrail down from five violations to four. The rest come from llm/cli and
diagram_generator.
Follows the core change: FsKeyProvider and discoverInstalledAgents are no longer
re-exported from the main barrel.
init-command.ts, audit-command.ts discoverInstalledAgents -> @gitgov/core/fs
dependency-injection.ts six KeyProvider.FsKeyProvider call sites
now use FsKeyProvider from the /fs import
that line 4 already had
And a test that was passing without exercising anything. audit-command.test.ts
mocked @gitgov/core/fs with two functions; once discoverInstalledAgents moved to
that subpath it resolved to undefined, the non-fatal try/catch in audit-command
swallowed the TypeError, and AORCH-P9 stayed green while never running discovery.
The mock now covers it.
AORCH-P9 is still vacuous for a separate, pre-existing reason: its EARS requires
appending a message listing discovered-but-unregistered agents, and the assertion
checks that message is ABSENT. Reported, not fixed — writing the real assertion is
new work, not part of this change.
tsc 0 in core and cli. 502 unit tests and 119 e2e passing.
Comments and assertion messages only — no behaviour, no assertions changed. cli/commands/indexer/indexer.test.ts header + inline comments cli/commands/login/login-command.ts ten LOGIN-T1/T2/T3 comment blocks e2e/tests/init_state_branch.test.ts header, step comments, two assertion messages Code and tests are English in this repo; specs are Spanish. These predate the convention.
…declared in three cli specs
Deploying monorepo with
|
| Latest commit: |
e6c6a77
|
| Status: | 🚫 Build failed. |
…e/fs resolve.ts imports CliLlmProvider statically and uses it in its switch, so exporting the resolver from the root barrel dragged node:child_process into dist/src/index.js. That was one of the EARS-CI02 violations. Both symbols now ship from @gitgov/core/fs via shared/fs/fs.ts. The interface, its types and AnthropicLlmProvider stay at the root, which keeps the barrel runtime-agnostic. There is deliberately no /llm subpath: subpaths here are named after the runtime they require, never a domain. Adds llm_provider_contract.test.ts so LLM-D1 and D2 are verified against every implementation rather than through the resolver, and D2 now calls query() instead of asserting the method exists. The review-advisor jest mapper needed a second entry: the root mapper is anchored with $, so @gitgov/core/fs bypassed it and resolved to dist while the barrel import beside it resolved to src. core/src/index.ts also drops the DiagramGenerator export; the module files are deleted in the following commit.
The module reached fs and path from the root entrypoint and accounted for two of the four EARS-CI02 violations. Removing it closes both. Deletes the core module and its diagnose-duplicates script, the CLI command and its TUI dashboard, and the e2e suite. Leaves a short comment at each removal site so the absence is explained rather than silent. CHANGELOG entries are historical records and stay untouched. Also corrects counts in cli/README.md that were already wrong before this change: 18 commands and 635 tests (504 unit + 131 e2e), measured.
The detector matched four shapes but all of them required from or require, so a bare side-effect import — which esbuild emits when it drops an unused binding but must keep the import — passed unreported. A negative control produced exactly that shape and the guardrail stayed green. Fixing the matcher does not fix the list. A denylist can only catch what somebody wrote down, and Node keeps adding builtins, each reachable as x and as node:x. Each entrypoint now declares which builtins it may reach and why; an unknown one fails by default. Scope stays at the two entrypoints already checked, so the change adds no new red. crypto is declared on the root as a known gap with its reason: the bundle reaches generateKeyPairSync and diffieHellman, which Web Crypto does not substitute. EARS-CI03 gets a real test — the report used to be handed to fail(), a Jasmine global Jest does not expose, so it threw before printing and the requirement was dead code. EARS-CI05 asserts parsed specifiers instead of a substring match that could not fail.
…sserts Six of fifteen requirements were marked green without an assert behind them. A1 measured src instead of dist and now reads the built bundle, with pnpm test running the build first. A3 replaces a fixture tautology with @ts-expect-error directives, so the suite stops compiling if a required field becomes optional. B2 adds the negative half as a source scan with its own vacuity and positive controls. C2 covers the branch where the fingerprint can only come from the input. Each close carries a negative control: production code was broken and exactly one test fell. Drops @anthropic-ai/claude-agent-sdk from dependencies. Nothing imports it in src or dist, and this package was the only one in the monorepo declaring it — leftover from before the provider abstraction, and a contradiction of the provider-agnostic requirement it sat under.
…r EARS prefix Closes the last two EARS-CI02 violations. The root bundle of @gitgov/core now imports only crypto, util and events — exactly its declared allowlist. ENGINE VALIDATOR ProjectModule imported validateAgentEngine directly, which reached backends/local_backend.ts and dragged `path` and `node:module` into the root bundle. It now receives an IEngineValidator as its seventh dependency — the other six already arrived through deps; this one was the only hardcoded collaborator. - IEngineValidator + EngineValidationResult: pure contract, @gitgov/core - FsEngineValidator: Node-only implementation, @gitgov/core/fs EARS-M1 was split rather than moved. engine_validator.ts had two imports and zero direct Node APIs — Node-only transitively, via resolveLocalEntrypoint. The contract half (never throw, non-local engines resolvable) stayed as ARUN-M1 in the pure spec; the resolution half became ARUN-M2 in the /fs spec. The four original tests survive, split 3/1, plus one new test for the "never throws" clause that nothing verified. The free function is deleted rather than kept beside the class: two implementations of the same 45 lines would have duplicated an EARS id across two test files. The dep is optional, which can silence PROJ-B6. Two tests cover it: one asserts the warning IS produced with a validator injected, the other pins down the degraded path. Both production wiring sites inject it. EARS PREFIX NORMALIZATION agent_runner used the generic EARS- prefix, which collides repo-wide: [EARS-A1] appears in 16 unrelated modules. That collision is what allowed the same id to carry contradictory requirements across the two agent_runner specs. Renamed to ARUN-, scoped by file path and an explicit id list — a global substitution would have touched 62 files owned by other modules. EARS-E9 is deliberately untouched inside agent_runner: it belongs to cli/agent_command.md and shares a letter with the MCP block (E1-E4). Two orphan tests, [EARS-B1b] and [EARS-B1c], existed in code and in no spec. They exercise what ARUN-B1 already specifies, so they are retagged [ARUN-B1] and listed in the traceability table. No renumbering: B1..B7 stay seven. SINGLE SOURCE OF TRUTH IN local_backend resolveLocalEntrypoint and executeEntrypoint each carried a byte-identical copy of the entrypoint classification, 55 lines apart, under a docblock claiming "single source of truth, no duplication". Extracted as isPackageEntrypoint. Documented a dormant consequence: the validator resolves against one root while execution uses two. Both come from the same value today, so validation predicts execution — if they ever diverge, ARUN-M2 stops predicting what happens. core 3086 passed, cli 502 passed, guardrail 5/5, tsc clean in core/cli/saas-api.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Parent-side counterpart of
gitgovernance/private#feat/index-generation-fencing. Mostly submodule pointer bumps; the code changes are limited tocore,cliande2e.Code changes
core—FileListerfreshness semantics declared (EARS-FL05,EARS-B9),RecordSignerrejects empty notes (RSIG-A5), repo-scoped agent keys inPrismaKeyProvider(PKP-H1..H4), and worktree sync-state retry under concurrent writers (WTSYNC-B18/B19).cli—LOGIN-T1: agent keys upload viasyncKey(agentKeys[]).core/integration/guardrails—EARS-CI01/CI02wereit.skipped behind a TODO referencing cycles nobody could identify. Enabling them exposed thatEARS-CI03usedfail(), a Jasmine global this runner does not expose: it raisedReferenceErrorbefore printing its report, so the guardrail could detect without being able to say what it found. Replaced withthrow. With the guardrail actually reporting, it found real violations in the published entrypoint — registered as a product finding, not closed by lowering the guard.Comments — process-only traceability (session ids, dated audit tags) removed from
coreandclicomments.[EARS-XX]stays: it is a contract with a permanent consumer. A session id is a row in a dated document, and an unresolvable id forces the reader to reconstruct someone else's process to read the test.Verification
tsc0 incoreandcli; full suites green. The e2e batch that exercises this code lives in the private submodule and is reported there:432 passed | 0 failures | exit 0.