tidy-api/oauthflow: one body per resolution, not one per caller - #240
Merged
Conversation
dataDir and devDataDir were the same twenty-line platform switch differing in one constant — the directory name. windowsDataDir already took that name as a parameter, so only the Unix branches were duplicated, and internal/platform (the resolver this package's comments promise to stay byte-identical to) has shared its own for the same stated reason: a rule applied to one copy and not the other is how the dev and release directories come to sit in different parents. dataDirNamed is that switch once, and the two entry points name their constant. The AGENTHUB_DATA_DIR lookup stays ahead of every branch, where both copies already had it — which is where this still differs from internal/platform, and the comment now says so. make ci green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DialOrStart and StartSupervised take the same StartOptions and each
opened with the same twenty lines of defaulting: socket path, daemon
binary, run directory, deadline, poll interval. StartOptions documents
those defaults on the fields themselves ("0 = 10s", "0 = 100ms", "" = dir
of SocketPath), so two copies meant that documentation could become true
of one entry point and false of the other, and the two ways of bringing a
daemon up would then look for it in different places.
StartOptions.resolve is that sequence once. DaemonArgs is the one thing
the two genuinely disagree about — `daemon start`, plus `--foreground`
for a supervised child — so it is the one thing passed in.
make ci green.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
postForm and postJSON each spelled out the same prologue for a credential-bearing POST: parse the endpoint, run checkURL, build the request, set Content-Type / Accept / User-Agent, add the caller's headers, Do, classify a transport failure. checkURL is the SSRF screen, so a third credential POST written by copying either one could lose it and nothing would say so. postCredential is that prologue once. It stops at the response rather than also refusing a 3xx: both callers refuse it, but with different errors — a token exchange redacts the Location it reports, because that header can echo back the credential just sent, while a registration does not read it — and a parameter for that would make the zero-redirect rule read as a caller's choice. One ordering change comes with it: postJSON now marshals its payload before parsing the endpoint, so an unmarshalable payload beats a bad endpoint. Both are programmer errors and the payloads are in-package structs, but it is a change rather than a rename. make ci green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Nightly tidy over
apiandinternal/oauthflow: three resolution/prologue bodies that existed twice, where the second copy is what a future third caller would be copied from.api: one body for the two data-directory flavours (dataDirNamed, mirroringinternal/platform)api: one place that fills in aStartOptions(resolve)internal/oauthflow:postCredential— the SSRF screen is not a step a credential POST can skip