Skip to content

The family's workstation layout: init lands at <into>/<Family>/<Family>, and make siblings clones each member beside the holder without ever moving one (#76) - #79

Merged
brettheap merged 4 commits into
mainfrom
feat/family-siblings-and-init-landing
Sep 9, 2026
Merged

brettheap merged 4 commits into
mainfrom
feat/family-siblings-and-init-landing

Conversation

@brettheap

@brettheap brettheap commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Lane: xfactory-2 (xFactory-2)
Claim: #76 (comment) (2026-09-09T15:40Z, on Brett Heap's "claim 76 and start on it"), slice (A) of two
Ruling: #76 (comment) (rulings 1a, 1b and 3, and the landing rule in the body)

Governing: #76 (the family side; the scaffold side is #78 from branch feat/scaffold-family-flag). Expect a textual conflict with #78 in README.md, AGENTS.md and the size caps in tests/test_repo_hygiene.py — both slices grew those files at the same lines; whichever lands second is rebased by this lane before merge.

family.py init lands the holder at <into>/<Family>/<Family>. --into is the PARENT directory (default: where you are standing); the family FOLDER <Family>/ is made in it and the holder is cloned inside, keeping its repository name — the doubled name ruling 3 kept, with family.yaml telling the two apart. Standing in <Family>/ already lands the holder at <dir>/<Family> (no folder nested in the folder you are in); standing in one that already holds its holder is a named refusal, family-holder-already-here, because that is an add, not an init. --work-dir keeps today's meaning untouched (<work-dir>/<Family>, no folder) and is refused together with --into by name. Nothing is ever moved, and the temp-directory default is gone: the plan prints folder and landing, and the closing block prints the family folder, the holder, the add command, make siblings, and the TWO-COPIES sentence. It no longer suggests git push of the tracking branch, which AGENTS.md forbids.

make siblings, implemented ONCE (ruling 1a/1b). templates/family-root/scripts/siblings.py — standard library only, copied and digest-pinned like bootstrap.py and validate-family.py, so an existing holder is offered it by update-shape.py check as upstream-added and takes it with apply --add scripts/siblings.py (tested). For each family.yaml member the sibling is <holder>/../<Project>: ABSENT → git clone --recurse-submodules from the url .gitmodules carries (the one make bootstrap uses; repository: is the fallback), then the member's own bootstrap places its legs on their tracking branches; PRESENT → verify origin and project.yaml id, then git fetch and NOTHING ELSE — no checkout, reset or pull, and a test leaves a dirty file and a checked-out branch in a sibling to prove it; WRONG (another repository, another id, or not a clone) → reported, skipped, exit 1. Then the parent-folder check: a parent not named after the family gets a WARNING with the exact mv for the human — the script moves nothing (grep it: clone and fetch are its only writes). Idempotent, --dry-run, a layout table at the end. Entry points: the holder's make siblings, and scripts/family.py siblings --family-root <holder>, which runs that same file through runpy exactly as the root bootstrap shim runs the assembly root's bootstrap — one implementation, two doors, and it works from a checkout of the standard before the holder is cloned anywhere else.

Docs: the workstation layout in the holder README (folder, two copies, which one you work in, the doubled name, where handoffs live), the openRepoShape README's Families section and InkRouter example (now --into .., since a bare init from a checkout would otherwise land the folder inside the checkout), AGENTS.md's family section, and the holder's AGENTS-shape.md.

Tests: 23 new — nine on init's landing (default, --into, standing in the folder, the already-here refusal, --work-dir unchanged, both flags refused, non-empty target refused, dry-run creates nothing, the pin covers siblings.py), twelve on siblings (clones on branch with legs placed; second run all present; a worked-in clone untouched; wrong repository / wrong id / not-a-clone reported and skipped; parent-folder warning moves nothing; dry-run; both entry points produce the same table; relative submodule urls; differently spelled remotes), two on update-shape.py (an existing holder is owed the file; a project is not offered it). Full suite in the worktree: 647 passed, 1 skipped.

