feat(pi-dotfiles): fork-pin-review governance and package ownership ledger - #52
Merged
Merged
Conversation
Adds validation.md and pre-mortem.md from /sdd:4-validate, and folds in the resulting plan.md/ux.md/ADR-002 amendments.
Adds the tracked .config/pi/extensions-manifest.json registry plus ExtensionManifestSource.load() and the frozen ManifestEntry dataclass that mirrors PiConfigSource's validation style. The approved-requires- approved_by/approved_date invariant is enforced in ManifestEntry's own __post_init__ so it fires on direct construction, not only via load().
… 2.1)
Temp-HOME spike (pi 0.84.4) with a real npm-hosted extension
(@gotgenes/pi-permission-system): `pi install` writes to
~/.pi/agent/npm/{package.json,package-lock.json,node_modules/<pkg>}, and
`pi remove` fully prunes it. Hand-editing settings.json's packages array to
drop an entry (the path llm-sync actually takes) and re-running Pi does
*not* prune the artifact in any invocation tried — confirming Epic 2.2's
PiPackageLedger is required, not optional.
…parity (Epic 5.3) Adds project_plans/pi-dotfiles/extensions/claude-tool-compat-matrix.md mapping every Claude tool name referenced by .claude/skills/**/SKILL.md to its Pi status (native, planned pi-claude-compat shim, extension-provided candidate, unsupported, or unknown-needs-verification), plus the Tier 4 deferred session/workflow-utility rows and the global/project-instructions row. Adds a .cfgcaddy.yml entry symlinking .claude/CLAUDE.md to .pi/agent/AGENTS.md, closing the global-instructions parity gap (project-level parity was already native via Pi's own AGENTS.md/CLAUDE.md loader).
… (Epic 1.1, Story 1.1.2) Tasks 1.1.2c/1.1.2d: an approved manifest entry's notes must cite >=5 distinct file paths, each confirmed to exist in the fork tree via `gh api repos/tstapler/<fork_repo>/git/trees/<fork_commit>?recursive=1`. Mechanical existence check only; does not judge review quality.
Add verify_pinned_sources_reviewed() in a new review_gate.py that cross-references rendered Pi packages/extensions against the extension review manifest: an unreviewed fork source, a stale commit, or a non-approved disposition all block sync_pi_settings() before PiSettingsTarget.save() ever runs, so no partial write can occur. Extract parse_fork_source()/normalize_fork_repo() in pi_config.py as the single shared fork-URL parser for both _is_allowed_package_source and the new gate, closing the drift risk from having two independent parsers. Add a duplicate-manifest-id check to ExtensionManifestSource.load() (two map keys whose own `id` field collide) and wire `make llm-sync-test` into CI's test job with paths triggers for stapler-scripts/llm-sync/** and .config/pi/**.
…(Epic 1.3) Adds fork_pin_extension.py, a standalone uv/typer script that forks an upstream Pi extension repo via gh and scaffolds a candidate manifest entry in .config/pi/extensions-manifest.json. disposition is always the literal "candidate" and approved_by/approved_date are always null -- the script has no flag or code path that can reach the approved state, which is enforced both structurally (no CLI parameter influences those fields) and by a regression test that scans the script's own source for the quoted literal "approved". --dry-run makes zero subprocess calls and previews the same manifest-entry shape a real run persists, verified by a shared entry-builder function and a test that replays the real run's commit through it to prove the two paths can't silently diverge.
Adds .claude/skills/pi-extension-review/SKILL.md documenting the fork-pin-review gate for third-party Pi extensions: the seven-step checklist from extension-audit.md plus a maintenance-liveness check recorded in the manifest notes field, the verify_pinned_sources_reviewed() enforcement point, the human-only approval-field invariant, how to run fork_pin_extension.py and make llm-sync-test, and the hold/rejected outcomes for a candidate that fails review. Cross-links from .config/pi/README.md.
…Epic 2.2) Add PiPackageLedger, a sibling to PiSettingsTarget's managedKeys pattern for on-disk package artifacts pi install writes (not just settings.json keys). Grounded in .config/pi/README.md's Package lifecycle spike: Pi does not garbage-collect installed packages when settings.json is hand-edited, only via its own `pi remove`/`pi uninstall` CLI, so removing an entry from config can silently orphan its files without this ledger. - save()/find_stale()/prune()/reconcile() mirror PiSettingsTarget's atomic write pattern. Only the npm source shape's artifact path is grounded in the spike (agent_dir/npm/node_modules/<package>); non-npm sources (git fork, local path) are recorded with an explicit null sentinel rather than a fabricated path. - Wire into sync_pi_settings(): ledger.save() runs after a changed settings write; --reconcile-pi-package-ledger recovers a ledger write that never ran (crash between the settings write and the ledger write, so a normal unchanged-settings rerun would never retry it); stale entries are always reported, --prune-stale-pi-packages actually deletes them. - bootstrap-pyinfra/deploys/llm_sync.py: confirm (and document) shell_capture already passes main.py's combined stdout through unmodified, so the new stale-report lines surface in bootstrap output with no pyinfra change.
Adds plugins/pi-claude-compat, a small Tyler-owned Pi extension that aliases Claude-named tools (AskUserQuestion, Grep, Glob, LS) onto Pi's native tools so synced Claude skills don't silently hit "unknown tool" on Pi. Grep/Glob/LS re-derive Pi's own createGrepToolDefinition/ createFindToolDefinition/createLsToolDefinition under the Claude name; AskUserQuestion wraps Pi's native ctx.ui.select() single-choice prompt (no native multi-select exists, so multiSelect questions still return one answer). Registered via .config/pi/config.d/40-claude-compat.json as a local path source, same pattern as kibitzer/ponytail/dotfiles-hooks. Adds a regression test proving path-sourced extensions are exempt from the manifest review gate by construction (Task 3.4.1d).
AGENTS.md's Pi section now names verify_pinned_sources_reviewed() as the fork-pin review enforcement point; the Pi README documents the --prune-stale-pi-packages / --reconcile-pi-package-ledger flags and the ledger's on-disk location.
…spec sweep gap 1)
…st state (spec sweep gap 3)
…bootstrap (spec sweep gap 2)
Surface 4 (bootstrap/pyinfra dry-run output) of design/ux.md's 5 UX surfaces had zero tests -- confirmed pre-existing gap from earlier /sdd:6-verify passes. The `--dry-run`/`--pi-dir` CLI plumbing itself predates this SDD project (cli.py's dry_run args landed 2026-02-25, 7 months before pi-dotfiles started 2026-09-15); ux.md was speccing exact copy against that pre-existing surface, not describing new build scope. Ran the real CLI against fixtures to compare actual output against ux.md's 6 acceptance criteria for this surface: destination path and the "No changes made to <path> (dry run)." closing line were never printed at all for the Pi-settings dry-run branch -- a real, narrow gap. Fixed precisely in sync_pi_settings() (src/cli.py): print the agent_dir destination on both the "would update" and closing lines, without touching PiSettingsTarget.save()'s bool return contract (which 3+ existing test assertions depend on). Verified against real fixture runs before and after (changed / unchanged / credential-rejected cases). Added test_pi_dry_run_ux.py covering the now-real destination-naming, no-credential-leak, and closing-line criteria, and one test in bootstrap-pyinfra/test_pi_install.py for the external-install-mode skip-reason criterion (pyinfra --dry against main.py isn't practical to test directly -- it needs sudo/homebrew and fails before reaching the Pi deploy -- so this tests plan_pi_install(), the pure function whose .reason the unconditional `print(f"Pi installation: ...")` line actually consumes). Per-key enumeration (ux.md criterion 2) remains a genuine, deferred gap: PiSettingsTarget.save() only returns a bool today, not which keys changed, so nothing exists for sync_pi_settings to print per-key without changing that method's return contract and the existing assertions built on it -- out of proportion for this pass.
…, gate coverage gap, missing tests)
# Conflicts: # stapler-scripts/llm-sync/src/cli.py
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a fork-pin-review governance system for Pi (pi.dev) coding-agent extensions on top of the existing
llm-synctiered-config renderer: no third-party extension source can render intosettings.jsonwithout a human-approved, exact-commit-pinned manifest entry. Also adds a package ownership ledger for on-disk artifacts Pi installs (which Pi does not garbage-collect on its own — confirmed by direct observation), a scaffolding helper for the fork-and-review workflow, a first-party Claude-tool-name compat shim, and the supporting docs/runbook.Context
Per
project_plans/pi-dotfiles/requirements.md, extending this repo's Claude-parity tooling to Pi requires that third-party extensions never silently activate — the security review step must stay unambiguously human.ADR-001andADR-002cover the design;project_plans/pi-dotfiles/implementation/plan.mdis the full implementation plan.Changes
stapler-scripts/llm-sync/src/sources/extension_manifest.py,review_gate.py):.config/pi/extensions-manifest.jsontracks review records;verify_pinned_sources_reviewed()blocks anygithub.com/tstapler/-fork-shaped source lacking anapprovedentry at the exact commit. Approval (disposition: "approved") can only be hand-edited by a human — no code path, including the scaffolding helper, can write it (enforced at the type level viaManifestEntry.__post_init__, and by a static-inspection regression test).stapler-scripts/llm-sync/scripts/fork_pin_extension.py): scripts the mechanical parts of forking + scaffolding acandidatemanifest entry; has no approval path.stapler-scripts/llm-sync/src/targets/pi_package_ledger.py): tracks installed package artifacts so removed config entries can be safely pruned (--prune-stale-pi-packages) and interrupted runs recovered (--reconcile-pi-package-ledger). Built on a VERIFIED spike (.config/pi/README.md's "Package lifecycle" section) showing Pi does not self-prune based onsettings.jsoncontent alone.plugins/pi-claude-compat/pi/index.ts): first-partyAskUserQuestion/Grep/Glob/LSaliases so synced Claude skills don't silently fail on Pi..claude/skills/pi-extension-review/SKILL.md): the seven-step (now eight, with a maintenance-liveness check) gate checklist, discoverable and cross-referenced from the enforcement code.make llm-sync-testnow gatesstapler-scripts/llm-sync/**and.config/pi/**changes in.github/workflows/ci.yml.claude-tool-compat-matrix.md, global-instructions parity via a new.cfgcaddy.ymlentry,rollout-runbook.md(staged rollout + rollback procedure),AGENTS.md/README.mdupdates.stapler-scripts/llm-sync, 20 inbootstrap-pyinfra), all passing — unit, integration (subprocess-level CLI invocation), and UX-acceptance (CLI-output) tests perproject_plans/pi-dotfiles/implementation/validation.md.Impact
stapler-scripts/llm-sync(new modules +cli.py/pi_config.pyextensions),bootstrap-pyinfra/deploys/llm_sync.py(docstring only), one new first-party Pi extension, CI config, andpi-dotfilesproject docs. No existing sync behavior (Gemini/OpenCode/Antigravity targets, Claude skill/prompt sync) is touched — regression-tested via the existingtest_cli_targeting.py/test_pi_sync.pysuites..config/pi/extensions-manifest.jsonstarts empty, so it only blocks a source once apackages/extensionsentry actually references agithub.com/tstapler/fork, which nothing in tracked config does yet._enabled_package_sources()incli.py, documented as a known follow-up to eliminate, see below).fork_pin_extension.pyis a standaloneuv runscript with inline PEP 723 deps (typer).Reviewer Notes
review_gate.py's commit-comparison logic (case-insensitive, exact-string, explicitly not prefix-tolerant) andparse_fork_source()'s fork-shape detection inpi_config.pyare the actual security boundary — a security review pass traced the full bypass surface and found it sound (see/sdd:6-verifyreport below).gotgenes/pi-packages, plan mode, the hooks bridge) and Epics 4.1/4.2 (1Password credential provider) are not in this PR — they require Tyler's owngh repo fork+ a multi-hour manual security review + a hand-edited approval, which is the whole point of the gate this PR builds.fork_pin_extension.pyand the review skill are ready for that follow-up work.~/.pi/agent/settings.jsoninto universal/work/machine-generated) needs data from a machine this environment doesn't have access to.ux.mdSurface 4 criterion 2) is a small, pre-existing (7-months-predates-this-plan) gap incli.py's dry-run verbosity — sized at under an hour, deferred rather than wideningPiSettingsTarget.save()'s bool return contract under this PR's time pressure._enabled_package_sources()incli.pyre-reads the tiered config a second time to recover package-id↔source mappingsPiConfigSource.load()'s rendering step discards — flagged during architecture review as worth folding intoLoadedPiConfigdirectly before Epic 3.2 adds a second consumer of the same derived data.pi_settings.py/pi_package_ledger.py) are noted refactor follow-ups, not correctness issues.config.dfragment withenabled: falseor requires an approved manifest entry that doesn't exist yet).Related
Implements
project_plans/pi-dotfiles/requirements.md,ADR-001,ADR-002. Full plan:project_plans/pi-dotfiles/implementation/plan.md./sdd:6-verifyproduced a ✅ PASS verdict (Layer 1+2 repair loop: 1/5 iterations, 9 findings fixed; Layer 3: clean; security review: no CRITICAL/HIGH/MEDIUM findings; Layer 4: 4/6 UX criteria closed, 1 deferred as documented above).