Skip to content

fix: make tunnel webhook mounts recoverable - #502

Draft
PerishCode wants to merge 1 commit into
mainfrom
codex/looper-owned-webhook-ensure
Draft

fix: make tunnel webhook mounts recoverable#502
PerishCode wants to merge 1 commit into
mainfrom
codex/looper-owned-webhook-ensure

Conversation

@PerishCode

Copy link
Copy Markdown
Contributor

Summary

  • add a looper_mount marker to tunnel webhook managed URLs, derived from the local per-repo HMAC secret
  • when the SQLite hook record is missing but the secret file survives, re-record exactly one mount-marked hook before returning to hook-id reconciliation
  • keep plain legacy URL matches non-adoptable and document the recovery boundary

Authority

The authority for re-recording a tunnel webhook is the locally held per-repo HMAC secret, materialized as a deterministic looper_mount marker in the GitHub hook URL. It is not the agent output and not a generic remote hook shape; plain URL, name, events, or user-supplied hook ids still do not authorize adoption.

Trade-off

This prevents Looper from creating duplicate tunnel webhooks when the SQLite row is lost but the local secret survives. The cost is a visible opaque query marker in managed URLs, one gh api repos/:repo/hooks list before first create, and a new recovery branch to keep in sync with the existing hook-id lifecycle. A simpler plain-URL adoption rule was rejected because it cannot prove Looper ownership and would repeat the unsafe direction from #487.

This is authority-bearing and should receive @oracle review before merge.

Validation

  • go test ./internal/runtime
  • go vet ./...
  • go build ./...
  • go test ./... was run; it failed only in internal/agent executor timeout tests (TestExecutorStartSanitizesChildEnvAndUsesWorkingDirectory, TestExecutorFallsBackAfterFailedNativeResumeAttempt, TestExecutorNativeResumeFailureAfterAttachDoesNotFallback), while internal/runtime and the remaining completed packages passed.

@quangdang46 quangdang46 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hermes QA Report | PR #502

Verdict: COMMENT β€” Code changes are clean and well-structured. Design follows AGENTS.md guidelines. Blocked on required @oracle review per PR body.

Quality Gates

Check Result
go vet ./... βœ… PASS
go build ./... βœ… PASS
go test ./internal/runtime/... βœ… PASS (all tunnel/webhook tests)
Targeted tunnel/webhook tests βœ… PASS

Code Review Summary

Positives:

  • Clean HMAC-SHA256 mount ID derivation from repo+secret β€” deterministic, stable, properly handles empty secrets
  • findReusableTunnelHook correctly guards against duplicate adoption (error on multiple mount-marked hooks) and rejects plain legacy URL matches outright
  • reconcileRecordedTunnelHook extraction keeps the logic readable and testable
  • ADR 0006, docs/configuration.md, and docs/users-guide.md all updated to document the new mount-marker recovery path
  • All existing tests updated to expect mount-marked URLs β€” no regressions

Design compliance: Authority is grounded in the local HMAC secret, not agent output or remote state inference. Trade-off (query marker in URL, list-before-create latency, new recovery branch) is documented. Matches AGENTS.md requirements.

Merge blocker: PR body requests @oracle review for this authority-bearing change. Oracle review must complete before merge.

Recommendation: Code ready. Merge after @oracle review approval.

Reviewed at 2026-06-21 from Hermes QA cron.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants