Preserve session-wide token totals during condensation - #6
Draft
entire[bot] wants to merge 73 commits into
Draft
Conversation
Entire-Checkpoint: 01M1K3SP30MWG6YHC2QZ9R34A1
Entire-Checkpoint: 01M1MDT7FYTX9025MWBCJE3ZPQ
Entire-Checkpoint: 01M1MF8R024N9A0FHGZK7M45PR
Entire-Checkpoint: 01M1MREADV8D9JS18B5CJA253E
Entire-Checkpoint: 01M1MX9YXDDG0H204M6XN4WB63
Entire-Checkpoint: 01M1MZD5TNZTQGX5W9Z02YDW9Z
Show installation progress and announce the forwarded command. Update the terminal renderer so completed confirmation prompts are cleared, with regression coverage for terminal cleanup and plugin install/dispatch behavior. Entire-Checkpoint: 01M211S73J1V1YPNB123826DE8
Entire-Checkpoint: 01M231BF0DXEPSR8G9AWF7NCMC
Entire-Checkpoint: 01M23DNRDKQ7Z7SYTXZ4PBT5SB
The control plane gained the branch-protection resource and several new operations since the last refresh. Two spec changes needed normalizer support before ogen would consume it: every operation now lists the interactive oauth2/oidc login schemes, which ogen cannot generate, and the repo read models gained required enum fields the CLI never reads. The normalizer drops the interactive schemes, loosens the new provider enums, and makes Repo.provider and Repo.capabilities optional, so the generated SecuritySource and the decoding tolerance stay as they were. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`entire repo protection list|add|remove` edits a native repository's
branch-protection rules through core's /repos/{repoId}/branch-protection
resource. Each rule is a branch pattern and a level: protected (no force
push, no delete) or, with --server-side-merge-only, no direct push at all.
add and remove are one PATCH each; add upserts by ref, so re-adding a
branch changes its level. A short name expands to refs/heads/, HEAD and
refs/... pass through. Every verb prints the resulting rules and honours
--json.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The same refresh made capabilities required on Org and Project, org and provider required on RepoIndexEntry, and added the RepoIndexEntry.permission enum. None is read by the CLI, so they get the same treatment as Repo's new fields: optional, and display enums open. An older core, or a value the server adds later, then cannot fail org list, project list, or repo routing. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Core reports no rules for a mirror, but "Nothing is protected yet" misstates it: the data plane protects a mirror's default branch regardless, and its rules are the upstream's. list now reads the repo's provider and prints that instead. --json keeps the plain array for scripts. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`list` fetched the repo unconditionally but read `provider` only when the rule list was empty and --json was off, so every call cost two control-plane round trips, --json paid for a response it discarded, and a GetRepo failure sank a list whose real answer was already in hand. The lookup moves into the empty-list rendering, which is the one branch that needs it. That branch now also serves --json: stdout stays the bare array a script parses, and the mirror caveat goes to stderr, because a script concluding "no rules ⇒ nothing is protected" is wrong on a GitHub mirror — the exact misreading the human-facing note exists to prevent. The note is therefore load-bearing for the text rendering and advisory for --json, so a failed provider lookup is fatal only to the former. Reading `Repo.provider` also needed a constant of its own. `providerGitHub` is documented as the *account* identity slug behind a grantee handle like "github:alice", and already carried a note distinguishing it from setup.go's `checkpointProviderGitHub`; which forge backs a repository is a third concern spelling GitHub the same way, so it gets `repoProviderGitHub` and the note now names all three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M25PZST8D96JE5769N2M4AEJ
… claim
Two corrections to spec/normalize.go, both in the transforms it documents.
dropInteractiveSecurity removes the alternatives naming only oauth2/oidc. If
an operation's *only* alternatives were interactive, the result was an empty
array — and in OpenAPI an empty operation-level "security" means the
operation needs no authentication at all, so ogen would generate it with no
security source and the client would quietly stop sending the bearer.
Nothing in today's spec has that shape (all 86 operations also offer
bearerAuth), but a device-code or authorize endpoint added upstream would,
and this is the only failure here that would not announce itself: the
sibling case, a requirement mixing oauth2 with bearerAuth, survives whole
and makes ogen abort at generate time on the now-dangling scheme.
filterSecurityRequirements now errors instead, naming the operation.
readModelOptionalFields said the client "never reads" its fields and that "a
field the CLI starts reading must leave this list", which the commit adding
Repo.provider to it also made false — `repo protection list` reads it. The
bar is not being read, it is being *depended on*: a field read through its
Opt accessor with a safe default degrades one branch instead of failing the
response, which is exactly how Repo.provider is used. readModelEnumFields
said the same thing in enum terms ("display-only … never branches on their
value") and is restated as an open-set contract, since the CLI tests one
value and treats every other as "not that one".
core.gen.json is unchanged: the guard does not fire on the current spec, and
comments are not part of the output.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M25Q0MET4PP2SPP5GSA3W6E3
UPSTREAM.md's convention is that each workaround is "Locked in by" a test,
and the new required-field loosening had none: the existing enum test was
extended for Repo, but the other three schemas went unpinned.
RepoIndexEntry gets a dedicated test because ListRepos is the widest
consumer of the loosening — the consolidated repos index is what
resolveRepoCellTarget routes repo-scoped requests with, and what search,
repo mirror and the dispatch wizard page through — so a decode failure
there takes out cell routing and search together rather than one command.
It omits org and provider (both required upstream, neither depended on) and
carries an unknown `permission`, pinning the matching enum loosening in the
same request. Org and Project get a smaller table test for capabilities.
Verified non-vacuous: the same fixture with a still-required field removed
fails to decode ("invalid: visibility (field required)"), so ogen really
does reject an absent required field and these tests pin the loosening
rather than passing regardless.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M25Q0WSHCKMJ40JPPG4FRTCM
"Nothing is protected yet." asserts that nothing protects this repository, and it was printed for every provider that was not "github" — which made it the else of the mirror test rather than a finding of its own. `provider` is optional (an older core omits it) and open (normalize.go strips its enum, so a forge added later decodes verbatim), and in --json mode the lookup can fail outright and was swallowed. Each of those is "we could not find out", and answering it with the native sentence is exactly how an empty list would come to hide a mirror's upstream rules — an unqualified [] for a script, and a flat contradiction of the note the mirror branch exists to print. All three outcomes are now identified positively: "entire" earns the native sentence, "github" the mirror note, and everything else — absent, unrecognized, or unread because the lookup failed — a third note saying the provider could not be determined and that protection may be governed elsewhere. --json puts whichever qualification applies on stderr, with the lookup error when there was one, and still writes [] to stdout. This is the invariant `.entire` validation and WorktreeRoot already state: identify each condition positively, because "we could not find out" is not evidence that there is nothing here. add and remove keep the bare sentence. They share one PATCH that the server refuses on a mirror, so reaching their empty rendering at all means the write landed on a repo that accepts rules — noted at both call sites. Also corrects the comments claiming these read-model enums are display-only and never branched on, which Repo.provider has made false since it joined the list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M25T2DWR6K1GTFCJQ79X8NXY
Two defects, each with a regression test verified red without its fix. An `entire-graph` already in the managed directory but unreachable through PATH — a local-dev symlink whose target moved, or a managed bin dir that could not be prepended at startup — dead-ended: the user was prompted, accepted, and after three network round-trips got "already installed; use --force to replace", because the on-demand path passes no --force. The managed entry is now found before the prompt and executed, which is what installMissingPlugin's own return contract already promised. A confirmation rendered to the supplied writer even when that writer was not a terminal. `entire graph 2>log` therefore put zero bytes on the terminal while holding it in raw mode waiting for a keypress, and with Yes as the default an Enter authorized a download-and-exec nobody was shown. The opener now keeps the TTY's output handle instead of closing it, and renders there when the caller's writer is not itself a terminal. Also narrows the plugin signal exit. It fired whenever any signal had reached this process, which discarded the plugin's own exit code — Ctrl-C reaches the whole foreground process group, so a TUI plugin that handles it and exits 0 was reported as killed. It is now gated on the plugin's outcome via ExitPluginSignalled, which incidentally fixes os.Exit(-1) truncating to 255 for a signalled child. Smaller: the announcement names the command rather than only its arguments (a bare `entire graph` printed "Running plugin with command:" and stopped); `plugin upgrade` opts into the install progress its own startPluginStep calls were emitting nowhere; Browse cancellation follows its prompt to stderr; the graph name gets a constant; the progress docs move out of the managed-install -directory section; and the prompt-clearing assertion stops pinning the row count of a huh form. TestMaybeRunPlugin_MissingGraphNonInteractive gains plugin-dir isolation: consulting the managed directory before the prompt made it read the developer's real plugins, and pluginParentDir is not covered by the testdirs fallback. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M25T41HTVXGY36EF3QXAF9YP
Removed JavaScript, TypeScript, and Python languages from CodeQL analysis configuration.
Go 1.26.6 -> 1.27.1 in go.mod and mise.toml, kept aligned per the comment
there.
The golangci-lint bump is forced rather than optional: 2.11.3 refuses to lint a
module targeting a newer Go than it was built with ("the Go language version
(go1.26) used to build golangci-lint is lower than the targeted Go version
(1.27.1)"), and 2.13.0 is the first release built with go1.27, so 2.13.x is the
floor. It arrives with three linter behaviour changes, resolved here rather than
suppressed.
goconst (2.13 scans composite literals, 2457 new findings). Two settings
restore the prior scope for the 2328 that are test fixtures and map-literal
keys; a Go toolchain bump should not also adopt a stricter linter. The
remaining 129 become named constants: a new names.go for command/verb names,
shared control-plane table headers and count-dependent display nouns; a
token-report vocabulary block shared by `session tokens`, `checkpoint tokens`
and `tokens profile`; canonical activity agent IDs; explain row labels; hook
categories; recap footer hints; and per-package constants in review,
claudecode, copilotcli, benchutil, strategy and transcript/compact.
Every substitution preserves the string value, so the --json contracts and test
assertions are untouched.
goconst's "such constant already exists" hints are deliberately not followed
where they point at a same-valued constant in an unrelated domain
(agentFlagName for the `agent` command, trailReviewSeverityLow for a
token-pressure level, sourceCheckpoint for the `checkpoint` command). Two
literals stay literals for the same reason: `gh repo create` and `gh <kind>
list` argv are the GitHub CLI's tokens, not Entire's.
nolintlint (22): deleted. Newer gosec no longer reports G115 on int(f.Fd()), so
those directives were dead.
staticcheck SA1019 (8): targeted //nolint with reasons. These are deliberate
reads of our own deprecated-for-backward-compat fields, so dropping the usage
would break the compatibility they exist for.
govet (1): Go 1.27's new inline analyzer flags the deprecated reflect.Ptr
alias; now reflect.Pointer.
Verified on go1.27.1: build clean; go vet clean on darwin, linux and windows
and under the integration and e2e tags; all five lint tasks at 0 issues;
test:ci green (91 packages under -race with -tags=integration, plus both canary
legs, vogon 56/56 and roger-roger 4/4).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M25TX5G5TZ9416EZ64JGSHDF
Two review findings from the Go 1.27 bump. The documented prerequisite still said Go 1.26 in five places (README, WINDOWS, CONTRIBUTING, CLAUDE.md/AGENTS.md, docs/first-time-contributors), so anyone building with GOTOOLCHAIN=local or without toolchain-download access would fail while satisfying the docs. They now say 1.27.1 rather than 1.27: the go directive carries a patch level, so 1.27.0 is not enough either, and CONTRIBUTING names the GOTOOLCHAIN=local case since that is the setup where the failure is not self-explanatory. AGENTS.md is a symlink to CLAUDE.md and follows automatically. The goconst comment claimed both settings "restore the prior scope". That is true of ignore-map-keys and false of ignore-tests: goconst did lint test logic under 2.11, and the proof is in the same commit -- the three //nolint:goconst directives it deleted as unused were suppressing real comparison and assignment findings in _test.go files. The comment now separates the two, says plainly which shapes are no longer checked in tests, and records why the narrower alternatives were rejected: goconst settings have no per-path variant, the remainder is 2016 findings across 62 packages so a path-scoped rule would name nearly all of them, and exclude-types replaces the default exclusion set instead of adding to it -- asking it to exclude CompositeLit re-enables Call findings and takes the count from 2457 to 6255. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M25VRC179S521B9MFR27TF71
Addresses all five review findings on the trail. Commit 1 suppressed the destination, source and count fields whenever push_sessions=false, on the reasoning that they were push promises. Only their phrasing was. - The elected remote is still the checkpoint READ source: CheckpointReadRemotesWithElection never consults push_sessions, and neither does resume's fetch path. Suppressing it left status as the only surface that names the read source, naming nothing. It is kept and reworded "Checkpoints read from:". - The unpushed count is the only signal that local-only checkpoint data is accumulating, which it keeps doing because push_sessions gates pushing and not checkpoint creation. Kept, phrased "N checkpoints stored locally only". - The remote-topology note told the user to run `entire status` for the elected destination and how many checkpoints wait for it, and described where a push delivers them. Both now carry a disabled-pushing caveat instead of dead-ending. - The CheckpointPushDisabled JSON doc claimed destination, error and count were all omitted; commit 2 kept the error and the ignored-remote fields were never mentioned. Rewritten to say the flag requalifies the fields rather than removing them. - The two rationale lines added after the info literal described neither the statement above nor the block below them; they are now the PushDisabled field's own contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M26RKRK1SC7YWAXVTZKVPTMG
The disabled-pushing caveat added to the remote-topology note claimed the
destination below it was "where they are read from today". That note lists
PUSH URLs, and a fan-out remote's reads use its fetch URL instead — as the
git-branch branch a few lines down already says ("only the fetch URL is ever
reconciled"). The caveat now says only where a push would go and points at
`entire status` for the read source, which names an elected remote rather
than a URL and so is accurate there.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Entire-Checkpoint: 01M26TCAWXDEXNZM1X624SDFV9
"N checkpoints stored locally only" claimed more than the number supports. Unpushed is measured against the ELECTED destination alone — a tracking-ref comparison on git-branch, the push queue on git-refs — so it says nothing about whether those checkpoints already reached another remote. The read chain's legacy origin tier is exactly that case, and stale tracking state over-reports as well. That is the wrong direction to be wrong in: it reassures someone asking whether checkpoint data has left their machine. The line now says what the count establishes and no more — "N checkpoints not on <remote>", or "not pushed to the checkpoint remote" in dedicated mode — keeping the scoping the enabled path already had and dropping only the push promise. The same overclaim is corrected in the PushDisabled and unpushed_checkpoints field docs, and a regression assertion rejects the local-only phrasings. Also documents why the read-source line names the elected remote rather than the whole read chain: rendering the chain would restate the resolver's rule here (and drift from it) or need a second election call. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M26VBQZATYM0WHY2PTKRCXW8
Re-adding a branch without --server-side-merge-only used to send false and, because the server upserts by ref, lowered a merge-only rule. The level now travels only when the flag was given; the server (entiredb #3135) keeps an existing rule's level when it is absent and protects a new branch, so an add that only names a branch can never lower it. --server-side-merge-only=false is the explicit way down. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Reads fail OPEN where the election fails closed: strategy.CheckpointReadRemotes falls back to origin on a misconfigured checkpoint_push_remote, because failing reads closed would only prevent FINDING data. The fail-closed early return left Remote empty, so with pushing disabled status named no read source at all — in the one configuration where status is the only surface that could. Reported as ReadFallback / checkpoint_read_fallback rather than through Remote. Remote means "the elected remote" and nothing was elected here, so putting a fallback in it would misstate checkpoint_sync_remote to JSON consumers. The value comes from strategy.LeadCheckpointReadRemote rather than from reproducing the resolver's fallback rule here; it re-runs the election, which is why it is on the error path only, and it stays local-only like the rest of status. Tests: the six-topology table gains the expectation, and the phrasing of formatUnpushedCheckpointsLine moves to a pure-function table — the previous fixture-based test rebuilt a repo and mutated CWD to assert a string that depends on nothing else, while covering fewer of its branches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M26W1Z8MJHMECYXYV14BQKVF
The refresh made it required and the CLI never reads it; an older core that omits it must not fail repo routing and search. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… elected remote Follow-up review of the previous commit found the fetch-side routing incomplete in two ways, one of them a bug. A fetch-probe ERROR is not the same as a false. False means reads resolve somewhere other than the dedicated store, so the elected remote is the answer; an error means no read URL resolves at all — reachable with a configured checkpoint_remote and no remote named origin, since the dedicated path derives from origin. Falling soft to the elected remote there reported a working read source for a repo whose checkpoint reads fail. Status now says it could not determine one (checkpoint_read_source_unknown) rather than naming a remote. The "not in use" diagnostic did not route by direction along with the verdict. It votes with the push identity set, which ACCEPTS a store the fetch side declines, so in that topology a configured checkpoint_remote served no reads and nothing said so — the silent ignore the warning exists to prevent. It is now reported for reads too, without a reason: the fetch side returns a verdict and not a cause, and its false covers ownership, an unparseable origin URL and an unmappable protocol alike. Doc corrections, all cases of a comment claiming more than the code establishes: PushDisabled no longer says it changes only phrasing (with a checkpoint_remote configured it changes which probe decides, so Remote and Source can differ in value); both ReadFallback docs state both preconditions, so absence is not read as "reads fall open to nothing"; ReadsDedicatedStore no longer describes leadReadRemote as the derivation source (it is an ownership-vote identity — the dedicated path derives from origin) and no longer implies "inherited" is the only false; and the remote-topology note points at `entire status` for the read source instead of promising one it cannot always give. Tests: the destination table gains the probe-failure row and pins it through ReadSourceUnknown, so it cannot pass for another reason, and moves to keyed fields — nine positional columns with runs of "" is how an arity error got in mid-edit. The integration case for the topology note becomes a unit test: describeCheckpointDestination takes an io.Writer and a plain struct, so it needed neither a TestEnv nor a bare remote. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M26YXZWMSEYHZ6B7EBN6XFAV
A failed election does not stop reads — they fail open — so with a configured checkpoint_remote the dedicated store can still be serving them with nothing elected. The previous commit skipped the read source entirely on that path, reasoning that ReadFallback names a git remote and so had nothing true to say. That was true of the field and let it decide what STATUS says: Remote/Source already carry a dedicated sense, which is exactly the shape this needs. The fail-closed path now reports the dedicated store (Remote/Source), the fail-open candidate when the fetch side does not own it (ReadFallback), or that nothing could be determined (ReadSourceUnknown) — the same three answers the resolved path gives, because both now go through one resolveDedicatedReadSource. The asymmetry between those two paths is what produced this finding and the previous one, so it is the asymmetry that is removed rather than the symptom. The renderer needed the matching change: the error line and the read source were arms of one switch, so a warning shadowed the destination whenever both applied. The warning is now emitted independently of the read-source switch. Docs follow the behavior: Remote can hold the dedicated slug on the fail-closed path, and ReadFallback's second precondition is "the dedicated store is not what serves reads" rather than "no checkpoint_remote configured". Tests: two rows for the failed-election-with-dedicated case, owned and disowned. Expectations move from tc.name matching to explicit wantErr / wantIgnored fields — several rows now reach each, and name matching silently mis-expects every row added after it, which it did twice while writing this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M26ZXE9SXJSVMWQYH5HE9FCN
finalizeCodexObservedAtSessionEnd closed every observed turn regardless of whether refreshCodexInventory had managed to read that child's rollout — an absent or unreadable rollout, a fallback scan that timed out or breached its budget, or an extraction that never ran at all all landed in the same branch as a successful analysis. Completing the record there is unrecoverable. It sets CompletedAt, which hides the record from the completeLiveTaskRecords sweep running three lines later — whose ResolveAgentTranscriptPath attempt and analyzer pass are a genuinely different resolution path — and condensation then materializes a path-free "unavailable" reason and drops the record via removeCompletedTaskRecords, with no later hook to reconcile it. Gate the loop on entry.ResolvedTranscriptPath, which refreshCodexInventory records only after loading the rollout and matching session_meta.id to AgentID, so it is the one piece of evidence here that the child was ever read. An unresolved child's turn stays pending and its record stays live, so the sweep retries it now and each later condensation re-materializes it. The inner path-carry is now unconditional, since a skipped entry cannot reach it. Collapses the two session-end finalize tests into one table over the resolved/unresolved contrast. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27EBV17CTACGSZPVYMFH0DR
analyzeRolloutForTurns read only the last token_count snapshot, so a child's exact usage always carried APICallCount 0 and checkpoints underreported API calls whenever Codex subagents ran. Codex was the only agent doing this. The Codex PARENT path already counts the same token_count events as apiCalls (CalculateTokenUsage), and both other subagent-capable agents roll their children's counts up (claudecode/transcript.go, factoryaidroid/transcript.go). The count is user-visible through session_tokens.go, which surfaces it as APICalls and gates its "nothing to report" check on it. Count each in-scope token_count event that carries a total_token_usage snapshot, which is the same test the parent applies. A token_count with no snapshot (a rate-limit-only update) is not a model turn and does not count. The subagent_history_start_ordinal scoping already skips out-of-scope lines before this point, so it bounds the count the way fromOffset bounds the parent's. The count is attached only when exact usage survives validation: an unusable final snapshot makes the child's whole total unavailable, and a bare call count with no tokens would read as a child that burned nothing. exactUsageFromInfo splits into decodeTotalTokenUsage (shared by the count and the usage read, so both agree on what a usage-bearing event is) and exactUsageFromSnapshot (the consistency checks, unchanged). The loop now carries the decoded snapshot rather than raw bytes, so there is still exactly one decode per event. Replaces the assertion that pinned the old behaviour, and adds a case for a snapshot-free token_count. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27FSCA1DWSY16E2VB7EVM17
repo: add the branch-protection subtree
…go-dependencies-e4e38e0420 build(deps): bump the go-dependencies group across 1 directory with 3 updates
build: bump Go to 1.27.1 and golangci-lint to 2.13.2
…eckpoint-pushing fix(status): report disabled checkpoint pushing
entireio/auth-go#25, merged as f3d099d. oauthhttp.HTTPClient — the one construction point behind sts, refresh, authcode and deviceflow — now refuses a redirect that leaves the host the caller targeted, and refuses any hop that leaves https once a chain has reached it. First in the series because the next commit's regression test depends on it: without this bump TestRoundTripper_RejectsRedirectedExchangePOST fails, reporting the login JWT reaching the redirect target. This replaces the local-path `replace` directive the branch carried while entireio#25 was in review. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27SH4XARVKD62BMEGSM42YF
The existing TestRoundTripper_RejectsOffOrigin401ExchangeURL covers a hint that names a foreign host, which validateExchangeURL refuses up front. This is the case that check cannot see: an impeccable hint — same origin as the 401, https-or-loopback, path exactly /oauth/token — whose origin then answers the exchange POST with a 307 pointing elsewhere. Validation has already passed by then, so the only thing between the user's login JWT and the redirect target is the exchange client's redirect policy. net/http strips sensitive headers on a cross-host redirect but replays the body, and the body is where subject_token lives. The guard itself belongs in auth-go, but the test belongs here, because "the CLI reaches a guarded client" is a property of the CLI's wiring, not of the library: entireio#2224 added the guard to httputil.PostOAuthToken, and entireio#2235 moved this path off that helper twelve hours later without anyone noticing, precisely because entireio#2224's regression test sits in httputil and kept passing. Anchored where the CLI actually makes the call, this fails against auth-go without the fix — reporting both the leaked JWT and the attacker-issued token being presented to the core. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1KDHGDWNKKJEEA6ZFSB4Z0A
The CLI had two RFC 8693 implementations. internal/coreapi's cross-jurisdiction transport moved onto auth-go's sts in entireio#2235; this moves the last caller of the other one — the jurisdictional identity token in cmd/entire/cli/auth — and deletes it. The duplicate had drifted in ways that mattered: - Its OAuthError.Error() rendered the server-supplied response body into the terminal verbatim. auth-go sanitises the OAuth error code and description for terminal escapes; the CLI's parallel copy never picked that up. - It was the CLI's only exchange whose cross-host redirect guard lived in the CLI rather than in the library, which is how entireio#2235 dropped that guard for coreapi without any test noticing. BEHAVIOUR CHANGE, deliberate but worth reviewing: plain-HTTP handling narrows. PostOAuthToken applied no scheme gate at all, so --insecure-http-auth reached a plain-http core on any host, and a misconfigured https-less core was dialled in the clear without one. sts permits http only on loopback, even with AllowInsecureHTTP. So a non-loopback plain-http core stops working under --insecure-http-auth, and a plain-http core without the flag now fails closed instead of shipping the login JWT unencrypted. This matches what the cross-juris path already does (cross_juris_client.go gates on isLoopbackHTTP) and what auth-go chose deliberately ("Restricted explicit insecure HTTP opt-in to loopback hosts"). subject_token_type stays access_token, not JWT: that is what the form this replaces sent and what entire-core matches on. Client.Timeout does not carry over — only the Transport does. sts applies the same 30s budget via context.WithTimeout, which unlike Client.Timeout does not cancel the body read after the response returns. git-remote-entire's wrong-cluster hint now digs *sts.ExchangeError out of the error chain instead of *httputil.OAuthError; the fields it reads (Code, Description) are the same, and TestFatalMessage still passes. httputil is left holding only OAuthClientID. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1KDJ19XTTYJKW0WD3ZZGR7Y
Follow-up cleanup on the two preceding commits, from /simplify. internal/entireclient/httputil was left holding a single const plus a paragraph describing the RFC 8693 implementation that no longer lives there — the residue of a deletion that stopped one file early. Its three importers are gone: package auth already aliased the const as oauthClientID (so cell_data_api.go was the only site in the package reaching around its own alias), and internal/coreapi already imports cmd/entire/cli/auth, so exporting auth.OAuthClientID adds no package edge. The const now sits in provider.go's block beside oauthTokenPath, which is where the rest of the OAuth wiring is. exchangeJurisdictionToken takes an http.RoundTripper rather than an *http.Client. It only ever used the client's Transport, and the nil check guarding that was unreachable — every caller routes through cellExchangeHTTPClient, whose every branch returns a non-nil client. The signature now states what the function needs, so "the Timeout does not carry over" is true by construction rather than by comment. The coreapi test keeps the entireio#2224/entireio#2235 rationale, which is the load- bearing half, and loses the paragraph restating what its own name and its sibling's already say. Its third assertion went too: an attacker-issued token can only reach the core if the redirect was followed, which the first assertion already catches, so it was a second spelling of the headline claim rather than independent coverage. Its doc comment is also trimmed. Measured against the rest of the repo the rationale here had outgrown its neighbours, and the narrative it carried is already in the PR body; what stays is the part a reader of this function cannot recover elsewhere. Folded in while doing that: a note that sts narrows plain HTTP to loopback on top of AllowInsecureHTTP, so that is the effective policy regardless of --insecure-http-auth — the line otherwise reads as permitting more than it does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M1XPFQQK00625TT7SRA41VSY
…ature-flags Remove retired semantic-search flag wording
…acking Codex subagent tracking
Entire-Checkpoint: 01M27VYXMNDNPV2B2JGFW182H2
…all-on-demand Install Graph on demand with visible plugin installation progress
…ard-followup auth: restore the cross-host redirect guard on the cross-juris exchange, and de-duplicate RFC 8693
A fork PR gets a read-only GITHUB_TOKEN, so `security-events: write` is not granted and the upload fails the job rather than the analysis completing with findings in the log. codeql-actions.yml guarded this explicitly; replacing that file dropped the guard, so restore it. The condition is NOT the predecessor's, and the difference matters. That workflow ran only on pull_request, so it could compare `github.event.pull_request.head.repo.full_name` directly. This one also runs on push and schedule, where that expression is null — copied verbatim it resolves to 'never' and silently stops main from ever being uploaded, which is the one thing this workflow exists to do. Testing `github.event_name` first keeps push and schedule on 'always' and narrows the skip to fork PRs alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Entire-Checkpoint: 01M27XXVZZ4RS1QAEN095Z9H41
Update CodeQL workflow to remove unused languages
…-file-close fix: handle Close on the two writable handles that discarded it
Entire-Checkpoint: 01M2848G1CNPKDA8YJDA1MDFJA
Entire-Checkpoint: 01M28J6MX4M92Y88PWYE3WPRFK
Entire-Checkpoint: 01M2A6HV96SV0206QW63R1V23N
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
7 participants
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.
https://entire.io/gh/MuskanPaliwal/cli/trails/5
This draft pull request was opened by Entire after CI was requested for the linked trail. Feel free to edit the title or body — the link above is what keeps the trail and PR connected.