From f49776a25e5ecc0fc277a24515627df92f72f444 Mon Sep 17 00:00:00 2001 From: Brian Krabach Date: Sun, 20 Sep 2026 13:17:48 -0700 Subject: [PATCH] Use Foundation native names and shared session settings --- amplifier_app_cli/commands/session.py | 7 ++-- amplifier_app_cli/lib/settings.py | 48 +++------------------------ amplifier_app_cli/main.py | 33 ++---------------- amplifier_app_cli/session_runner.py | 4 ++- amplifier_app_cli/session_store.py | 21 ++++++------ docs/SHARED-SESSION-METADATA.md | 21 ++++++++++++ pyproject.toml | 4 +-- tests/test_session_runner.py | 9 ++--- tests/test_shared_root_state.py | 5 ++- tests/test_shared_session_metadata.py | 30 +++++++++++++++++ uv.lock | 5 +-- 11 files changed, 89 insertions(+), 98 deletions(-) create mode 100644 docs/SHARED-SESSION-METADATA.md create mode 100644 tests/test_shared_session_metadata.py diff --git a/amplifier_app_cli/commands/session.py b/amplifier_app_cli/commands/session.py index b3ad551d..1fdb9d97 100644 --- a/amplifier_app_cli/commands/session.py +++ b/amplifier_app_cli/commands/session.py @@ -174,6 +174,7 @@ def _prepare_resume_context( # Get project slug for session-scoped settings project_slug = get_project_slug() + app_settings = app_settings.with_session(session_id, project_slug) # Resolve configuration using unified function (single source of truth) config_data, prepared_bundle = resolve_config( @@ -1117,8 +1118,10 @@ def sessions_delete(session_id: str, force: bool): root = SharedRootSession.acquire(session_id) try: root.held.check() - if session_path.exists(): - shutil.rmtree(session_path) + from amplifier_foundation.session.metadata import metadata_lock + with metadata_lock(session_path): + if session_path.exists(): + shutil.rmtree(session_path) root.delete_checkpoint() finally: root.release() diff --git a/amplifier_app_cli/lib/settings.py b/amplifier_app_cli/lib/settings.py index 35316492..47f8377c 100644 --- a/amplifier_app_cli/lib/settings.py +++ b/amplifier_app_cli/lib/settings.py @@ -118,25 +118,10 @@ def with_session(self, session_id: str, project_slug: str) -> "AppSettings": def get_merged_settings(self) -> dict[str, Any]: """Load and merge settings from all scopes.""" - result: dict[str, Any] = {} - # Order: global -> project -> local -> session (most specific wins) - paths_to_check = [ - self.paths.global_settings, - self.paths.project_settings, - self.paths.local_settings, - ] - if self.paths.session_settings: - paths_to_check.append(self.paths.session_settings) + from amplifier_foundation.settings import read_settings - for path in paths_to_check: - if path.exists(): - try: - with open(path, encoding="utf-8") as f: - content = yaml.safe_load(f) or {} - result = self._deep_merge(result, content) - except Exception: - pass # Skip malformed files - return result + return read_settings((self.paths.global_settings, self.paths.project_settings, + self.paths.local_settings, self.paths.session_settings)) # ----- UI settings ----- @@ -382,32 +367,7 @@ def get_provider_overrides(self) -> list[dict[str, Any]]: More-specific scopes override less-specific: global < project < local < session. Providers not present in higher scopes pass through from lower scopes. """ - from .merge_utils import _provider_key, merge_module_items # noqa: F401 - - result: list[dict[str, Any]] = [] - - # Scope priority order: global (lowest) → project → local → session (highest) - paths_to_check: list[Path | None] = [ - self.paths.global_settings, - self.paths.project_settings, - self.paths.local_settings, - self.paths.session_settings, - ] - - for path in paths_to_check: - if path is None or not path.exists(): - continue - try: - with open(path, encoding="utf-8") as f: - content = yaml.safe_load(f) or {} - scope_providers = content.get("config", {}).get("providers", []) - if not isinstance(scope_providers, list) or not scope_providers: - continue - result = self._merge_provider_lists(result, scope_providers) - except Exception: - pass - - return result + return self.get_merged_settings().get("config", {}).get("providers", []) def _merge_provider_lists( self, diff --git a/amplifier_app_cli/main.py b/amplifier_app_cli/main.py index 1598711b..042ef041 100644 --- a/amplifier_app_cli/main.py +++ b/amplifier_app_cli/main.py @@ -1849,40 +1849,11 @@ async def _rename_session(self, new_name: str) -> str: session_id = self.session.coordinator.session_id try: - from datetime import UTC, datetime - from .session_store import SessionStore - from .shared_root_state import update_root_metadata store = SessionStore() - updates = { - "name": new_name[:50], # Limit name length - "name_generated_at": datetime.now(UTC).isoformat(), - } - root_state = self.session.coordinator.get_capability("cli.shared_root_state") - if root_state is not None: - # The active root already owns the lock. Preserve complete - # provider context while retaining the shared writer lock. - context = self.session.coordinator.get("context") - if context is None or not hasattr(context, "get_messages"): - return "Cannot rename: root context is unavailable." - messages = await context.get_messages() - metadata = { - **store.get_metadata_if_exists(session_id), - **updates, - "session_id": session_id, - "bundle": self.bundle_name, - } - root_state.checkpoint( - store, messages, bundle=self.bundle_name, metadata=metadata - ) - elif self.session.config.get("root_session_id", session_id) != session_id: - # Spawned children never participate in the root writer lock. - store.update_metadata(session_id, updates) - else: - update_root_metadata(store, session_id, updates) - - return f"✓ Session renamed to: {new_name[:50]}" + metadata = store.rename(session_id, new_name) + return f"✓ Session renamed to: {metadata['name']}" except Exception as e: return f"Failed to rename session: {e}" diff --git a/amplifier_app_cli/session_runner.py b/amplifier_app_cli/session_runner.py index 1f0a3df4..2b1aa1a7 100644 --- a/amplifier_app_cli/session_runner.py +++ b/amplifier_app_cli/session_runner.py @@ -425,7 +425,9 @@ async def create_initialized_session( from amplifier_foundation.configurator import SessionConfigurator configurator = SessionConfigurator(session, config.prepared_bundle) - app_settings = AppSettings() + from .project_utils import get_project_slug + + app_settings = AppSettings().with_session(session_id, get_project_slug()) merged = app_settings.get_merged_settings() configurator_settings = merged.get("configurator") or {} await configurator.apply_saved_settings(configurator_settings) diff --git a/amplifier_app_cli/session_store.py b/amplifier_app_cli/session_store.py index ea96bb43..8c219f82 100644 --- a/amplifier_app_cli/session_store.py +++ b/amplifier_app_cli/session_store.py @@ -21,6 +21,7 @@ from amplifier_foundation import sanitize_message from amplifier_foundation import write_with_backup from amplifier_foundation.session.history import SessionHistoryStore +from amplifier_foundation.session.metadata import SessionMetadataStore from amplifier_app_cli.project_utils import get_project_slug from amplifier_foundation.paths.resolution import get_amplifier_home @@ -125,7 +126,7 @@ def save(self, session_id: str, transcript: list, metadata: dict) -> None: # Foundation validates both payloads before writing either native file. SessionHistoryStore(session_dir).save( - transcript, redact_secrets(metadata), sanitizer=sanitize_message + transcript, redact_secrets(metadata), sanitizer=sanitize_message, merge_metadata=True ) logger.debug(f"Session {session_id} saved successfully") @@ -151,7 +152,7 @@ def save_new(self, session_id: str, transcript: list, metadata: dict) -> None: session_dir = self.reserve_session(session_id) try: SessionHistoryStore(session_dir).save( - transcript, redact_secrets(metadata), sanitizer=sanitize_message + transcript, redact_secrets(metadata), sanitizer=sanitize_message, merge_metadata=True ) except BaseException: # We created this directory exclusively, so removing a partial @@ -173,7 +174,7 @@ def _save_transcript(self, session_dir: Path, transcript: list) -> None: def _save_metadata(self, session_dir: Path, metadata: dict) -> None: """Save native metadata with the CLI's existing credential redaction.""" - SessionHistoryStore(session_dir).save_metadata(redact_secrets(metadata)) + SessionHistoryStore(session_dir).save_metadata(redact_secrets(metadata), merge_metadata=True) def load(self, session_id: str) -> tuple[list, dict]: """Load session state with corruption recovery. @@ -252,18 +253,16 @@ def update_metadata(self, session_id: str, updates: dict) -> dict: if not session_dir.exists(): raise FileNotFoundError(f"Session '{session_id}' not found") - # Load current metadata - metadata = self._load_metadata(session_dir) - - # Apply updates - metadata.update(updates) - - # Save updated metadata - self._save_metadata(session_dir, metadata) + metadata = SessionMetadataStore(session_dir).update(redact_secrets(updates)) logger.debug(f"Session {session_id} metadata updated: {list(updates.keys())}") return metadata + def rename(self, session_id: str, name: str) -> dict: + """Rename through Foundation without replacing transcript/runtime state.""" + self.get_metadata(session_id) # Keep strict identity/existence validation. + return SessionMetadataStore(self.base_dir / session_id).set_name(name) + def get_metadata(self, session_id: str) -> dict: """Get session metadata without loading transcript. diff --git a/docs/SHARED-SESSION-METADATA.md b/docs/SHARED-SESSION-METADATA.md new file mode 100644 index 00000000..01c669fb --- /dev/null +++ b/docs/SHARED-SESSION-METADATA.md @@ -0,0 +1,21 @@ +# Shared session metadata + +The CLI and Unified share the native session directory. `metadata.json` is the +authority for names and descriptions; Unified's display cache is not a separate +name store. `/rename` uses Foundation's `SessionMetadataStore.set_name`, with a +shared limit of 200 characters. Delayed automatic names and later runtime saves +preserve an explicit rename from either application. + +Foundation serializes metadata updates with a short sibling lock. Session +execution still uses its separate ownership lock. Both applications must be +updated before relying on concurrent metadata edits; an already-running older +process retains its older save behavior until restarted. + +Resuming a native session also selects its `settings.yaml` scope. CLI and Unified +use Foundation's shared settings reader with global, project, local, and native +session paths. Provider instances merge by ID or module; unrelated keys and +credential references remain intact. Reading never rewrites configuration. + +This preserves session identity, transcript context, and event history. It does +not replay tools or make web-only pending operations, canvas state, or private +runtime-control files executable by the CLI. diff --git a/pyproject.toml b/pyproject.toml index 493353ed..87a35d3d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -13,7 +13,7 @@ dependencies = [ "pydantic>=2.0.0", "amplifier-core>=1.5.3", # Tool installs consume published metadata, not this repository's uv.lock. - "amplifier-foundation @ git+https://github.com/microsoft/amplifier-foundation@2c0063a187181173dfe2438ce031079e8723f894", + "amplifier-foundation @ git+https://github.com/microsoft/amplifier-foundation@main", "pyyaml>=6.0.3", "prompt-toolkit>=3.0.52", "httpx[socks]>=0.28.1", @@ -67,7 +67,7 @@ build-backend = "hatchling.build" dev = ["pytest>=9.0.3", "pytest-asyncio>=0.24.0"] [tool.uv.sources] -amplifier-foundation = { git = "https://github.com/microsoft/amplifier-foundation", rev = "2c0063a187181173dfe2438ce031079e8723f894" } +amplifier-foundation = { git = "https://github.com/microsoft/amplifier-foundation", rev = "main" } [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/tests/test_session_runner.py b/tests/test_session_runner.py index a3d4b5b4..fd5adac4 100644 --- a/tests/test_session_runner.py +++ b/tests/test_session_runner.py @@ -763,6 +763,8 @@ async def test_configurator_apply_saved_settings_called(self): saved_settings = {"key": "value"} merged = {"configurator": saved_settings, "other": "stuff"} fake_module = self._make_fake_configurator_module(mock_configurator_cls) + settings = MagicMock(get_merged_settings=MagicMock(return_value=merged)) + settings.with_session.return_value = settings with ExitStack() as stack: for p in _configurator_patches(mock_sess): @@ -776,13 +778,12 @@ async def test_configurator_apply_saved_settings_called(self): stack.enter_context( patch( f"{_MODULE}.AppSettings", - return_value=MagicMock( - get_merged_settings=MagicMock(return_value=merged) - ), + return_value=settings, ) ) - await create_initialized_session(cfg, console) + initialized = await create_initialized_session(cfg, console) + assert settings.with_session.call_args.args[0] == initialized.session_id mock_configurator.apply_saved_settings.assert_called_once_with(saved_settings) @pytest.mark.anyio diff --git a/tests/test_shared_root_state.py b/tests/test_shared_root_state.py index fcfe8a4a..cd0df4f7 100644 --- a/tests/test_shared_root_state.py +++ b/tests/test_shared_root_state.py @@ -509,7 +509,10 @@ def test_held_read_refreshes_older_cli_native_writes(shared_api, tmp_path): try: assert root.read(native)[0][0]["content"] == "first" # Simulate a transcript-compatible host writing through the shared layer. - native.save("root", [{"role": "user", "content": "second"}], {"name": "renamed"}) + from amplifier_foundation.session.history import SessionHistoryStore + SessionHistoryStore(native.base_dir / "root").save( + [{"role": "user", "content": "second"}], {"name": "renamed"} + ) messages, metadata = root.read(native) assert messages[0]["content"] == "second" assert metadata["name"] == "renamed" diff --git a/tests/test_shared_session_metadata.py b/tests/test_shared_session_metadata.py new file mode 100644 index 00000000..e9049de4 --- /dev/null +++ b/tests/test_shared_session_metadata.py @@ -0,0 +1,30 @@ +"""A native session can move between updated hosts without losing its name.""" + +import pytest + +from amplifier_app_cli.session_store import SessionStore +from amplifier_foundation.session.metadata import SessionMetadataStore + + +def test_external_name_survives_cli_checkpoint_and_cli_rename_is_shared(tmp_path): + store = SessionStore(tmp_path) + messages = [{"role": "user", "content": "Existing conversation"}] + store.save("web-session", messages, {"bundle": "anchors", "other_host": True}) + metadata = SessionMetadataStore(tmp_path / "web-session") + metadata.set_name("Created in the web") + stale = store.get_metadata("web-session") + metadata.set_name("Changed in another client") + store.save("web-session", messages, {**stale, "turn_count": 1}) + assert store.get_metadata("web-session")["name"] == "Changed in another client" + result = store.rename("web-session", "CLI name " + "x" * 100) + assert metadata.read()["name"] == result["name"] + assert metadata.read()["name_source"] == "manual" + assert metadata.read()["other_host"] is True + assert store.load("web-session")[0] == messages + + +def test_missing_session_rename_does_not_create_it(tmp_path): + store = SessionStore(tmp_path) + with pytest.raises(FileNotFoundError): + store.rename("missing", "A name") + assert not (tmp_path / "missing").exists() diff --git a/uv.lock b/uv.lock index 631fcf49..6c6717a5 100644 --- a/uv.lock +++ b/uv.lock @@ -30,7 +30,7 @@ dev = [ [package.metadata] requires-dist = [ { name = "amplifier-core", specifier = ">=1.5.3" }, - { name = "amplifier-foundation", git = "https://github.com/microsoft/amplifier-foundation?rev=2c0063a187181173dfe2438ce031079e8723f894" }, + { name = "amplifier-foundation", git = "https://github.com/microsoft/amplifier-foundation?rev=main" }, { name = "click", specifier = ">=8.1.0" }, { name = "filelock", specifier = ">=3.29.6" }, { name = "httpx", extras = ["socks"], specifier = ">=0.28.1" }, @@ -72,9 +72,10 @@ wheels = [ [[package]] name = "amplifier-foundation" version = "1.0.0" -source = { git = "https://github.com/microsoft/amplifier-foundation?rev=2c0063a187181173dfe2438ce031079e8723f894#2c0063a187181173dfe2438ce031079e8723f894" } +source = { git = "https://github.com/microsoft/amplifier-foundation?rev=main#52dec7e276db62f720448c8e2ec176b9cbffde2d" } dependencies = [ { name = "amplifier-core" }, + { name = "filelock" }, { name = "pyyaml" }, ]