You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Defect: ADR-0032 removed every sibling daemon's HTTP listener but explicitly left open how an external GitHub webhook reaches a service that no longer listens and is not resident. That gap is what blocks milestone tm 1.3.5 criterion (c) (undaemonize trusty-analyze/trusty-review). ADR-0034 (owner ruling 2026-08-07, "Console relays over UDS.") closes the design question; this issue is the implementation.
Evidence:
Two webhook handlers assume an always-listening HTTP server: crates/trusty-review/src/service/webhook.rs:141 and crates/trusty-analyze/src/service/handlers/review.rs:128.
The two disagree on the unset-secret policy. trusty-review returns 401 and rejects every delivery (webhook.rs:147-161). trusty-analyze logs "no webhook secret configured — skipping webhook signature verification" and processes the payload anyway (review.rs:153-157) — a live fail-open.
Both return 202 before the work runs, then tokio::spawn and downgrade failure to a log line (review.rs:210-214; webhook.rs:305-309). GitHub does not retry an acknowledged delivery, so a post-ack failure loses the event while health reports fine. Same shape as ensure_project_indexed (crates/trusty-code/src/run_task/mod.rs:127: let _ = trusty_common::search_index::ensure_project_indexed(&project, true);) and trusty-review: handle_review's in_flight counter leaks on disconnect/panic — /status is not a trustworthy load signal #5020's in_flight counter, measured pinned at 9 for ten days.
The 0600 socket permission that ADR-0031 and ADR-0032 both cite as UDS's access-control advantage is not implemented: no production file in the workspace calls set_permissions/PermissionsExt on a socket (every hit is a test fixture). Sockets are created at the default umask, and embedder_client/uds.rs + bm25_client.rs place them in $TMPDIR with a /tmp fallback.
Resolution: implement ADR-0034 in four steps, each gated at the rung named.
Shared UDS listener/dial module in trusty-common — newline-framed JSON-RPC 2.0, matching the existing clients. Socket in a 0700 user-owned state dir (following trusty-agents/src/ctrl/socket.rs, not $TMPDIR), chmod 0600 after bind, peer-uid check on accept (SO_PEERCRED / getpeereid). Migrate embedder_client/uds.rs, bm25_client.rs, and ctrl/socket.rs onto it. Rung 4.
UdsServiceSupervisor in trusty-common, generalised from Bm25Supervisor, plus an external-mode opt-out so an operator running the target under tctl can suppress spawn. Rung 5 — concurrency tests for double-spawn and socket adoption.
Console ingress: /api/webhooks/{source}, single HMAC verification over the exact received bytes, fsync'd spool before the 202, 5xx when the spool write fails, durable pending entry plus attempt count on relay failure, oldest-pending-age exported through /api/console/metrics/* as a red health state. Rung 5 — failure-path coverage is the point of the step.
Retire both HTTP webhook routes; targets accept the relayed frame over UDS and trust the provenance record. Unify the unset-secret policy to fail-closed. Rung 5 + code-critic.
Criterion (c) is rung 5 overall (cross-crate contract + process lifecycle): --include-ignored integration coverage, failure-path and concurrency tests, and a code-critic round.
fix(trusty-review): serve --stdio opens the daemon's redb dedup store, colliding with serve (HTTP) #5064 blocks criterion (c), not only the deferred d2. A console-spawned webhook worker and a concurrent serve --stdio session both call build_app_state against the same --log-dir, so the dedup.redb exclusive-flock collision recurs — and lands on the webhook path, which unlike the MCP path runs with allow_posting: true and actually needs the store.
ADR: docs/adr/0034-webhook-ingress-console-relays-over-uds-to-a-supervised-on-demand-process.md. Design question settled by ADR-0034; supersedes the open carve-out in ADR-0032. Related: #5028 (direction question that prompted it), #3193 (DOC-47 external event ingestion epic — broader scope, this is the criterion-(c) slice).
Defect: ADR-0032 removed every sibling daemon's HTTP listener but explicitly left open how an external GitHub webhook reaches a service that no longer listens and is not resident. That gap is what blocks milestone
tm 1.3.5criterion (c) (undaemonize trusty-analyze/trusty-review). ADR-0034 (owner ruling 2026-08-07, "Console relays over UDS.") closes the design question; this issue is the implementation.Evidence:
crates/trusty-review/src/service/webhook.rs:141andcrates/trusty-analyze/src/service/handlers/review.rs:128.trusty-reviewreturns 401 and rejects every delivery (webhook.rs:147-161).trusty-analyzelogs"no webhook secret configured — skipping webhook signature verification"and processes the payload anyway (review.rs:153-157) — a live fail-open.202before the work runs, thentokio::spawnand downgrade failure to a log line (review.rs:210-214;webhook.rs:305-309). GitHub does not retry an acknowledged delivery, so a post-ack failure loses the event while health reports fine. Same shape asensure_project_indexed(crates/trusty-code/src/run_task/mod.rs:127:let _ = trusty_common::search_index::ensure_project_indexed(&project, true);) and trusty-review: handle_review's in_flight counter leaks on disconnect/panic — /status is not a trustworthy load signal #5020'sin_flightcounter, measured pinned at 9 for ten days.0600socket permission that ADR-0031 and ADR-0032 both cite as UDS's access-control advantage is not implemented: no production file in the workspace callsset_permissions/PermissionsExton a socket (every hit is a test fixture). Sockets are created at the default umask, andembedder_client/uds.rs+bm25_client.rsplace them in$TMPDIRwith a/tmpfallback.crates/trusty-memory/src/bm25_supervisor.rs(ensure_running) already implements spawn-gate serialisation, socket adoption, LRU cap, RSS ceiling, backoff probing, and SIGTERM→SIGKILL, hardened by [trusty-search] Add a default max fan-out count and a --serial flag for multi-index search fan-out #2845/[trusty-search] Memory limit (rss_limit_mb) not enforced — unbounded RSS growth leads to OOM-kill #2846.Resolution: implement ADR-0034 in four steps, each gated at the rung named.
trusty-common— newline-framed JSON-RPC 2.0, matching the existing clients. Socket in a0700user-owned state dir (followingtrusty-agents/src/ctrl/socket.rs, not$TMPDIR),chmod 0600after bind, peer-uid check on accept (SO_PEERCRED/getpeereid). Migrateembedder_client/uds.rs,bm25_client.rs, andctrl/socket.rsonto it. Rung 4.UdsServiceSupervisorintrusty-common, generalised fromBm25Supervisor, plus an external-mode opt-out so an operator running the target undertctlcan suppress spawn. Rung 5 — concurrency tests for double-spawn and socket adoption./api/webhooks/{source}, single HMAC verification over the exact received bytes, fsync'd spool before the 202, 5xx when the spool write fails, durablependingentry plus attempt count on relay failure, oldest-pending-age exported through/api/console/metrics/*as a red health state. Rung 5 — failure-path coverage is the point of the step.Criterion (c) is rung 5 overall (cross-crate contract + process lifecycle):
--include-ignoredintegration coverage, failure-path and concurrency tests, and a code-critic round.Prerequisites:
serve --stdiosession both callbuild_app_stateagainst the same--log-dir, so thededup.redbexclusive-flock collision recurs — and lands on the webhook path, which unlike the MCP path runs withallow_posting: trueand actually needs the store.ADR:
docs/adr/0034-webhook-ingress-console-relays-over-uds-to-a-supervised-on-demand-process.md. Design question settled by ADR-0034; supersedes the open carve-out in ADR-0032. Related: #5028 (direction question that prompted it), #3193 (DOC-47 external event ingestion epic — broader scope, this is the criterion-(c) slice).🤖🤖🤖 Generated with trusty-mpm — https://github.com/bobmatnyc/trusty-tools