Skip to content

fix(repository): log native draft not-submitted and attributable head refusals, fix helper log advice - #3602

Merged
freeqaz merged 2 commits into
mainfrom
fix-2429
Oct 25, 2026
Merged

freeqaz merged 2 commits into
mainfrom
fix-2429

Conversation

@freeqaz

@freeqaz freeqaz commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator

Problem

Three low findings from a post-land review of #3585 and #3589:

  • 2429: a native draft create that the broker settled as not sent before any write (for example, the GitHub connection failed) returns a recorded 409 receipt with not-submitted. Neither that answer nor a same-operation replay of the receipt wrote a repository-credentials.denied line, although prepared-native-git.md promises one line per refused credentialed request and lists the failed GitHub connection under not_dispatched. A request whose head was refused before authentication (wrong Host, bad framing) was also never logged, even when it carried the session credential.
  • 2430: the page said every line carries the session ID, Namespace and repositoryRef. The broker can attribute a line only when the credential names an open session, so session_unavailable lines (for example after a worker Pod restart) carry none of them.
  • 2431: for a native unavailable refusal, oce-github pr-create told the Agent to "check the worker log". The native line is in the worker Pod's repository-credentials container, and kubectl logs on the worker defaults to the worker container. The capacity advice said "for this Agent" although the broker-wide request limit can also refuse.

Change

  • transport/agent.ts: a recorded receipt whose error is not-submitted writes one line (status 409, reasonCode not-submitted, reason not_dispatched, operation api), on the first answer and on a replay. Created and outcome_unknown receipts are still not refusals.
  • transport/request.ts + agent.ts: a head refused after its fields were validated (wrong Host, connection/upgrade fields, framing, encoding, Expect) now returns its single Authorization value with the denial. The transport logs it as request_refused only when that credential names an open session. It uses the same presentedToken parse as the existing attribution and nothing more. Earlier head refusals (start line, field syntax, sizes, repeated fields) carry no single credential and stay unlogged. Unauthenticated traffic stays silent, and the Agent's answer is unchanged.
  • 2430, docs rather than code: a record of closed sessions would not help the main case. A worker Pod restart replaces the broker process, so its in-memory sessions are gone anyway. prepared-native-git.md now says the identity fields appear only when the credential names an open session, and that a session_unavailable line is matched by time to the worker Pod restart or the session's end. It also names the not-submitted receipt and the attributable head refusals, and says uncertain outcomes are not logged. The flow doc names the head-refusal case.
  • client/draft-pr.ts: native-only advice. unavailable now says to check the repository-credentials container log in the worker Pod. exchange-capacity says too many requests are running for this Agent or across the broker, so retry after a short wait. The OpenShell route's advice is unchanged, and the Agent still learns no more than the OpenShell path tells it.

Verification

  • postgres-prepared-repository-git-broker.test.mjs (real broker listener, PostgreSQL owner, GitHub fixture):
    • a new connection failed before any write mode drops GitHub's connection before TLS and asserts the 409 not-submitted receipt plus one not_dispatched line for the create and for its replay;
    • the Git test asserts one request_refused line, with the session identity, for a wrong Host carrying the session credential;
    • the withdrawn draft case pins the new advice.
    • The GitHub fixture gains refuseNextConnection().
  • On main with only the test changes, this file fails 4 cases on a remote test VM: wrong-Host line missing, not-submitted line missing (the receipt itself reproduces), and the old advice text in both created modes. With the fix, the postgres-platform lane passes.
  • repository-credentials-service-loss.test.mjs (real broker process): a wrong-Host request without a credential, and one with a credential that no open session issued, write no line.
  • repository-credentials-draft-pr-cli, -transport-bounds, -http and -service-loss pass locally; tsc --build, eslint and prettier are clean on the touched files.
  • Docs word counts: prepared-native-git.md stays under 1,500 words and docs/flows/repository-credentials.md is at 2,398.

Risks

  • Low. A head refusal is logged only for a credential that names an open session, so the log volume is still bounded by the Agent's own request rate, as before. The denial object now carries the raw Authorization value inside the process. Its only consumer uses it for attribution, and the other inspectRequestHead callers refuse any denial without reading it.
  • The advice text change affects only the native helper's unavailable and exchange-capacity lines.

…ttributable head refusals

A native draft create the broker settles as not-submitted (GitHub's connection failed before
any write) and its replay now write a repository-credentials.denied line (not_dispatched). A
request head refused after its fields were checked (wrong Host, bad framing) is logged as
request_refused when its credential names an open session. The draft helper's native advice
names the repository-credentials container and no longer blames this Agent for the
broker-wide request limit (findings 2429, 2431).
… to read session_unavailable

The session ID, Namespace and repositoryRef appear only when the credential names an open
session; session_unavailable lines carry none and are matched by time to a worker Pod restart
or the session's end. Name the not-submitted draft receipt and the attributable head
refusals, and say uncertain outcomes are not logged (findings 2429, 2430).
@freeqaz
freeqaz requested a review from a team as a code owner October 25, 2026 08:26
@clawsweeper

clawsweeper Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

🦞👀
ClawSweeper picked this up.

Pull request received. I will update this pull request when review starts.

ClawSweeper review complete

ClawSweeper finished reviewing this revision. The review result is being finalized.

View the workflow run.

@clawsweeper

clawsweeper Bot commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

ClawSweeper status: review started.

I am starting a fresh review of this pull request: fix(repository): log native draft not-submitted and attributable head refusals, fix helper log advice This is item 1/1 in the current shard. Shard 0/1.

This temporary status tracks the active review worker. The completed review will appear in the durable ClawSweeper review comment.

Crustacean status: shell secured, claws on keyboard, evidence pebbles being sorted.

@clawsweeper clawsweeper Bot added clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending. labels Oct 25, 2026
@freeqaz
freeqaz merged commit e720a82 into main Oct 25, 2026
53 checks passed
@freeqaz
freeqaz deleted the fix-2429 branch October 25, 2026 08:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clawsweeper:intake:bug Preliminary submission classification; full source review pending. clawsweeper:intake:needs-environment Preliminary submission classification; full source review pending.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant