Bug: batch-wire-webhooks.py creates hooks with empty secret, breaking ingress for orphan repos #40

New issue

Closed

opened 2026-08-27 19:17:25 +00:00 by Mark · 1 comment

Mark commented

2026-08-27 19:17:25 +00:00

Owner

Closed via watcher recovery after kilo-gsd.py push-failure recovery.

Fix landed on origin/main:

866f351 fix(worker): implement --force-recreate for Mark/org_mode#40 — wires the previously-documented-but-unimplemented --force-recreate <repo> flag in workers/batch-wire-webhooks.py. This is the operational remediation: deletes every daemon-pointing hook on the named repo and creates a single fresh one using WEBHOOK_SECRET. Lazies env-var reads so tests can monkeypatch. Adds tests/test_batch_wire_webhooks.py covering find_daemon_hooks, create_webhook (verifies the created hook carries WEBHOOK_SECRET, not ''), force_recreate_repo (incl. the broken-secret and all-duplicates-deleted paths), argparse wiring, and the underlying HTTP DELETE method. 600 insertions / 60 deletions across the worker and tests.
0b7abc7 Apply changes from kilo-gsd.py run for Mark/org_mode#40 — adds --dry-run argparse flag and a dry-run short-circuit in main() that prints the planned action without making any Forgejo API calls. 13 insertions.

kilo-gsd.py push-failure root cause:

A sibling run for Mark/org_mode#29 pushed commit bf0d318 (touches vikunja-webhook-to-kanban.sh and test_vikunja_webhook_to_kanban.py) while the #40 pipeline was mid-Commit. bf0d318 and the #40 commits share no file overlap (zero conflicts), so a clean git rebase origin/main resolved the non-fast-forward. commit ids above are post-rebase.

Recovery procedure (so the next watcher knows):

git fetch origin main — confirm remote has advanced.
Inspect git show --stat <remote_tip> vs the local pending commits — check for file overlap.
If no overlap: git rebase origin/main then git push origin main.
If overlap: surface to operator (file-level merge needed).
After push: close the Forgejo issue with a comment summarizing landed commits.

Review-flagged concerns (not blocking, worth a follow-up):

The Phase 7/9 Review agent flagged 23 items, mostly pre-existing or test-coverage gaps rather than functional defects. Two worth tracking:

Item 17: --dry-run short-circuit does not enumerate which hook IDs would be deleted under --force-recreate. A real dry-run should call find_daemon_hooks(repo) and print the planned deletions and secret length.
Items 8/3/1: silent-duplicate-creation risk under specific race conditions. Worth a follow-up issue.

The core fix for this ticket is correct and well-tested for the happy path. The deeper Forgejo-internal bug for repo 170 (no hook_task rows generated despite valid hook row) is out-of-scope per the original plan and remains an open operational concern.

Issue auto-close via watcher card t_6128733e recovery.

Closed via watcher recovery after kilo-gsd.py push-failure recovery. Fix landed on origin/main: - 866f351 fix(worker): implement --force-recreate for Mark/org_mode#40 — wires the previously-documented-but-unimplemented `--force-recreate <repo>` flag in workers/batch-wire-webhooks.py. This is the operational remediation: deletes every daemon-pointing hook on the named repo and creates a single fresh one using WEBHOOK_SECRET. Lazies env-var reads so tests can monkeypatch. Adds tests/test_batch_wire_webhooks.py covering find_daemon_hooks, create_webhook (verifies the created hook carries WEBHOOK_SECRET, not ''), force_recreate_repo (incl. the broken-secret and all-duplicates-deleted paths), argparse wiring, and the underlying HTTP DELETE method. 600 insertions / 60 deletions across the worker and tests. - 0b7abc7 Apply changes from kilo-gsd.py run for Mark/org_mode#40 — adds --dry-run argparse flag and a dry-run short-circuit in main() that prints the planned action without making any Forgejo API calls. 13 insertions. kilo-gsd.py push-failure root cause: A sibling run for Mark/org_mode#29 pushed commit bf0d318 (touches vikunja-webhook-to-kanban.sh and test_vikunja_webhook_to_kanban.py) while the #40 pipeline was mid-Commit. bf0d318 and the #40 commits share no file overlap (zero conflicts), so a clean `git rebase origin/main` resolved the non-fast-forward. commit ids above are post-rebase. Recovery procedure (so the next watcher knows): 1. git fetch origin main — confirm remote has advanced. 2. Inspect `git show --stat <remote_tip>` vs the local pending commits — check for file overlap. 3. If no overlap: git rebase origin/main then git push origin main. 4. If overlap: surface to operator (file-level merge needed). 5. After push: close the Forgejo issue with a comment summarizing landed commits. Review-flagged concerns (not blocking, worth a follow-up): The Phase 7/9 Review agent flagged 23 items, mostly pre-existing or test-coverage gaps rather than functional defects. Two worth tracking: - Item 17: --dry-run short-circuit does not enumerate which hook IDs would be deleted under --force-recreate. A real dry-run should call find_daemon_hooks(repo) and print the planned deletions and secret length. - Items 8/3/1: silent-duplicate-creation risk under specific race conditions. Worth a follow-up issue. The core fix for this ticket is correct and well-tested for the happy path. The deeper Forgejo-internal bug for repo 170 (no hook_task rows generated despite valid hook row) is out-of-scope per the original plan and remains an open operational concern. Issue auto-close via watcher card t_6128733e recovery.

Mark added the

triage-approved

bug

labels

2026-08-27 19:17:25 +00:00

Mark referenced this issue from a commit

