Improve rejected-update evidence and verify real model diagnosis #207

Merged

nimmo merged 5 commits from fix/remediation-evaluation-evidence into main 2026-09-09 18:33:30 +01:00

Conversation 1 Commits 5 Files changed 7 +149 -5

nimmo commented 2026-09-09 05:13:33 +01:00

Owner

Copy link

Rejected-update diagnoses lacked useful evaluation context and could misstate affected hosts as observed failures. Host builds now request traces; the incident helper retains named context and final errors; reports distinguish the recorded failed host from validation scope. A read-only text companion prevents OpenCode from clipping the JSON diagnostic string at its 2000-character line limit.

Refs #205. Updater MR #1 is merged. The lock now resolves the same immutable helper revision through main. Review fix c907fda also names the evidence wrapping constant; the exact pinned emitter's ShellCheck-suppressed assignments were verified as consumed. Full flake and remediation regression checks pass.

Validation: both regression suites, ShellCheck, formatting, full flake/VM checks with the published input pin. Exact #204 real-model replay identified pi-coding-agent nativeBuildInputs and the typescript-go rename in 239.353 seconds without timeout or publication. Historical #172 replay correctly found Moonlight/FFmpeg incompatibility but still could not propose a pin; separate follow-up will address failed-builder attribution and leaf patch scope. Detailed model dependency-chain claims remain advisory.

Scheduled live deployment/no-op evidence remains pending; #205 stays open. Existing upstream deprecation warnings remain outside this change.

Conflict resolution: merged main at 35bbf3a and retained its complete flake.lock. The updater merge revision a514b03 has the same source narHash as reviewed cd61d13. Resolution head 1eb13d2 passes full flake check, remediation regressions, and CI test run 856.

Final review triage: the automated review repeated the sourced-variable suppression concern. Standalone ShellCheck of the exact a514b03 incident emitter passes, and its reads of LAST_BUILD_RC/FAILED_HOST/FAILED_COMMAND were verified. Suppressions remain assignment-scoped. The suggested named constant and explanatory comment already exist; the fixed diagnostic assertion intentionally tests retained fixture evidence. No unresolved functional defect or failed deterministic check was identified.

Rejected-update diagnoses lacked useful evaluation context and could misstate affected hosts as observed failures. Host builds now request traces; the incident helper retains named context and final errors; reports distinguish the recorded failed host from validation scope. A read-only text companion prevents OpenCode from clipping the JSON diagnostic string at its 2000-character line limit. Refs #205. Updater MR #1 is merged. The lock now resolves the same immutable helper revision through main. Review fix c907fda also names the evidence wrapping constant; the exact pinned emitter's ShellCheck-suppressed assignments were verified as consumed. Full flake and remediation regression checks pass. Validation: both regression suites, ShellCheck, formatting, full flake/VM checks with the published input pin. Exact #204 real-model replay identified pi-coding-agent nativeBuildInputs and the typescript-go rename in 239.353 seconds without timeout or publication. Historical #172 replay correctly found Moonlight/FFmpeg incompatibility but still could not propose a pin; separate follow-up will address failed-builder attribution and leaf patch scope. Detailed model dependency-chain claims remain advisory. Scheduled live deployment/no-op evidence remains pending; #205 stays open. Existing upstream deprecation warnings remain outside this change. Conflict resolution: merged main at 35bbf3a and retained its complete flake.lock. The updater merge revision a514b03 has the same source narHash as reviewed cd61d13. Resolution head 1eb13d2 passes full flake check, remediation regressions, and CI test run 856. Final review triage: the automated review repeated the sourced-variable suppression concern. Standalone ShellCheck of the exact a514b03 incident emitter passes, and its reads of LAST_BUILD_RC/FAILED_HOST/FAILED_COMMAND were verified. Suppressions remain assignment-scoped. The suggested named constant and explanatory comment already exist; the fixed diagnostic assertion intentionally tests retained fixture evidence. No unresolved functional defect or failed deterministic check was identified.

nimmo added this to the Automated Update Remediation milestone 2026-09-09 05:13:33 +01:00

nimmo added 3 commits 2026-09-09 05:13:36 +01:00

