feat(shelf): T1 ERGON — 48 products on the shelf, class P shelf-parity live - #1670
Conversation
…parity green The library's third shelf is populated: 48 register-marked products transferred organvm → organvm-iii-ergon (redirects live; both private products included). Books: shelf_assignments block (bare names), 21 row re-keys + 27 new judgment rows (class carried from census — placement changed, posture never), orgs split shelves/reserved (repos: shelf_derived replaces the 0-ceiling; a-organvm stays the venture reserve). Engine: class P shelf-parity rung (declared == census owner, both directions), owners() enumerates declared shelf orgs, census reads non-canonical owners user-scoped — the App-token blindness hid shelf privates and personal rolls; collaborator census now completes (complete=true, all four partner lanes read write). Live doctor: P clean 48/48, N+O real verdicts, zero custody drift. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VHjMkFUZJ1pK21oyv1ropN
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Multi-agent review roll call (CodeRabbit and Claude review automatically. Reviewers: post substantive findings only. Authors/agents: address every thread, push fixes to this branch, reply and resolve, then re-request review.) |
|
To use Codex here, create a Codex account and connect to github. |
📝 WalkthroughWalkthroughAdds ERGON shelf declarations and repository overrides, routes shelf and personal-owner census queries through user-scoped GitHub access, and introduces shelf parity drift detection in ChangesShelf parity
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant GitHub as GitHub API
participant Observe as observe()
participant Doctor as doctor()
participant Drift as shelf_drift()
Observe->>GitHub: Fetch owner repositories and collaborators
GitHub-->>Observe: Return census facts
Doctor->>Drift: Compare shelf assignments with census facts
Drift-->>Doctor: Return shelf parity drift entries
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/gitvs.py`:
- Around line 984-987: Update the later /orgs/{org}/outside_collaborators
request in the collaborator-roll flow to select authentication by organization
class: use _gh for canonical organizations and _gh_user for non-canonical
organizations, matching the _roll helper’s behavior. Ensure shelf-org
collaborator rolls remain readable instead of being skipped.
- Around line 1659-1663: The ownership validation around owners_by_name must
require exact parity with the declared shelf: flag any repo where owners_
differs from the singleton set {org}, including duplicate ownership by another
organization. Preserve the absent-owner message where appropriate, update the
drift message for non-exact ownership, and add a regression case covering a repo
owned by both org and another owner.
- Around line 1925-1935: Update the control flow in the surrounding
doctor/parity execution path so the P shelf-parity check runs before either
early return, including under --offline --parity-only. Gate shelf_drift using
persisted census evidence that is explicitly known complete, skipping P for
absent or partial census data rather than treating missing facts as drift;
preserve fail-open, idempotent, offline-safe behavior and add regressions for
parity-only reachability and partial-census skipping.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 13db66de-4eea-47fc-9e4e-51bb5ac10df9
📒 Files selected for processing (3)
cli/tests/test_gitvs.pyinstitutio/github/estate.yamlscripts/gitvs.py
| def _roll(repo: str, path: str, jq: str) -> subprocess.CompletedProcess: | ||
| # Personal-estate rolls are invisible to the org-installed App identity (dual-estate | ||
| # custody: partner lanes live on the personal account) — read them user-scoped. | ||
| if repo.split("/", 1)[0] not in org_set: | ||
| return _gh_user(["api", path, "--jq", jq], timeout=30) | ||
| return _gh(["api", path, "--jq", jq], token, timeout=30) | ||
| if _owner_org_class(repo) == "canonical": | ||
| return _gh(["api", path, "--jq", jq], token, timeout=30) | ||
| return _gh_user(["api", path, "--jq", jq], timeout=30) |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Route organization collaborator rolls through user-scoped auth too.
_roll() fixes repository endpoints, but the later /orgs/{org}/outside_collaborators call still always uses _gh. Shelf-org collaborator rolls therefore become unreadable and class N skips them. Select _gh_user for non-canonical orgs there as well.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/gitvs.py` around lines 984 - 987, Update the later
/orgs/{org}/outside_collaborators request in the collaborator-roll flow to
select authentication by organization class: use _gh for canonical organizations
and _gh_user for non-canonical organizations, matching the _roll helper’s
behavior. Ensure shelf-org collaborator rolls remain readable instead of being
skipped.
| owners_ = owners_by_name.get(n) or set() | ||
| if not owners_: | ||
| out.append(f"{org}/{n}: declared on the shelf but absent from the census") | ||
| elif org not in owners_: | ||
| out.append(f"{n}: declared shelf {org}, census owner {'/'.join(sorted(owners_))} — transfer owed") |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Flag duplicate ownership, not only an absent expected owner.
A repo present in both organvm-iii-ergon and another owner passes because org in owners_. Shelf parity requires exact ownership equality, so treat owners_ != {org} as drift and add a duplicate-owner regression case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/gitvs.py` around lines 1659 - 1663, The ownership validation around
owners_by_name must require exact parity with the declared shelf: flag any repo
where owners_ differs from the singleton set {org}, including duplicate
ownership by another organization. Preserve the absent-owner message where
appropriate, update the drift message for non-exact ownership, and add a
regression case covering a repo owned by both org and another owner.
| # P — shelf parity (custody v4.0.0 Phase 2): declared shelf membership vs census owner, | ||
| # both directions. Real drift is RED with the exact remediation named; the rung never | ||
| # fires a transfer itself. | ||
| shelves_reg = ((estate.get("shelf_assignments") or {}).get("shelves")) or {} | ||
| if shelves_reg: | ||
| if not rows: | ||
| skips.append("[P shelf-parity] no census facts (run census online first)") | ||
| else: | ||
| for d in shelf_drift(shelves_reg, rows): | ||
| fails.append(f"[P shelf-parity] {d}") | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Make class P reachable and require complete census evidence.
The configured parity gate runs doctor --offline --parity-only, but both early returns occur before this block. Also, partial census facts are evaluated as drift because only an empty rows skips P. Run P before those exits using known-complete persisted census evidence; otherwise skip it and add parity-only/partial-census regressions. As per path instructions, fleet scripts must be fail-open, idempotent, and offline-safe.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@scripts/gitvs.py` around lines 1925 - 1935, Update the control flow in the
surrounding doctor/parity execution path so the P shelf-parity check runs before
either early return, including under --offline --parity-only. Gate shelf_drift
using persisted census evidence that is explicitly known complete, skipping P
for absent or partial census data rather than treating missing facts as drift;
preserve fail-open, idempotent, offline-safe behavior and add regressions for
parity-only reachability and partial-census skipping.
Source: Path instructions
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48e3246522
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| timeout=180, | ||
| ) | ||
| args = ["api", route, "--paginate", "-X", "GET", "-F", "per_page=100", "--jq", jq] | ||
| r = _gh_user(args, timeout=180) if user_scoped else _gh(args, token, timeout=180) |
There was a problem hiding this comment.
When owner is the personal account and user_scoped=True, the org route fails and this still falls back to /users/{owner}/repos, which lists only that user's public repositories. Switching to _gh_user does not change the endpoint's visibility semantics, so private personal repositories remain absent while _owner_repos returns a successful, supposedly complete list; use /user/repos with owner affiliation or an equivalent authenticated GraphQL query for this case.
Useful? React with 👍 / 👎.
| if shelves_reg: | ||
| if not rows: | ||
| skips.append("[P shelf-parity] no census facts (run census online first)") | ||
| else: | ||
| for d in shelf_drift(shelves_reg, rows): |
There was a problem hiding this comment.
When any owner read fails, observe() records led["repos"]["complete"] = False but may still write partial facts—or leave old facts when every read fails—and this condition runs shelf_drift anyway. For example, if native authentication cannot enumerate the new shelf org while the canonical App-token read succeeds, all 48 shelf entries are reported as custody drift rather than an unavailable sensor; gate class P on the current census completeness flag before comparing ownership.
Useful? React with 👍 / 👎.
| for org in (estate.get("shelf_assignments") or {}).get("shelves") or {}: | ||
| o = str(org) | ||
| if o and o not in derived: | ||
| derived.append(o) |
There was a problem hiding this comment.
Adding shelf orgs to owners() also sends them through _collaborator_census's org-level outside-collaborator loop, which still calls _gh with the App token at lines 1027–1035. Because this change explicitly treats shelf orgs as outside that App installation, those reads become None and class N skips them, leaving undeclared outside collaborators on the new shelf org undetected; route non-canonical org rolls through _gh_user as the per-repository rolls already do.
Useful? React with 👍 / 👎.
| shelves: | ||
| match: ["organvm-*", "meta-organvm"] | ||
| plan_ok: ["free"] | ||
| repos: shelf_derived | ||
| enterprise_subscription: none | ||
| owner: gitvs | ||
| note: "The backbone SHELF orgs (custody v4.0.0 Phase 2): populated from shelf_assignments per tranche; membership is class P's shelf-parity (declared == census owner, BOTH directions) — repos is derived, never a hand count. T1 ERGON populated 2026-07-30; remaining shelves fill on their tranches." |
There was a problem hiding this comment.
Moving every organvm-* and meta-organvm org into this shelf_derived class removes the previous repos: 0 ceiling from the seven shelves that have not received a tranche. Class L only checks integer ceilings, while class P and owners() only inspect orgs explicitly present in shelf_assignments—currently just organvm-iii-ergon—so an accidental repo in any other shelf org is now invisible to both predicates; retain the zero ceiling for unassigned shelves or include empty assignments for every shelf org in class P's census.
Useful? React with 👍 / 👎.
| # ── ERGON shelf rows (Phase 2 T1, 2026-07-30) — movers without prior judgment rows; class | ||
| # carried verbatim from the census at move time, so the tranche changed placement, never posture. | ||
| organvm-iii-ergon/a-i-council--coliseum: {class: governed_public, why: "product-ledger row — ERGON shelf T1 2026-07-30 (class carried from census at move time)"} | ||
| organvm-iii-ergon/agent--claude-smith: {class: governed_public, why: "product-ledger row — ERGON shelf T1 2026-07-30 (class carried from census at move time)"} |
There was a problem hiding this comment.
All 48 moved repositories are assigned to governed_public, portal_public, or operation_private, and each of those classes declares app_installed: required, yet the accompanying census changes explicitly state that shelf orgs sit outside the App installation. The transfer therefore leaves every shelf repository off its declared App posture—and App-backed automation unavailable—while class F only checks for an installation globally; install the App on organvm-iii-ergon or avoid assigning App-required classes until that installation exists.
Useful? React with 👍 / 👎.
| elif org not in owners_: | ||
| out.append(f"{n}: declared shelf {org}, census owner {'/'.join(sorted(owners_))} — transfer owed") |
There was a problem hiding this comment.
When a declared repository also exists under another owner, this condition treats it as clean as long as the declared shelf owner is one member of owners_; the reverse pass ignores the extra copy because that other owner is not itself a declared shelf. Recreating an old organvm/<name> repository after transfer can therefore leave both copies live while class P reports no drift, contrary to the declared-owner equality invariant; require owners_ == {org} for each declared name.
Useful? React with 👍 / 👎.
| organvm-iii-ergon: | ||
| - a-i-chat--exporter |
There was a problem hiding this comment.
The transfer changes the canonical name of the highest-ranked value product to organvm-iii-ergon/a-i-chat--exporter, but value-repos.json and revenue-ladder.json still key it as organvm/a-i-chat--exporter (and do the same for rank-2 public-record-data-scrapper). These registries are compared by exact string—for example, scripts/merge-ready.py looks up the live PR repository name directly at lines 94–106—so PRs from the transferred repos lose their revenue rank/value status and backlog generation continues emitting work against stale identities; re-key all exact-owner registries in the same transfer commit.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46922757ff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for org in (estate.get("shelf_assignments") or {}).get("shelves") or {}: | ||
| o = str(org) | ||
| if o and o not in derived: | ||
| derived.append(o) |
There was a problem hiding this comment.
Once this adds organvm-iii-ergon to owners(), observe() also passes that owner to _owner_open_pr_counts, which still performs every GraphQL query through _gh with the canonical App token. Because this change explicitly treats shelf orgs as outside that App installation, the query either fails the entire PR census or returns only public repositories, so open PRs on the transferred private products disappear while prs.complete may remain true; route non-canonical owners through _gh_user here as the repository census now does.
Useful? React with 👍 / 👎.
| note: "The one repo-holding org (307 repos) — sole owner of the SYSTEM estate. Dual-estate custody (2026-07-29): collaboration-born products are personally-owned assets whose custody home is the personal account (see the repo_custody resource type; membership derives from the constellation register) — sole_owner reads 'all SYSTEM code lives here', not 'no repo may live on the personal estate'. Spending limit + plan are account-level his-hand acts (GitHub billing UI, cited via L-CARD-FRAUD-HOLD / the runbook) — the doctor observes, never mutates billing." | ||
| shelves: | ||
| match: ["organvm-*", "meta-organvm"] | ||
| plan_ok: ["free"] |
There was a problem hiding this comment.
Constraining every shelf org to the Free plan makes the newly transferred operation_private repositories unable to satisfy their declared branch_protection: required posture. The repository's own plan states that the Team upgrade is needed for private-org rulesets (docs/plans/2026-07-29-custody-dual-estate-semver-lineage.md:37-39), but moving these repos out of organvm also removes them from that planned upgrade; permit Team for populated shelves or assign a posture achievable on Free.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f5826f8c60
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| derived.append(owner) | ||
| # Shelf orgs are declared registry data (shelf_assignments) — enumerate them too, or the | ||
| # census never sees shelf repos and class P reads every declared shelf row as absent. | ||
| for org in (estate.get("shelf_assignments") or {}).get("shelves") or {}: |
There was a problem hiding this comment.
When the documented LIMEN_GITVS_OWNERS override is nonempty, owners() returns the requested subset before reaching this new shelf loop, while class P still compares every shelf assignment against the resulting facts. For example, LIMEN_GITVS_OWNERS=organvm produces a successful census marked complete but reports all 48 ERGON repositories as absent; unlike an owner-read failure, gating on repos.complete would not fix this. Append the declared shelf owners when class P is active or skip shelf parity for a narrowed census so unscanned owners remain coverage debt rather than false custody drift.
AGENTS.md reference: AGENTS.md:L255-L259
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4b1d2d450d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - vox | ||
| - vox--publica | ||
| - vulnpulse | ||
| - writelens |
There was a problem hiding this comment.
After a fresh estate census records the transferred nameWithOwner values, scripts/check-convergence.py::_owner_reachable compares repository owners by exact identity, so the unchanged organvm/writelens and organvm/vox owners in institutio/governance/convergence.yaml become missing. This makes the worker-toolkit, text-quality-scoring, and voice-infrastructure capabilities fail the convergence predicate; re-key those owner fields to the shelf identities in the same transfer.
AGENTS.md reference: AGENTS.md:L296-L300
Useful? React with 👍 / 👎.
| organvm-iii-ergon/a-i-council--coliseum: {class: governed_public, why: "product-ledger row — ERGON shelf T1 2026-07-30 (class carried from census at move time)"} | ||
| organvm-iii-ergon/agent--claude-smith: {class: governed_public, why: "product-ledger row — ERGON shelf T1 2026-07-30 (class carried from census at move time)"} |
There was a problem hiding this comment.
These repositories retain governed_public only through repo_overrides after moving outside the class's organvm/** glob. scripts/sync-marketplace-config.py resolves every integration's governed_public scope solely through _class_globs() and _governed_repos(), so all 26 transferred public repositories are excluded from future CodeRabbit, Claude-review, review-fanout, Renovate, and similar config PRs even after the required Apps are installed; derive targets through classify_repo and the census or otherwise include override members.
Useful? React with 👍 / 👎.
1 participant
Phase 2 T1 of custody v4.0.0 — the first advertised shelf gets its contents.
Fired and verified: 48 register-marked products transferred organvm → organvm-iii-ergon (redirects live, both private products included; the org now holds exactly 48). Pre-flight: zero org-level secrets/vars, repo-level secrets travel with their repos, no Pages custom domains.
Books:
shelf_assignmentsblock (bare names, transfer-proof) · 21 override re-keys + 27 new judgment rows (class carried verbatim from census — placement changed, posture never; surface-engine got its owned exposure decision) · orgs splitshelves/reserved(repos: shelf_derived; a-organvm stays the venture reserve).Engine: class P shelf-parity rung (declared == census owner, BOTH directions) ·
owners()enumerates declared shelf orgs · census reads non-canonical owners user-scoped — fixing shelf-org private blindness AND the personal-roll routing the #1668 heal missed (owners() includes the personal glob, so its org-set test sent personal rolls back through the App token). Collaborator census: complete=true for the first time since dual-estate began; all four partner lanes readwrite.Live doctor after: P clean 48/48 · N + O real verdicts, zero custody drift · the sole remaining live drift is the pre-existing landed-branch backlog (Phase 3's reap).
Gates: pytest 20 passed · gitvs.test.sh PASS (32) · parity drift==∅ · ruff-lint env false-red is #1658 (identical on clean main; CI's pinned ruff adjudicates).
🤖 Generated with Claude Code
https://claude.ai/code/session_01VHjMkFUZJ1pK21oyv1ropN
Summary by CodeRabbit
New Features
Tests