2026-09-06 06:14:55 +00:00

batch-wire-webhooks.py: self-heal empty-secret hooks

Mark referenced this issue from a commit

2026-09-06 06:14:55 +00:00

fix(worker): drop false-positive broken-hook detection in batch-wire-webhooks.py

Mark referenced this issue from a commit

2026-09-06 06:14:55 +00:00

fix(worker): implement --force-recreate for Mark/org_mode#40

Mark referenced this issue from a commit

2026-09-06 06:14:55 +00:00

Apply changes from kilo-gsd.py run for Mark/org_mode#40

Mark closed this issue

2026-09-06 06:15:47 +00:00

Mark commented

2026-09-06 06:17:33 +00:00

Author

Owner

Closing — script-level bug fixed and verified on origin/main.

Commits referencing this issue (all on origin/main at 0b7abc7):

3fae479 (2026-08-31) — batch-wire-webhooks.py: self-heal empty-secret hooks — when the existing daemon hook on a repo has secret='', delete it and create a fresh one with WEBHOOK_SECRET instead of skipping it as "already wired"
7abe70f (2026-09-05) — fix(worker): drop false-positive broken-hook detection — Forgejo API GET responses hide the secret field, so the prior "flag and recreate" branch falsely flagged every existing hook on every run; new behavior: a repo is "already wired" iff any hook points at DAEMON_URL with type=forgejo and active=true
164bb5b (2026-09-05) — fix(worker): use blackhole.local (avahi) for wrapper SSH target — addresses a sibling ingress bug: the org-subscriber daemon on unstable resolves "blackhole" via public DNS which has no tailnet mapping; switching to blackhole.local (avahi mDNS) restores ingress from forgejo webhook → daemon → wrapper
866f351 (2026-09-05) — fix(worker): implement --force-recreate — --force-recreate <repo> deletes every daemon-pointing hook on the named repo and creates a single fresh one using WEBHOOK_SECRET. This is the operational fix for Mark/easystitch-service (repo_id=170) and any other repo whose webhook row carries an empty secret and has no working duplicate
0b7abc7 (2026-09-05) — Apply changes from kilo-gsd.py run for Mark/org_mode#40 — squash commit

Verification:

18/18 tests in tests/test_batch_wire_webhooks.py pass (locally)
create_webhook() confirmed to read _webhook_secret() from env, never hardcoded empty
Force-recreate path verified by automated tests (force_recreate_repo covered including broken-secret and all-duplicates-deleted cases)

Downstream issue tracked separately:
The Mark/easystitch-service (repo_id=170) ingress regression — where forgejo-web refuses to generate hook_task rows for repo 170 even with a valid DB secret — is a separate rootful-podman investigation that requires operator-scope SSH access to forgejo.tail018ac4.ts.net. It is NOT a script-level bug and is being tracked under kanban card t_408c8c29 (rootful-host fixup). The --force-recreate flag added in this fix is the remediation operator can run against repo 170 once the forgejo-web delivery worker starts queuing hook_tasks again.

Closing this issue; rootful-podman follow-up lives on t_408c8c29.

Closing — script-level bug fixed and verified on origin/main. **Commits referencing this issue (all on origin/main at 0b7abc7):** - `3fae479` (2026-08-31) — `batch-wire-webhooks.py: self-heal empty-secret hooks` — when the existing daemon hook on a repo has `secret=''`, delete it and create a fresh one with `WEBHOOK_SECRET` instead of skipping it as "already wired" - `7abe70f` (2026-09-05) — `fix(worker): drop false-positive broken-hook detection` — Forgejo API GET responses hide the secret field, so the prior "flag and recreate" branch falsely flagged every existing hook on every run; new behavior: a repo is "already wired" iff any hook points at `DAEMON_URL` with `type=forgejo` and `active=true` - `164bb5b` (2026-09-05) — `fix(worker): use blackhole.local (avahi) for wrapper SSH target` — addresses a sibling ingress bug: the org-subscriber daemon on unstable resolves "blackhole" via public DNS which has no tailnet mapping; switching to `blackhole.local` (avahi mDNS) restores ingress from forgejo webhook → daemon → wrapper - `866f351` (2026-09-05) — `fix(worker): implement --force-recreate` — `--force-recreate <repo>` deletes every daemon-pointing hook on the named repo and creates a single fresh one using `WEBHOOK_SECRET`. This is the operational fix for `Mark/easystitch-service` (repo_id=170) and any other repo whose webhook row carries an empty secret and has no working duplicate - `0b7abc7` (2026-09-05) — `Apply changes from kilo-gsd.py run for Mark/org_mode#40` — squash commit **Verification:** - 18/18 tests in `tests/test_batch_wire_webhooks.py` pass (locally) - `create_webhook()` confirmed to read `_webhook_secret()` from env, never hardcoded empty - Force-recreate path verified by automated tests (`force_recreate_repo` covered including broken-secret and all-duplicates-deleted cases) **Downstream issue tracked separately:** The `Mark/easystitch-service` (repo_id=170) ingress regression — where forgejo-web refuses to generate `hook_task` rows for repo 170 even with a valid DB secret — is a separate rootful-podman investigation that requires operator-scope SSH access to `forgejo.tail018ac4.ts.net`. It is NOT a script-level bug and is being tracked under kanban card `t_408c8c29` (rootful-host fixup). The `--force-recreate` flag added in this fix is the remediation operator can run against repo 170 once the forgejo-web delivery worker starts queuing hook_tasks again. Closing this issue; rootful-podman follow-up lives on `t_408c8c29`.