fix(remediation): link ntfy follow-ups to Forgejo work #169

Merged

nimmo merged 6 commits from fix/remediation-follow-up-notifications into main 2026-08-13 19:56:25 +01:00

Conversation 6 Commits 6 Files changed 6 +216 -20

nimmo commented 2026-08-13 19:04:04 +01:00

Owner

Copy link

Summary

• publish a bounded remediation issue/PR report from the controller
• send a separate post-publication ntfy follow-up with direct Forgejo links
• retain the Automated Update Remediation milestone and keep ntfy secrets out of the model-capable job
• normalize live Forgejo JSON issue indices before producing the report

Validation

• ./tests/nixos-update-remediation-regressions.sh
• Forgejo workflow lint with actionlint
• git diff --check

Rollout

No NixOS deployment is required. Once merged, the next update-mode workflow with published remediation will send the additional ntfy follow-up.

Refs #168

## Summary - publish a bounded remediation issue/PR report from the controller - send a separate post-publication ntfy follow-up with direct Forgejo links - retain the `Automated Update Remediation` milestone and keep ntfy secrets out of the model-capable job - normalize live Forgejo JSON issue indices before producing the report ## Validation - `./tests/nixos-update-remediation-regressions.sh` - Forgejo workflow lint with `actionlint` - `git diff --check` ## Rollout No NixOS deployment is required. Once merged, the next update-mode workflow with published remediation will send the additional ntfy follow-up. Refs #168

nimmo added this to the Automated Update Remediation milestone 2026-08-13 19:04:04 +01:00

nimmo added the

project:auto-update-remediation

service:auto-update

labels 2026-08-13 19:04:04 +01:00

nimmo self-assigned this 2026-08-13 19:04:04 +01:00

nimmo added 1 commit 2026-08-13 19:04:05 +01:00

fix(remediation): notify Forgejo follow-ups ... a1be00de25

Refs #168

nimmo commented 2026-08-13 19:09:08 +01:00

Author

Owner

Copy link

Automated code review

Reviewed commit: 4baab2c3b332c13bc505afbaa3ddf1b20dd1b073

Verdict: Ready to merge

No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory.

Overall assessment

The pull request successfully decouples ntfy notifications from the model-capable remediation job, introduces a bounded public issue/PR report via a new notify-remediation workflow step, and normalizes Forgejo JSON issue indices in the Python controller. Validation includes regression tests, actionlint, and diff checks. The changes align with security boundaries by keeping ntfy secrets out of the model sandbox and correctly migrate GitHub Actions output variables to Forgejo's native FORGEJO_OUTPUT.

Blocking findings

None.

Important findings

None.

Suggestions

• Bash empty array expansion safety under set -u (.forgejo/workflows/attic-cache.yml:398; medium confidence)
  curl_args=() followed by "${curl_args[@]}" inside a step with set -eu enabled.
  Impact: Bash versions prior to 4.4 trigger an unbound variable error when expanding an empty array with set -u, potentially aborting the notification step.
  Suggested fix: Guard the expansion with a length check (e.g., if [ ${#curl_args[@]} -gt 0 ]; then) or use the POSIX-safe pattern "${curl_args[@]+"${curl_args[@]}"}" to ensure compatibility across runner environments.

Tests and validation

• No configured deterministic checks.

Questions

• Does the target ntfy instance version support raw newline characters in --data-raw payloads without truncation or encoding normalization, given that $message is constructed with literal newlines?

Review limitations

• Review is based solely on the provided diff and repository context; cannot execute workflow runs, validate Forgejo output variable resolution in the actual runner environment, or verify runtime ntfy payload delivery.
• Cannot assess cross-host deployment impact or live auto-update behavior without access to the running NixOS configuration or CI logs.

Diff coverage

• .forgejo/workflows/attic-cache.yml: reviewed — included in a context-limited batch
• README.md: reviewed — included in a context-limited batch
• scripts/auto-update-workflow-route.sh: reviewed — included in a context-limited batch
• scripts/nixos-update-remediation.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: 6
• Previous automated review: updated
• Findings: 1

<!-- forgejo-ai-review:v1 --> ## Automated code review Reviewed commit: `4baab2c3b332c13bc505afbaa3ddf1b20dd1b073` ## Verdict: Ready to merge No blocking or important findings were retained, and deterministic checks passed. Suggestions remain advisory. ### Overall assessment The pull request successfully decouples ntfy notifications from the model-capable remediation job, introduces a bounded public issue/PR report via a new `notify-remediation` workflow step, and normalizes Forgejo JSON issue indices in the Python controller. Validation includes regression tests, actionlint, and diff checks. The changes align with security boundaries by keeping ntfy secrets out of the model sandbox and correctly migrate GitHub Actions output variables to Forgejo's native `FORGEJO_OUTPUT`. ### Blocking findings None. ### Important findings None. ### Suggestions - **Bash empty array expansion safety under set -u** (.forgejo/workflows/attic-cache.yml:398; medium confidence) curl_args=() followed by "${curl_args[@]}" inside a step with set -eu enabled. Impact: Bash versions prior to 4.4 trigger an unbound variable error when expanding an empty array with set -u, potentially aborting the notification step. Suggested fix: Guard the expansion with a length check (e.g., if [ ${#curl_args[@]} -gt 0 ]; then) or use the POSIX-safe pattern "${curl_args[@]+"${curl_args[@]}"}" to ensure compatibility across runner environments. ### Tests and validation - No configured deterministic checks. ### Questions - Does the target ntfy instance version support raw newline characters in --data-raw payloads without truncation or encoding normalization, given that $message is constructed with literal newlines? ### Review limitations - Review is based solely on the provided diff and repository context; cannot execute workflow runs, validate Forgejo output variable resolution in the actual runner environment, or verify runtime ntfy payload delivery. - Cannot assess cross-host deployment impact or live auto-update behavior without access to the running NixOS configuration or CI logs. ### Diff coverage - `.forgejo/workflows/attic-cache.yml`: **reviewed** — included in a context-limited batch - `README.md`: **reviewed** — included in a context-limited batch - `scripts/auto-update-workflow-route.sh`: **reviewed** — included in a context-limited batch - `scripts/nixos-update-remediation.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: 6 - Previous automated review: updated - Findings: 1 </details>

nimmo added 1 commit 2026-08-13 19:11:31 +01:00

fix(remediation): preserve partial follow-up reports ... 3d683f2ee7

Refs #168

nimmo commented 2026-08-13 19:12:00 +01:00

Author

Owner

Copy link

Responding to the automated review:

• Accepted the defensive JSON-handling suggestion. 3d683f2 validates the atomic report with jq -ce; malformed output is logged and replaced with an empty report, so no ntfy follow-up is sent.
• The requested scan_status == 0 guard is intentionally not applied. The controller writes the report atomically only after its bounded scan loop. A non-zero exit can therefore follow successful publication for earlier incidents; suppressing those verified links would hide the manual follow-up that the notification is meant to surface. Each workflow run uses a new mktemp report path, so the report cannot be stale from a prior run.

Revalidated with ./tests/nixos-update-remediation-regressions.sh, actionlint, and git diff --check.

Responding to the automated review: - Accepted the defensive JSON-handling suggestion. `3d683f2` validates the atomic report with `jq -ce`; malformed output is logged and replaced with an empty report, so no ntfy follow-up is sent. - The requested `scan_status == 0` guard is intentionally not applied. The controller writes the report atomically only after its bounded scan loop. A non-zero exit can therefore follow successful publication for earlier incidents; suppressing those verified links would hide the manual follow-up that the notification is meant to surface. Each workflow run uses a new `mktemp` report path, so the report cannot be stale from a prior run. Revalidated with `./tests/nixos-update-remediation-regressions.sh`, actionlint, and `git diff --check`.

nimmo commented 2026-08-13 19:17:26 +01:00

Author

Owner

Copy link

Review response before the follow-up push:

• I will accept the ntfy payload hardening: construct real newline-separated text and pass it with printf %s / curl --data-raw so backslashes in public URLs are preserved literally.
• I will also trim a string index before decimal validation; this is a harmless robustness improvement.
• I am rejecting the proposed downgrade of publication_report_record validation failures. A state marked published but lacking a valid public issue/PR reference is an integrity violation in the cross-job hand-off. Failing the scan is intentional fail-closed behavior; logging and continuing would lose required follow-up work.

I will rerun the remediation regressions, workflow lint, and diff check before pushing the resulting commit.

Review response before the follow-up push: - I will accept the ntfy payload hardening: construct real newline-separated text and pass it with `printf %s` / `curl --data-raw` so backslashes in public URLs are preserved literally. - I will also trim a string index before decimal validation; this is a harmless robustness improvement. - I am rejecting the proposed downgrade of `publication_report_record` validation failures. A state marked published but lacking a valid public issue/PR reference is an integrity violation in the cross-job hand-off. Failing the scan is intentional fail-closed behavior; logging and continuing would lose required follow-up work. I will rerun the remediation regressions, workflow lint, and diff check before pushing the resulting commit.

nimmo added 1 commit 2026-08-13 19:18:21 +01:00

fix(remediation): preserve notification URLs ... aaf5419dd5

Refs #168

nimmo commented 2026-08-13 19:22:31 +01:00

Author

Owner

Copy link

Review response before the next push:

Accepted. The notifier intentionally uses Bash array and string features, so I will set shell: bash explicitly on that step rather than relying on runner defaults. I will extend the workflow regression assertion, rerun the remediation suite, actionlint, and diff check, then push.

Review response before the next push: Accepted. The notifier intentionally uses Bash array and string features, so I will set `shell: bash` explicitly on that step rather than relying on runner defaults. I will extend the workflow regression assertion, rerun the remediation suite, actionlint, and diff check, then push.

nimmo added 1 commit 2026-08-13 19:23:10 +01:00

fix(remediation): declare notifier shell ... e34ae367ed

Refs #168

nimmo commented 2026-08-13 19:28:39 +01:00

Author

Owner

Copy link

Review response before the next push:

• Accepted: report-validation failures will include the incident fingerprint and published status, while retaining fail-closed behavior.
• Accepted: I will assert that the compact JSON output contains no literal newline before writing GITHUB_OUTPUT.
• Rejected: the integration test’s exact fake URL is intentional protocol coverage. It verifies that the controller carries the Forgejo URL returned by Tea into the bounded report; a broad URL regex would weaken that contract and conceal a producer/consumer mismatch.

I will run the remediation regressions, workflow lint, and diff check before pushing.

Review response before the next push: - Accepted: report-validation failures will include the incident fingerprint and published status, while retaining fail-closed behavior. - Accepted: I will assert that the compact JSON output contains no literal newline before writing `GITHUB_OUTPUT`. - Rejected: the integration test’s exact fake URL is intentional protocol coverage. It verifies that the controller carries the Forgejo URL returned by Tea into the bounded report; a broad URL regex would weaken that contract and conceal a producer/consumer mismatch. I will run the remediation regressions, workflow lint, and diff check before pushing.

nimmo added 1 commit 2026-08-13 19:29:37 +01:00

fix(remediation): diagnose invalid follow-up reports ... caae61b258

Refs #168

nimmo added 1 commit 2026-08-13 19:33:28 +01:00

refactor(ci): use Forgejo output files ... 4baab2c3b3

Refs #168

nimmo commented 2026-08-13 19:38:24 +01:00

Author

Owner

Copy link

Final readiness summary:

• Automated review of 4baab2c is Ready to merge with no blocking or important findings.
• The remaining empty-array note is advisory; this notifier explicitly runs under Bash on the NixOS runner, where the empty-array expansion is supported.
• Validation passed: ./tests/nixos-update-remediation-regressions.sh, actionlint, and git diff --check.
• No live deployment is required; once merged, the next update-mode remediation publication will emit the new issue/PR follow-up notification.

The PR is ready for merge approval.

Final readiness summary: - Automated review of `4baab2c` is **Ready to merge** with no blocking or important findings. - The remaining empty-array note is advisory; this notifier explicitly runs under Bash on the NixOS runner, where the empty-array expansion is supported. - Validation passed: `./tests/nixos-update-remediation-regressions.sh`, actionlint, and `git diff --check`. - No live deployment is required; once merged, the next update-mode remediation publication will emit the new issue/PR follow-up notification. The PR is ready for merge approval.

nimmo merged commit 999961b155 into main 2026-08-13 19:56:25 +01:00

nimmo referenced this pull request from a commit 2026-08-13 19:56:26 +01:00

Merge pull request 'fix(remediation): link ntfy follow-ups to Forgejo work' (#169) from fix/remediation-follow-up-notifications into main

nimmo deleted branch fix/remediation-follow-up-notifications 2026-08-13 19:56:26 +01:00

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

nimmo

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!169

No description provided.