diff --git a/README.md b/README.md index 7af5172..2c1c4d6 100644 --- a/README.md +++ b/README.md @@ -4,8 +4,9 @@ **The package manager for AI agents.** -Share AI agent skills across your team like code packages — from any Git repo, -into Claude Code, Cursor, Codex, and more. +For teams who want to manage agent skills like software packages — the way npm, +PyPI, and uv manage code. Install skills from any Git repo into Claude Code, +Cursor, Codex, and more, then share them across your team like real dependencies. [![PyPI](https://img.shields.io/pypi/v/agr?color=blue)](https://pypi.org/project/agr/) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://opensource.org/licenses/MIT) @@ -19,25 +20,58 @@ into Claude Code, Cursor, Codex, and more. --- -## Getting started +## Why agr -Install the CLI: +agr is for **teams** who want to manage their agent skills as seriously as they +manage their code — the way npm, PyPI, and uv manage software packages. + +Skills make AI agents better at your work. But today they're copied around by +hand, drift between machines, and live in tool-specific folders. agr treats them +like real dependencies: declared in one manifest, locked to a version, installed +with one command, and identical for every teammate, on every machine, in every +tool. + +That brings the same wins you already get from a package manager for code: + +- **Version & pin.** `agr.lock` records the exact version of every skill, so a + skill that works today keeps working tomorrow — no silent upstream changes + breaking your agents. Upgrade on purpose, when you choose, with `agr upgrade`. +- **Distribute effortlessly.** Publishing a skill is just pushing to a Git repo; + installing one is `agr add owner/repo/skill`. No registry to set up, no files + to email around. +- **One source of truth for the team.** The skills your agents use are part of + your repo — reviewed in PRs, versioned in Git, and shared like any other + dependency. Everyone runs the same skills, so your agents behave consistently + across the whole team. +- **Onboard in one command.** A new teammate clones the repo, runs `agr sync`, + and their agents are set up exactly like everyone else's — same skills, same + standards, day one. + +## Install ```bash uv tool install agr ``` -Install your first skill: +--- + +## What you can do with it + +Five things. That's the whole tool. + +### 1. Install a skill from a Git repo ```bash agr add anthropics/skills/pdf ``` -Handles follow the pattern `owner/repo/skill` — pointing to a directory inside -a GitHub repo. `anthropics/skills/pdf` means the `pdf/` directory inside -[github.com/anthropics/skills](https://github.com/anthropics/skills). +Handles are just a path into GitHub: **`owner/repo/skill`**. `anthropics/skills/pdf` +is the `pdf/` directory inside +[github.com/anthropics/skills](https://github.com/anthropics/skills). Any public +repo works — no registry, no publishing step. -Then invoke it in your AI tool: +`agr add` auto-creates `agr.toml`, detects which AI tools you use, and installs +the skill into each. Then invoke it in your tool: | Tool | Invoke with | |------|-------------| @@ -48,17 +82,22 @@ Then invoke it in your AI tool: | GitHub Copilot | `/pdf` | | Antigravity | *(via IDE)* | -No setup required — `agr add` auto-creates `agr.toml` and detects which tools -you use. +### 2. Use your own local skills ---- +Point at a directory on disk instead of a repo: -## Built for teams +```bash +agr add ./skills/my-internal-skill +``` + +Great for skills you're still writing, or ones that never leave your codebase. +They sync into every tool exactly like remote skills. -agr is opinionated: skill directories (`.claude/skills/`, `.cursor/skills/`, …) -are build artifacts — like `.venv/` or `node_modules/`. Add them to `.gitignore`. -Commit `agr.toml` and `agr.lock` instead. `agr sync` rebuilds the environment -from the manifest on every machine. +### 3. Share one skill environment with your team + +`.claude/skills/`, `.cursor/skills/`, … are build artifacts — like `.venv/` or +`node_modules/`. Add them to `.gitignore`. Commit **`agr.toml`** and +**`agr.lock`** instead: ```toml tools = ["claude", "cursor"] @@ -66,51 +105,51 @@ tools = ["claude", "cursor"] dependencies = [ {handle = "anthropics/skills/pdf", type = "skill"}, {handle = "anthropics/skills/frontend-design", type = "skill"}, + {path = "./skills/my-internal-skill", type = "skill"}, ] ``` +A new teammate clones the repo and runs: + ```bash -agr sync # Like npm install, but for AI agents +agr sync # like `npm install`, but for AI agents ``` -New teammate? `agr sync` and they're productive on day one — same skills, -same standards, every tool. +Now everyone has the same skills, the same standards, in every tool. ---- - -## Keep skills up to date +### 4. Keep skills up to date ```bash agr upgrade # all skills -agr upgrade pdf # one skill +agr upgrade pdf # just one ``` ---- +Re-fetches skills at their latest upstream version and updates `agr.lock`. -## Example skills +### 5. Try a skill without installing it ```bash -agr add anthropics/skills/pdf # Read, extract, create PDFs -agr add anthropics/skills/frontend-design # Production-grade interfaces -agr add anthropics/skills/claude-api # Build apps with the Claude API -agr add anthropics/skills/skill-creator # Create, modify, and improve skills +agrx anthropics/skills/pdf ``` +Downloads and runs a skill once, then throws it away — nothing added to +`agr.toml`, nothing left behind. + --- ## All commands -| Command | Description | -|---------|-------------| -| `agr add ` | Install a skill | +| Command | What it does | +|---------|--------------| +| `agr add ` | Install a skill and add it to `agr.toml` | | `agr remove ` | Uninstall a skill | -| `agr sync` | Install all from `agr.toml` | -| `agr upgrade [handle...]` | Re-fetch deps at latest version | +| `agr sync` | Install everything in `agr.toml` | +| `agr upgrade [handle...]` | Re-fetch skills at their latest version | | `agr list` | Show installed skills | -| `agrx ` | Run a skill temporarily without installing | +| `agrx ` | Run a skill once, without installing | -Add `-g` to `add`, `remove`, `sync`, or `list` for global skills (available in -all projects). +Add `-g` to `add`, `remove`, `sync`, or `list` to manage **global** skills, +available across all your projects. --- diff --git a/agr/commands/add.py b/agr/commands/add.py index c6a184b..de85d6e 100644 --- a/agr/commands/add.py +++ b/agr/commands/add.py @@ -14,6 +14,7 @@ Dependency, ) from agr.console import get_console +from agr.features import feature_enabled from agr.exceptions import ( INSTALL_ERROR_TYPES, AgrError, @@ -68,7 +69,7 @@ def _detect_local_type(source_path: Path) -> str: If neither marker exists, checks for a [package] section in agr.toml. If nothing matches, defaults to skill (existing behaviour). """ - has_ralph = is_valid_ralph_dir(source_path) + has_ralph = is_valid_ralph_dir(source_path) and feature_enabled("ralph") has_skill = is_valid_skill_dir(source_path) if has_ralph and has_skill: @@ -192,20 +193,21 @@ def _install_dependency( except SkillNotFoundError: pass - try: - installed_path, install_result = fetch_and_install_ralph( - handle, - repo_root, - overwrite, - resolver=resolver, - source=source, - default_repo=default_repo, - ) - return AddInstallResult( - [str(installed_path)], install_result, DEPENDENCY_TYPE_RALPH - ) - except RalphNotFoundError: - pass + if feature_enabled("ralph"): + try: + installed_path, install_result = fetch_and_install_ralph( + handle, + repo_root, + overwrite, + resolver=resolver, + source=source, + default_repo=default_repo, + ) + return AddInstallResult( + [str(installed_path)], install_result, DEPENDENCY_TYPE_RALPH + ) + except RalphNotFoundError: + pass return _install_package( handle, @@ -270,6 +272,8 @@ def _install_package( (DEPENDENCY_TYPE_PACKAGE, entry) for entry in expanded.package_entries ] for dep in expanded.dependencies: + if dep.is_ralph and not feature_enabled("ralph"): + continue sub_handle = dep.to_parsed_handle(config.default_owner) sub_source = dep.resolve_source_name(config.default_source) if dep.is_ralph: diff --git a/agr/commands/sync.py b/agr/commands/sync.py index 467d07e..3d1dafc 100644 --- a/agr/commands/sync.py +++ b/agr/commands/sync.py @@ -25,6 +25,7 @@ find_config, require_repo_root, ) +from agr.features import feature_enabled from agr.package import ExpandedDeps, detect_conflicts, expand_packages from agr.console import error_exit, get_console, print_error from agr.exceptions import ( @@ -604,6 +605,11 @@ def _classify_dependencies( results[index] = SyncResult.up_to_date() continue + if dep.is_ralph and not feature_enabled("ralph"): + # Ralph install is gated off: skip silently as if absent. + results[index] = SyncResult.up_to_date() + continue + if dep.is_ralph: # Ralphs are tool-agnostic: check project-level ralphs dir. if not forced and is_ralph_installed( @@ -893,6 +899,10 @@ def _sync_dep_from_lockfile( handle, source_name = dep.resolve(config.default_source, config.default_owner) is_ralph_dep = dep.is_ralph + # Ralph install is gated off: skip silently as if absent. + if is_ralph_dep and not feature_enabled("ralph"): + return SyncResult.up_to_date() + # Check installation status — initialise tools_needing_install # unconditionally so it is always bound regardless of code path. tools_needing_install: list[ToolConfig] = [] diff --git a/agr/features.py b/agr/features.py new file mode 100644 index 0000000..33e80f8 --- /dev/null +++ b/agr/features.py @@ -0,0 +1,36 @@ +"""Environment-variable-backed feature flags. + +A small registry maps feature names to the environment variables that enable +them. A feature is *off* unless its env var is set to a recognised truthy +value. The registry keeps adding a new gated feature to a single line. + +Truthy values (case-insensitive, surrounding whitespace ignored): +``1``, ``true``, ``yes``, ``on``. Anything else — including unset — is off. +""" + +import os + +# Feature name -> environment variable that enables it. +_FEATURE_ENV_VARS: dict[str, str] = { + "ralph": "AGR_ENABLE_RALPH", +} + +# Recognised truthy env-var values (compared lowercase, stripped). +_TRUTHY_VALUES = frozenset({"1", "true", "yes", "on"}) + + +def _is_truthy(value: str | None) -> bool: + """Return True if an env-var value counts as enabling a feature.""" + if value is None: + return False + return value.strip().lower() in _TRUTHY_VALUES + + +def feature_enabled(name: str) -> bool: + """Return whether the named feature is enabled via its env var. + + Raises: + KeyError: if ``name`` is not a registered feature. + """ + env_var = _FEATURE_ENV_VARS[name] + return _is_truthy(os.environ.get(env_var)) diff --git a/agr/ralph_installer.py b/agr/ralph_installer.py index 2424706..00f3684 100644 --- a/agr/ralph_installer.py +++ b/agr/ralph_installer.py @@ -31,6 +31,7 @@ InvalidLocalPathError, RalphNotFoundError, ) +from agr.features import feature_enabled from agr.handle import ( INSTALLED_NAME_SEPARATOR, ParsedHandle, @@ -273,6 +274,12 @@ def fetch_and_install_ralph( Returns: Tuple of (installed path, InstallResult with lockfile metadata). """ + # Defense-in-depth: with the ralph feature off, no caller can install a + # ralph. Raise the same not-found error a missing ralph would, so nothing + # reveals that a feature flag is involved. + if not feature_enabled("ralph"): + raise RalphNotFoundError(handle.name) + if repo_root is None: raise AgrError("repo_root is required for ralph installation") diff --git a/docs/contributing/feature-flags.md b/docs/contributing/feature-flags.md new file mode 100644 index 0000000..397dcb3 --- /dev/null +++ b/docs/contributing/feature-flags.md @@ -0,0 +1,81 @@ +# Feature flags + +`agr` has a small, env-var-backed feature-flag system for shipping work that +should stay dark by default — code that lands on `main` and is fully tested, but +behaves as if it doesn't exist until an operator opts in. + +This page is for contributors. It is **not** part of the published docs site +(it's excluded from the build), so a gated feature stays invisible to users +until you decide to document it. + +## How it works + +The whole mechanism lives in [`agr/features.py`](https://github.com/computerlovetech/agr/blob/main/agr/features.py): + +- A registry maps a **feature name** to the **environment variable** that + enables it. +- A feature is **off** unless its env var is set to a recognised truthy value: + `1`, `true`, `yes`, or `on` — case-insensitive, surrounding whitespace + ignored. Anything else, including unset, is off. +- `feature_enabled(name)` returns the on/off state. An unregistered name raises + `KeyError` (a programming error, surfaced loudly rather than silently off). + +```python +from agr.features import feature_enabled + +if feature_enabled("ralph"): + ... # gated behaviour +``` + +Both `agr` and `agrx` import the same module, so a flag resolves identically +across the two CLIs. + +## Adding a new gated feature + +1. **Register it** — one line in `_FEATURE_ENV_VARS`: + + ```python + _FEATURE_ENV_VARS: dict[str, str] = { + "ralph": "AGR_ENABLE_RALPH", + "my_feature": "AGR_ENABLE_MY_FEATURE", + } + ``` + +2. **Gate the behaviour at every decision point**, not just at one choke point. + Wrap each branch that turns the feature on in `feature_enabled("my_feature")`. + +3. **Add a defense-in-depth guard** at the lowest-level entry point (the + function that actually does the work), so the feature can't activate even if + a caller forgets a gate. + +## Keeping a dark feature dark + +The hard part isn't the flag — it's making the *off* path indistinguishable +from a world where the feature was never built: + +- **No leakage.** Off-path errors and help text must never mention the flag, the + env var, or the feature's name. Reuse the generic errors that already exist + for "not found" / "unsupported" cases. +- **Gate at the right altitude.** A single hard `raise` deep in the call stack + can produce the wrong surface behaviour (e.g. a caught exception that triggers + a fallback). Gate at the *decision* point so the resulting error reads + naturally, and keep the deep guard only as a backstop. +- **Silent skips.** Where a feature would expand into work (package expansion, + sync over pinned deps), drop the gated items silently rather than warning — + a warning reveals that something was skipped. + +The ralph-install feature is the worked example: it's gated at local type +detection, the remote install fallback, package expansion, and sync, with a +backstop guard in the installer. See +[`agr/commands/add.py`](https://github.com/computerlovetech/agr/blob/main/agr/commands/add.py), +[`agr/commands/sync.py`](https://github.com/computerlovetech/agr/blob/main/agr/commands/sync.py), +and [`agr/ralph_installer.py`](https://github.com/computerlovetech/agr/blob/main/agr/ralph_installer.py). + +## Testing + +In tests, set or unset the env var to exercise both states. The suite enables +ralph by default via an autouse fixture in `tests/conftest.py` +(`AGR_ENABLE_RALPH=1`), so existing tests reflect "flag on = today's +behaviour"; off-path tests unset it explicitly. Cover, at minimum: flag +resolution (set/unset/truthy/non-truthy), each gated path blocked when off and +working when on, and the absence of any flag-related text in off-path output. diff --git a/docs/diary/2026-06-08-readme-jobs-to-be-done.md b/docs/diary/2026-06-08-readme-jobs-to-be-done.md new file mode 100644 index 0000000..4e6c4c1 --- /dev/null +++ b/docs/diary/2026-06-08-readme-jobs-to-be-done.md @@ -0,0 +1,170 @@ +# Diary: Simplify the README around the top jobs to be done + +Reshape `README.md` so a newcomer immediately understands what agr is, who it's +for, and what to do. Center it on the handful of core jobs (install from a repo, +use local skills, share a team environment, upgrade, run once) and make the +team / package-manager-for-skills framing explicit. + +## Step 1: Rewrite README around five jobs to be done + +**Author:** main + +### Prompt Context + +**Verbatim prompt:** "Okay so we need to simplify the README file. This is +because we want to center it around the top five jobs to be done and how to get +started with AGR and make it very clear for people how to use it and how it +works with the handles and the whole installing from repos and how it works with +the local skills and stuff like that. Please please give it a shot. It should be +very easy for people to understand it and know that they need AGR and it should +feel simple and it should feel like they actually know what to do. Just really +crisp." + +**Interpretation:** Restructure the README so the value proposition and usage +are obvious, organized around ~5 concrete jobs, with handles, repo installs, and +local skills all clearly explained. + +**Inferred intent:** Convert the README from a feature list into a "here's what +you can do and how" document that converts a curious reader into a confident +user. + +### What I did + +Before writing, I confirmed the real capabilities so the doc wouldn't lie: +read `/agr/handle.py` (`parse_handle`, ~line 253) to confirm the handle forms +(`owner/repo/skill`, shorter remote forms, and local `./path`), and read +`/docs/index.md` and `/docs/managing.md` to align messaging and confirm local +skills use a `path = "./…"` entry in `agr.toml`. + +Then I rewrote `/README.md` around five jobs under the banner "Five things. +That's the whole tool.": + +1. Install a skill from a Git repo — handles explained as "just a path into + GitHub", plus the per-tool invoke table. +2. Use your own local skills — `agr add ./skills/my-internal-skill` (this job + was absent from the old README). +3. Share one skill environment with your team — gitignore the tool dirs, commit + `agr.toml` + `agr.lock`, `agr sync`. The example manifest now shows both a + remote `handle` and a local `path`. +4. Keep skills up to date — `agr upgrade`. +5. Try a skill without installing — `agrx`, promoted from a table footnote to a + first-class job. + +Kept the header, badges, demo SVG, full command table, community-skills block, +and footer. + +### Why + +The old README mixed framing ("Built for teams", "Keep skills up to date", +"Example skills") in a way that read as a feature tour rather than a task guide. +Organizing by job answers the reader's real question — "what do I do with this?" +— and surfaces capabilities (local skills, `agrx`) that were buried. + +### What worked + +Reading `handle.py` and the docs first meant every command and handle example in +the rewrite is accurate rather than guessed — e.g. local skills really are a +`path` entry, and the `owner/repo/skill` → GitHub-directory mapping is exactly +how `parse_handle` documents it. + +### What didn't work + +Nothing broke — this is a docs-only change, no build or test failures. The first +draft was structurally complete; subsequent edits were additive framing, not +fixes. + +### What I learned + +agr's local-skill support (`agr add ./path`, `path = "./…"` in `agr.toml`) is a +genuine first-class feature that the previous README never mentioned — easy to +miss because it's only visible in `docs/managing.md` and the handle parser. + +### What was tricky + +Balancing "crisp" against the user's later asks to *add* the package-manager +rationale and the versioning/distribution benefits — more words, but the request +was explicitly to make the "why" clearer. I kept the additions as a tight bullet +list so the document still scans quickly rather than turning into prose walls. + +### What warrants review + +Read `/README.md` top to bottom as a new user would and check: (1) the five jobs +are genuinely distinct and ordered by likelihood of use; (2) the handle +explanation lands before any handle is used; (3) the command table matches the +real CLI surface. Cross-check the command table against `/docs/reference.md` if +in doubt. + +### Future work + +`docs/index.md` and `docs/managing.md` could adopt the same jobs-to-be-done +framing for consistency with the README, but that's out of scope for this change. + +## Step 2: Add team + package-manager framing and the "why version/distribute" rationale + +**Author:** main + +### Prompt Context + +**Verbatim prompt:** "Also make sure to make it clear that this is for teams who +want to manage their skills and agent skills in a great way, just like software +packages like npm, PyPI, UV, and stuff like that. I really like the current +thing right now. The current tech is really great." — followed by: "Also explain +why it is nice to version, to distribute to, all of that stuff. Also make sure to +make it clear why it's nice to do this and use this tool as a team." + +**Interpretation:** Add an explicit "for teams, like npm/PyPI/uv" positioning to +the header and intro, then expand on the concrete benefits of versioning, +distribution, and team use. + +**Inferred intent:** Make sure the reader doesn't just see *how* to use agr but +understands *why* a team would want package-manager discipline for their skills. + +### What I did + +Edited the header subtitle of `/README.md` to lead with the team + +npm/PyPI/uv analogy. Opened the "Why agr" section with "agr is for **teams**…" +and the same analogy. Then added a four-bullet benefits list: **Version & pin** +(`agr.lock`, intentional upgrades), **Distribute effortlessly** (publish = push +to a repo, install = `agr add`), **One source of truth for the team** (skills in +PRs, versioned in Git, consistent agent behavior), and **Onboard in one command** +(`agr sync` on clone). + +### Why + +The reorganized README explained *what* and *how* well, but the user wanted the +*why-as-a-team* made explicit — the payoff of treating skills like packages +(reproducibility, no drift, easy onboarding) wasn't spelled out. + +### What worked + +Framing the benefits as a direct parallel to "wins you already get from a package +manager for code" lets readers transfer their existing npm/uv intuition straight +onto agr, which is exactly the analogy the user asked for. + +### What didn't work + +Nothing failed — purely additive edits to prose. + +### What I learned + +The user values the package-manager mental model strongly enough to want it in +two places (header + intro); positioning, not just instructions, is part of what +makes this README "feel simple." + +### What was tricky + +Adding rationale without bloating the doc. Resolved by keeping each benefit to a +one-line bold lead + one sentence, so the section adds substance while still +scanning fast. + +### What warrants review + +Check that the header subtitle and the "Why agr" opener don't feel redundant +reading them back-to-back — they intentionally repeat the npm/PyPI/uv analogy, +which should reinforce rather than annoy. Confirm the benefits list claims are +all true (e.g. `agr.lock` really pins versions — yes, per `docs/managing.md`). + +### Future work + +None beyond the Step 1 follow-up (aligning the docs site pages with the same +framing). diff --git a/mkdocs.yml b/mkdocs.yml index ca586ed..60a1515 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -79,6 +79,7 @@ not_in_nav: | exclude_docs: | diary/** + contributing/** validation: nav: diff --git a/tasks/2026-06-07-feature-flags-gate-ralph-install/LOG.md b/tasks/2026-06-07-feature-flags-gate-ralph-install/LOG.md new file mode 100644 index 0000000..dfb20b3 --- /dev/null +++ b/tasks/2026-06-07-feature-flags-gate-ralph-install/LOG.md @@ -0,0 +1,107 @@ +# Log + +## Iteration 1 — Feature-flag foundation + installer guard (2026-06-07) + +**Shipped (Option A: foundation only):** + +- New `agr/features.py`: env-var-backed feature-flag registry. + - `feature_enabled(name)` resolves a feature from a name→env-var map + (`ralph → AGR_ENABLE_RALPH`). + - Truthy parsing: `1`/`true`/`yes`/`on`, case-insensitive, whitespace-stripped; + unset or anything else is off. Unknown feature name raises `KeyError`. +- Defense-in-depth guard in `fetch_and_install_ralph` + (`agr/ralph_installer.py`): when `feature_enabled("ralph")` is false it raises + `RalphNotFoundError(handle.name)` before any install work — no message + mentions a flag/feature/env var (no leakage). +- Tests: + - `tests/test_features.py` — set/unset/truthy/non-truthy resolution + unknown + feature. + - `tests/test_ralph_installer.py::TestFetchAndInstallRalphFeatureGate` — + flag-off blocks install with no leakage; flag-on installs as today. + - `tests/conftest.py` — autouse `enable_ralph_feature` fixture sets + `AGR_ENABLE_RALPH=1` by default so the suite reflects "flag on = today's + behaviour"; off-path tests unset it explicitly. + +**Acceptance criteria covered:** 1, 2, 7, 8 (local path), and the slice of 10/11 +that applies (flag resolution + installer guard tests; `pytest`/`ruff`/`ty` +green). `agr`/`agrx` share `agr/features.py`, so unification holds. + +**Verification:** `uv run ruff check`, `uv run ty check` pass. Full suite: +1298 passed, 8 skipped. 6 pre-existing failures in `tests/test_docs.py` (missing +`docs/creating.md`/`llms.txt` from the earlier docs-distill commit) — confirmed +present on a clean tree, unrelated to this change. + +**Postponed to later iterations (the decision-point gating — Option B remainder):** + +- `_detect_local_type` gate so `agr add ./local-ralph` ends in a generic + not-a-skill error (AC 3). +- Remote skill→ralph fallback skip in `_install_dependency` (AC 4). +- Package-expansion silent ralph skip (AC 5). +- `agr sync` silent ralph skip (AC 6). +- Leakage audit across those off-path branches (AC 9). + +Note: the installer guard currently raises `RalphNotFoundError`, which the remote +add fallback catches and then tries package — that is acceptable backstop +behaviour, but the primary non-leaking semantics for AC 3–6 still need the +decision-point gates above. + +## Iteration 2 — Decision-point gates (2026-06-07) + +**Shipped (the Option B remainder — primary gates at every ralph-install +decision point):** + +- `agr/commands/add.py`: + - `_detect_local_type`: `has_ralph = is_valid_ralph_dir(...) and + feature_enabled("ralph")`. Off → a `RALPH.md` dir (and a both-markers dir) + resolves to skill and falls through to the existing not-a-skill error; no + "contains both" raise, no flag mention (AC 3). + - `_install_dependency`: remote skill→ralph fallback wrapped in + `if feature_enabled("ralph")`. Off → skill→package only, normal not-found + (AC 4). + - `_install_package`: `if dep.is_ralph and not feature_enabled("ralph"): + continue` before dispatch — ralph leaves dropped silently, non-ralph leaves + install as today (AC 5). +- `agr/commands/sync.py`: + - `_classify_dependencies`: ralph dep with flag off → `SyncResult.up_to_date()` + and `continue` (never enters `pending_ralph`, prints nothing) (AC 6). + - `_sync_one_dependency` (locked path): ralph dep with flag off returns + `up_to_date()` before any clone/install work (AC 6). +- Leakage audit (AC 9): no user-facing strings added; off-paths reuse existing + generic errors and silent up-to-date results. Only "gated"/"flag" words are + code comments. + +**Tests:** +- `tests/unit/test_add.py`: `TestDetectLocalTypeFeatureGate` (ralph dir → skill + off / ralph on; both-markers no raise off) + off-path remote-fallback-skip and + package-ralph-leaf-skip tests asserting `fetch_and_install_ralph` not called. +- `tests/unit/test_sync.py` (new): `_classify_dependencies` ralph skipped + silently + skill still queued when off; ralph queued when on. + +**Acceptance criteria covered:** 3, 4, 5, 6, 9, and the remainder of 10/11. With +iteration 1 this closes the full task. + +**Verification:** `uv run ruff check .`, `uv run ruff format --check`, `uv run ty +check` pass. `uv run pytest`: 1308 passed, 5 skipped. Same 6 pre-existing +`tests/test_docs.py` failures (missing `docs/creating.md`/`llms.txt`), unrelated. +`agr`/`agrx` both consume `agr.features`, so unification holds. + +## Iteration 3 — Contributor doc for the feature-flag registry (2026-06-07) + +**Shipped (polish; the task's ACs were already fully covered by iterations 1–2):** + +- New `docs/contributing/feature-flags.md`: explains the env-var registry, the + truthy-value set, how to add a new gated feature (one-line registry entry + + decision-point gates + defense-in-depth guard), and the "keep a dark feature + dark" rules (no leakage, gate at the right altitude, silent skips). Uses ralph + as the worked example with links to the gated files. +- `mkdocs.yml`: added `contributing/**` to `exclude_docs` so the note ships in + the repo for contributors but is **not** published to agr.run — keeping the + ralph feature dark for end users (no public mention of `AGR_ENABLE_RALPH`). + +**Verification:** `uv run mkdocs build --strict` succeeds; `site/contributing/` +is absent, confirming the doc is excluded from the published site. No code +changed, so the test suite is unaffected (the 6 pre-existing `tests/test_docs.py` +failures remain out of scope). + +This makes the "adding a second flag is one line" promise discoverable for +contributors and closes out the task. diff --git a/tasks/2026-06-07-feature-flags-gate-ralph-install/TASK.md b/tasks/2026-06-07-feature-flags-gate-ralph-install/TASK.md new file mode 100644 index 0000000..b83ad81 --- /dev/null +++ b/tasks/2026-06-07-feature-flags-gate-ralph-install/TASK.md @@ -0,0 +1,127 @@ +## Task + +Introduce a general, reusable feature-flag mechanism for `agr`, resolved from +environment variables, and make "installing ralph loops" the first feature +behind it. By default the flag is off and ralph installation is invisible: any +attempt to install a ralph behaves as if ralphs aren't a concept — a generic +not-found, with no hint that a flag exists. The system is built as a small +registry so adding a second flag later is a one-line change. + +## Relevant Codebase + +The slice this touches is the dependency-install machinery. There is **no +existing feature-flag or env-var gating system** today — config comes entirely +from `agr.toml` via `AgrConfig` (`agr/config.py`), and the only env-var reads in +the codebase are incidental (`$EDITOR` in `agr/commands/config_cmd.py`, git +helpers in `agr/git.py`). This is net-new. + +**Ralph is a first-class dependency type but installs through several paths:** + +- **Type constant & predicate**: `DEPENDENCY_TYPE_RALPH = "ralph"` + (`agr/config.py:44`); `Dependency.is_ralph` (`agr/config.py:244`). +- **Single install choke point**: `fetch_and_install_ralph` + (`agr/ralph_installer.py`) — called from **add.py** (`add.py:166`, `:196`, + `:276`) and **sync.py** (`sync.py:478`, `:919`). +- **Local type detection**: `_detect_local_type` (`agr/commands/add.py:64`) + returns `DEPENDENCY_TYPE_RALPH` when a local dir contains `RALPH.md` + (`is_valid_ralph_dir`, `agr/ralph.py:22`). +- **Remote skill→ralph fallback**: `_install_dependency` + (`agr/commands/add.py:176-208`) tries skill first, then falls back to ralph, + then package. +- **Package expansion**: `_install_package` installs ralph-typed transitive + deps in its loop (`agr/commands/add.py:275`). +- **Sync**: installs ralph deps already pinned in `agr.toml`/lockfile + (`agr/commands/sync.py:538`, `:607`, `:894-919`); existence check via + `is_ralph_installed` (`agr/ralph_installer.py:347`). + +**How it works (control flow):** `agr add ` → `run_add` +(`add.py:387`) → per-ref `parse_handle` → type detection (local via +`_detect_local_type`, remote defaults to skill) → `_install_dependency` → +ralph branch calls `fetch_and_install_ralph`. `agr sync` reads deps from +`agr.toml`/lockfile and dispatches each by `dep.is_ralph` to the same installer. + +**Patterns to follow:** +- New runtime config is read in small dedicated modules with focused functions + (mirror the style of `agr/config.py` helpers). +- Errors raised via `agr/exceptions.py` types; user-facing messaging through + `agr/console.py` (`error_exit`, `get_console`). +- Keep runtime deps minimal; prefer stdlib (per `CLAUDE.md`). +- `agr` and `agrx` must stay unified/synced; tests required for new behavior; + no external services or API keys in tests. + +**Integration points:** the gate consumes the new feature-flag module and is +applied at the ralph type-detection and install decision points listed above. +Removal (`agr remove`), upgrade (`agr upgrade`), listing (`agr list`), and +*running* ralphs are unaffected. + +## Goal + +A user who has not set the ralph feature env var experiences `agr` as if ralph +installation does not exist: `agr add`, `agr sync`, and package expansion never +install a ralph, and nothing in the output reveals that a hidden feature or flag +is involved. A user who sets the env var gets today's full ralph-install +behavior back, unchanged. Adding a second gated feature later requires only +registering its name and env var. + +## Acceptance Criteria + +1. A reusable feature-flag module exists (e.g. `agr/features.py`) exposing a way + to check whether a named feature is enabled (e.g. `feature_enabled("ralph")`), + resolved from an environment variable via a name→env-var registry. +2. Env-var truthiness is parsed consistently (documented set of truthy values, + e.g. `1`/`true`/`yes`, case-insensitive); unset or non-truthy means off. +3. With the flag **off** (default), `agr add ./local-ralph` (a dir containing + `RALPH.md`) does NOT install a ralph — `_detect_local_type` does not return + the ralph type, and the command ends in a generic not-found/not-a-skill + error with no mention of any flag. +4. With the flag **off**, `agr add owner/repo/x` (remote) skips the skill→ralph + fallback entirely (skill→package only) and produces a normal not-found error + with no mention of any flag. +5. With the flag **off**, `agr add ` whose expansion contains ralph + deps installs the non-ralph leaves and skips the ralph leaves silently. +6. With the flag **off**, `agr sync` with ralph deps pinned in + `agr.toml`/lockfile skips them silently and installs the rest. +7. Defense-in-depth: `fetch_and_install_ralph` cannot install a ralph when the + flag is off, regardless of caller. +8. With the flag **on**, every path above installs ralphs exactly as it does + today (no behavior change). +9. No user-facing error or output text, in any off-path, reveals the existence + of the flag or feature. +10. Tests cover: flag resolution (set/unset/truthy/non-truthy values); each + install path blocked when off and working when on; package-skip and + sync-skip behavior; and absence of flag-related leakage in error text. +11. `agr` and `agrx` remain unified; `uv run pytest`, `uv run ruff check .`, + and `uv run ty check` pass. + +## Scope + +### In scope +- A general env-var-backed feature-flag system with a registry. +- Gating all ralph **install** paths: local add, remote add fallback, package + expansion, and sync — plus a guard at `fetch_and_install_ralph`. +- Silent/not-found off-behavior with no flag leakage. +- Tests for the above. + +### Out of scope +- `agr.toml`-based flag configuration (env var only for now). +- Gating non-install ralph operations: `agr remove`, `agr upgrade`, + `agr list` rendering, and *running* ralphs. +- Gating any feature other than ralph install (system is reusable, but ralph is + the only consumer in this task). + +## Risks + +- **Multiple choke points, not one.** Correct "silent not-found" semantics need + the gate at the *type-detection / fallback* decision points (so errors read + naturally), not only as a raise inside `fetch_and_install_ralph`. A single + hard raise there would produce wrong error semantics (e.g. the remote add + fallback catches `RalphNotFoundError` then tries package). The installer + guard is defense-in-depth, not the primary gate. +- **Leakage of flag existence.** Off-path error/help text must be audited so no + message hints at the flag — easy to miss in fallback or skip branches. +- **Two small details to settle during implementation** (left open by design): + (a) for sync/package, silent skip (the chosen assumption) vs. a one-line + warning; (b) the exact env var name (e.g. `AGR_ENABLE_RALPH`) and the precise + truthy-value set. +- **agr/agrx sync.** Ensure the flag check is reachable/consistent across both + CLIs per the repo's unification rule. diff --git a/tests/conftest.py b/tests/conftest.py index a57518f..82722fd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -13,6 +13,17 @@ def pytest_configure(config): config.addinivalue_line("markers", "network: tests that make real network requests") +@pytest.fixture(autouse=True) +def enable_ralph_feature(monkeypatch): + """Enable the ralph feature flag by default for the test suite. + + Ralph install is gated behind AGR_ENABLE_RALPH. Most tests assume the + feature is on (today's behaviour), so enable it globally. Tests that + exercise the flag-off path unset it explicitly via monkeypatch. + """ + monkeypatch.setenv("AGR_ENABLE_RALPH", "1") + + @pytest.fixture(autouse=True) def skip_e2e_in_ci(request): """Auto-skip E2E tests in CI based on SKIP_E2E env var.""" diff --git a/tests/test_features.py b/tests/test_features.py new file mode 100644 index 0000000..e1dcf12 --- /dev/null +++ b/tests/test_features.py @@ -0,0 +1,27 @@ +"""Tests for the env-var-backed feature-flag module.""" + +import pytest + +from agr.features import feature_enabled + + +class TestFeatureEnabled: + """Resolution of the ralph feature flag from its env var.""" + + def test_unset_is_off(self, monkeypatch): + monkeypatch.delenv("AGR_ENABLE_RALPH", raising=False) + assert feature_enabled("ralph") is False + + @pytest.mark.parametrize("value", ["1", "true", "TRUE", "Yes", "on", " true "]) + def test_truthy_values_enable(self, monkeypatch, value): + monkeypatch.setenv("AGR_ENABLE_RALPH", value) + assert feature_enabled("ralph") is True + + @pytest.mark.parametrize("value", ["", "0", "false", "no", "off", "maybe"]) + def test_non_truthy_values_disable(self, monkeypatch, value): + monkeypatch.setenv("AGR_ENABLE_RALPH", value) + assert feature_enabled("ralph") is False + + def test_unknown_feature_raises(self): + with pytest.raises(KeyError): + feature_enabled("does-not-exist") diff --git a/tests/test_ralph_installer.py b/tests/test_ralph_installer.py index 38a963c..ba367cd 100644 --- a/tests/test_ralph_installer.py +++ b/tests/test_ralph_installer.py @@ -110,3 +110,46 @@ def test_rollback_on_content_hash_failure(self, tmp_path, ralph_fixture): # The installed path should be cleaned up by rollback assert not installed_path.exists() + + +class TestFetchAndInstallRalphFeatureGate: + """Defense-in-depth: the installer refuses when the flag is off.""" + + def test_flag_off_blocks_install_without_leaking( + self, tmp_path, ralph_fixture, monkeypatch + ): + """With AGR_ENABLE_RALPH unset, no ralph is installed and the error + text reveals nothing about a feature flag.""" + monkeypatch.delenv("AGR_ENABLE_RALPH", raising=False) + repo_root = tmp_path / "repo" + repo_root.mkdir() + (repo_root / ".git").mkdir() + + handle = ParsedHandle( + is_local=True, name=ralph_fixture.name, local_path=ralph_fixture + ) + + with pytest.raises(RalphNotFoundError) as exc_info: + fetch_and_install_ralph(handle, repo_root) + + message = str(exc_info.value).lower() + assert "flag" not in message + assert "AGR_ENABLE_RALPH".lower() not in message + assert "feature" not in message + # Nothing should have been installed. + assert not get_ralphs_dir(repo_root).exists() + + def test_flag_on_allows_install(self, tmp_path, ralph_fixture, monkeypatch): + """With the flag on, a local ralph installs as it does today.""" + monkeypatch.setenv("AGR_ENABLE_RALPH", "1") + repo_root = tmp_path / "repo" + repo_root.mkdir() + (repo_root / ".git").mkdir() + + handle = ParsedHandle( + is_local=True, name=ralph_fixture.name, local_path=ralph_fixture + ) + + path, _ = fetch_and_install_ralph(handle, repo_root) + assert path.exists() + assert (path / RALPH_MARKER).exists() diff --git a/tests/unit/test_add.py b/tests/unit/test_add.py index 491e904..9c32c4e 100644 --- a/tests/unit/test_add.py +++ b/tests/unit/test_add.py @@ -6,11 +6,17 @@ from agr.exceptions import RalphNotFoundError, SkillNotFoundError from agr.commands.add import ( AddInstallResult, + _detect_local_type, _install_dependency, _install_package, _update_lockfile_for_adds, ) -from agr.config import AgrConfig, Dependency +from agr.config import ( + DEPENDENCY_TYPE_RALPH, + DEPENDENCY_TYPE_SKILL, + AgrConfig, + Dependency, +) from agr.handle import ParsedHandle from agr.lockfile import LockedEntry, load_lockfile from agr.package import ExpandedDeps @@ -95,6 +101,88 @@ def test_install_package_returns_lock_entries_for_package_and_children(self): "owner/repo/bundle" ) + def test_remote_ralph_fallback_skipped_when_flag_off(self, monkeypatch): + """Off-path: remote add goes skill->package, never trying ralph.""" + monkeypatch.delenv("AGR_ENABLE_RALPH", raising=False) + with ( + patch( + "agr.commands.add.fetch_and_install_to_tools", + side_effect=SkillNotFoundError("no skill"), + ), + patch( + "agr.commands.add.fetch_and_install_ralph", + ) as fetch_ralph, + patch( + "agr.commands.add._install_package", + return_value=AddInstallResult([], InstallResult(), "package"), + ) as install_package, + ): + result = _install_dependency( + ParsedHandle(username="owner", repo="repo", name="bundle"), + "skill", + MagicMock(), + [], + False, + MagicMock(), + None, + None, + "skills", + config=AgrConfig(), + ) + + assert result.dep_type == "package" + install_package.assert_called_once() + fetch_ralph.assert_not_called() + + def test_package_ralph_leaf_skipped_when_flag_off(self, monkeypatch): + """Off-path: ralph leaves are silently dropped, skill leaves install.""" + monkeypatch.delenv("AGR_ENABLE_RALPH", raising=False) + expanded = ExpandedDeps( + dependencies=[ + Dependency(type="skill", handle="owner/repo/child-skill"), + Dependency(type="ralph", handle="owner/repo/child-ralph"), + ], + parents={ + "owner/repo/child-skill": "owner/repo/bundle", + "owner/repo/child-ralph": "owner/repo/bundle", + }, + package_entries=[], + ) + + with ( + patch("agr.commands.add.expand_packages", return_value=expanded), + patch( + "agr.commands.add.detect_conflicts", + side_effect=lambda deps, parents, direct_ids: deps, + ), + patch( + "agr.commands.add.fetch_and_install_to_tools", + return_value=( + {"claude": MagicMock()}, + InstallResult( + commit="a" * 40, + content_hash="sha256:aaa", + source_name="github", + ), + ), + ) as fetch_skill, + patch("agr.commands.add.fetch_and_install_ralph") as fetch_ralph, + ): + _install_package( + ParsedHandle(username="owner", repo="repo", name="bundle"), + MagicMock(), + [MagicMock()], + False, + MagicMock(), + None, + None, + "skills", + config=AgrConfig(), + ) + + fetch_skill.assert_called_once() + fetch_ralph.assert_not_called() + def test_remote_dependency_falls_back_to_package(self): with ( patch( @@ -300,3 +388,27 @@ def test_skill_handle_rewritten_with_resolved_repo(self, tmp_path): assert lockfile is not None assert len(lockfile.skills) == 1 assert lockfile.skills[0].handle == "owner/skills/my-skill" + + +class TestDetectLocalTypeFeatureGate: + """Off-path: a local RALPH.md dir is invisible when the flag is off.""" + + def test_ralph_dir_detected_as_skill_when_flag_off(self, tmp_path, monkeypatch): + monkeypatch.delenv("AGR_ENABLE_RALPH", raising=False) + (tmp_path / "RALPH.md").write_text("# ralph\n") + + assert _detect_local_type(tmp_path) == DEPENDENCY_TYPE_SKILL + + def test_ralph_dir_detected_as_ralph_when_flag_on(self, tmp_path, monkeypatch): + monkeypatch.setenv("AGR_ENABLE_RALPH", "1") + (tmp_path / "RALPH.md").write_text("# ralph\n") + + assert _detect_local_type(tmp_path) == DEPENDENCY_TYPE_RALPH + + def test_both_markers_does_not_raise_when_flag_off(self, tmp_path, monkeypatch): + """With ralph gated off, a both-markers dir is just a skill (no error).""" + monkeypatch.delenv("AGR_ENABLE_RALPH", raising=False) + (tmp_path / "RALPH.md").write_text("# ralph\n") + (tmp_path / "SKILL.md").write_text("# skill\n") + + assert _detect_local_type(tmp_path) == DEPENDENCY_TYPE_SKILL diff --git a/tests/unit/test_sync.py b/tests/unit/test_sync.py new file mode 100644 index 0000000..27bec00 --- /dev/null +++ b/tests/unit/test_sync.py @@ -0,0 +1,56 @@ +"""Unit tests for agr.commands.sync feature-gating of ralph install.""" + +from pathlib import Path +from unittest.mock import patch + +from agr.commands.sync import SyncStatus, _classify_dependencies +from agr.config import AgrConfig, Dependency + + +class TestClassifyDependenciesRalphGate: + """Off-path: pinned ralph deps are skipped silently during classification.""" + + def _config(self) -> AgrConfig: + return AgrConfig( + dependencies=[ + Dependency(type="skill", handle="owner/repo/my-skill"), + Dependency(type="ralph", handle="owner/repo/my-ralph"), + ] + ) + + def test_ralph_skipped_silently_when_flag_off(self, monkeypatch): + monkeypatch.delenv("AGR_ENABLE_RALPH", raising=False) + config = self._config() + + # The skill still needs installing so we can confirm "installs the rest". + with ( + patch( + "agr.commands.sync._resolve_tools_needing_install", + return_value=["claude"], + ), + patch("agr.commands.sync.is_ralph_installed", return_value=False), + ): + classified = _classify_dependencies(config, Path("/repo"), []) + + # Ralph dep (index 1) is marked up-to-date and never queued. + assert classified.results[1].status == SyncStatus.UP_TO_DATE + assert classified.pending_ralph == [] + # The skill leaf is still queued for install. + assert len(classified.pending_remote) == 1 + assert classified.pending_remote[0].index == 0 + + def test_ralph_queued_when_flag_on(self, monkeypatch): + monkeypatch.setenv("AGR_ENABLE_RALPH", "1") + config = self._config() + + with ( + patch( + "agr.commands.sync._resolve_tools_needing_install", + return_value=["claude"], + ), + patch("agr.commands.sync.is_ralph_installed", return_value=False), + ): + classified = _classify_dependencies(config, Path("/repo"), []) + + assert len(classified.pending_ralph) == 1 + assert classified.pending_ralph[0].index == 1