fix: improve rejected-update evidence and diagnosis boundaries ... 57a086a085

Refs #205. Requires the coordinated nixos-auto-update evidence selector update.

fix: expose diagnostic text without read-tool truncation ... 230190d356

Refs #205. Verified with a real non-publishing Nova replay of incident #204.

chore: pin published remediation evidence helper ... e3ece68efa

Refs #205; depends on nixos-auto-update MR #1.

nimmo commented 2026-09-09 05:14:58 +01:00

Author

Owner

Copy link

Automated code review

Reviewed commit: 1eb13d2fccdd88ce2b5a07862abe5ba3fdefbc6d

Verdict: Requires further work

Resolve the blocking or important findings and investigate failed deterministic checks before merging.

Overall assessment

The PR introduces a build-output.txt companion file to preserve full diagnostic traces that would otherwise be truncated by OpenCode's per-line read limit, adds --show-trace to Nix build commands, and clarifies documentation regarding host failure scope and model confidence. The changes are well-supported by tests and documentation updates.

Blocking findings

None.

Important findings

• ShellCheck suppression in production script without corresponding fix (scripts/attic-cache-build.sh:511; high confidence)

  shellcheck disable=SC2034 # consumed by the sourced incident emitter

  NIXOS_UPDATE_LAST_BUILD_RC="$build_rc"
  Impact: Suppressing SC2034 hides unused variable warnings. While justified here due to dynamic sourcing, it reduces static analysis coverage for this file.
  Suggested fix: Ensure the incident emitter script is well-documented or linted separately to maintain confidence in these suppressed assignments.

Suggestions

• Hardcoded line length constant in Python code (scripts/nixos_update_remediation/model.py:35; high confidence)
  EVIDENCE_LINE_CHARACTERS = 512
  Impact: The value 512 is a magic number. If the OpenCode read limit changes or if a different wrapping strategy is desired, this constant must be updated manually.
  Suggested fix: Consider defining this as a named constant with a comment explaining its origin (e.g., 'OpenCode line limit headroom') or making it configurable via environment variable for easier tuning.

• Test assertion dependency on specific error message (tests/nixos-update-remediation-integration.py:295; high confidence)
  assert 'renamed to/replaced by' in preview
  Impact: The test relies on a specific substring from the fake evidence. If the model or upstream package names change, this assertion might break even if the logic is correct.
  Suggested fix: Consider using a more generic assertion or ensuring the fake evidence is generated dynamically to match current upstream states if possible.

Tests and validation

• No configured deterministic checks.

Questions

• Is the 512-character line wrap length optimal for readability in build-output.txt, or should it be configurable?
• Are there any other tools or readers besides OpenCode that might have similar per-line limits that could benefit from this text companion?

Review limitations

• I cannot run the NixOS flake checks or integration tests to verify the changes in a live environment.
• I am relying on the provided diff and documentation for evidence; I cannot verify the actual behavior of OpenCode's read tool.

Diff coverage

• README.md: reviewed — included in a context-limited batch
• remediation/worker-prompt.md: reviewed — included in a context-limited batch
• scripts/attic-cache-build.sh: reviewed — included in a context-limited batch
• scripts/nixos_update_remediation/model.py: reviewed — included in a context-limited batch
• scripts/nixos_update_remediation/publication.py: reviewed — included in a context-limited batch
• tests/nixos-update-remediation-integration.py: reviewed — included in a context-limited batch
• tests/nixos-update-remediation-regressions.sh: reviewed — included in a context-limited batch

Review metadata

• Reviewer: forgejo-ai-review v1
• Model: qwen3.6:35b-a3b-q8_0
• Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md
• Commits considered: 5
• Previous automated review: updated
• Findings: 3

