Repository navigation
session: the standing isolation test compares the canonical repository root - #1703
Closed
ZeroPoint95 wants to merge 2 commits into
Closed
ZeroPoint95 wants to merge 2 commits into
ZeroPoint95 wants to merge 2 commits into
Conversation
…y root TestStandingIsolationRecordsItsCopyBeforeWorkerInitialization compared the recorded Root, which repositoryRoot canonicalizes, against the raw t.TempDir(). On macOS those spell /private/var/folders and /var/folders for the same directory, so a correct record failed. The test now compares against canonicalPath(repo), as its siblings already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ZeroPoint95
marked this pull request as draft
October 1, 2026 01:16
ZeroPoint95
marked this pull request as ready for review
October 1, 2026 02:09
ZeroPoint95
marked this pull request as draft
October 1, 2026 14:02
Collaborator
Author
|
Superseded by #1722, which carries this commit and fixes the other nine macOS-only reds in the same change. |
AbirAbbas
added a commit
that referenced
this pull request
Oct 2, 2026
… marshals itself (#1722) * changes: entry for #1703 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * session: the standing isolation test compares the canonical repository root TestStandingIsolationRecordsItsCopyBeforeWorkerInitialization compared the recorded Root, which repositoryRoot canonicalizes, against the raw t.TempDir(). On macOS those spell /private/var/folders and /var/folders for the same directory, so a correct record failed. The test now compares against canonicalPath(repo), as its siblings already do. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * tests: the suite is green on macOS, and the allocation law prices its marshals itself Three back-to-back `make pr-ready BASE=origin/dev~5` runs on a clean dev (df7022d) failed with the same ten tests every time on a Mac, while CI on ubuntu was green for the same commit. None of them flaked; each was a Linux assumption in a test. Five internal/session tests hold a task under a 1 TiB memory floor, and the governor reads /proc/meminfo, which a Mac does not have — an unknown reading admits everything, so the hold never came. They now go through requireHostMemoryReading, the skip two sibling tests already carried inline. Four cmd/codeaf engine tests compared the envelope's files against a raw t.TempDir(), which macOS spells through the /var -> /private/var link while the engine records the canonical root. beltRepoWorkspace and the unborn-branch test resolve the folder, as team_launch_test and update_test already do. The warm breakpoints encode law said 8 allocations; that figure belonged to encoding/json on Go 1.26 and is 11 on Go 1.27 with the memo unchanged. The test now prices the two marked marshals in-process and demands the result slice and nothing more on top of them, which is the actual promise. PERF.md says so. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * changes: entry for #1722 Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * changes: the entry's surface is engine and its claims end in full stops Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * session: the held-run tests state their machine and run on every host Skipping the four held-run tests on a host without /proc/meminfo meant nobody on a Mac ever ran them, and a regression in holding, reopening on the same admission, keeping the accepted crew or reading settings before creation would show up only on CI. hostReading was the one place both /proc files are read, so the governors are now built over a package-level readHost, and littleMemoryHost states a quiet machine with one gibibyte available for the rest of a test — the 1 TiB floor holds everywhere, deterministically, and the seam is restored when the test ends. requireHostMemoryReading stays for the one test whose claim is about this host's own reading. With the hold finally reachable on a Mac, the reopen test met the same /var -> /private/var mismatch the others did on its recorded ground, and compares against canonicalPath now. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> * tests: the warm encode law prices encoding/json, not marshalMarked The breakpoints law measured marshalMarked itself and allowed the result slice on top, so an allocation added inside marshalMarked raised its own allowance and passed: a defensive strings.Clone of the tool content failed the old constant (9 against 8) and passed the new form. The law now prices only the encoding/json calls the two marked positions are designed to make, built from the package's own wire types with their inputs prebuilt, names this package's own allocations (the result slice, the parts slice of the marked system message, nothing for a tool result), and checks each marked position before the whole warm path. It stays toolchain-independent: 8 on Go 1.26 and 11 on Go 1.27, both measured. The equality between 8 and 81 turns is unchanged. PERF.md and the change entry now say exactly that, call the allocation failure a toolchain assumption rather than a Linux one, and give the held runs' memory floor in MiB. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * tests: the held-run memory floor is an exbibyte, not a tebibyte The littleMemoryHost comment called the 1 << 40 floor 1 TiB, but TaskMinFreeMB is in MiB, so the floor is 1 EiB. Only the comment changes. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * session: the held belt stop test reads the run under its lock TestHeldBeltRunStopsWithoutPreparingRepository polled agent.beltRun without agent.beltMu while releaseBeltRun clears it under that lock, so -race reported a data race in about one run in three (on dev as well; CI does not run -race). The poll now takes the lock, as the test's other read already does; -race -count=10 is clean. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * tests: the warm encode law refuses a wire type with its own marshaler The law prices encoding/json by marshalling the package's own wire types, so a MarshalJSON added to one of them would put this package's allocations inside the measured price and raise its own allowance: a defensive copy in such a method took the warm encode from 8 to 11 with identical wire bytes, and the law passed. The price now refuses json.Marshaler and encoding.TextMarshaler on markedTextPart, markedMessage and markedToolMessage, as values and as pointers, before measuring; a marshaler added on purpose has to be priced as named overhead here and in PERF.md together. The per-position failure now says which way the count moved, and names what to update together when the change is deliberate. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Abir Abbas <abirabbas1998@gmail.com>
santoshkumarradha
added
the
area:session
santoshkumarradha
pushed a commit
that referenced
this pull request
Oct 24, 2026
Co-Authored-By: Claude Opus 5.5 <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
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.
TestStandingIsolationRecordsItsCopyBeforeWorkerInitializationfailed on a cleandevon macOS withlost recovery identity.Cause. The test compared the recorded
Rootwith the rawt.TempDir().repositoryRoot(internal/session/task_run.go) deliberately canonicalizes the root so every later path and repository lock share one spelling, and on macOSt.TempDir()answers under/var/folderswhile its canonical spelling is/private/var/folders. A correct record failed the comparison. No production code compares a stored root with an unresolved folder; everyRootcomes fromrepositoryRoot.Fix. The test compares against
canonicalPath(repo), as its siblings already do (placescontext_test.go,delegate_stop_test.go).Proof. Failed on an untouched
origin/dev; passes-count=3;make test-focus PKGS=./internal/session RUN='^TestStandingIsolation|^TestStandingBranchIsolation'is green. Not added to the known-red ledger.🤖 Generated with Claude Code