diff --git a/.github/workflows/launchpad-agents-tests.yml b/.github/workflows/launchpad-agents-tests.yml new file mode 100644 index 00000000000..0a97c40b917 --- /dev/null +++ b/.github/workflows/launchpad-agents-tests.yml @@ -0,0 +1,81 @@ +name: launchpad — agents tests + +# Runs the unit tests for launchpad/agents/*.py. +# +# WHY THIS WORKFLOW EXISTS. Until it did, NO CI job ran anything under +# launchpad/agents/ — review-code found the same High finding independently on +# two separate pull requests (#260's 20 tests and #262's 25 tests, both green +# locally and both never executed by CI). A test suite nothing runs is a claim, +# not a check: it cannot fail, so it cannot protect anything. +# +# It also catches the failure mode that made the gap visible. goose_config.py +# imports ruamel.yaml, a third-party package the repo recorded nowhere. On a +# machine without it the module raises ImportError before a single test runs, and +# nothing would have reported that. Installing from the requirements file here +# means an unrecorded dependency now breaks CI rather than breaking a reader. +# +# `pull_request`, deliberately NOT `pull_request_target` — the suite under test +# lives in the repository, so a pull request can modify the very code this job +# runs. On `pull_request` that code executes with the fork's own permissions and +# no access to repository secrets. Same reasoning as +# launchpad-review-agent-controls.yml, which this mirrors. + +on: + pull_request: + paths: + - "launchpad/agents/**" + - ".github/workflows/launchpad-agents-tests.yml" + push: + branches: [launchpad] + paths: + - "launchpad/agents/**" + +# Read-only. These are pure unit tests against temp-directory fixtures: they +# reach no network, no relay, and no GitHub API, so they need no token scope. +permissions: + contents: read + +jobs: + tests: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Install agent script dependencies + run: pip install -r launchpad/agents/requirements.txt + + # Fails if the suite contains no test CASES, rather than reporting success + # for a run that executed nothing. `unittest discover` exits 0 on an empty + # suite, which is indistinguishable from a pass in the CI summary — the + # exact shape of the gap this workflow was added to close. + # + # COUNTS CASES, NOT FILES, and the first version of this guard counted + # files — which left the very hole it existed to close. Found by + # cross-vendor review: leave test_goose_config.py in place but rename every + # `test_*` method to `check_*`, and a file-counting guard reports one test + # file while `unittest discover` collects zero cases and exits 0. A guard + # that can be satisfied by a filename is not a guard. + # + # A module that fails to import counts as one case here (the loader + # substitutes a `_FailedTest`), so it passes this step and then fails the + # run below — which is the correct split: this step answers "is there + # anything to run", the run answers "does it pass". + - name: Confirm test cases were discovered + run: | + python3 - <<'PY' + import sys, unittest + suite = unittest.defaultTestLoader.discover("launchpad/agents", pattern="test_*.py") + n = suite.countTestCases() + print(f"discovered {n} test case(s)") + if n == 0: + print("::error::launchpad/agents has no discoverable test cases — " + "this job would have passed vacuously.") + sys.exit(1) + PY + + - name: Run agents unit tests + run: python3 -m unittest discover -s launchpad/agents -p "test_*.py" -v diff --git a/launchpad/agents/goose_config.py b/launchpad/agents/goose_config.py new file mode 100644 index 00000000000..a24f440ad95 --- /dev/null +++ b/launchpad/agents/goose_config.py @@ -0,0 +1,253 @@ +#!/usr/bin/env python3 +"""Read-merge-write goose's config.yaml to enable the `developer` extension. + +STEP 4 of issue #239 (the Route 3 projector): goose's write/shell capability +is a config-FILE toggle, not an env var -- +`desktop/src-tauri/src/managed_agents/config_bridge/goose.rs` only ever +*reads* `extensions.developer` out of this file, and nothing in this +repository writes it. This module builds read-merge-write from scratch: read +the existing file if present (empty mapping otherwise), preserve every +existing key untouched, set `extensions.developer = {type: builtin, enabled: +true}`, and write atomically (temp file + rename in the same directory) so a +crash mid-write cannot leave a half-written config an operator's next +`goose` invocation trips over. Running it twice against the same file is a +no-op, not an append-again. + +Uses ruamel.yaml's round-trip mode (not PyYAML) specifically so an operator's +comments and quoting style survive a merge -- a plain load+dump strips both +on every write, confirmed to lose a hand-written "# managed by ansible" +comment and turn a quoted host string unquoted. That dependency is recorded +in `launchpad/agents/requirements.txt` and installed in CI by +`.github/workflows/launchpad-agents-tests.yml`; locally, either +`pip install -r launchpad/agents/requirements.txt` or the +`python3-ruamel.yaml` apt package works. + +Known limitation: no file locking across the read-modify-write window, so a +goose process rewriting its own config.yaml at the same moment this script +runs could lose one side's update. Each individual write stays atomic +(temp file + rename), so this is a lost-update race, not corruption -- the +plan's own atomicity requirement is about surviving a crash mid-write, not +about concurrent writers, and this module does not attempt the latter. + +Does not wire into project-pack.py (STEP 5) -- this is the goose-config half +only. + +Usage: + python3 launchpad/agents/goose_config.py --enable-developer +""" + +from __future__ import annotations + +import argparse +import os +import stat +import sys +import tempfile +from pathlib import Path + +from ruamel.yaml import YAML +from ruamel.yaml.comments import CommentedMap + + +class GooseConfigError(RuntimeError): + """goose's config.yaml could not be read or merged -- always fails + loudly, never silently discards or guesses at data the caller would + act on.""" + + +def _yaml() -> YAML: + y = YAML() + y.preserve_quotes = True + return y + + +def goose_config_path(env: dict | None = None) -> Path: + """Mirrors goose.rs's `goose_config_path()` exactly, including its + edge case: `std::env::var("GOOSE_PATH_ROOT")` returns `Ok("")` for a + set-but-empty variable, so Rust does NOT treat empty as unset -- an + explicitly set (even empty) GOOSE_PATH_ROOT wins here too, rather than + silently falling back to a different path than the one goose itself + would resolve.""" + env = env if env is not None else os.environ + if "GOOSE_PATH_ROOT" in env: + return Path(env["GOOSE_PATH_ROOT"]) / "config" / "config.yaml" + return Path.home() / ".config" / "goose" / "config.yaml" + + +def read_config(path: Path) -> CommentedMap: + """The parsed mapping at `path`, or an empty mapping if the file does + not exist or is empty. Raises GooseConfigError on invalid YAML or a + non-mapping top-level value, rather than crashing with a raw + exception or silently discarding the file's contents.""" + if not path.exists(): + return CommentedMap() + try: + with path.open("r", encoding="utf-8") as f: + loaded = _yaml().load(f) + except Exception as exc: + raise GooseConfigError(f"{path} is not valid YAML: {exc}") from exc + if loaded is None: + return CommentedMap() + if not isinstance(loaded, dict): + raise GooseConfigError( + f"{path}'s top-level YAML value is a {type(loaded).__name__}, " + "not a mapping -- refusing to merge into it" + ) + return loaded + + +def merge_developer_extension(config: dict) -> dict: + """Returns a NEW mapping with `extensions.developer` enabled. Every + other top-level key, and every other extension, is preserved untouched + (comments and quoting included, when `config` came from `read_config`). + Idempotent: merging an already-merged mapping returns an equal + mapping -- `developer`'s existing position in `extensions` is kept + rather than moved to the end, so a second write matches the first + byte-for-byte. + + Raises GooseConfigError if an existing `extensions` key is present but + is not itself a mapping.""" + merged = config.copy() if isinstance(config, CommentedMap) else CommentedMap(config) + raw_extensions = merged.get("extensions") + if raw_extensions is None: + extensions = CommentedMap() + elif isinstance(raw_extensions, dict): + extensions = ( + raw_extensions.copy() + if isinstance(raw_extensions, CommentedMap) + else CommentedMap(raw_extensions) + ) + else: + raise GooseConfigError( + f"'extensions' is a {type(raw_extensions).__name__}, not a " + "mapping -- refusing to merge into it" + ) + extensions["developer"] = {"type": "builtin", "enabled": True} + merged["extensions"] = extensions + return merged + + +def write_config_atomic(path: Path, config: dict) -> None: + """Writes `config` to `path` via a temp file in the same directory, + then an atomic rename over the original -- a crash mid-write leaves + either the old file or the new one, never a half-written one. + + If `path` is a symlink, writes through it (onto the resolved real + target) rather than replacing the symlink itself -- otherwise a + dotfile-managed config (Stow, chezmoi, a manual symlink into a synced + repo) silently loses its symlink on the first run. + + If `path` already exists, the new file keeps its permission mode + (`tempfile.mkstemp` otherwise always creates at 0600, which would + silently narrow an existing 0644 file's permissions on every write) + and its LF-or-CRLF line-ending convention (a CRLF file written by a + Windows operator was silently rewritten to LF, which is the same class + of unasked-for edit as losing their comments -- and this repository does + support Windows, so it is not hypothetical). A file containing any CRLF + is treated as a CRLF file; mixed endings are normalised to CRLF rather + than preserved per-line. LF and CRLF are the only conventions handled: + a lone-CR (pre-OS X Mac) file normalises to LF, which is a limitation + of this function rather than a preserved convention.""" + path.parent.mkdir(parents=True, exist_ok=True) + real_path = path.resolve() if path.is_symlink() else path + real_path.parent.mkdir(parents=True, exist_ok=True) + + existing_mode = None + newline = "\n" + if real_path.exists(): + existing_mode = real_path.stat().st_mode + # Read in full rather than sampling a prefix: a file can be LF for a + # hundred lines and CRLF after. These configs are a few KB at most. + if b"\r\n" in real_path.read_bytes(): + newline = "\r\n" + + fd, tmp_name = tempfile.mkstemp( + dir=str(real_path.parent), prefix=f".{real_path.name}.", suffix=".tmp" + ) + try: + if existing_mode is not None: + os.chmod(fd, stat.S_IMODE(existing_mode)) + # newline= is what preserves the convention: the YAML dumper always + # emits "\n", and Python translates it on the way out. + with os.fdopen(fd, "w", encoding="utf-8", newline=newline) as f: + _yaml().dump(config, f) + os.replace(tmp_name, real_path) + except Exception: + try: + os.unlink(tmp_name) + except OSError: + pass + raise + + +def enable_developer_extension( + path: Path | None = None, env: dict | None = None +) -> Path: + """Read-merge-write entry point: enables goose's `developer` extension + at `path` (default: `goose_config_path(env)`). Returns the path + written. + + WHAT THIS GRANTS, STATED WHERE IT HAPPENS. goose's `developer` extension is + its shell-and-filesystem tool: enabling it gives the agent that loads this + config the ability to run commands and write files as the invoking user. + That is the entire point for #239 (The Professor could draft a page but had + no tool that could save it), and it is also a real expansion of blast + radius. + + The issue-#239 plan's OPEN item 2 records that this decision is NOT settled + for live or unattended operation, and names who must settle it: "Who + arbitrates whether goose's `developer` extension is safe enough to enable + for a live/unattended run later -- not decided here [...] a live, unattended + agent with real shell access is a materially different blast radius than a + human-triggered local session under BYOK." + + So this function is scoped to the human-triggered local proof the plan + sanctions (STEP 7). It must not be wired into an unattended or + cohort-facing runtime until that OPEN item has an answer, and this docstring + is deliberately the place a reader finds that out -- the plan is not in + scope for someone reading the module. + """ + target = path if path is not None else goose_config_path(env) + current = read_config(target) + merged = merge_developer_extension(current) + write_config_atomic(target, merged) + return target + + +def main(argv: list[str] | None = None) -> int: + # First line only, matching project-pack.py's own `__doc__.splitlines()[0]`. + # The full docstring is 40+ lines of rationale aimed at a reader of the + # source; dumping all of it into `--help` buries the two flags a caller + # actually needs to see. + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--enable-developer", + action="store_true", + help="enable goose's developer (shell/write) extension in config.yaml", + ) + parser.add_argument( + "--path", + type=Path, + default=None, + help=( + "override goose's config.yaml path (default: GOOSE_PATH_ROOT or " + "~/.config/goose/config.yaml)" + ), + ) + args = parser.parse_args(argv) + + if not args.enable_developer: + parser.error("nothing to do -- pass --enable-developer") + + try: + target = enable_developer_extension(path=args.path) + except GooseConfigError as exc: + print(f"error: {exc}", file=sys.stderr) + return 1 + + print(f"enabled developer extension in {target}", file=sys.stderr) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/launchpad/agents/requirements.txt b/launchpad/agents/requirements.txt new file mode 100644 index 00000000000..14d412cacae --- /dev/null +++ b/launchpad/agents/requirements.txt @@ -0,0 +1,37 @@ +# Third-party dependencies for launchpad/agents/*.py. +# +# WHY THIS FILE EXISTS. `goose_config.py` imports ruamel.yaml, and until this file +# existed that dependency was recorded nowhere: not in a manifest, not in a CI +# install step, only in prose inside the module's own docstring. A reader who +# cloned the repo and ran the script got an ImportError, and CI could not have +# caught it because no CI job ran these tests at all +# (.github/workflows/launchpad-agents-tests.yml now does). +# +# ruamel.yaml rather than PyYAML, deliberately, and it is not interchangeable: +# goose_config.py rewrites an operator's own ~/.config/goose/config.yaml, and +# PyYAML's load+dump silently strips every comment and re-quotes every scalar on +# the way through. Measured on a fixture carrying `# managed by ansible` and a +# quoted host value: both were gone after one round trip. ruamel's round-trip +# mode preserves them. Anything that edits a human-authored YAML file in place +# needs the round-trip parser; PyYAML is fine for files only machines write. +# +# BOUNDED RANGE, both ends deliberate. +# +# Floor 0.17.21: that is what the python3-ruamel.yaml apt package ships on +# Ubuntu 24.04, and pinning above it would make the apt route unusable for no +# gain. (An earlier revision of this comment claimed an exact pin "would reject +# the newer wheel pip installs" — that was simply wrong, pip installs whatever +# version you name if it is still on the index. Corrected by cross-vendor +# review; the real reason for a range is the apt/pip split, not pip behaviour.) +# +# Ceiling <0.20: a conservative "do not enter an untested release series" +# boundary, NOT a major-version boundary. ruamel.yaml's own documentation puts +# its eventual major transition at 1.0, so 0.20 is simply the next 0.x release +# series -- an earlier revision of this comment called it "the next major +# series", which was wrong (corrected by cross-vendor review). The reason for +# any ceiling at all is that this module's whole purpose is round-tripping an +# operator's hand-written config, so a serializer change in a release series +# nobody here has tested could silently alter their file with no commit in this +# repo to point at. Raise it deliberately, after checking round-tripping still +# holds, rather than leaving it open. +ruamel.yaml>=0.17.21,<0.20 diff --git a/launchpad/agents/test_goose_config.py b/launchpad/agents/test_goose_config.py new file mode 100644 index 00000000000..278ee04f9af --- /dev/null +++ b/launchpad/agents/test_goose_config.py @@ -0,0 +1,423 @@ +#!/usr/bin/env python3 +"""Controls for goose_config.py's read-merge-write logic. + +STEP 4 of issue #239 (the Route 3 projector): goose's `developer` extension +(write/shell capability) is a config-FILE toggle +(`~/.config/goose/config.yaml`, or `$GOOSE_PATH_ROOT/config/config.yaml`), +and nothing in this repository writes that file today +(`desktop/src-tauri/src/managed_agents/config_bridge/goose.rs` is +read-only). These tests drive the module's pure functions directly, plus one +end-to-end pass against a real temp file for the atomic-write and +idempotency guarantees the plan's own done-when demands. + +Run: python3 -m unittest discover -s launchpad/agents -p "test_*.py" +""" + +from __future__ import annotations + +import importlib.util +import os +import stat +import tempfile +import unittest +from pathlib import Path + +_SPEC = importlib.util.spec_from_file_location( + "goose_config", Path(__file__).resolve().parent / "goose_config.py" +) +m = importlib.util.module_from_spec(_SPEC) +_SPEC.loader.exec_module(m) + + +class GooseConfigPathTests(unittest.TestCase): + def test_uses_goose_path_root_when_set(self): + path = m.goose_config_path({"GOOSE_PATH_ROOT": "/custom/root"}) + self.assertEqual(path, Path("/custom/root/config/config.yaml")) + + def test_defaults_to_home_config_goose_when_unset(self): + path = m.goose_config_path({}) + self.assertEqual(path, Path.home() / ".config" / "goose" / "config.yaml") + + def test_empty_goose_path_root_mirrors_rust_treating_it_as_set(self): + # Rust's std::env::var("GOOSE_PATH_ROOT") returns Ok("") for a + # set-but-empty variable, so goose.rs's own resolution does NOT + # treat empty as unset. This module mirrors that exactly, even + # though it is arguably a footgun -- diverging would mean this + # script patches a different file than the one goose itself reads. + path = m.goose_config_path({"GOOSE_PATH_ROOT": ""}) + self.assertEqual(path, Path("config/config.yaml")) + + +class ReadConfigTests(unittest.TestCase): + def test_missing_file_returns_empty_dict(self): + with tempfile.TemporaryDirectory() as d: + self.assertEqual(m.read_config(Path(d) / "does-not-exist.yaml"), {}) + + def test_reads_existing_mapping(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_text("active_provider: anthropic\n", encoding="utf-8") + self.assertEqual(m.read_config(path), {"active_provider": "anthropic"}) + + def test_empty_file_returns_empty_dict(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_text("", encoding="utf-8") + self.assertEqual(m.read_config(path), {}) + + def test_invalid_yaml_raises_goose_config_error(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_text("{{{{not valid", encoding="utf-8") + with self.assertRaises(m.GooseConfigError): + m.read_config(path) + + def test_non_mapping_top_level_raises_goose_config_error(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_text("- just\n- a\n- list\n", encoding="utf-8") + with self.assertRaises(m.GooseConfigError): + m.read_config(path) + + def test_preserves_comments_on_round_trip(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_text( + "# managed by ansible -- do not edit by hand\n" + 'active_provider: databricks_v2 # production creds\n', + encoding="utf-8", + ) + cfg = m.read_config(path) + m.write_config_atomic(path, cfg) + written = path.read_text(encoding="utf-8") + self.assertIn("# managed by ansible -- do not edit by hand", written) + self.assertIn("# production creds", written) + + +# The fixture below is RAW TEXT on purpose, not built by calling this module's +# own writer. An earlier version of these controls built every fixture with +# write_config_atomic(), so both the "before" and the "after" had already been +# through the same serializer -- which is exactly the shape that cannot detect a +# serializer that mangles human-authored YAML. review-code found this twice +# independently. It carries a top-of-file comment, an inline comment on a +# top-level key, a comment nested two levels deep, a deliberately quoted scalar, +# and an inline comment inside `extensions` -- the block this module writes into. +OPERATOR_AUTHORED_CONFIG = """\ +# top-of-file: managed by ansible, do not edit by hand +active_provider: databricks_v2 # inline on a top-level key +providers: + databricks_v2: + # a comment nested two levels deep + model: goose-claude-4-6-opus + host: "https://dbc.example" # quoted on purpose +extensions: + my-mcp: + type: stdio # inline inside the block this module writes into + enabled: false +""" + + +class OperatorAuthoredConfigTests(unittest.TestCase): + """The guarantee this module exists for, exercised through the REAL entry + point (`enable_developer_extension`, merge included) against a file a human + wrote by hand. + + The distinction matters and was a genuine coverage gap: comment + preservation was previously only asserted across read_config -> + write_config_atomic, which skips merge_developer_extension entirely. Since + the merge copies the mapping (`config.copy()` / `CommentedMap(...)`), a copy + that dropped ruamel's comment attachments would have lost every comment on + the real path while the old test still passed.""" + + def _write_fixture(self, d: str) -> Path: + path = Path(d) / "config.yaml" + path.write_text(OPERATOR_AUTHORED_CONFIG, encoding="utf-8") + return path + + def test_merge_preserves_every_comment_and_quoting_style(self): + with tempfile.TemporaryDirectory() as d: + path = self._write_fixture(d) + m.enable_developer_extension(path=path) + written = path.read_text(encoding="utf-8") + + for probe in ( + "# top-of-file: managed by ansible, do not edit by hand", + "# inline on a top-level key", + "# a comment nested two levels deep", + "# quoted on purpose", + "# inline inside the block this module writes into", + ): + self.assertIn(probe, written, f"comment lost through merge: {probe}") + + # The quoting style itself, not just the value: PyYAML's dumper + # re-emits this unquoted, which is how the original Blocker was found. + self.assertIn('host: "https://dbc.example"', written) + + def test_merge_adds_developer_without_disturbing_existing_extension(self): + with tempfile.TemporaryDirectory() as d: + path = self._write_fixture(d) + m.enable_developer_extension(path=path) + cfg = m.read_config(path) + self.assertEqual( + cfg["extensions"]["my-mcp"], {"type": "stdio", "enabled": False} + ) + self.assertEqual( + cfg["extensions"]["developer"], {"type": "builtin", "enabled": True} + ) + self.assertEqual(cfg["active_provider"], "databricks_v2") + + def test_second_run_on_operator_authored_file_is_byte_for_byte_no_op(self): + with tempfile.TemporaryDirectory() as d: + path = self._write_fixture(d) + m.enable_developer_extension(path=path) + after_first = path.read_bytes() + m.enable_developer_extension(path=path) + self.assertEqual( + after_first, + path.read_bytes(), + "a second run against a human-authored file must be a no-op", + ) + + def test_only_addition_is_the_developer_block(self): + """BYTE-EXACT: the output is the input with the developer block appended + and nothing else changed at all. + + An earlier version of this control compared line *membership* + (`[ln for ln in written.splitlines() if ln in original_lines]`), which + cross-vendor review correctly called weaker than its own docstring + claimed: `splitlines()` discards a missing trailing newline, hides a + CRLF/LF conversion, and would tolerate the three new lines being + interleaved anywhere among the originals. Comparing the whole string + closes all three at once and needs no separate ordering argument.""" + with tempfile.TemporaryDirectory() as d: + path = self._write_fixture(d) + m.enable_developer_extension(path=path) + + expected = OPERATOR_AUTHORED_CONFIG + ( + " developer:\n" " type: builtin\n" " enabled: true\n" + ) + # read_BYTES, not read_text: `read_text` normalises newlines, so a + # CRLF/LF rewrite is invisible to it. Cross-vendor review caught the + # text-mode version of this assertion passing while the + # implementation was in fact rewriting CRLF to LF. + self.assertEqual( + path.read_bytes(), + expected.encode("utf-8"), + "output must be the operator's file byte-for-byte, plus only " + "the appended developer block", + ) + + def test_preserves_crlf_line_endings(self): + """A Windows operator's CRLF file stays CRLF. + + Measured before this was fixed: a 5-CRLF input came back with 0, and + the original bytes were not a prefix of the output — the module was + silently converting the whole file to LF, which is the same class of + unasked-for edit as dropping their comments.""" + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + original = ( + b"# managed by hand\r\n" + b"active_provider: anthropic\r\n" + b"extensions:\r\n" + b" my-mcp:\r\n" + b" type: stdio\r\n" + ) + path.write_bytes(original) + + m.enable_developer_extension(path=path) + after = path.read_bytes() + + self.assertNotIn(b"\n", after.replace(b"\r\n", b""), + "no bare LF may survive in a CRLF file") + self.assertTrue( + after.startswith(original), + "the operator's original bytes must be an exact prefix of the result", + ) + self.assertEqual( + after, + original + b" developer:\r\n type: builtin\r\n enabled: true\r\n", + ) + + def test_lf_file_stays_lf(self): + """The mirror of the CRLF case: preserving CRLF must not mean emitting + CRLF into a file that never had it.""" + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_bytes(b"active_provider: anthropic\n") + m.enable_developer_extension(path=path) + self.assertNotIn(b"\r", path.read_bytes()) + + +class MergeDeveloperExtensionTests(unittest.TestCase): + def test_adds_developer_extension_when_absent(self): + merged = m.merge_developer_extension({}) + self.assertEqual( + merged["extensions"]["developer"], {"type": "builtin", "enabled": True} + ) + + def test_preserves_unrelated_top_level_keys(self): + original = { + "active_provider": "databricks_v2", + "providers": {"databricks_v2": {"model": "goose-claude-4-6-opus"}}, + } + merged = m.merge_developer_extension(original) + self.assertEqual(merged["active_provider"], "databricks_v2") + self.assertEqual( + merged["providers"]["databricks_v2"]["model"], "goose-claude-4-6-opus" + ) + + def test_preserves_other_extensions(self): + original = { + "extensions": {"my-mcp": {"type": "stdio", "enabled": False}}, + } + merged = m.merge_developer_extension(original) + self.assertEqual( + merged["extensions"]["my-mcp"], {"type": "stdio", "enabled": False} + ) + self.assertEqual( + merged["extensions"]["developer"], {"type": "builtin", "enabled": True} + ) + + def test_does_not_mutate_the_input(self): + original = {"extensions": {"my-mcp": {"type": "stdio", "enabled": False}}} + m.merge_developer_extension(original) + self.assertNotIn("developer", original["extensions"]) + + def test_is_idempotent_when_developer_already_enabled(self): + once = m.merge_developer_extension({}) + twice = m.merge_developer_extension(once) + self.assertEqual(once, twice) + + def test_overwrites_a_disabled_developer_entry_to_enabled(self): + original = {"extensions": {"developer": {"type": "builtin", "enabled": False}}} + merged = m.merge_developer_extension(original) + self.assertEqual( + merged["extensions"]["developer"], {"type": "builtin", "enabled": True} + ) + + def test_non_mapping_extensions_key_raises_goose_config_error(self): + with self.assertRaises(m.GooseConfigError): + m.merge_developer_extension({"extensions": ["not", "a", "mapping"]}) + + +class WriteConfigAtomicTests(unittest.TestCase): + def test_creates_parent_directory_and_file(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "nested" / "config.yaml" + m.write_config_atomic(path, {"a": 1}) + self.assertTrue(path.is_file()) + self.assertEqual(m.read_config(path), {"a": 1}) + + def test_leaves_no_temp_files_behind_on_success(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + m.write_config_atomic(path, {"a": 1}) + self.assertEqual(os.listdir(d), ["config.yaml"]) + + def test_overwrites_existing_file(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_text("stale: true\n", encoding="utf-8") + m.write_config_atomic(path, {"fresh": True}) + self.assertEqual(m.read_config(path), {"fresh": True}) + + def test_preserves_existing_file_permissions(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + path.write_text("a: 1\n", encoding="utf-8") + os.chmod(path, 0o644) + m.write_config_atomic(path, {"a": 2}) + self.assertEqual(stat.S_IMODE(path.stat().st_mode), 0o644) + + def test_writes_through_a_symlink_rather_than_replacing_it(self): + with tempfile.TemporaryDirectory() as d: + real_path = Path(d) / "real-config.yaml" + m.write_config_atomic(real_path, {"stale": True}) + link_path = Path(d) / "config.yaml" + link_path.symlink_to(real_path) + + m.write_config_atomic(link_path, {"fresh": True}) + + self.assertTrue(link_path.is_symlink(), "the symlink must survive the write") + self.assertEqual(link_path.resolve(), real_path) + self.assertEqual(m.read_config(real_path), {"fresh": True}) + + +class EnableDeveloperExtensionEndToEndTests(unittest.TestCase): + """The plan's own done-when for STEP 4: run twice against a fixture + carrying an unrelated provider block and one other extension -- both + survive byte-for-byte except the added/updated developer entry, and the + second run is a no-op (idempotent, not append-again).""" + + def _fixture_path(self, d: str) -> Path: + path = Path(d) / "config.yaml" + fixture = { + "active_provider": "databricks_v2", + "providers": { + "databricks_v2": { + "model": "goose-claude-4-6-opus", + "host": "https://dbc.example", + } + }, + "extensions": {"my-mcp": {"type": "stdio", "enabled": False}}, + } + m.write_config_atomic(path, fixture) + return path + + def test_creates_file_when_none_exists(self): + with tempfile.TemporaryDirectory() as d: + path = Path(d) / "config.yaml" + self.assertFalse(path.exists()) + written = m.enable_developer_extension(path=path) + self.assertEqual(written, path) + cfg = m.read_config(path) + self.assertEqual( + cfg["extensions"]["developer"], {"type": "builtin", "enabled": True} + ) + + def test_preserves_unrelated_provider_block_and_other_extension(self): + with tempfile.TemporaryDirectory() as d: + path = self._fixture_path(d) + m.enable_developer_extension(path=path) + cfg = m.read_config(path) + self.assertEqual(cfg["active_provider"], "databricks_v2") + self.assertEqual( + cfg["providers"]["databricks_v2"]["model"], "goose-claude-4-6-opus" + ) + self.assertEqual( + cfg["extensions"]["my-mcp"], {"type": "stdio", "enabled": False} + ) + self.assertEqual( + cfg["extensions"]["developer"], {"type": "builtin", "enabled": True} + ) + + def test_second_run_is_a_byte_for_byte_no_op(self): + with tempfile.TemporaryDirectory() as d: + path = self._fixture_path(d) + m.enable_developer_extension(path=path) + after_first = path.read_bytes() + m.enable_developer_extension(path=path) + after_second = path.read_bytes() + self.assertEqual( + after_first, + after_second, + "a second run must be a no-op, not append-again", + ) + + def test_defaults_to_goose_config_path_when_no_path_given(self): + with tempfile.TemporaryDirectory() as d: + fixture_path = Path(d) / "config" / "config.yaml" + fixture_path.parent.mkdir(parents=True) + m.write_config_atomic(fixture_path, {"active_provider": "anthropic"}) + written = m.enable_developer_extension(env={"GOOSE_PATH_ROOT": d}) + self.assertEqual(written, fixture_path) + cfg = m.read_config(fixture_path) + self.assertEqual(cfg["active_provider"], "anthropic") + self.assertEqual( + cfg["extensions"]["developer"], {"type": "builtin", "enabled": True} + ) + + +if __name__ == "__main__": + unittest.main()