<!-- forgejo-ai-review:v1 --> ## Automated code review Reviewed commit: `1eb13d2fccdd88ce2b5a07862abe5ba3fdefbc6d` ## Verdict: Requires further work Resolve the blocking or important findings and investigate failed deterministic checks before merging. ### Overall assessment The PR introduces a `build-output.txt` companion file to preserve full diagnostic traces that would otherwise be truncated by OpenCode's per-line read limit, adds `--show-trace` to Nix build commands, and clarifies documentation regarding host failure scope and model confidence. The changes are well-supported by tests and documentation updates. ### Blocking findings None. ### Important findings - **ShellCheck suppression in production script without corresponding fix** (scripts/attic-cache-build.sh:511; high confidence) # shellcheck disable=SC2034 # consumed by the sourced incident emitter NIXOS_UPDATE_LAST_BUILD_RC="$build_rc" Impact: Suppressing SC2034 hides unused variable warnings. While justified here due to dynamic sourcing, it reduces static analysis coverage for this file. Suggested fix: Ensure the incident emitter script is well-documented or linted separately to maintain confidence in these suppressed assignments. ### Suggestions - **Hardcoded line length constant in Python code** (scripts/nixos_update_remediation/model.py:35; high confidence) EVIDENCE_LINE_CHARACTERS = 512 Impact: The value 512 is a magic number. If the OpenCode read limit changes or if a different wrapping strategy is desired, this constant must be updated manually. Suggested fix: Consider defining this as a named constant with a comment explaining its origin (e.g., 'OpenCode line limit headroom') or making it configurable via environment variable for easier tuning. - **Test assertion dependency on specific error message** (tests/nixos-update-remediation-integration.py:295; high confidence) assert 'renamed to/replaced by' in preview Impact: The test relies on a specific substring from the fake evidence. If the model or upstream package names change, this assertion might break even if the logic is correct. Suggested fix: Consider using a more generic assertion or ensuring the fake evidence is generated dynamically to match current upstream states if possible. ### Tests and validation - No configured deterministic checks. ### Questions - Is the 512-character line wrap length optimal for readability in `build-output.txt`, or should it be configurable? - Are there any other tools or readers besides OpenCode that might have similar per-line limits that could benefit from this text companion? ### Review limitations - I cannot run the NixOS flake checks or integration tests to verify the changes in a live environment. - I am relying on the provided diff and documentation for evidence; I cannot verify the actual behavior of OpenCode's read tool. ### Diff coverage - `README.md`: **reviewed** — included in a context-limited batch - `remediation/worker-prompt.md`: **reviewed** — included in a context-limited batch - `scripts/attic-cache-build.sh`: **reviewed** — included in a context-limited batch - `scripts/nixos_update_remediation/model.py`: **reviewed** — included in a context-limited batch - `scripts/nixos_update_remediation/publication.py`: **reviewed** — included in a context-limited batch - `tests/nixos-update-remediation-integration.py`: **reviewed** — included in a context-limited batch - `tests/nixos-update-remediation-regressions.sh`: **reviewed** — included in a context-limited batch <details><summary>Review metadata</summary> - Reviewer: forgejo-ai-review v1 - Model: qwen3.6:35b-a3b-q8_0 - Guidance loaded: .forgejo/ai-review.md, .forgejo/ai-review.yml, AGENTS.md, README.md - Commits considered: 5 - Previous automated review: updated - Findings: 3 </details>

nimmo referenced this pull request from a commit 2026-09-09 17:04:55 +01:00

Address automated review of remediation evidence MR

nimmo added 1 commit 2026-09-09 17:04:57 +01:00

Address automated review of remediation evidence MR ... c907fda69b

Resolve the merged updater dependency through main without changing its immutable revision. Name the evidence wrapping width and keep an independent regression check against the OpenCode read limit. Refs #205, MR #207.

nimmo referenced this pull request 2026-09-09 18:23:33 +01:00

Enable and validate leaf-package remediation proposals #210

nimmo referenced this pull request 2026-09-09 18:24:16 +01:00

Enable correctly attributed leaf-package remediation proposals #208

nimmo added 1 commit 2026-09-09 18:30:27 +01:00

Merge main into remediation evidence branch and retain current lock ... 1eb13d2fcc

Resolve the flake.lock conflict using main’s merged updater revision and current automatic input updates. The updater source hash is identical to the reviewed helper. Full flake check and remediation regressions pass.

nimmo merged commit 4ba715cb38 into main 2026-09-09 18:33:30 +01:00

nimmo referenced this pull request from a commit 2026-09-09 18:33:32 +01:00

