Add LHP NOC callback support#23
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b78179332a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| def _signature(secret: str, method: str, path: str, timestamp: str, body: Any) -> str: | ||
| message = "\n".join([_token(method.upper()) or "GET", path, _token(timestamp), json.dumps(body, sort_keys=True, separators=(",", ":"), ensure_ascii=False)]).encode() |
There was a problem hiding this comment.
For every signed LHP request, datetime.now(UTC).isoformat() includes +00:00, but _token(timestamp) strips the + before building the HMAC message while the unmodified value is sent in X-NOC-Loop-Timestamp. A CaseService verifier that signs the header value it receives will compute a different digest and reject both payload fetches and callbacks, so NOC-origin LHP issues will always block once signature verification is enabled.
Useful? React with 👍 / 👎.
| post_lhp_update( | ||
| lhp_pointer, | ||
| lhp_config, | ||
| update_type="change_planned", | ||
| status="change_planned", | ||
| summary=report.detail, | ||
| evidence=evidence, | ||
| ) |
There was a problem hiding this comment.
When callbacks are enabled, a transient webhook/network error from the optional change_planned callback propagates out of post_lhp_update; at this point the draft PR has already been published, but the outer except converts the daemon result to error and skips the ledger update/reporting of the PR URL. Callback delivery should be isolated or caught here so a NOC outage does not make an otherwise successful engineering run look failed.
Useful? React with 👍 / 👎.
Svaag
force-pushed
the
feat/lhp-v1-callback-support
branch
from
b781793 to
81149f2
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 81149f2fdd
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| status="blocked", | ||
| summary=f"Could not fetch authoritative NOC LHP payload: {type(exc).__name__}: {exc}", | ||
| ) | ||
| return _finish( |
There was a problem hiding this comment.
When an approved LHP issue is selected but CaseService is unavailable or the LHP client is misconfigured, this early return exits before the update_ledger call later in daemon_once. Because the issue remains approved and the daily run count is unchanged, every timer invocation can immediately retry the same failing fetch and emit another accepted/blocked callback plus notifications, bypassing the daemon's per-day run throttle for this new failure path.
Useful? React with 👍 / 👎.
1 participant
Summary
request.mdfrom structured LHP fields while keeping issue prose as sanitized untrusted backgroundaccepted,investigating,change_planned,needs_human, andblockedcallbacks to NOCValidation
Notes
loop:approvedgate remains unchangedENGINEERING_LOOP_NOC_LHP_BASE_URL,ENGINEERING_LOOP_NOC_LHP_SECRET, andENGINEERING_LOOP_LHP_CALLBACK_ENABLED=1