[eumemic]

Automated dev-pipeline build for issue #2538.

[eumemic-bot]

Code review

No actionable correctness, security, or regression findings identified in the reviewed changes.

[eumemic]

Code review

Canonical review record (reconciler-written, company#383)

Verdict: NEEDS-CHANGES at f7dc115afd84a1c10bcd5dee8b8599a1be28a5dc.

1. BLOCKING — F1-degradation-alarm-masked-by-sibling-successes (src/aios/sandbox/backends/docker.py:1971 and :2001 recordmeasured, src/aios/sandbox/snapshottelemetry.py:78-82 process-global per-reason streak). There is one process-global streak per reason, and every successful imagediskcost resets it to 0, including chain-cache hits line 1971 that measure nothing new. Each snapshot probes several images: base, parent, and the committed image. So one image's persistent failure always sits next to sibling successes and the streak never reaches ALERTSTREAK=5. This is case c from the checklist, a healthy aggregate hiding one item's failure. Probe A: the base's history is unparseable on every snapshot and the parent parses, so the dead-history trigger is off for every session on that base. After 20 snaps Property: A degradation that persists across many consecutive snapshots must raise sandbox.chainaccountingdegradedalarm within a bounded number of snapshots. This must hold when the degraded probe is interleaved with successful probes of other images, and when some images are served from the chain cache. A cache hit, or a successful measurement of a different image, must not count as evidence that the degra

   On head f7dc115afd84a1c10bcd5dee8b8599a1be28a5dc: `uv run pytest tests/unit/sandbox/test_zz_probe_2545.py -q -s -p no:randomly` prints `degraded_total= 40 streak= 1 alarms= 0` and then `degraded_total= 40 streak= 0 alarms= 0`. Result: 2 failed, with AssertionError 'base unparseable on 20/20 snapshots yet no alarm' and 'every new image unparseable for 20 snapshots yet no alarm'.
   

Canonical record written by the reconciler from the reviewer's structured return (company#383); the reviewer's own prose comment is discussion.

[eumemic-bot]

Code review

No actionable correctness, security, or regression findings identified.

[eumemic]

Code review

Reviewed head e52016f40241690f0368ef94350cc15def820884 (matches the live PR head).

Verdict: NEEDS-CHANGES (one blocking finding)

F1: type-check gate is red. CI lint on this head reports a failure. Running locally, uv run mypy src tests ... (the same invocation as code-validation.yml) gives 4 errors, all in tests/unit/sandbox/test_snapshot_verb.py at lines 1711, 1730, 1750, and 1761. The cause is that _alarms(logs: list[dict[str, Any]]) (line 1674) receives the list[MutableMapping[str, Any]] returned by structlog.testing.capture_logs(). On master, the same mypy run passes.

• Property: uv run mypy src tests (the CI invocation) exits 0 on the PR head.
• Failing test: the CI lint job / uv run mypy tests/unit/sandbox/test_snapshot_verb.py.
• Remedy (advisory only): annotate _alarms so it accepts list[MutableMapping[str, Any]], or a Sequence[Mapping[str, Any]].

What was executed:

• ruff check and ruff format --check are green.
• pytest tests/unit/sandbox passes: 729 passed, 1 skipped.
• Mutation probes on docker.py each turned a test red:
  • verified-absent base reduced to an rw-only projection;
  • the 4x unknown-base byte backstop removed;
  • the vanished-parent view-only fallback removed (snapshot chain accounting follow-ups from #2388 review (unknown-base byte bound, vanished-parent pin, unparseable-history alerting) #2538 N2 is now pinned);
  • verified absence reported as indeterminate;
  • a transient error reported as absence;
  • the record_measured call removed;
  • the history_unavailable telemetry call removed;
  • the base_unmeasurable telemetry call removed.

I found no correctness defects in the N1/N2/N3 logic.

[eumemic]

Code review

Canonical review record (reconciler-written, company#383)

Verdict: NEEDS-CHANGES at e52016f40241690f0368ef94350cc15def820884.

1. BLOCKING — F1 (tests/unit/sandbox/testsnapshotverb.py:1674 callers at 1711, 1730, 1750, 1761). TestChainAccountingDegradationTelemetry.alarms is annotated logs: listdictstr, Any. structlog.testing.capturelogs yields listMutableMappingstr, Any, and because list is invariant, mypy raises an arg-type error at each of the 4 call sites. The CI lint check on the head SHA concluded failure. On master the same mypy run is clean. Property: The repository type-check gate uv run mypy src tests packages/aios-sdk/aiossdk connectors//src packages/aios-connector-http/aiosconnectorhttp packages/aios-browser-driver/aiosbrowserdriver, as invoked in .github/workflows/code-validation.yml exits 0 on the PR head, and the CI lint check concludes success.

   On e52016f4, the full CI mypy invocation printed 'Found 4 errors in 1 file (checked 1824 source files)' with test_snapshot_verb.py:1711/1730/1750/1761 [arg-type]. Same command on master: 'Success: no issues found'. GitHub check-runs for e52016f4: lint=failure; unit, integration, connectors and e2e (non-docker) = success; e2e (docker) still in_progress.
   

Canonical record written by the reconciler from the reviewer's structured return (company#383); the reviewer's own prose comment is discussion.

[eumemic-bot]

Code review

• [P2] Do not report a verified-absent base as a degraded probe — docker.py. _base_disk_cost() explicitly distinguishes a confirmed “No such image” with base_absent=True, but this branch still calls record_degraded(). A legitimately deleted base therefore increments the degradation metric on every snapshot and raises an error-level alarm after five observations, despite accounting having a definitive result. Restrict degradation telemetry to not base_known and not base_absent, or treat verified absence as a successful measurement/reset.

[eumemic]

Code review

Canonical review record (reconciler-written, company#383)

Verdict: MERGE-SAFE at 36a7abd00a036546c0fd483ce4fec77177fe8987.

No blocking findings at this head.

Canonical record written by the reconciler from the reviewer's structured return (company#383); the reviewer's own prose comment is discussion.

[eumemic]

Risk Assessment

Tier 3

Follow-ups to the #2388 snapshot chain accounting (+587/-9 across 3 files). Two changes to sandbox/backends/docker.py affect when snapshots flatten or commit. First, when the base image is confirmed gone, the budget is now charged the parent's full cost. Second, when the base can't be inspected because of a transient error, the snapshot flattens once the parent chain exceeds 4x the budget. This changes how flattening behaves in the core snapshot verb that runs on every idle exit, and the old absent-base test now checks only the no-budget case. Also adds a new snapshot_telemetry.py module: a Prometheus counter, per-subject consecutive-failure streak alarms and sliding-window alarms, with module-global thread-locked state. It's meant to be observation-only and fail-safe. There are about 340 lines of new unit tests. Rated tier 3 rather than 2 because the change touches disk accounting and flatten logic on a hot path, adds global state, and could cause more flattening in production. It is not tier 4: there's no migration and nothing security-sensitive.

[eumemic]

Auto-merge receipt

The dev-pipeline reconciler auto-merged PR #2545 at head 36a7abd00a036546c0fd483ce4fec77177fe8987 after proving: risk tier 3 (cap 4), reviewed-green provenance for this head, the complete required check set green, no hold/fix/human veto, and behind_by=0 against master.

[eumemic]

Post-merge: a pipeline PR was merged WITHOUT the maker≠checker gate

pipeline:v2 PR #2545 MERGED without the reconciler's maker≠checker gate (risk:tier-3 is ABOVE the auto-merge cap (2)). Auto-merge requires reviewed:green + an in-cap risk tier — this merge bypassed it.