Enforce literal substrate convergence - #1705
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Important Review skippedToo many files! This PR contains 143 files, which is 43 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. This review couldn't start because sufficient usage credits or metered capacity aren't available. Add credits or update usage-based reviews in the billing tab, then retry. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (143)
You can disable this status message by setting the 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 |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b4ee52d699
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0b13a8cf9d
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f77931aa67
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a8541912f1
ℹ️ 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".
| if candidate.get("restoration_passed") is True | ||
| and _copy_count(candidate.get("copy_count", 0)) >= 2 | ||
| and candidate.get("independent_physical_devices") is True | ||
| ), |
There was a problem hiding this comment.
Bind restoration receipts to the sealed inventory
When a custody label has an older successful restoration receipt and the inventory is subsequently replaced or resealed under the same label, this check still accepts the old receipt because it validates only the three booleans/count and never compares the emitted inventory_sha256, plan_sha256, or content identity with the sealed inventory. The court can therefore authorize moving a private root without proving that either restored copy covers the current inventory; require the receipt to cryptographically identify the inventory and plan being approved.
AGENTS.md reference: AGENTS.md:L326-L327
Useful? React with 👍 / 👎.
| violations.append( | ||
| Violation( | ||
| "undeclared_nested_repository", | ||
| f"{manifest_path}/{rel}", | ||
| "nested checkout is not a registered Git submodule; use the canonical " | ||
| "Workspace row or runtime/worktrees", |
There was a problem hiding this comment.
Reconcile the court with the mandated worktree launcher
Fresh evidence after the prior thread is that this exact tree still sets wt="$repo/.worktrees/$slug" in scripts/start-worktree-session.sh:610, while this branch classifies the resulting worktree's .git file as an undeclared nested repository. Thus every capsule created through the repository-mandated launcher makes convergence fail by construction; place launcher worktrees under the declared ephemeral root or explicitly recognize that sanctioned location.
AGENTS.md reference: AGENTS.md:L396-L400
Useful? React with 👍 / 👎.
| configured = os.environ.get("WORKSPACE_ROOT", str(Path.home() / "Workspace")) | ||
| return Path(os.path.abspath(os.path.expandvars(configured))).expanduser() |
There was a problem hiding this comment.
Expand a tilde before canonicalizing the live root
When WORKSPACE_ROOT is exported as a literal tilde path such as ~/Workspace, audit() correctly expands it, but this helper calls abspath first, producing a path like /current/dir/~/Workspace that expanduser() can no longer fix. A live audit is then marked workspace_root_is_canonical_live: false, and its receipt retains the unredacted local path instead of the canonical $WORKSPACE_ROOT identity; expand the user component before computing the absolute path.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 525b059525
ℹ️ 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".
| namespace = root / "runtime" / "worktrees" / repository_storage_key(repo) | ||
| _validate_runtime_container(root, namespace) | ||
| return namespace / slug |
There was a problem hiding this comment.
Register runtime worktrees with the lifecycle reaper
When this launcher creates a capsule for any repository outside the default Limen/Portvs set, the new $WORKSPACE_ROOT/runtime/worktrees/<repo-key>/<slug> location is absent from every default source in iter_worktree_targets(): the inventory scans legacy dispatch roots, repo-local .worktrees, and linked worktrees of only its registered repositories. Such capsules therefore never appear in debt reports or reclaim-worktrees.py, allowing stale worktrees to accumulate while the exact-zero lifecycle predicate reports success; enumerate this runtime root or register every launcher repository.
AGENTS.md reference: AGENTS.md:L388-L400
Useful? React with 👍 / 👎.
| hostname = (parsed.hostname or "").lower() | ||
| port = f":{parsed.port}" if parsed.port else "" | ||
| path_text = parsed.path.rstrip("/") | ||
| if path_text.endswith(".git"): | ||
| path_text = path_text[:-4] | ||
| identity = urlunsplit((parsed.scheme.lower(), f"{hostname}{port}", path_text, "", "")) |
There was a problem hiding this comment.
Normalize transport-equivalent origins before hashing
When an existing repository changes its origin between equivalent HTTPS and SSH forms, such as https://github.com/organvm/limen.git and git@github.com:organvm/limen.git, this preserves the scheme and produces different storage keys. The launcher then searches a new namespace for an already-existing slug and can fail because the expected branch remains checked out at the old path; canonicalize equivalent provider transports to one repository identity before hashing.
Useful? React with 👍 / 👎.
| def _root() -> Path: | ||
| return Path(os.environ.get("LIMEN_ROOT", str(Path.home() / "Workspace" / "limen"))) | ||
| return Path( | ||
| os.environ.get("LIMEN_ROOT", str(Path.home() / "Workspace" / "library" / "engine" / "organvm" / "limen")) | ||
| ) |
There was a problem hiding this comment.
Derive capacity state from WORKSPACE_ROOT
When Limen runs from a non-default WORKSPACE_ROOT without an explicit LIMEN_ROOT—a supported configuration for direct, non-host-mutating installs—this helper still reads ~/Workspace/library/engine/organvm/limen/logs/usage.json. _load_usage() consequently returns an empty meter, so capacity_census() can classify depleted or rate-limited lanes using only stale board budgets; derive the fallback Limen path from WORKSPACE_ROOT, as the dispatch root helper already does.
Useful? React with 👍 / 👎.
| repo_common="$(git -C "$repo" rev-parse --path-format=absolute --git-common-dir)" | ||
| wt_common="$(git -C "$wt" rev-parse --path-format=absolute --git-common-dir)" | ||
| if [[ "$(cd "$repo_common" && pwd -P)" != "$(cd "$wt_common" && pwd -P)" ]]; then |
There was a problem hiding this comment.
Reject symlinked worktree leaf paths
When the canonical slug path is a symlink to a linked worktree outside runtime/worktrees, these repository-identity and branch checks both succeed because git -C follows the symlink. The launcher then reports the path as reused and writes the capsule into the external target, bypassing the physical-container validation performed only for the namespace; reject a symlinked $wt and verify its resolved top level is the canonical leaf before reuse.
Useful? React with 👍 / 👎.
| try: | ||
| if path.suffix == ".jsonl": | ||
| rows: list[Mapping[str, Any]] = [] | ||
| for line in path.read_text(encoding="utf-8").splitlines(): |
There was a problem hiding this comment.
When a referenced append-only custody JSONL grows large, this materializes the entire file with read_text().splitlines() and retains every parsed row, and _audit_private_custody() repeats that work for each private manifest row sharing the ledger. Such a ledger can make the aggregate convergence command consume unbounded memory and runtime despite its declared scan limits; stream with a shared byte/row/deadline ceiling and report excess as unmeasured state.
AGENTS.md reference: AGENTS.md:L436-L440
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7911425f8f
ℹ️ 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".
| fi | ||
| created="reused" | ||
| elif git -C "$repo" show-ref --verify --quiet "refs/heads/$branch"; then | ||
| git -C "$repo" worktree add "$wt" "$branch" >/dev/null |
There was a problem hiding this comment.
Reuse legacy worktrees before adding the branch
Fresh evidence after relocating the launcher is that an existing pre-upgrade capsule still has work/<slug> checked out at <repo>/.worktrees/<slug>; the canonical target is absent, so this branch runs git worktree add for a branch Git refuses to check out twice and the mandated launcher cannot resume that capsule. Detect the branch's registered legacy worktree and migrate or reuse it during cutover.
AGENTS.md reference: AGENTS.md:L396-L400
Useful? React with 👍 / 👎.
| min_age_h=min_age_h, | ||
| source=f"canonical-runtime-worktree:{namespace.name}", | ||
| ) | ||
| for unit in units |
There was a problem hiding this comment.
Reap empty repository namespaces
When the reaper removes the last <slug> from a canonical <repo-key> namespace, the empty namespace directory remains, but this inventory creates targets only for entries in units. The convergence court then emits ephemeral_empty_namespace, while subsequent reaper runs see nothing to remove, so normal accepted cleanup can never reach the required idempotent fixed point; explicitly own and remove empty physical namespaces.
AGENTS.md reference: AGENTS.md:L184-L187
Useful? React with 👍 / 👎.
| def provider_outcome_ledger_path() -> Path: | ||
| root = Path(os.environ.get("LIMEN_ROOT", Path.home() / "Workspace" / "limen")).expanduser() | ||
| root = Path( | ||
| os.environ.get("LIMEN_ROOT", Path.home() / "Workspace" / "library" / "engine" / "organvm" / "limen") |
There was a problem hiding this comment.
Derive the provider ledger from WORKSPACE_ROOT
When a direct installation sets a non-default WORKSPACE_ROOT without exporting LIMEN_ROOT, this fallback still selects ~/Workspace/library/.../limen. Provider outcomes are consequently read from and appended to a newly created stale tree rather than the installed checkout, so dispatch can lose current cooldown and failure state; derive this fallback from WORKSPACE_ROOT, as the corrected capacity resolver does.
AGENTS.md reference: AGENTS.md:L194-L197
Useful? React with 👍 / 👎.
| if common.name != ".git" or not common.is_dir(): | ||
| raise SpawnError("lead capsule does not share a primary non-bare Git checkout") |
There was a problem hiding this comment.
Resolve primary checkouts with separate Git directories
When a lead capsule belongs to a valid non-bare checkout created with git init --separate-git-dir, --git-common-dir names that external administration directory rather than a directory literally named .git. This check therefore rejects the lead before it can spawn a battle, although the previous repo-local layout worked for such checkouts; derive the primary worktree from git worktree list --porcelain and verify its common-directory identity instead of assuming the basename.
Useful? React with 👍 / 👎.
| for line in proc.stdout.splitlines(): | ||
| _, _, value = line.partition(" ") | ||
| if value.strip(): | ||
| result.add((repo / value.strip()).resolve(strict=False)) |
There was a problem hiding this comment.
Validate declared submodules against index gitlinks
When .gitmodules names a path that is not actually a mode-160000 gitlink in the repository index, this code still exempts any nested .git found there. A stale or fabricated declaration can therefore hide an ad-hoc clone from undeclared_nested_repository—including unique commits the outer repository's custody checks never inspect—and let convergence authorize cleanup without proving that nested state is remote-preserved; only exempt paths verified as current index gitlinks.
AGENTS.md reference: AGENTS.md:L301-L305
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03c0046624
ℹ️ 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".
| violations.extend(_audit_local_branch_custody(repo, row.path, remote_refs)) | ||
| violations.extend(_audit_stash_custody(repo, row.path, remote_refs)) |
There was a problem hiding this comment.
Audit all local refs before accepting repository custody
When a declared checkout has a unique commit reachable only from refs/tags, refs/notes, or the reflog—for example after resetting a branch while retaining a local tag—the report can still be green because this audits only local branches and stashes. The repository's own scripts/reap-clones.py:273-285 treats those namespaces and --reflog as local-only data-loss sources; cleanup authorized by this report can therefore discard the objects. Compare every local ref and reflog object with the freshly fetched origin refs before accepting custody.
AGENTS.md reference: AGENTS.md:L301-L305
Useful? React with 👍 / 👎.
| Violation("repository_custody_missing", row.path, f"live custody ref is absent: {custody_ref}") | ||
| ) | ||
|
|
||
| status = _run_git(repo, "status", "--porcelain=v1", "--untracked-files=all") |
There was a problem hiding this comment.
Bound repository-status output before buffering it
When a repository has a very large untracked or ignored tree, these status probes bypass ScanBudget; local git status -h confirms that --untracked-files=all enumerates untracked files and --ignored=matching includes ignored entries, while _run_git uses capture_output=True and buffers the complete stdout. Thus max_scan_entries does not bound memory or subprocess output, and the aggregate court can be killed before producing its receipt. Apply an output ceiling or use a bounded counting/streaming probe.
AGENTS.md reference: AGENTS.md:L436-L440
Useful? React with 👍 / 👎.
| return creation_root.parent / "_limen-worktree-abandonment" | ||
| candidate = Path(ABANDONMENT_QUARANTINE).expanduser() | ||
| else: | ||
| candidate = xdg_data_home() / "limen" / "worktree-abandonment" |
There was a problem hiding this comment.
Keep quarantine on the source filesystem
When WORKSPACE_ROOT or a worktree root is on an external SSD while the default $XDG_DATA_HOME remains on the internal home volume, this new default always selects a cross-device quarantine. quarantine_path() then rejects it with cross-filesystem-quarantine-denied, so both generated-payload cleanup and orphan_quarantine_root() fail for exactly the external-substrate configuration the repository supports, preventing the reaper from reaching its fixed point. Derive a persistent quarantine on the source device or provide a safe same-device fallback.
AGENTS.md reference: AGENTS.md:L317-L325
Useful? React with 👍 / 👎.
| violations: list[Violation] = [] | ||
| if not (repo / ".git").exists(): | ||
| return [Violation("repository_missing_git", row.path, "declared repository is not a Git worktree")] | ||
| top = _run_git(repo, "rev-parse", "--show-toplevel") |
There was a problem hiding this comment.
Audit every registered linked worktree
When a declared repository has a linked worktree outside the Workspace with dirty or untracked changes, this audit can still pass because git status is run only in the primary checkout; a local probe confirms the primary status remains empty while the linked worktree reports its modification. The repository's own scripts/reclaim-worktrees.py:958-963 treats registered sibling worktrees as a cleanup blocker, but this court never enumerates them, so moving or deleting the primary Git directory can strand the linked worktree's local state. Audit every git worktree list --porcelain entry or report it as unmeasured.
AGENTS.md reference: AGENTS.md:L388-L392
Useful? React with 👍 / 👎.
| import re | ||
|
|
||
|
|
||
| _SCAN_ROOTS = ( |
There was a problem hiding this comment.
Include tracked agent settings in the legacy-path scan
After the compatibility doorway is removed, .gemini/settings.json:4 still launches the MCP server with uv --directory /Users/4jp/Workspace/limen/mcp, and .claude/settings.json:94 still falls back to $HOME/Workspace/limen. The new checker nevertheless reports zero findings because _SCAN_ROOTS omits both tracked settings directories, so the path gate can authorize a cutover that breaks Gemini MCP startup and Claude closeout outside a Limen checkout. Include these operational settings in the scan and migrate their commands to the canonical root contract.
Useful? React with 👍 / 👎.
Summary
Verification
Live migration receipt
The implementation is ready; physical convergence intentionally remains red and owner-recorded. The current court records 42 manifest rows, 1 discovered canonical repository location, 3 compatibility paths, 4 private roots, 77 violations, and zero unmeasured state. No repository checkout or private payload is moved by this PR.
Merge condition
Merge only through the repository merge queue from this exact reviewed head after pr-gate is terminal green. The live red migration gates remain inputs to the separately receipted successor workstream; they are not represented as completed Omega convergence.