Merge pull request 'Improve rejected-update evidence and verify real model diagnosis' (#207) from fix/remediation-evaluation-evidence into main

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:authentication

Issues related to authentication area

  

area:flake-utilities

Issues related to flake utilities area

  

area:performance

Issues related to performance area

  

area:tbd

Area to be determined

  

host:chaos

Issues specific to the chaos host.

  

host:electra

Issues specific to the electra host.

  

host:fleet

Issues that span multiple hosts or the fleet as a whole.

  

host:lyra

Issues specific to the lyra host.

  

host:nova

Issues specific to the nova host.

  

host:vega

Issues specific to the vega host.

  

investigation

Issue is an investigation task

  

phase:cutover

The service cutover execution phase.

  

phase:deploy

  

phase:mcp

  

phase:module

  

phase:packaging

  

phase:prep

Preparation and preflight work before service cutover.

  

phase:validation

Post-cutover validation and rollback-window work.

  

priority:high

High priority issue

  

priority:medium

Medium priority issue

  

project:attic-postgres-lyra-rollout

Issues in the Attic PostgreSQL migration and Lyra rollout workstream.

  

project:auto-update-reliability

Work arising from the automated update reliability review.

  

project:auto-update-remediation

Guarded Nova-assisted diagnosis and remediation of failed automated updates.

  

project:declarative-purity-cleanup

Issues in the declarative purity cleanup workstream.

  

project:external-review

Review of external configurations for ideas and improvements

  

project:fleet-boundary-cleanup

Maintainability work that clarifies fleet configuration and automation boundaries.

  

project:host-facts-refactor

Issues in the host facts refactor workstream.

  

project:lyra-nixos-deploy

Issues in the Lyra NixOS deployment workstream.

  

project:lyra-service-stack-migration

Issues in the lyra service stack migration workstream.

  

project:nebula-mesh-network

Issues in the Nebula mesh network workstream.

  

project:nixos-build-deployment-pipeline

Issues in the NixOS Build & Deployment Pipeline workstream.

  

project:security-hardening

Work belonging to the Security Hardening milestone

  

project:service-stack-migration

Issues in the service stack migration workstream.

  

project:vega-sillytavern-cutover

Issues in the Vega SillyTavern cutover workstream.

  

project:wiki-rebuild

Re-found the repository wiki as the fleet's curated knowledge architecture.

  

repo:numtide/flake-utils

Issues related to numtide/flake-utils repository

  

repo:numtide/nix-auth

Issues related to numtide/nix-auth repository

  

repo:numtide/nixos-passthru-cache

Issues related to numtide/nixos-passthru-cache repository

  

repo:numtide/nix-relay

Issues related to numtide/nix-relay repository

  

service:auto-update

NixOS flake update, cache build, and consumer deployment automation.

  

service:mem0

  

service:nix

Issues concerning Nix build, store, cache, or deployment infrastructure.

  

service:sillytavern

Issues related to the SillyTavern service.

  

service:slskd

Soulseek daemon (slskd) work.

  

service:synthseek

Synthseek music discovery/library manager work.

No labels

area:authentication

area:flake-utilities

area:performance

area:tbd

host:chaos

host:electra

host:fleet

host:lyra

host:nova

host:vega

investigation

phase:cutover

phase:deploy

phase:mcp

phase:module

phase:packaging

phase:prep

phase:validation

priority:high

priority:medium

project:attic-postgres-lyra-rollout

project:auto-update-reliability

project:auto-update-remediation

project:declarative-purity-cleanup

project:external-review

project:fleet-boundary-cleanup

project:host-facts-refactor

project:lyra-nixos-deploy

project:lyra-service-stack-migration

project:nebula-mesh-network

project:nixos-build-deployment-pipeline

project:security-hardening

project:service-stack-migration

project:vega-sillytavern-cutover

project:wiki-rebuild

repo:numtide/flake-utils

repo:numtide/nix-auth

repo:numtide/nixos-passthru-cache

repo:numtide/nix-relay

service:auto-update

service:mem0

service:nix

service:sillytavern

service:slskd

service:synthseek

Milestone

Clear milestone

No items

No milestone

Automated Update Remediation

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

nimmo/nixos-config!207

No description provided.