diff --git a/robot-docs/settings.md b/robot-docs/settings.md index 5c4df653..83058cba 100644 --- a/robot-docs/settings.md +++ b/robot-docs/settings.md @@ -36,6 +36,8 @@ The existing files remain authoritative on disk: `config.json`, `extensions.json `daemon/settings.gleam` owns HTTP decoding and provider/UI operations. `harness/session_settings.gleam` owns capability/MCP changes, and `harness/albedo_settings_store.erl` provides their persistence and the shared transaction support. The runtime depends on these harness operations rather than daemon settings. Provider validation stays in `daemon/configuration`; MCP validation stays in its extension. `harness/albedo_settings_lock.erl` provides one mutation lock per home, including provider defaults and extension defaults or raised caps saved through existing commands. Credential operations, OAuth account refreshes, and settings save/reload/restoration all use the same per-home lock. There are no nested locks on different settings files. Same-process nested operations retain the outer lock. The lock is released if its holder exits. +`harness/capabilities.gleam` decodes capability preferences, applies session overrides to global defaults, and validates every known group before settings mutations or snapshots. Readers validate the requested choice. Unscoped discovery ignores session preferences. `harness/albedo_capabilities.erl` provides bounded file reads. + The daemon validates files before changing them. A malformed or unreadable document fails the operation rather than becoming an empty default. Writes seal an exclusive temporary file to mode `0600` before writing its contents, sync it, and rename it into place. Session settings messages enter the session actor, which checks idleness before persistence. The session then asks the runtime actor to save and reload. The runtime actor holds the mutation lock while saving, preparing the replacement composition, and restoring settings after preparation fails. The session actor never holds this lock across a call to the runtime actor. Restoration affects the settings file and credential entry involved in the mutation and preserves other credential entries. The runtime retains the previous composition on preparation failure. Errors explicitly report restoration failures. These operations are not crash-atomic transactions across multiple files. diff --git a/src/albedo/harness/albedo_capabilities.erl b/src/albedo/harness/albedo_capabilities.erl index 1286582a..f2102e34 100644 --- a/src/albedo/harness/albedo_capabilities.erl +++ b/src/albedo/harness/albedo_capabilities.erl @@ -1,30 +1,11 @@ -module(albedo_capabilities). --export([enabled/4, optional/4]). +-export([read/1]). -%% A session override wins over a global default. Malformed preferences never -%% silently change the model's capabilities; composition fails and keeps the old one. -enabled(Home, Session, Kind, Name) -> +read(Home) -> Path = filename:join(Home, <<"capabilities.json">>), case file:read_file(Path) of - {error, enoent} -> {ok, true}; + {error, enoent} -> {ok, <<"{}">>}; {ok, Bytes} when byte_size(Bytes) =< 1048576 -> - try - Config = json:decode(Bytes), - Scoped = maps:get(Session, maps:get(<<"sessions">>, Config, #{}), #{}), - Default = lookup(maps:get(<<"global">>, Config, #{}), Kind, Name, true), - {ok, lookup(Scoped, Kind, Name, Default)} - catch _:_ -> {error, <<"invalid capabilities.json">>} end; + {ok, Bytes}; _ -> {error, <<"could not read capabilities.json">>} end. - -lookup(Scope, Kind, Name, Default) -> - case maps:find(Name, maps:get(Kind, Scope, #{})) of - {ok, Value} when is_boolean(Value) -> Value; - error -> Default; - _ -> erlang:error(invalid_preference) - end. - -%% A capability check only a selected session performs: an unscoped reader is -%% enabled unconditionally. -optional(undefined, _Home, _Kind, _Name) -> {ok, true}; -optional(Session, Home, Kind, Name) -> enabled(Home, Session, Kind, Name). diff --git a/src/albedo/harness/albedo_instruction_files.erl b/src/albedo/harness/albedo_instruction_files.erl index 1115d790..410c0c20 100644 --- a/src/albedo/harness/albedo_instruction_files.erl +++ b/src/albedo/harness/albedo_instruction_files.erl @@ -69,7 +69,8 @@ select_files([File = {Scope, Display, _} | Rest], Selection, Selected) -> undefined -> {ok, true}; {Home, Session} -> Name = <<(atom_to_binary(Scope))/binary, ":", Display/binary>>, - albedo_capabilities:optional(Session, Home, <<"instructions">>, Name) + SelectedSession = case Session of undefined -> none; _ -> {some, Session} end, + 'albedo@harness@capabilities':optional(SelectedSession, Home, <<"instructions">>, Name) end, case Enabled of {ok, true} -> select_files(Rest, Selection, [File | Selected]); diff --git a/src/albedo/harness/albedo_settings_store.erl b/src/albedo/harness/albedo_settings_store.erl index cccc694e..8b8112ae 100644 --- a/src/albedo/harness/albedo_settings_store.erl +++ b/src/albedo/harness/albedo_settings_store.erl @@ -92,13 +92,7 @@ capability(Home, Session, Kind, Name, Value, After) -> end, After). validate_caps(Config) -> - Groups = [object(<<"global">>, Config) | maps:values(object(<<"sessions">>, Config))], - lists:foreach(fun(Group) -> - true = is_map(Group), - lists:foreach(fun(Kind) -> - true = lists:all(fun({K, V}) -> is_binary(K) andalso is_boolean(V) end, maps:to_list(object(Kind, Group))) - end, [<<"skills">>, <<"instructions">>, <<"mcp">>]) - end, Groups). + check('albedo@harness@capabilities':validate(Config)). mcp(Home, Name, ServerJSON, SecretsJSON, After) -> transaction(Home, <<"extensions.json">>, {<<"mcp">>, Name}, fun validate_mcp/1, fun(Prior) -> diff --git a/src/albedo/harness/capabilities.gleam b/src/albedo/harness/capabilities.gleam new file mode 100644 index 00000000..61b2b12f --- /dev/null +++ b/src/albedo/harness/capabilities.gleam @@ -0,0 +1,109 @@ +//// Capability preferences: session choices override global defaults. + +import gleam/dict.{type Dict} +import gleam/dynamic.{type Dynamic} +import gleam/dynamic/decode +import gleam/json +import gleam/list +import gleam/option.{type Option, None, Some} +import gleam/result + +/// Read one choice. Unspecified capabilities are enabled. +pub fn enabled( + home: String, + session: String, + kind: String, + name: String, +) -> Result(Bool, String) { + use bytes <- result.try(read(home)) + json.parse_bits(bytes, selected_decoder(session, kind, name)) + |> result.replace_error("invalid capabilities.json") +} + +/// Unscoped discovery does not apply a session's preferences. +pub fn optional( + session: Option(String), + home: String, + kind: String, + name: String, +) -> Result(Bool, String) { + case session { + None -> Ok(True) + Some(session) -> enabled(home, session, kind, name) + } +} + +@external(erlang, "albedo_capabilities", "read") +fn read(home: String) -> Result(BitArray, String) + +// Readers check only the requested choice; persisted settings are validated +// in full before a mutation or snapshot. +fn selected_decoder( + session: String, + kind: String, + name: String, +) -> decode.Decoder(Bool) { + use default <- decode.optional_field( + "global", + True, + choice_decoder(kind, name, True), + ) + let session_decoder = { + use enabled <- decode.optional_field( + session, + default, + choice_decoder(kind, name, default), + ) + decode.success(enabled) + } + use enabled <- decode.optional_field("sessions", default, session_decoder) + decode.success(enabled) +} + +fn choice_decoder(kind: String, name: String, default: Bool) { + let named_decoder = { + use enabled <- decode.optional_field(name, default, decode.bool) + decode.success(enabled) + } + use enabled <- decode.optional_field(kind, default, named_decoder) + decode.success(enabled) +} + +/// Validate every known capability group while allowing unrelated fields. +pub fn validate(config: Dynamic) -> Result(Nil, String) { + use fields <- result.try(object(config)) + use global <- result.try(section(fields, "global")) + use sessions <- result.try(section(fields, "sessions")) + use _ <- result.try(validate_scope(global)) + list.try_each(dict.values(sessions), fn(scope) { + use fields <- result.try(object(scope)) + validate_scope(fields) + }) +} + +fn validate_scope(fields: Dict(String, Dynamic)) -> Result(Nil, String) { + list.try_each(["skills", "instructions", "mcp"], fn(kind) { + use choices <- result.try(section(fields, kind)) + list.try_each(dict.values(choices), fn(value) { + decode.run(value, decode.bool) + |> result.replace_error("invalid settings") + |> result.map(fn(_) { Nil }) + }) + }) +} + +fn object(value: Dynamic) -> Result(Dict(String, Dynamic), String) { + decode.run(value, decode.dict(decode.string, decode.dynamic)) + |> result.replace_error("invalid settings") +} + +fn section( + fields: Dict(String, Dynamic), + key: String, +) -> Result(Dict(String, Dynamic), String) { + case dict.get(fields, key) { + Error(_) -> Ok(dict.new()) + Ok(value) -> + object(value) |> result.replace_error("invalid settings section") + } +} diff --git a/src/albedo/harness/extensions/mcp/albedo_mcp.erl b/src/albedo/harness/extensions/mcp/albedo_mcp.erl index cdfe1b95..01665646 100644 --- a/src/albedo/harness/extensions/mcp/albedo_mcp.erl +++ b/src/albedo/harness/extensions/mcp/albedo_mcp.erl @@ -14,7 +14,8 @@ prepare(ConfigJson, Session) -> Config = json:decode(ConfigJson), Servers = maps:get(<<"servers">>, Config, #{}), true = is_map(Servers), - open_servers(lists:sort(maps:to_list(Servers)), [], Session) + SelectedSession = case Session of undefined -> none; _ -> {some, Session} end, + open_servers(lists:sort(maps:to_list(Servers)), [], SelectedSession) catch _:_ -> {error, <<"MCP configuration is invalid">>} end. @@ -27,7 +28,7 @@ open_servers([], Opened, _) -> end; open_servers([{Name, Config} | Rest], Opened, Session) -> case {maps:get(<<"enabled">>, Config, true), - albedo_capabilities:optional(Session, albedo_extension_settings:home(), <<"mcp">>, Name)} of + 'albedo@harness@capabilities':optional(Session, albedo_extension_settings:home(), <<"mcp">>, Name)} of {true, {ok, true}} -> case open_server(Name, Config) of {ok, Server} -> open_servers(Rest, [Server | Opened], Session); diff --git a/src/albedo/harness/extensions/skills/extension.gleam b/src/albedo/harness/extensions/skills/extension.gleam index fb300c35..40bd10f7 100644 --- a/src/albedo/harness/extensions/skills/extension.gleam +++ b/src/albedo/harness/extensions/skills/extension.gleam @@ -6,6 +6,7 @@ //// unified command catalog, where a user invocation submits the activation as //// one turn and a model invocation returns it as data. +import albedo/harness/capabilities import albedo/harness/command import albedo/harness/extension as harness_extension import albedo/harness/extensions/skills/catalog @@ -30,7 +31,7 @@ pub fn extension_at(home: String) -> harness_extension.Extension { use discovered <- result.try(catalog.scan_at(workspace, home)) use names <- result.try( list.try_fold(discovered.skills, [], fn(acc, skill) { - use enabled <- result.try(item_enabled( + use enabled <- result.try(capabilities.enabled( settings.home(), session, "skills", @@ -64,14 +65,6 @@ pub fn extension_at(home: String) -> harness_extension.Extension { ) } -@external(erlang, "albedo_capabilities", "enabled") -fn item_enabled( - home: String, - session: String, - kind: String, - name: String, -) -> Result(Bool, String) - const instructions = "Agent Skills are cataloged session commands. Invoke one through the `commands` object (commands.catalog() maps slash names to methods): it returns the skill's instructions as data and never submits a turn or executes bundled scripts. `await skills.resources(name)` returns a record whose .resources lists bundled resource names, and `await skills.read(name, resource=\"SKILL.md\", offset=0, limit=16384)` reads one bounded page as a record: page.content, page.next_offset, and page.truncated (page[\"content\"] works too). Failures raise SkillsError. A user may explicitly run the listed slash command, which submits the activation as one user turn." /// One cataloged skill as a session command: both callers resolve the same diff --git a/test/e2e/settings_test.py b/test/e2e/settings_test.py index 11e5f3a5..e861ee36 100644 --- a/test/e2e/settings_test.py +++ b/test/e2e/settings_test.py @@ -273,6 +273,60 @@ class SettingsTest(unittest.TestCase): self.assertEqual(after["mcp"], before["mcp"]) self.assertEqual(after["capabilities"], before["capabilities"]) + def test_capability_defaults_and_session_overrides_control_context_and_commands( + self, + ): + instruction = "CAPABILITY_INSTRUCTION_MARKER" + description = "CAPABILITY_SKILL_MARKER" + (self.app.workspace / "AGENTS.md").write_text(instruction) + skill = self.app.workspace / ".albedo/skills/preference-demo/SKILL.md" + skill.parent.mkdir(parents=True) + skill.write_text( + "---\nname: preference-demo\ndescription: " + + description + + "\n---\nSkill body.\n" + ) + + def prompt_context(session): + self.app.prompt(session, "inspect capability preferences").close() + self.app.idle(session) + return json.dumps(self.provider.requests[-1]["request"]) + + default_session = self.app.session() + context = prompt_context(default_session) + self.assertIn(instruction, context) + self.assertIn(description, context) + + self.capability(scope="global", enabled=False) + self.capability( + kind="skills", name="preference-demo", scope="global", enabled=False + ) + disabled_session = self.app.session() + context = prompt_context(disabled_session) + self.assertNotIn(instruction, context) + self.assertNotIn(description, context) + self.assertNotIn( + "/preference-demo", + { + item["name"] + for item in self.request(f"/sessions/{disabled_session}/commands") + }, + ) + + self.capability(enabled=True) + self.capability(kind="skills", name="preference-demo", enabled=True) + context = prompt_context(self.session) + self.assertIn(instruction, context) + self.assertIn(description, context) + commands_route = f"/sessions/{self.session}/commands" + self.assertIn( + "/preference-demo", {item["name"] for item in self.request(commands_route)} + ) + self.capability(kind="skills", name="preference-demo", enabled=None) + self.assertNotIn( + "/preference-demo", {item["name"] for item in self.request(commands_route)} + ) + def test_mcp_save_delete_and_failed_connection_restore_settings_and_credentials( self, ): @@ -322,6 +376,12 @@ class SettingsTest(unittest.TestCase): lambda: self.request("/settings/ui", {"tools": True}, "PATCH"), ), ("capabilities.json", {"global": {"skills": []}}, self.capability), + ("capabilities.json", {"sessions": []}, self.capability), + ( + "capabilities.json", + {"sessions": {"other-session": {"mcp": {"broken": "yes"}}}}, + self.capability, + ), ( "extensions.json", {"mcp": {"servers": []}},