Decisions the ruling did not settle, for the reviewer: the clone url is .gitmodules's, not derived from repository:; siblings.py imports bootstrap's credential and member helpers rather than copying them (bootstrap added to the hygiene test's local-module allowlist); the parent-folder check compares against the holder's repository: basename, since name: is a display name; exit codes follow validate-family.py's 0/1/2 grammar. docs/handbook.html now lags the README's Families section and is left for a follow-up — it is a hand-designed page with no test tying it to the README.

🤖 Generated with Claude Code

Summary by Sourcery

Establish the family workstation layout and add a safe, idempotent workflow for cloning member repositories beside the family holder.

New Features:

  • Add family workstation layout initialization that places the holder inside a family-named folder and clearly reports the landing paths.
  • Add a make siblings workflow, with an equivalent family.py siblings entry point, to create and maintain member working clones beside the holder.
  • Propagate the siblings utility into family holders with digest pinning and support existing holders through shape updates.

Bug Fixes:

  • Prevent sibling setup from modifying existing working clones beyond fetching, overwriting unrelated directories, or moving checkouts.
  • Resolve relative member repository URLs across URL, filesystem, and Windows path spellings.

Enhancements:

  • Preserve the existing --work-dir behavior while making --into the explicit parent-directory option for family initialization.
  • Document the two-copy family layout, pinned versus working member repositories, refusal behavior, and non-moving workstation rules.

Documentation:

  • Update family guidance in the main README, holder README, AGENTS instructions, and holder shape guidance with the workstation layout and sibling workflow.

Tests:

  • Add coverage for family landing behavior, dry runs, refusal cases, sibling cloning and idempotence, safety around existing repositories, alternate entry points, URL handling, and shape updates.

…y>, and `make siblings` clones each member beside the holder without ever moving one (#76)

Brett Heap's RULING of 2026-09-09 on issue #76, walked through as multiple
choice: (1a) the holder's target is `make siblings`, which clones each
`family.yaml` member as a working copy beside the holder, on its tracking
branch, idempotently, warns and prints the `mv` when the parent folder is not
`<Family>`, and never moves anything; (1b) ONE implementation, TWO entry
points, no second code path; (3) the doubled `<Family>/<Family>` layout
stands. Unchanged from the body: `init` lands at `<into>/<Family>/<Family>`,
and nothing ever moves a checkout. This is slice (A), the family side; the
scaffold side (`--family` in setup-project.py) is a separate branch.

WHY. The plain folder in the original family ruling — "like a holder FOLDER
and some utilities for the family of services" (2026-09-04) — was never
written down, and existed anyway, arranged by hand on every workstation.
`init` materialized the holder in a TEMP directory unless `--work-dir` said
otherwise, so the person moved it afterwards; nothing produced the members'
working clones at all, and the first question anybody asked — "is
InkRouter/InkRouter the family, and what is the parent folder?" — had no
answer in any file.

`init` now lands where you are standing: `--into <dir>` is the PARENT
directory (the invocation directory by default), the plain folder `<Family>/`
is created in it, and the holder is cloned inside that. Standing in the
family folder already, it does not nest a second one — and when that folder
already holds its holder the run is refused by name rather than nesting a
holder inside a holder. `--work-dir` keeps exactly its old meaning
(`<work-dir>/<Family>`, no folder), because every rehearsal in this suite
passes it; the two flags together are refused. The plan and the `--dry-run`
print the landing path, and the closing block names the folder, the holder,
both next commands and which of the two copies of a member you work in.

`templates/family-root/scripts/siblings.py` is the implementation, copied
into holders and digest-pinned like `bootstrap.py` and `validate-family.py`
— which is what carries it to InkRouter/InkRouter, the holder that already
exists, through `update-shape.py check` -> `upstream-added` -> `apply --add
scripts/siblings.py`, per file, on a human's word. `make siblings` runs the
copy; `family.py siblings --family-root <path>` runs THAT SAME FILE through
`runpy`, the way the root `bootstrap` shim runs the assembly root's
bootstrap, so it works from a checkout of the standard before the holder is
anywhere else. It reads the member urls the way `make bootstrap` does — out
of `.gitmodules`, `family.yaml`'s `repository:` as the fallback — and
imports the credential resolution and the member rows from the `bootstrap.py`
beside it rather than defining either twice.

What it refuses to do is most of it. An existing clone is FETCHED and nothing
else: no checkout, no reset, no pull, no branch switch, because somebody is
working in there — and it is first verified to BE that member (origin, and
its own `project.yaml` id), with a mismatch reported and skipped rather than
written into. A directory that is not a clone at all is left alone. When the
holder's parent folder is not named after the family it WARNS and prints the
exact `mv` — including the staging name `mv A A/A` cannot be spelled without
— and moves nothing: relocating a checkout breaks an open shell, an editor
and any linked worktree, whose `.git` file carries an absolute path. The
doctor pattern, not a mover, which is the half of the ruling that keeps this
from being the thing the issue deliberately did not propose.

ONE COMMIT, not two: the init landing and the siblings target are separable
in code and not in prose. `init`'s own closing block names `make siblings`,
and the README, AGENTS.md, the holder's README and its AGENTS-shape.md each
explain the folder, the two copies of every member and the never-moves rule
as one thing. A commit whose output named a target that did not exist yet
would be a commit that has to be read with the next one open.

Docs: the holder's README gains a "Workstation layout" section (the tree, the
two copies and which one you work in, the doubled name, where handoff files
live) and a `make siblings` row; its AGENTS-shape.md now has four commands
and says what the utility never does; the README's Families section gains the
layout, the landing rule and the extended InkRouter tree; AGENTS.md's family
procedure gains the landing and the rule that the assistant does not run that
`mv` either.

Tests, all offline into bare repositories in a temporary directory: init's
default landing, `--into`, standing in the family folder, the refusal when
that folder already holds a holder, `--work-dir` unchanged, both flags
together, a non-empty landing, the dry run; and for siblings — clones beside
the holder on their tracking branch with the legs placed (through `make
siblings`), a second run all `present`, a clone somebody is working in
untouched (branch and uncommitted file), a clone of another repository
reported and skipped non-zero, a directory that is not a clone, a remote
spelled differently accepted, an id that is not the row's skipped, a relative
submodule url resolved against this remote, the parent-folder warning with
the exact `mv` and nothing moved, the dry run, and `family.py siblings`
producing the same layout table as `make siblings`. `test_family.py` asserts
the fresh holder's `shape-pin.yaml` carries the new row and that the copy is
byte-identical and executable; `test_update_shape_add.py` proves an existing
holder is offered it as `upstream-added` and a project is offered nothing,
because only the family's list names it.

Caps moved with their rationale: README 1014 -> 1059, AGENTS.md 277 -> 303.

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 9, 2026 16:06
@brettheap

Copy link
Copy Markdown
Contributor Author

@codex review

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 1 day and 10 hours by commenting @sourcery-ai review. Upgrade to get a review now.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

@sourcery-ai

sourcery-ai Bot commented Sep 9, 2026

Copy link
Copy Markdown

Reviewer's Guide

This PR establishes the family workstation layout, updates init to place holders accordingly, and adds a digest-pinned make siblings utility that safely clones absent members, fetches recognized existing clones without altering work, reports layout issues without moving files, and is available through both holder and standard-repository entry points.

Sequence diagram for creating a family workstation

sequenceDiagram
    actor User
    User->>FamilyCLI: init --org --family --into
    FamilyCLI->>FamilyCLI: _landing(args, family)
    FamilyCLI->>Git: clone holder into <into>/<Family>/<Family>
    Git-->>FamilyCLI: holder repository
    FamilyCLI-->>User: print folder, landing, add command, make siblings
    User->>Holder: make siblings
    Holder->>Siblings: run scripts/siblings.py
    Siblings->>FamilyManifest: read family.yaml members
    loop each member
        Siblings->>Git: clone --recurse-submodules to ../<Project>
        Git-->>Siblings: working sibling clone
        Siblings->>MemberBootstrap: run scripts/bootstrap.py
    end
    Siblings-->>User: layout table
Loading

Sequence diagram for safe existing sibling synchronization

sequenceDiagram
    actor User
    User->>Siblings: make siblings
    Siblings->>Sibling: inspect origin and project.yaml
    alt recognized clone
        Siblings->>Git: fetch origin
        Git-->>Siblings: remote updates
        Siblings-->>User: present, branch and worktree unchanged
    else wrong repository, id, or not a clone
        Siblings-->>User: report finding and skip
    end
    Siblings->>Siblings: check parent folder name
    alt parent name differs from family
        Siblings-->>User: warning and exact mv command
    end
    Note over Siblings,Sibling: No checkout, reset, pull, or move
Loading

File-Level Changes

Change Details Files
Changed family initialization to create the documented workstation layout and preserve the legacy work-directory override.
  • Added --into parent-directory semantics with <Family>/<Family> holder landing and special handling when already inside the family folder.
  • Removed the temporary-directory default while retaining --work-dir behavior and rejecting both landing flags together.
  • Expanded plans, completion output, and refusal messages to explain the folder, holder, add flow, siblings command, and intentional two-copy layout.
scripts/family.py
AGENTS.md
README.md
Added a single, reusable siblings implementation for cloning and maintaining working member checkouts beside the family holder.
  • Clone absent members using .gitmodules URLs with repository fallback, relative-URL resolution, credentials, recursive submodules, and each member's bootstrap.
  • Verify existing siblings by origin and project identity, then fetch only; report and skip mismatches without overwriting or moving anything.
  • Added dry-run support, idempotent layout reporting, parent-folder warnings with human-run move commands, and 0/1/2 exit behavior.
templates/family-root/scripts/siblings.py
templates/family-root/Makefile
templates/family-root/README.md
templates/family-root/AGENTS-shape.md
scripts/family.py
Shipped and digest-pinned siblings.py in generated family holders with two entry points sharing the same implementation.
  • Added the utility to family materialization's copied and executable file lists and shape-pin handling.
  • Added make siblings and a family.py siblings --family-root runpy shim that delegates to the standard's implementation.
  • Extended hygiene and update-shape behavior so existing family holders are offered the new file while projects are not.
scripts/shape_materialize.py
scripts/family.py
tests/test_repo_hygiene.py
tests/test_update_shape_add.py
Updated family documentation and regression coverage for the new layout and sibling safety guarantees.
  • Documented the plain family folder, doubled holder name, pinned versus working copies, handoff location, and non-moving behavior.
  • Added tests for initialization landing/refusals, dry runs, sibling cloning/fetch-only behavior, identity validation, URL handling, both entry points, and update-shape propagation.
README.md
AGENTS.md
templates/family-root/README.md
templates/family-root/AGENTS-shape.md
tests/test_family.py
tests/test_repo_hygiene.py
tests/test_update_shape_add.py

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

…han a literal list (SonarCloud S5332 on #79) (#76)

SonarCloud failed #79's quality gate on one finding, python:S5332 ("Using
HTTP protocol is insecure. Use HTTPS instead.") against the literal list of
schemes `same_repository`'s normaliser stripped before comparing two remote
urls. Nothing there fetches anything — the prefix is removed from BOTH sides
so that `git@github.com:Org/Repo.git` and `https://github.com/Org/Repo` are
one answer — so the finding is about the literal and not about a transport.
It is still worth taking, and not by an exemption: a list of schemes has to
be kept in step with git's transports, and this file was already carrying a
list that named five of them and would have to grow for the sixth.

One `re.sub` on `^[A-Za-z][A-Za-z0-9+.-]*://` replaces the list, which is
more general as well as quieter: `ssh://`, `git://`, `git+ssh://`, `file://`
and anything an estate's own remote helper registers all normalise the same
way now, and `file:///srv/mirrors/Repo.git` and `/srv/mirrors/Repo.git`
became one answer rather than two. The scp-like `host:owner/repo` handling,
the `.git` suffix, the case folding and the trailing `owner/repo` comparison
are untouched.

The "spelled differently" test becomes six spellings of one repository —
scp-like, https, ssh, git+ssh, `file://` over this family's own mounted bare
repository, and that same path with a trailing slash — and asserts each is
recognised as the member rather than reported as another repository. It now
runs the second pass with `--dry-run`: `place` verifies the origin and the
member's own `project.yaml` id BEFORE the dry run decides not to fetch, so
the identity is what is under test and the run reaches for no network, which
the previous single case did when it re-spelled the origin as a github.com
url and then fetched it.

110 passed (tests/test_family.py, tests/test_repo_hygiene.py).

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

There are a few concrete edge-case and URL-resolution bugs (notably in family.py _landing() and siblings.py resolve_relative() / missing-git handling) that can lead to incorrect behavior in real workstations.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR implements the “family workstation layout” workflow by (1) changing family.py init to land the holder under a plain <Family>/ folder (i.e. <into>/<Family>/<Family>), and (2) adding a single, digest-pinned make siblings utility (also invokable via family.py siblings) to place member working clones beside the holder without moving or overwriting existing checkouts.

Changes:

  • Add templates/family-root/scripts/siblings.py and wire it into the holder Makefile plus a family.py siblings entry point (one implementation, two doors).
  • Update family.py init landing semantics to remove the temp-dir default and support --into while keeping --work-dir unchanged.
  • Expand docs and tests to cover the layout rules, siblings behavior, and update-shape propagation.
File summaries
File Description
scripts/family.py Adds --into landing logic for init and a siblings subcommand that runpy-executes the pinned template script.
scripts/shape_materialize.py Pins and chmod-marks scripts/siblings.py for family holders via FAMILY_COPIED_VERBATIM/FAMILY_EXECUTABLE.
templates/family-root/scripts/siblings.py New utility to clone/fetch member working copies beside the holder and emit a layout table + warnings (no move/overwrite).
templates/family-root/Makefile Adds make siblings target invoking scripts/siblings.py.
templates/family-root/README.md Documents the workstation layout, “two copies” model, and the no-move/no-overwrite rules.
templates/family-root/AGENTS-shape.md Adds the make siblings command and reiterates safety constraints (no move, fetch-only).
README.md Adds a new “workstation layout” subsection and updates the InkRouter example commands/diagram.
AGENTS.md Updates the family procedure to include siblings and the new landing rule.
tests/test_family.py Adds extensive tests for init landing behavior and siblings idempotent, safe clone/fetch semantics.
tests/test_update_shape_add.py Extends update-shape.py --add tests to cover scripts/siblings.py being owed to existing holders only.
tests/test_repo_hygiene.py Updates local-module allowlist (bootstrap import) and bumps README/AGENTS line caps with rationale.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread scripts/family.py
Comment on lines +405 to +417
into = (args.into or Path.cwd()).expanduser().resolve()
if into.name == family:
if (into / family / MANIFEST).is_file():
raise Refusal(
"family-holder-already-here",
f"{into} is already the {family} family folder and its holder "
f"is at {into / family}, so there is nothing to init",
"Remediation: `family.py add --family-root "
f"{into / family} --member <org>/<Project>` grows it, and "
"`make siblings` in the holder puts each member's working "
"clone beside it. There is no --force.")
return into, into / family
return into / family, into / family / family
Comment thread templates/family-root/scripts/siblings.py Outdated
Comment on lines +466 to +467
try:
root = find_repo_root(args.root or Path(__file__).resolve().parents[1])
Copilot AI review requested due to automatic review settings September 9, 2026 16:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

There are two confirmed edge-case bugs (init run from inside an existing holder mis-classifies the “plain folder” landing, and sibling origin normalisation breaks Windows drive-letter paths) that should be fixed before merge.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

templates/family-root/scripts/siblings.py:188

  • same_repository() normalisation treats any ':' in the first path segment as scp-like syntax, which breaks Windows local paths like 'C:/repos/Proj.git' (drive-letter ':') and can cause valid clones to be misidentified as WRONG ORIGIN.

scripts/family.py:409

  • _landing() treats a cwd named as the family folder even when that directory is actually the holder repo itself (/, i.e. it already has family.yaml and .git). In that case it will print that the “folder” is a plain directory and may clone a second holder repo nested inside the existing checkout.
    into = (args.into or Path.cwd()).expanduser().resolve()
    if into.name == family:
        if (into / family / MANIFEST).is_file():
            raise Refusal(
                "family-holder-already-here",
  • Files reviewed: 11/11 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

…hygiene caps carry both slices' entries

A merge rather than a rebase because the organisation's ruleset refuses a force-push to any branch, and a ruleset refusal is not something to work around. The one conflict was tests/test_repo_hygiene.py's README.md and AGENTS.md size caps, which both slices raised with commentary; both entries are kept, #78's first, and the caps are the merged files' exact counts (1073 and 308). README.md and AGENTS.md merged cleanly. The tree is byte-identical to the rebased form of this branch. The squash on merge folds this commit away.

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 9, 2026 16:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The new siblings/init paths have a couple of concrete crash/safety gaps (notably unhandled “path exists but is a file” cases and missing validation for member row project names) that should be fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 3
  • Review effort level: Lite

Comment on lines +298 to +302
project = str(row.get("project") or "?")
repository = str(row.get("repository") or "")
target = root.parent / project
sibling = Sibling(project, target)
url, source = clone_url(root, row, urls)
Comment thread scripts/family.py
Comment on lines +516 to +520
(folder or work.parent).mkdir(parents=True, exist_ok=True)
if work.exists() and any(work.iterdir()):
raise Refusal(
"family-target-exists", f"{work} already exists and is not empty",
"Remediation: choose an empty --work-dir. There is no --force.")
"Remediation: choose another --into (or an empty --work-dir); if "
Comment on lines +319 to +323
present = (target / ".git").exists()
if not present and target.exists() and any(target.iterdir()):
sibling.state = "NOT A CLONE"
sibling.finding = (
f"{project}: {target} exists, is not empty and is not a git "
…parated remote too (Windows, #79) (#76)

PR #79's `windows-latest` job, at the merged head, on one test:

    FINDING IRRS: git clone exited 128:
    fatal: 'D:\a\_temp\t\family0\remotes\InkRouter.git/IRRS.git' does not
    appear to be a git repository

A `.gitmodules` url spelled `../IRRS.git` is resolved against the
SUPERPROJECT'S REMOTE, which is git's own rule and a TEXTUAL one: one
trailing component dropped per `..`. The remote on that runner is a bare
repository on disk, and `git remote get-url` hands it over as
`D:\a\_temp\t\family0\remotes\InkRouter.git` — with no forward slash anywhere
in it. The walk split on `/` alone, so the `..` consumed nothing,
`InkRouter.git` stayed, and `IRRS.git` was appended to the whole string. On
Linux the same test passes because the same path has slashes; the assumption
was invisible on the platform it was written on, which is the same shape as
the three cases `tests/test_windows_paths.py` already exists for.

THE SEPARATOR IS NOW THE BASE'S OWN, decided by what the base IS. A
FILESYSTEM PATH — no `<scheme>://`, and any colon in its first segment is a
DRIVE LETTER rather than git's `host:path` scp syntax, the same distinction
`repo_shape.SAFE_PATH_RE` draws and for the same reason — is normalised to
`/` for the walk and handed back in the spelling it arrived in. A URL keeps
what it had: a url's separator is `/` on every platform, so an scp url's
slashes must never be rewritten into backslashes on a Windows workstation.
Backslashes rather than forward slashes on the way out because git on Windows
accepts either and a human comparing the value against `git remote -v` or
`.gitmodules` should not have to translate it — and `same_repository` folds
both spellings to one answer, so identity does not depend on the choice.

The arithmetic moved into `join_relative`, a PURE STRING function over two
strings that touches no filesystem and asks nothing about this platform,
which is what lets the Windows answer be asserted from Linux — the trick
`family_landing` and `root_key` are already held to. `_one_level_up` carries
the edges: a `..` too many consumes NOTHING, so a scheme (`https:/`) and an
scp host (`git@host:`) survive it, while the parent of `/Repo.git` is the
POSIX root and a Windows DRIVE is a component of its own (`D:/Repo.git` ->
`D:` -> `D:/Other.git`). The drive is tested BEFORE the scheme, because `D:`
is also what a one-letter scheme looks like.

Eight spellings are asserted from Linux in `tests/test_windows_paths.py`,
starting with the runner's own remote verbatim: the backslash path, a
forward-slash Windows path, a UNC share, POSIX, `https://`, `file:///`,
`ssh://` and the scp form, each resolving `../IRRS.git` beside the holder's
own remote. Two more cover the path-or-url decision and the edges, and one
covers the case that is every family this standard has actually made: an
ABSOLUTE url goes through untouched, without asking git anything. The
end-to-end `test_siblings_resolves_a_relative_submodule_url` is unchanged —
it is the test that caught this, and it stays the one that runs the real
clone.

148 passed (tests/test_family.py, tests/test_windows_paths.py,
tests/test_repo_hygiene.py).

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings September 9, 2026 16:41
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

templates/family-root/scripts/siblings.py has a couple of concrete behavioral issues (misreporting missing member id: as “WRONG PROJECT”, and not actually producing the documented “no git” refusal) that should be corrected before approval.

Review details

Suppressed comments (2)

Previously missed (1) — in code that hasn't changed since the last review.

templates/family-root/scripts/siblings.py:450

  • When the member row has no id: (which validate-family.py treats as its own named finding), this check currently reports a misleading "WRONG PROJECT" mismatch against the string 'None'. It should instead detect the missing row id explicitly, report that the clone cannot be verified, and skip without implying the project.yaml id is wrong.

templates/family-root/scripts/siblings.py:556

  • The docstring says exit code 2 covers the "no git" case, but missing git currently raises an uncaught FileNotFoundError (traceback + exit 1). Add an explicit check inside the existing try so this becomes a named Refusal and consistently returns 2.
    try:
        root = find_repo_root(args.root or Path(__file__).resolve().parents[1])
        # `members_of` is `bootstrap.py`'s reading of the rows and stays the
        # only one; the manifest is then read again for the two fields the
        # LAYOUT needs and bootstrap has no use for.
  • Files reviewed: 12/12 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@brettheap
brettheap merged commit b6df51b into main Sep 9, 2026
6 checks passed
brettheap added a commit that referenced this pull request Sep 9, 2026
…se Speckit's verbs, and refuse by name when the overlay is not installed (#77) (#80)

* make park / make resume: the assembly root and the family holder expose Speckit's verbs, and refuse by name when the overlay is not installed (#77)

Slice S2 of #77, under Brett Heap's RULING of 2026-09-09 (ruling 1: Speckit
owns the mechanics and the root exposes the verbs; ruling 5: the family
holder gets them too) and the claim of 2026-09-09T17:55Z.

The assembly-root template Makefile gains `park` and `resume`, three lines
each: test the extension script, or print `make park needs the Speckit
worktree overlay; install it with: setup-openspeckit` and exit 2. `ARGS`
reaches the script, so `make park ARGS=--dry-run` is the rehearsal both
documents promise. NOTHING of park or resume is reimplemented here — the WIP
commit, the push, the `git worktree add`, the `.specify/feature.json`
rewrite and the soft reset are the Speckit git extension's, one
implementation, and a second one in a Makefile would have no tests of its
own. One `OVERLAY_MISSING` message serves both targets so the two refusals
cannot drift apart.

The family holder gains the same two verbs and runs each member's own IN THE
MEMBER'S WORKING CLONE BESIDE THE HOLDER — `scripts/siblings.py --make
<target>`, with `--make-arg` appended verbatim to that member's `make` line,
which is how `make park ARGS=--dry-run` reaches every member. NOT
`members/<Project>`: that copy is pinned, DETACHED and holds nobody's
features, so a holder that parked it would report "nothing to park" for every
member while the person's work sat untouched next door — the wrong question,
answered confidently. `siblings.py` is where the dispatch belongs because it
already knows where a sibling is and how to prove the clone there IS that
member (its `origin`, its own `project.yaml` id); that check now has ONE
definition, `verify_sibling`, used by the placement half and by the dispatch,
and the 17 existing siblings scenarios pass unedited. With `--make` it clones
nothing and fetches nothing: dispatching is all it does. A member with no
working clone is reported and SKIPPED naming `make siblings`, a member whose
verb refuses has that refusal printed where it happened, every member is
tried, and the run exits non-zero if any was skipped or refused — a `make
resume` on a fresh machine that skipped every member must never read as
success. `scripts/bootstrap.py` is untouched, so the `update-shape.py`
surface for an existing holder is `Makefile`, `AGENTS-shape.md` and
`scripts/siblings.py`.

GNU make reports ANY non-zero recipe status as its own exit 2 and prints
`Error <code>`, so the extension's 1/2/3 cannot come back out of `make`
as 1/2/3. Nothing here swallows or rewrites a code; the Makefile comment,
the new test file's header and its assertions all say so plainly rather
than pretending otherwise.

Docs: the two PINNED `AGENTS-shape.md` copies gain the verbs (park before
leaving a workstation; clone, `make bootstrap`, `make resume` on the next;
never commit under `worktrees/`; the holder's verbs run in the working clones
beside it, and a member that is skipped or refuses is reported and the rest
still run) — that is what reaches existing roots as `upstream-changed`
through `update-shape.py apply`. The templated READMEs gain a "Parking work
for another workstation" block and two rows in the holder's target table.
This repository's own README gains `## Carrying in-flight work to another
workstation`: the two verbs, who implements them, that the record is the
person's in a repository they own and named once in `~/.agents/workspace.yaml`,
that `resume` refuses on divergence, the holder's half and that it runs in
the working clones beside it, that nothing under `worktrees/` is committed or
synced, and that native Windows parks nothing
because the PowerShell mirror has neither script. `AGENTS.md` gains a short
`## Parking and resuming` procedure: the commands, the `setup-openspeckit`
prerequisite, never force-push a parked branch on a person's behalf, and
that the divergence refusal is not a thing to work around.

Tests: `test_make_targets_exist` gains `park:`/`resume:`; the new
`tests/test_park_resume_targets.py` (27 cases) covers the refusal by name in
a scaffolded root with no `.specify/`, the `ARGS` pass-through and the exit
code through an executable stub, `.PHONY`, `make help`, and a real
workstation layout — the holder at `<F>/<F>`, a working clone per member
beside it, bare repositories in a temporary directory — proving the verb runs
in both clones with `ARGS`, that `members/<Project>` (stubbed as `PINNED-*`
on purpose) is NEVER invoked, that a missing clone is skipped by name and the
next member still runs, that a member's exit 3 is reported and the rest still
run, that `--dry-run` runs nothing, and that the dispatch clones and fetches
nothing. The `README.md` and `AGENTS.md` line caps move with a dated entry
each, in the style of the two entries #78 and #79 wrote today.

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* siblings.py --make: the target and every make-arg are validated by pattern before they reach the subprocess (SonarCloud S8705 on #80) (#77)

SonarCloud's quality gate on PR #80 raised `pythonsecurity:S8705` on the
dispatch's `subprocess.run([make, target, *make_args], cwd=...)`: both values
arrive on this command's own command line and reach a subprocess's argv with
nothing between them. Nothing was injectable — the call is a LIST with no
shell, so `park; rm -rf /` would be ONE argument make has no rule for, never
a second command — but the check is cheap, it is right, and a file that runs
a subprocess should say on the same screen what constrains its argv.

Two patterns at module level, with the WHY beside them:

    MAKE_TARGET_RE     [A-Za-z][A-Za-z0-9_.-]*
    MAKE_ASSIGNMENT_RE [A-Za-z_][A-Za-z0-9_]*=[^\x00-\x1f\x7f]*

A make TARGET NAME, and a `NAME=value` make ASSIGNMENT whose value carries no
control character — the newline especially. They are narrow because the
holder only ever passes one shape of either, `make park` and `ARGS=<flags>`,
so anything else is a mistake worth refusing by name rather than a case worth
carrying.

`validated_make_call` refuses AT PARSE TIME, before anything runs, with
`make-target-invalid` or `make-arg-invalid` (exit 2) naming the pattern and
the offending value `repr()`'d and saying "Nothing was run." Not at the first
member: a bad value discovered after one member's verb had already committed
and pushed somebody's work would be a refusal that came too late to be one.
`--make ''` reaches that refusal rather than quietly becoming a placement run
— the dispatch branch now tests `is not None` instead of truthiness.

`is_a_make_call` is the one predicate both the parse-time refusal and the
call site use, and the call site restates it: a reader (or a scanner) looking
at the `subprocess.run` finds the constraint beside it instead of three
hundred lines up, and the sanitized values go into named locals immediately
before the call. That branch cannot fire, and says so.

`make_program` now resolves `$MAKE` through `shutil.which` as well as PATH,
so what reaches the subprocess is an absolute executable this file resolved
rather than a string somebody left in an environment variable; `$MAKE` set to
something unexecutable is `make-not-executable` (exit 2) instead of an
exec failure per member.

Tests: `--make` refused for `park; rm -rf /`, the empty string, `../park`,
`park park`, an embedded newline and `park$(id)`; `--make-arg` refused for an
embedded newline, `not an assignment`, `=x`, `9ARGS=x` and a bare `ARGS`;
accepted for `ARGS=--dry-run`, `ARGS=` (what `make park` with no ARGS
produces), a multi-flag value and `SPECKIT_LANE=xfactory-2`; `$MAKE` pointed
at a non-executable refused. Each refusal asserts exit 2, the code by name,
the value in the message, and that no member's stub ran. `-park` is
deliberately not in the target list: argparse refuses a value that looks like
an option before this file's check ever sees it.

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
brettheap added a commit that referenced this pull request Sep 10, 2026
…he family workstation layout, --family, park/resume and the user commands, the workspace form; a guard that the page follows the README's outline (#89) (#90)

* docs/handbook.html: the designed reading of the README at cbca5b4 — the family workstation layout, --family, park/resume and the user commands, the workspace form; a guard that the page follows the README's outline (#89)

The page was cut from README.md at `c093813` (#69, PR #71) and #88 fixed one
word in it. Everything the README gained in between was missing, so the
designed page told a newcomer less than the file it is a reading of. This is
the regeneration, at `cbca5b4`, in the page's own design and voice.

WHAT THE PAGE GAINED

- `#park`, a new section between "Keeping shape current" and "The degrade
  rule" — the whole of `## Carrying in-flight work to another workstation`
  (#80): the two root verbs and `ARGS=--dry-run`; `### park <Name>` and
  `resume <Name>`, from anywhere (#83) with the estate resolution, `--repo`,
  the fresh-machine `resume --workspace`, and what the two leave behind
  (including READ THE LINES, not the exit code); `### Who implements it, and
  what the record is` — the Speckit git extension rather than this
  repository, the person's record named once in `~/.agents/workspace.yaml`,
  `<user>-wip` with the home-org default and the opt-in `orgs:` map (#86),
  nothing under `worktrees/` ever committed, `resume` refusing on divergence,
  the family holder's dispatch into the WORKING clones, and native Windows
  parking nothing. One rail entry, `Park and resume`.
- `#families` gained two `<h3>`s: "The workstation layout: a plain folder,
  and two copies of every member" (#79) — the plain `<Family>/`, the holder
  at `<Family>/<Family>`, `--into` as the PARENT and `--work-dir` as the
  override, `make siblings`, which copy you work in, and that nothing ever
  moves a checkout — and "The InkRouter example", whose five `family.py`
  commands and two trees are the README's current ones (the block it
  replaced still named `../InkRouter` as the holder). Plus the holder's own
  `make validate` and its COPY pin.
- `#start` gained "Joining a family" (`--family <Family>`, and that standing
  in the folder needs no flag at all — AGENTS.md) and "What setup.sh actually
  runs", the seven-command flow including `mkdir -p <into>/<Family>` (#78),
  with the developer path from a checkout. The install step and the
  masthead's fastest path now say THREE commands, `unchanged` per file, and
  what `park` and `resume` are (#83). The rehearsal flag's Windows spelling
  is the README's own `py setup-project.py … --local-remote-dir .\rehearsal`
  (#70), replacing an aside that told the reader to go and read `--help`.
- `#legs` links `<user>-wip` to where such a repository lives, and gained the
  paragraph a reader of `openDox` needs: a neutral product may elect the
  shape and be its own assembly root, `also_matches` recorded rather than
  thrown away. `#layout` lists `park` and `resume`. `#agents` gained the rule
  that a refused `park` or `resume` is never worked around with a reset, a
  stash or a force-push, and that `--workspace` is nobody else's flag to
  pass.

FIDELITY. 39 code blocks on the page: 31 byte-identical to a README block
(whole, or a contiguous run of its lines), 0 re-wrapped, 5 quoted from README
prose, 2 from README inline code, 1 assembled by the page from three quoted
commands (the masthead). Every one of the 13 GitHub anchors resolves to a
real README or AGENTS.md heading, all 27 in-page hrefs resolve to an id on
the page, and the file still parses with `html.parser` with every tag
balanced: one file, inline CSS and one inline script, one Google Fonts link
with real fallback stacks, no images and no other network call.

THE GUARD. `tests/test_repo_hygiene.py::test_the_handbook_follows_the_readmes_outline`
ties the two files together, because nothing did: an explicit
`README_HEADING_TO_HANDBOOK_ID` maps every `## ` heading of README.md to the
id of the page element that carries it, so a new README section with no entry
fails NAMING THE HEADING and a heading deliberately folded into a neighbour
is a named decision; `HANDBOOK_SECTION_WITHOUT_A_README_H2` accounts for the
four page sections that are not a README `## ` at all, so a page section
cannot become unexplained. And the page names the README commit it is a
reading of, exactly once, whose README must be byte-identical to the one in
the tree — `git diff <sha> -- README.md`, against the working tree, so the
page goes stale the moment the README changes rather than one commit later.
That last check is skipped, and only that one, when the named commit is not
in the clone: a `fetch-depth: 1` CI checkout has one commit and the object is
absent there, while the outline checks are tree-only and run everywhere.

Evidence: `python3 -m pytest tests/test_repo_hygiene.py -q` 46 passed;
`python3 -m pytest tests -q` 889 passed, 1 skipped (`SHAPE_MEDXEHR_CLONE` is
unset — pre-existing and unrelated). The five ways the new guard can fail
were each exercised in a throwaway clone. Docs and one test only; no README,
script, template or workflow change, and both size caps are untouched.

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* tests.yml: fetch full history so the handbook's staleness check bites in CI (#89)

Ruled by Brett Heap on 2026-09-10: *set fetch-depth 0 so the staleness check
bites in CI.*

`test_the_handbook_follows_the_readmes_outline` compares README.md against
the commit `docs/handbook.html` says it is a reading of, which needs that
commit's OBJECT present. `actions/checkout`'s default `fetch-depth: 1`
fetches one commit, so the object was absent and that one check skipped
itself: green in CI, red only on the machine where the page would be
regenerated, which is the wrong way round. All three jobs — `tests`,
`tests-windows`, `tests-macos`, the only three `actions/checkout` steps in
the file — now check out with `fetch-depth: 0`, and each says why in a
comment naming the test.

The test's BEHAVIOUR is unchanged: the "skipped when the named commit is not
in this clone" branch stays, because a `git clone --depth 1` still hits it
and a failure about the clone is not a failure about the page. Only its
docstring moves, to say that CI is no longer such a clone.

Evidence: `python3 -m pytest tests/test_repo_hygiene.py -q` 46 passed (with
tests/test_windows_paths.py, 84 passed). The workflow parses under PyYAML and
compares equal to HEAD's parse with the three checkout `with:` blocks removed
— nothing else in the file moved. No actionlint on this machine; CI's own
actionlint step lints `templates/assembly-root/.github/workflows/validate.yml`
only, which is untouched.

Lane: xfactory-2 (xFactory-2)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
@brettheap
brettheap deleted the feat/family-siblings-and-init-landing branch September 11, 2026 20:06
brettheap pushed a commit that referenced this pull request Oct 15, 2026
…amily>` refuses a holder this user cannot search by name instead of raising `PermissionError`, and a folder a run makes that is only unwritable is told to be given write and search permission or replaced by another `--work-dir`, `--local-remote-dir` or `--into`, so neither ends in a traceback or tells a person to move aside what is not in the way (#609)

rv622r's re-verify of the L2-60 change found two things.

Problem 1. With `--into` named for the family (or the working directory
named for it), `Orbit/Orbit` is the holder, and `_landing` asked
`(into / family / MANIFEST).is_file()` of it. For a holder at mode 600, with
no search bit, `Path.is_file()` raised `PermissionError` (3.12 ignores only
ENOENT, ENOTDIR, EBADF and ELOOP), so both runs exited 1 with a traceback
before `_refuse_a_taken_holder` could name the folder; with the search bit
(mode 555) it was already refused by name. It is the same on main 396c0ff
(#79). `_landing` now asks `os.path.isfile`, which answers False, as
`_standard_checkout` beside it already does. Every other `.is_file()` or
`.exists()` on that path is already guarded: the folder and tree questions
use `os.path.lexists` and `stat`/`iterdir` inside `try`, and the two
`bare.exists()` calls (`_local_remote_is_empty` in both tools) come after
the `--local-remote-dir` question, which refuses a folder with no `w` or no
`x` first (probed at modes 600 and 000, the folder itself and a path below
it).

Problem 2. `FOLDER_REMEDIATION` says "Move what stands in the way aside",
which is wrong for an existing folder (`--work-dir`, `--local-remote-dir`,
the family folder, or one above it) whose only fault is its permissions.
`refuse_unmakeable_folder` now takes the flag to name (`--into` for the
family folder, which `named` spells "the family folder") and chooses by what
`folder_cannot_be_made` said: for `CANNOT_WRITE_IN` it says what the trees'
`target_remediation` says, from one `_unwritable_remediation`; every other
answer keeps `FOLDER_REMEDIATION` exactly. `family.py` names the flag by
`_landing_flag`, also used for the holder's refusal.

tests/test_dry_run_parity.py: the `Orbit/Orbit` layout at modes 555 and 600,
both runs; one row per flag (scaffold `--work-dir` and `--local-remote-dir`,
`init` `--into`, `--work-dir` and `--local-remote-dir`), both runs; the
ancestor case; eight files-in-the-way cases that keep the old sentence; the
chooser.

Lane: openreposhape-2 (openRepoShape-2)
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants