Repository navigation
family.yaml values are read the way openRepoShape v1.1.0 reads them, status names one family laid out twice, and resume finds a holder by its id when its record names another folder (#243) - #251
Conversation
…status names one family laid out twice, and resume finds a holder by its id when its record names another folder (#243) 1. Value shapes. The shared `trim_unquote` reads a scalar as v1.1.0's `parse_yaml` does: a quoted value is the text up to its closing quote (a `\` or a `''` does not close it) and nothing after the close is the value; an unquoted one is cut at a `#` that opens it or follows a space or tab. `yaml_scalar` sets `YAML_QUOTED`, and the repository arm of `is_family_folder`, now `named_for_repository`, reads an UNQUOTED `null`, `Null`, `NULL` or `~` as no repository. 2. Two layouts of one family are not de-duplicated. AGENTS.md says that parking both writes one record key, the family id, and to keep the folder named for the repository; `status` reports a finding on the holder's row when two family folders under the projects directory carry the same `family.yaml` `id:`, naming both and the one to keep. 3. `resume` finds the holder by `family.yaml` `id:` against the record's `family:` when the name typed and the record's folder both miss: one match is the estate (no clone), two or more are refused by name and nothing is cloned, none is the clone it always was. 4. Tests for the cwd walk-up of a repository-named family, and `named_apart` files only the record park writes, `quill.yaml`. The shared block gains `named_for_repository` and `family_folders_with_id` and stays byte-identical in park, resume and status. `leg_branch_of`'s comment names everyone who asks it, park's not-carried report (#242, PR #249) included; its SC2329 disable stays until #249 lands. AGENTS.md's cap moves 323 -> 330 with a dated entry. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Sorry @brettheap, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 2 days and 5 hours by commenting |
Reviewer's GuideThis PR synchronizes the shared shell YAML readers with openRepoShape v1.1.0, preserves and diagnoses families present in both supported folder layouts, and teaches Sequence diagram for resume finding a stale family by idsequenceDiagram
participant User
participant Resume as resume
participant Projects as Projects directory
participant Holder as Family holder
participant Clone as git clone
User->>Resume: resume NAME
Resume->>Projects: estate_by_name(NAME)
Projects-->>Resume: no estate
Resume->>Projects: find family folders by family.yaml id
alt exactly one holder
Projects-->>Resume: holder folder and repository
Resume->>Resume: is_safe_folder_name
Resume-->>User: found by id, no clone planned
else more than one holder
Projects-->>Resume: multiple folders
Resume-->>User: REFUSED with folders and folder to keep
else no holder
Projects-->>Resume: no matching folder
Resume->>Clone: clone repository into record root
Clone-->>Resume: clone result
end
Flow diagram for duplicate family layouts in statusflowchart TD
A[status scans projects directory] --> B[family_folders_with_id]
B --> C{Same family id in multiple folders?}
C -- No --> D[Continue normal status]
C -- Yes --> E[Keep repository-named folder]
E --> F[Report read-only FINDING naming every folder]
F --> G[Move nothing]
Flow diagram for YAML scalar parsing and family discoveryflowchart LR
A[family.yaml scalar] --> B{Value starts with quote?}
B -- Yes --> C[Read through matching quote; preserve escapes]
B -- No --> D[Cut at comment marker after whitespace]
C --> E[trim_unquote and yaml_scalar]
D --> E
E --> F{repository is unquoted null or ~?}
F -- Yes --> G[Fall back to name]
F -- No --> H[Use repository value]
G --> I[is_family_folder]
H --> I
I --> J[Discover family estate]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Independent review of #251 at Tested base: the merged tree I found no defect that should stop the landing. One LOW defect is confirmed with a repro and a verified fix (finding 1): take it in this PR or file it. The rest is INFO or NIT. What I verifiedScratch homes and basetemps under a private review directory. Every
Findings1. LOW, confirmed. Two spellings of ONE folder are counted as two layouts. Repro (scratch home, the
It needs a deliberate link between the two spellings, so it is rare, but the false finding then repeats on every Fix, in the shared block (all three copies, byte-identical): family_folders_with_id() {
local want="$1" dir child prior dup found=()
[ -n "$want" ] || return 0
for dir in "${PROJECTS_DIRS[@]}"; do
[ -d "$dir" ] || continue
for child in "$dir"/*; do
[ -d "$child" ] || continue
is_family_folder "$child" || continue
yaml_scalar "$child/${child##*/}/family.yaml" id || continue
[ "$YAML_VALUE" = "$want" ] || continue
dup=0
for prior in ${found[@]+"${found[@]}"}; do
if [ "$child" -ef "$prior" ]; then dup=1; fi
done
[ "$dup" -eq 0 ] || continue
found+=("$child")
printf '%s\n' "$child"
done
done
}Verified on a scratch copy of the merged tree with that change in all three scripts: 2. INFO, confirmed, by design. A bare 3. INFO, not reproduced. The id match ignores the record's org. 4. INFO, unmeasured. Cost of the new 5. NIT, not run on macOS. 🤖 Generated with Claude Code |
…f the projects directory reach it, and the second-spelling test skips where ~/Projects is ~/projects (#243, review of #251) The review of #251, finding 1: `PROJECTS_DIRS` is deduped by inode, but the folders inside two distinct projects directories were not compared, so a real `~/Projects` holding a link `Quill -> ~/projects/Quill` listed one holder twice. `resume quill` refused "2 family folders" and `status Quill` reported the family "laid out 2 times", with a remedy that would have taken the link away. A candidate that is `-ef` one already listed is now skipped (the reviewer's verified edit, with `local -a found=()` as the block's neighbours spell it), in the shared estate resolver, byte-identical in park, resume and status. Two real holders in two directories still refuse. The review's NIT: `second_projects_spelling` makes `~/Projects` beside the fixture's `~/projects`, and SKIPS, saying why, where the two names are one directory (a case-insensitive filesystem, a stock Mac) instead of failing on the `mkdir`. The two-holder refusal's `under-the-other-spelling` row and the new symlink test both use it; Linux runs both. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Delta re-check of #251, My finding 1 (a symlinked second spelling counted as two layouts) and the macOS NIT are both taken, and I found nothing new. Tested base: the merged tree What I checked
No new findings. The INFO items in my earlier comment stand unchanged (the bare- 🤖 Generated with Claude Code |
…wups-243 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Fixes #243, the four loose ends the independent review of #240 (review 5561239588) named. Brett Heap's word (2026-10-18) asked lane 1 to take both follow-up issues of the #238 and #240 reviews. #242 is a separate PR (#249), which touches only
park'sreport_repos; this one owns the shared estate resolver this round. That block stays byte-identical inpark,resumeandstatus(one md5 across the three; the byte-identity tests pass).1. Value shapes:
trim_unquotereads a scalar the way openRepoShape v1.1.0 doesDecision: fix it in the SHARED
trim_unquote, not inis_family_folderalone. A cut there would have left the quotes on the quoted-with-comment form.What changed.
trim_unquotenow follows v1.1.0'sparse_yamlat190133d(scripts/repo_shape.py:_strip_comment,_quote_end,_quoted_scalar; opensoft/openRepoShape#629):"…"a\escape does not close it, and inside'…'a doubled''does not. Anything after the close, a# commentincluded, is not the value, so a#inside quotes is text.#that opens it or follows a space or tab, then trimmed.a#bstaysa#b.yaml_scalarnow also setsYAML_QUOTED. The repository arm ofis_family_foldermoved into a new sharednamed_for_repository. It treats an UNQUOTEDnull,Null,NULLor~as no repository (v1.1.0's_scalarspellings), so the folder falls back toname:. A quoted"null"is still the text.family.yamllinerepository: testorg/Quill # the holderinQuillQuillrepository: "testorg/Quill" # the holderinQuillQuillrepository: nullinnull(nameOther)repository: "null"innullnullrepository: "testorg/Quill #2"inQuillQuill #2#stays literal)Caller audit. These are every caller of
trim_unquoteandyaml_scalarin the three scripts. "Changes" means a value with a trailing comment, or a quoted value followed by more text, now reads differently.Shared block (all three):
kind:(is_pinned_member,is_family_folder,is_project_root) andname:(is_family_folder): change. A trailing comment no longer keeps a holder or root from being recognised. That is right because v1.1.0 drops the comment andfamily.pynever writes one.repository:(named_for_repository): changes, per the table above.id:(family_folders_with_id, new): comment cut.park.sh'sworkspace_read_scalarcuts at any#, so the id park files under never carries one.tracking_branch:(tracking_branch_of, fromfamily.yamlandproject.yaml): changes.main # xis nowmain, which is whatbootstrap.pyandsiblings.pyread: both userepo_shape.load_yaml.- project:(family_members): changes.Alpha # xis now the memberAlpha, assiblings.pyreads it.role:andpath:(legs_of,leg_branch_of): change. A commentedpath: spec # the spec legis nowspec, asbootstrap.pyreads it.branch:(leg_branch_of): unchanged. Its own${branch%%[[:space:]]#*}cut runs first and is kept, and its 22-case table passes untouched.resumeonly:workspace.yaml'srepository:andpath:(yaml_scalar), andorgs:with itspath:(config_org_paths): change. A trailing comment is cut. This now agrees with the extension'sworkspace_read_scalar; before, it read the comment into the path.family:(yaml_scalar) and itsroot:andrepository:(manifest_project_field): change only for a commented value. Park writes none, and the extension's loader cuts at every#.repository:on therecordsline: comment cut. A holder with an unquotedrepository: nullwould still printnullthere. That line is print-only, the brief scopes the null rule tois_family_folder, andfamily.pyalways writes a repository. Noted, not changed.statusonly:commit:andsource_repository:(yaml_scalar),shape:'scommit:(nested_scalar), workspacepath:(workspace_checkouts) and the estateid:(locate_record): comment cut.update-shape.pyandparkwrite none.shape_pin_rows(path:,sha256:): unchanged except a tab before a comment, which is now cut. Its own#re-cut aftertrim_unquoteis now redundant for an unquoted path. For a quoted path holding#it still cuts, as before, andupdate-shape.pywrites no such path. Left as it was.record_scalarandread_root_shape: these cut at EVERY#first, which is the extension loader's rule, sotrim_unquotenever sees one. The only difference is text after a closing quote ("a" bis nowa), which is not YAML and which park never writes.config_scalardoes not usetrim_unquote, and is unchanged.2. Two layouts of one family: said, not de-duplicated
Decision: do NOT de-duplicate estates.
park <Name>, …".Quill/QuillandQuillWorks/QuillWorksare both found, as two estates. Parking both writes ONE record key, the family'sid:, so the later park replaces the first one's member blocks. Keep only the folder named for the repository, which is openRepoShape v1.1.0's rule and wheremake siblingsmoves a holder (make siblingsandfamily.py siblings: clone each member on its tracking branch, warn before cloning outside a<Family>folder, and run the holder's pinned siblings.py openRepoShape#543).park.shsetsMANIFEST_NAME="$WORKSPACE_FAMILY"(thefamily.yamlid:), andworkspace_write_manifestmerges by each project's- id:. A member's block is therefore replaced whichever layout parks it.test_agents_md_is_short_enough_to_be_read. The assembly root's 347 does not move.assembly-0001already fails on main's AGENTS.md (hunks 6 and 8), and applies or fails hunk for hunk the same with these lines (checked withpatch --dry-run), so the re-cut sets that cap.tests/test_test_roots.pyholds 347 until then.statusreports a read-only FINDING on the holder's row when two family folders under the projects directory carry the samefamily.yamlid:. It names every folder of that id and the one to keep, the folder named for its repository, and moves nothing. This uses a new sharedfamily_folders_with_id.3. A stale record found by id (
resume)When
estate_by_name "$NAME"and the record-folder lookup both miss,resumenow looks for family folders under the projects directory, by either naming rule, whosefamily.yamlid:equals the record'sfamily:.recordsline names the holder's own repository, andpresentnames its folder, with one line saying it was found by id and that the record'sroot:names another folder until the next park.A folder reached under both spellings of the projects directory counts ONCE. For example, a real
~/Projectsmay hold a linkQuill -> ~/projects/Quill. A candidate that is-efone already listed is skipped, and the first spelling, inPROJECTS_DIRSorder, is the one named. Soresumetakes that holder, andstatus's two-layout finding (item 2b, which reads the same list) has nothing to report. Before this, the review of #251 (finding 1) showedresume quillrefusing "2 family folders" andstatus Quillreporting the family "laid out 2 times" for one folder. Two real holders in two directories still refuse.The existing
is_safe_folder_namechecks on<Name>and the record'sroot:still run before any path is built. The holder found by id is held to it too, and refused by name if its folder could not be an estate name.4. Test gaps
Quill/Quill/scripts,Quill/QuillandQuill,park --dry-runanswers the holder; fromQuill/Alphait answers the member.named_apartnow files onlyquill.yaml, the record park writes. Thequillworks.yamlpark never writes forid: quillis gone.test_resume_names_the_repository_the_holders_family_yaml_givesnow asksresume quillagainstquill.yaml(root: Quill/Alpha) with the holder inQuillWorks/QuillWorks. This is item 3 in the other direction: before,resumeplanned a clone oftestorg/QuillintoQuill/Quill, beside the holder.resume QuillWorks --dry-runnow finds no record and refreshes the estate that is there.Red and green
All runs below were under the shared pytest lock, with
/usr/bin/python3 -m pytest, everyLANES_*andTMUX*variable unset, and a privateHOME,TMPDIR,--basetempandPYTHONPYCACHEPREFIX.At
a6156dc. New and changed tests, plus everynamed_apartuser (47 tests):park,resumeandstatus: 20 failed, 27 passed.trim_unquotetable; the on-disk test (a clone ofQuill/Quillwas planned); the found-by-id test (a clone ofQuillWorks/QuillWorkswas planned); both two-holder refusals and the unsafe-name refusal (main ranmkdirand thengit clone, exit 128); and the status two-layout finding (exit 0, no finding)."null", the literal#), the 11 shapes main already read right, the 4 walk-up cases, the clone plan for an absent family, and the 9named_apartusers.Whole modules at
a6156dc:tests/test_park_resume_commands.py: 217 passed (181 on main plus 36 new).tests/test_status_command.py: 210 passed (209 plus 1).tests/test_repo_hygiene.py: 189 passed, the byte-identity tests included.tests/test_test_roots.py::test_the_length_caps_are_the_assembly_roots_where_one_is_named: passes.Fix round for the review of #251, at
39b8a5d. This adds one test:test_one_holder_reached_under_both_spellings_is_one_holder_not_two, next to the two-holder refusal.tests/test_park_resume_commands.pynow holds 218 tests and the new and changed set 48.a6156dc's scripts: the new test fails (status Quillexit 1, "familyquillis laid out 2 times"). Its two refusal neighbours pass:1 failed, 2 passed.test_the_three_commands_carry_the_same_resolver_byte_for_byte: 10 passed, 0 skipped.tests/test_status_command.py: 210 passed.tests/test_repo_hygiene.py: 189 passed.second_projects_spelling, makes~/Projectsbeside the fixture's~/projects. Where the two names are one directory (a case-insensitive filesystem, such as a stock Mac), it SKIPS with a stated reason instead of failing on themkdir. Theunder-the-other-spellingrow and the new test both use it, and Linux runs both. The skip was shown with~/Projectslinked to~/projects, the samesamefileanswer a case-folding filesystem gives; no macOS run was made.Static checks:
bash -nandshellcheck -s bashare clean on all three scripts.${x,,},mapfile,declare -A,local -n,awk -v,[[or<<<in the added lines. The new code uses${x:off:len},$(( )),caseandwhile, all of which bash 3.2 has.leg_branch_of's comment in the shared block now names everyone who asks it, includingpark's not-carried report (#242, PR #249). Its# shellcheck disable=SC2329stays because #249 has not landed: without it,parkdraws exactly one SC2329 on this base.🤖 Generated with Claude Code
Summary by Sourcery
Align estate resolution with openRepoShape v1.1.0 and safely handle duplicate or stale family layouts across park, resume, and status.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests: