diff --git a/Cargo.lock b/Cargo.lock index 15e2d3e2..7afdcff1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5812,9 +5812,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.43" +version = "0.23.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "0283386ce02abc0151e1761d08802dfe86c173b0b494af5cbc086574e453da06" +checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" dependencies = [ "aws-lc-rs", "once_cell", @@ -6609,7 +6609,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.4.3", + "getrandom 0.3.4", "once_cell", "rustix 1.1.4", "windows-sys 0.61.2", diff --git a/docs/configuration/policy.md b/docs/configuration/policy.md new file mode 100644 index 00000000..7a6f7a26 --- /dev/null +++ b/docs/configuration/policy.md @@ -0,0 +1,86 @@ +# Organization policy + +Cortex reads a managed policy document for organizations that need to pin +behavior for every member. The document lives in one directory, named by +`CORTEX_ORG_POLICY_DIR`, and is called `policy.json`. + +```bash +export CORTEX_ORG_POLICY_DIR=/etc/cortex/policy +``` + +```json +{ + "fast_mode": false, + "plugin_install": "require_accept_command" +} +``` + +Both keys are optional. A missing document, or a document without a key, leaves +that key on the host default. + +## Resolution rules + +Every key resolves **fail-closed**. A document that exists but cannot be read, +cannot be parsed, or carries a value that is not recognized denies the +restricted behavior. Policy failures never grant a permission. + +| Situation | `fast_mode` | `plugin_install` | +|---|---|---| +| No directory, or no `policy.json` | host default | host default | +| Key absent | host default | host default | +| Key `true`, `"on"`, `"enabled"`, `"allowed"` | allowed | optional | +| Key `false`, `"off"`, any other value | disabled | required | +| Document unreadable or unparseable | disabled | required | + +## `fast_mode` + +When `fast_mode` is `false`, members cannot turn fast mode on. + +- `/fast` and `/fast on` show `Fast mode is disabled for your organization. + Contact your admin.` followed by `Staying on Standard.` +- The session keeps its current model and settings. Nothing is re-sent. +- There is no client flag that bypasses the policy. +- Turning fast mode **off** is always allowed. +- A remote session on Standard shows `Remote · Standard`. The Fast chip appears + only while fast mode is actually on. + +`/fast off` is unaffected by policy, so a member can always return to Standard. + +## `plugin_install` + +When `plugin_install` is `require_accept_command`, `cortex plugin install` and +`cortex plugin update` refuse to run without `--accept-command`: + +``` +This organization requires --accept-command for plugin installs. +Run `cortex plugin install --json` to review the commands, +then pass --accept-command . +``` + +The pin itself is enforced the same way for every organization, so a review +that does not match the package fails closed whether or not a policy document +exists. See [Plugins](../customization/plugins.md#pinned-command-installs). + +## Audit journal + +Fail-closed decisions are appended to `{cortex_home}/audit/events.jsonl`, one +JSON object per line: + +```json +{"schema":1,"ts":"2026-09-14T09:26:11Z","kind":"plugin_command_accepted","detail":{"plugin":"cortex-review","version":"1.2.0","accepted_hash":"8f4c…","actual_hash":"8f4c…","action":"install","policy":"require_accept_command"}} +``` + +| `kind` | Written when | +|---|---| +| `plugin_command_accepted` | A reviewed command hash was accepted for an install or update | +| `instructions_omitted` | A subagent skipped user, project, or local instruction documents | +| `managed_policy_never_omitted` | A request named managed policy; it loaded anyway | + +Records carry the plugin or source, the hashes, and the scope names. They never +carry prompt text, file bodies, or secrets. + +## See also + +- [Environment variables](../configuration/env.md) +- [Plugins](../customization/plugins.md) +- [Agents](../customization/agents.md#omitting-instruction-documents) diff --git a/docs/customization/agents.md b/docs/customization/agents.md index 5c42f853..7d01a740 100644 --- a/docs/customization/agents.md +++ b/docs/customization/agents.md @@ -74,6 +74,7 @@ edit anything. | `max_steps` | integer | Cap on tool-calling steps | | `color` | string | Colour used in the TUI | | `hidden` | bool | Hide from the default listing | +| `omit_instructions` | list | Instruction documents this agent skips. See below. | ### Tool access @@ -100,6 +101,46 @@ tools: Tool names are the ones in the [tools reference](../reference/tools.md). +### Omitting instruction documents + +Cortex merges instruction Markdown from several places. A subagent can be told +to skip some of them, which keeps a narrow task from pulling in unrelated +context. + +| Scope | Documents | +|---|---| +| `user` | `{cortex_home}/AGENTS.md` | +| `project` | the repository-root `AGENTS.md` | +| `local` | `AGENTS.md` between the repository root and the working directory | +| `managed` | organization policy. **Never omitted.** | + +```yaml +--- +name: reviewer +description: Reviews a diff without project-wide context +omit_instructions: [user, project] +--- +``` + +Omission is opt-in and applies to that run only. A skipped document is never +opened, so it cannot reach the prompt by another path. + +**Organization-managed policy always loads.** A request that names `managed` is +accepted, recorded, and ignored: the managed document still loads. The same is +true when the main agent passes `omit_instructions` to the `Task` tool. + +```json +{"mode": "worker", "prompt": "review src/auth", "omit_instructions": ["user", "project"]} +``` + +An unknown scope name is an error rather than a silent no-op, so a typo cannot +omit the wrong documents. + +Each omission is appended to `{cortex_home}/audit/events.jsonl` as +`instructions_omitted`, and a request that named managed policy is recorded as +`managed_policy_never_omitted`. See +[Organization policy](../configuration/policy.md#audit-journal). + ## Where agent files are found Searched in order; the first file defining a given name wins: diff --git a/docs/customization/plugins.md b/docs/customization/plugins.md index 64bb3648..5beb976e 100644 --- a/docs/customization/plugins.md +++ b/docs/customization/plugins.md @@ -10,6 +10,7 @@ Welcome to the Cortex Plugin System! This guide provides comprehensive documenta ## Table of Contents - [Managing plugins](#managing-plugins) +- [Pinned command installs](#pinned-command-installs) - [Introduction](#introduction) - [Plugin Architecture](#plugin-architecture) - [Plugin Manifest](#plugin-manifest-plugintoml) @@ -43,6 +44,38 @@ cortex plugin publish --dry-run In the TUI, `/plugins` manages them without leaving the session. +### Pinned command installs + +`plugin install` and `plugin update` can print exactly what a package would +register, then install only that reviewed set. + +```bash +cortex plugin install cortex-review --json # prints the review + command_hash +cortex plugin install cortex-review \ + --accept-command 8f4c2a71e0b6d3a5c19f7b204e8a1d6f30c5b9a7e2d4816f0a3c7b5d9e1f2a46 +``` + +`--json` prints the plugin id and version, every command with its aliases and +arguments, any hooks and tools, and a `command_hash`. The hash covers all of +them, so a manifest that changed in any way a user could notice produces a +different value. + +When the hash does not match the package under install, the install stops: + +``` +Command hash mismatch. Manifest may have changed. +Re-run with --json and accept the new hash. +``` + +The mismatch is fail-closed. Nothing is written to the plugin root, the +previously installed package is left untouched, no trust is renewed, and there +is no `-y` shortcut that accepts a changed manifest. An organization can also +require the pin for every member; see +[Organization policy](../configuration/policy.md). + +Every accepted hash is appended to `{cortex_home}/audit/events.jsonl` so a +review can be traced afterwards. + ## Introduction The Cortex plugin system allows developers to extend the CLI with custom functionality including: diff --git a/docs/media/tui-lock-v2/runtime/120x40/command-hash-mismatch.png b/docs/media/tui-lock-v2/runtime/120x40/command-hash-mismatch.png new file mode 100644 index 00000000..99c267d6 Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/120x40/command-hash-mismatch.png differ diff --git a/docs/media/tui-lock-v2/runtime/120x40/omit-instructions.png b/docs/media/tui-lock-v2/runtime/120x40/omit-instructions.png new file mode 100644 index 00000000..32dbfa2c Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/120x40/omit-instructions.png differ diff --git a/docs/media/tui-lock-v2/runtime/120x40/org-disabled-fast.png b/docs/media/tui-lock-v2/runtime/120x40/org-disabled-fast.png new file mode 100644 index 00000000..225821b7 Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/120x40/org-disabled-fast.png differ diff --git a/docs/media/tui-lock-v2/runtime/120x40/plugin-accept-command.png b/docs/media/tui-lock-v2/runtime/120x40/plugin-accept-command.png new file mode 100644 index 00000000..b9f6786e Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/120x40/plugin-accept-command.png differ diff --git a/docs/media/tui-lock-v2/runtime/120x40/remote-fast.png b/docs/media/tui-lock-v2/runtime/120x40/remote-fast.png new file mode 100644 index 00000000..6510213b Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/120x40/remote-fast.png differ diff --git a/docs/media/tui-lock-v2/runtime/120x40/remote-standard.png b/docs/media/tui-lock-v2/runtime/120x40/remote-standard.png new file mode 100644 index 00000000..55fec82e Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/120x40/remote-standard.png differ diff --git a/docs/media/tui-lock-v2/runtime/40x12/command-hash-mismatch.png b/docs/media/tui-lock-v2/runtime/40x12/command-hash-mismatch.png new file mode 100644 index 00000000..0a45cdbc Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/40x12/command-hash-mismatch.png differ diff --git a/docs/media/tui-lock-v2/runtime/40x12/omit-instructions.png b/docs/media/tui-lock-v2/runtime/40x12/omit-instructions.png new file mode 100644 index 00000000..a6922411 Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/40x12/omit-instructions.png differ diff --git a/docs/media/tui-lock-v2/runtime/40x12/org-disabled-fast.png b/docs/media/tui-lock-v2/runtime/40x12/org-disabled-fast.png new file mode 100644 index 00000000..6d4f4f7a Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/40x12/org-disabled-fast.png differ diff --git a/docs/media/tui-lock-v2/runtime/40x12/plugin-accept-command.png b/docs/media/tui-lock-v2/runtime/40x12/plugin-accept-command.png new file mode 100644 index 00000000..9b1f38da Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/40x12/plugin-accept-command.png differ diff --git a/docs/media/tui-lock-v2/runtime/40x12/remote-fast.png b/docs/media/tui-lock-v2/runtime/40x12/remote-fast.png new file mode 100644 index 00000000..bc2ff86e Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/40x12/remote-fast.png differ diff --git a/docs/media/tui-lock-v2/runtime/40x12/remote-standard.png b/docs/media/tui-lock-v2/runtime/40x12/remote-standard.png new file mode 100644 index 00000000..6f31ffd5 Binary files /dev/null and b/docs/media/tui-lock-v2/runtime/40x12/remote-standard.png differ diff --git a/docs/reference/cli.commands.json b/docs/reference/cli.commands.json index 3ccdf6d8..01917353 100644 --- a/docs/reference/cli.commands.json +++ b/docs/reference/cli.commands.json @@ -6424,6 +6424,20 @@ "required": false, "short": null }, + { + "help": "Accept the exact command hash a prior `--json` review printed (sha256)", + "id": "accept_command", + "long": "accept-command", + "required": false, + "short": null + }, + { + "help": "Print the command review and hash without installing", + "id": "json", + "long": "json", + "required": false, + "short": null + }, { "help": "Local package path or registry plugin ID", "id": "name", @@ -7061,6 +7075,20 @@ "required": false, "short": null }, + { + "help": "Accept the exact command hash a prior `--json` review printed (sha256)", + "id": "accept_command", + "long": "accept-command", + "required": false, + "short": null + }, + { + "help": "Print the command review and hash without updating", + "id": "json", + "long": "json", + "required": false, + "short": null + }, { "help": "Plugin name to update", "id": "name", diff --git a/scripts/readiness/release_age.py b/scripts/readiness/release_age.py index cc973650..fe8d4c7f 100644 --- a/scripts/readiness/release_age.py +++ b/scripts/readiness/release_age.py @@ -13,6 +13,19 @@ ROOT = Path(__file__).resolve().parents[2] MIN_AGE = timedelta(days=7) +# Security-advisory exceptions. A patched release published inside the +# seven-day window is allowed only while no older release carries the fix, +# and only until the release ages out on its own. Each entry is temporary: +# delete it once `expires` passes. The general age rule is not weakened, and +# yanked releases are never excused here. +ADVISORY_EXCEPTIONS = { + ("rustls", "0.23.45"): { + "advisory": "RUSTSEC-2026-0285", + "alias": "GHSA-2mjx-qc3c-rqvc", + "expires": "2026-09-21T15:11:18+00:00", + }, +} + def registry_packages(lock): return { (p["name"], p["version"]) for p in lock["package"] @@ -25,12 +38,32 @@ def old_enough(created_at, now): raise ValueError("Registry timestamp is missing its timezone") return now - created >= MIN_AGE +def advisory_exception(name, version, now): + """Return the live advisory exception for a release, or None. + + The caller checks `yanked` first: this only excuses the age rule, and only + until the exception's expiry date. + """ + entry = ADVISORY_EXCEPTIONS.get((name, version)) + if entry is None: + return None + raw = entry["expires"] + # Date-only values mean start-of-day UTC; full timestamps keep their offset. + if "T" in raw: + expires = datetime.fromisoformat(raw.replace("Z", "+00:00")) + if expires.tzinfo is None: + expires = expires.replace(tzinfo=timezone.utc) + else: + expires = datetime.fromisoformat(raw).replace(tzinfo=timezone.utc) + return entry if now < expires else None + def run(base): base = subprocess.check_output(["git", "-C", str(ROOT), "rev-parse", "--verify", f"{base}^{{commit}}"], text=True).strip() previous = subprocess.check_output(["git", "-C", str(ROOT), "show", f"{base}:Cargo.lock"], text=True) added = registry_packages(tomllib.loads((ROOT / "Cargo.lock").read_text())) - registry_packages(tomllib.loads(previous)) now = datetime.now(timezone.utc) failures = [] + excused = [] for name, version in sorted(added): request = Request( f"https://crates.io/api/v1/crates/{name}/{version}", @@ -39,9 +72,21 @@ def run(base): # Fail closed when registry evidence is unavailable. Never substitute now. with urlopen(request, timeout=30) as response: release = json.load(response)["version"] - if release["yanked"] or not old_enough(release["created_at"], now): + if release["yanked"]: failures.append(f"{name}@{version}: yanked or younger than seven days") + continue + if not old_enough(release["created_at"], now): + exception = advisory_exception(name, version, now) + if exception is None: + failures.append(f"{name}@{version}: yanked or younger than seven days") + else: + excused.append( + f"{name}@{version}: younger than seven days, allowed by " + f"{exception['advisory']} ({exception['alias']}) until {exception['expires']}" + ) time.sleep(1) + for line in excused: + print(f"advisory exception: {line}") print("\n".join(failures) or f"Release-age policy passed for {len(added)} newly locked releases") return int(bool(failures)) diff --git a/scripts/readiness/test_release_age.py b/scripts/readiness/test_release_age.py index 72fb65d0..059a6e75 100644 --- a/scripts/readiness/test_release_age.py +++ b/scripts/readiness/test_release_age.py @@ -1,7 +1,7 @@ from datetime import datetime, timedelta, timezone import unittest -from release_age import old_enough, registry_packages +from release_age import ADVISORY_EXCEPTIONS, advisory_exception, old_enough, registry_packages class ReleaseAgeTests(unittest.TestCase): def test_boundary_and_future_releases(self): @@ -18,3 +18,43 @@ def test_only_crates_io_releases_are_queried(self): {"name": "crate", "version": "2", "source": "registry+https://github.com/rust-lang/crates.io-index"}, ]} self.assertEqual(registry_packages(lock), {("crate", "2")}) + + def test_rustls_exception_is_narrow_and_expires(self): + # Only the patched release for the advisory is excused. + inside = datetime(2026, 9, 16, tzinfo=timezone.utc) + entry = advisory_exception("rustls", "0.23.45", inside) + self.assertIsNotNone(entry) + self.assertEqual(entry["advisory"], "RUSTSEC-2026-0285") + self.assertEqual(entry["alias"], "GHSA-2mjx-qc3c-rqvc") + self.assertEqual(entry["expires"], "2026-09-21T15:11:18+00:00") + # The unpatched release is never excused. + self.assertIsNone(advisory_exception("rustls", "0.23.44", inside)) + # Another crate is unaffected. + self.assertIsNone(advisory_exception("serde", "0.23.45", inside)) + + def test_rustls_exception_stops_applying_after_its_expiry(self): + # Exception must cover the full seven-day age window (~15:11:17Z), not + # midnight on the calendar day. + just_before = datetime(2026, 9, 21, 15, 11, 17, tzinfo=timezone.utc) + at_expiry = datetime(2026, 9, 21, 15, 11, 18, tzinfo=timezone.utc) + self.assertIsNotNone(advisory_exception("rustls", "0.23.45", just_before)) + self.assertIsNone(advisory_exception("rustls", "0.23.45", at_expiry)) + self.assertIsNone( + advisory_exception("rustls", "0.23.45", at_expiry + timedelta(seconds=1)) + ) + # Midnight on the expiry calendar day is still inside the window. + midnight = datetime(2026, 9, 21, 0, 0, 0, tzinfo=timezone.utc) + self.assertIsNotNone(advisory_exception("rustls", "0.23.45", midnight)) + + def test_every_exception_names_an_advisory_and_an_expiry(self): + for (name, version), entry in ADVISORY_EXCEPTIONS.items(): + self.assertTrue(name and version, (name, version)) + self.assertTrue(entry["advisory"].startswith("RUSTSEC-"), entry) + self.assertTrue(entry["alias"].startswith("GHSA-"), entry) + # A timezone-aware expiry parses; an exception must not be open ended. + raw = entry["expires"] + if "T" in raw: + parsed = datetime.fromisoformat(raw.replace("Z", "+00:00")) + self.assertIsNotNone(parsed.tzinfo or timezone.utc) + else: + datetime.fromisoformat(raw).replace(tzinfo=timezone.utc) diff --git a/src/cortex-agents/src/agent.rs b/src/cortex-agents/src/agent.rs index fbaefb6c..84f8b501 100644 --- a/src/cortex-agents/src/agent.rs +++ b/src/cortex-agents/src/agent.rs @@ -56,6 +56,9 @@ pub struct AgentInfo { /// Whether this agent should use a small/lightweight model. #[serde(default)] pub use_small_model: bool, + /// Instruction scopes to omit when this agent runs. Managed is never omitted. + #[serde(default)] + pub omit_instructions: Vec, } impl AgentInfo { @@ -78,6 +81,7 @@ impl AgentInfo { max_steps: None, max_tokens: None, use_small_model: false, + omit_instructions: Vec::new(), } } @@ -207,6 +211,7 @@ pub fn create_general_agent() -> AgentInfo { max_steps: Some(20), // Max 20 steps max_tokens: None, use_small_model: false, + omit_instructions: Vec::new(), } } @@ -312,6 +317,7 @@ pub fn create_explore_agent() -> AgentInfo { max_steps: Some(15), max_tokens: None, use_small_model: false, + omit_instructions: Vec::new(), } } @@ -415,6 +421,7 @@ pub fn create_research_agent() -> AgentInfo { max_steps: Some(15), max_tokens: None, use_small_model: false, + omit_instructions: Vec::new(), } } diff --git a/src/cortex-agents/src/custom/config.rs b/src/cortex-agents/src/custom/config.rs index db21f410..cd3cc245 100644 --- a/src/cortex-agents/src/custom/config.rs +++ b/src/cortex-agents/src/custom/config.rs @@ -69,6 +69,66 @@ pub struct CustomAgentConfig { /// Whether this agent is hidden from listings. #[serde(default)] pub hidden: bool, + + /// Instruction documents this agent skips. + /// + /// Opt-in. Organization-managed policy is never omitted, even when + /// `managed` is named here. + #[serde(default, alias = "omit-instructions")] + pub omit_instructions: Vec, +} + +/// One instruction scope named in agent frontmatter. +/// +/// Unknown names are rejected at parse time so a typo cannot silently omit the +/// wrong documents. The engine applies the same names through +/// `cortex_engine::instruction_scopes`. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum OmitScope { + /// Personal documents under the Cortex home. + User, + /// The repository-root document. + Project, + /// Documents between the repository root and the working directory. + Local, + /// Organization-managed policy. Accepted, never omitted. + Managed, +} + +impl OmitScope { + /// Stable scope name, matching the engine and the audit journal. + pub const fn as_str(self) -> &'static str { + match self { + Self::User => "user", + Self::Project => "project", + Self::Local => "local", + Self::Managed => "managed", + } + } + + /// True for the scope that is never omitted. + pub const fn is_managed(self) -> bool { + matches!(self, Self::Managed) + } + + /// Scope names that will actually be skipped. + pub fn skippable(scopes: &[Self]) -> Vec { + scopes.iter().copied().filter(|s| !s.is_managed()).collect() + } +} + +impl CustomAgentConfig { + /// Stable scope names for the engine `SubagentConfig`. + /// + /// Task-level `omit_instructions` takes precedence when non-empty; otherwise + /// these frontmatter scopes are applied at execution. + pub fn engine_omit_scopes(&self) -> Vec { + self.omit_instructions + .iter() + .map(|s| s.as_str().to_string()) + .collect() + } } fn default_model() -> String { @@ -88,6 +148,7 @@ impl Default for CustomAgentConfig { max_steps: None, color: None, hidden: false, + omit_instructions: Vec::new(), } } } @@ -488,4 +549,68 @@ tools: read-only < ReasoningEffort::High.suggested_max_steps() ); } + + #[test] + fn omit_instructions_defaults_to_loading_everything() { + let config = CustomAgentConfig::default(); + assert!(config.omit_instructions.is_empty()); + let parsed: CustomAgentConfig = serde_yaml::from_str("name: quiet\n").unwrap(); + assert!(parsed.omit_instructions.is_empty()); + } + + #[test] + fn omit_instructions_parses_scope_names_and_rejects_typos() { + let parsed: CustomAgentConfig = serde_yaml::from_str( + "name: quiet\nomit_instructions: [user, project, local, managed]\n", + ) + .unwrap(); + assert_eq!( + parsed.omit_instructions, + vec![ + OmitScope::User, + OmitScope::Project, + OmitScope::Local, + OmitScope::Managed + ] + ); + // The kebab-case alias is accepted too. + let aliased: CustomAgentConfig = + serde_yaml::from_str("name: quiet\nomit-instructions: [user]\n").unwrap(); + assert_eq!(aliased.omit_instructions, vec![OmitScope::User]); + assert!( + serde_yaml::from_str::( + "name: quiet\nomit_instructions: [projekt]\n" + ) + .is_err(), + "an unknown scope name must not parse" + ); + } + + #[test] + fn engine_omit_scopes_matches_frontmatter() { + let parsed: CustomAgentConfig = + serde_yaml::from_str("name: quiet\nomit_instructions: [user, project]\n").unwrap(); + assert_eq!( + parsed.engine_omit_scopes(), + vec!["user".to_string(), "project".to_string()] + ); + } + + #[test] + fn managed_is_never_skippable() { + let scopes = [ + OmitScope::User, + OmitScope::Managed, + OmitScope::Project, + OmitScope::Local, + ]; + assert_eq!( + OmitScope::skippable(&scopes), + vec![OmitScope::User, OmitScope::Project, OmitScope::Local] + ); + assert!(OmitScope::Managed.is_managed()); + assert_eq!(OmitScope::skippable(&[OmitScope::Managed]), Vec::new()); + assert_eq!(OmitScope::User.as_str(), "user"); + assert_eq!(OmitScope::Managed.as_str(), "managed"); + } } diff --git a/src/cortex-agents/src/custom/registry.rs b/src/cortex-agents/src/custom/registry.rs index 81763747..0993eabf 100644 --- a/src/cortex-agents/src/custom/registry.rs +++ b/src/cortex-agents/src/custom/registry.rs @@ -160,6 +160,13 @@ fn custom_agent_to_agent_info(agent: &CustomAgentConfig) -> AgentInfo { .unwrap_or_else(|| agent.reasoning_effort.suggested_max_steps()); info = info.with_max_steps(max_steps); + // Instruction omissions — transferred so the engine SubagentConfig can apply them. + info.omit_instructions = agent + .omit_instructions + .iter() + .map(|s| s.as_str().to_string()) + .collect(); + // Color if let Some(ref color) = agent.color { info = info.with_color(color); diff --git a/src/cortex-cli/src/plugin_cmd.rs b/src/cortex-cli/src/plugin_cmd.rs index be1c841d..4aeca805 100644 --- a/src/cortex-cli/src/plugin_cmd.rs +++ b/src/cortex-cli/src/plugin_cmd.rs @@ -96,6 +96,12 @@ pub struct PluginInstallArgs { /// Trust this exact package to execute native Node code (NOT a sandbox) #[arg(long)] pub trust_code: bool, + /// Accept the exact command hash a prior `--json` review printed (sha256) + #[arg(long, value_name = "SHA256")] + pub accept_command: Option, + /// Print the command review and hash without installing + #[arg(long)] + pub json: bool, /// Local package path or registry plugin ID pub name: String, @@ -263,6 +269,12 @@ pub struct PluginUpdateArgs { /// Local replacement package; otherwise use the registry #[arg(long)] pub source: Option, + /// Accept the exact command hash a prior `--json` review printed (sha256) + #[arg(long, value_name = "SHA256")] + pub accept_command: Option, + /// Print the command review and hash without updating + #[arg(long)] + pub json: bool, /// Plugin name to update pub name: String, } @@ -298,6 +310,8 @@ impl PluginCli { PluginSubcommand::Search(args) => args.json, PluginSubcommand::Browse(args) => args.json, PluginSubcommand::Run(args) => args.json, + PluginSubcommand::Install(args) => args.json, + PluginSubcommand::Update(args) => args.json, _ => false, }; if json { diff --git a/src/cortex-cli/src/plugin_cmd/install.rs b/src/cortex-cli/src/plugin_cmd/install.rs index 810bcb03..8337b64d 100644 --- a/src/cortex-cli/src/plugin_cmd/install.rs +++ b/src/cortex-cli/src/plugin_cmd/install.rs @@ -27,22 +27,142 @@ impl Drop for InstallLock { } } +/// Command review a `--json` dry run prints and `--accept-command` pins. +fn command_review(manifest: &runtime::PluginManifest) -> serde_json::Value { + serde_json::json!({ + "id": manifest.plugin.id, + "version": manifest.plugin.version, + "commands": manifest.commands.iter().map(|command| serde_json::json!({ + "name": command.name, + "aliases": command.aliases, + "description": command.description, + "usage": command.usage, + "args": command.args.iter().map(|arg| serde_json::json!({ + "name": arg.name, + "required": arg.required, + "default": arg.default, + })).collect::>(), + "hidden": command.hidden, + })).collect::>(), + "hooks": manifest.hooks.iter().map(|hook| serde_json::json!({ + "type": hook.hook_type.to_string(), + "priority": hook.priority, + "pattern": hook.pattern, + "function": hook.function, + })).collect::>(), + "tools": manifest.tools.iter().map(|tool| tool.name.clone()).collect::>(), + "command_hash": runtime::command_pin::command_hash(manifest), + }) +} + +/// Print the review for a package without installing it. +fn review_only(manifest: &runtime::PluginManifest) -> Result<()> { + println!( + "{}", + serde_json::to_string_pretty(&command_review(manifest))? + ); + Ok(()) +} + +/// Enforce the reviewed hash and record an accepted pin. +/// +/// Fails closed: a mismatch or a malformed hash stops the install before the +/// package reaches the plugin root, and organization policy that requires a +/// pin refuses an install that carries none. +fn enforce_command_pin( + manifest: &runtime::PluginManifest, + accept_command: Option<&str>, + action: &str, +) -> Result<()> { + let policy = cortex_engine::org_policy::current().plugin_install; + enforce_command_pin_with(manifest, accept_command, action, policy) +} + +/// Policy-explicit form of [`enforce_command_pin`], so the decision can be +/// exercised without a process-global policy directory. +fn enforce_command_pin_with( + manifest: &runtime::PluginManifest, + accept_command: Option<&str>, + action: &str, + policy: cortex_engine::org_policy::PluginInstallPolicy, +) -> Result<()> { + match accept_command { + Some(accepted) => { + runtime::command_pin::verify_command_hash(manifest, accepted)?; + audit_command_pin(manifest, accepted, action, policy.as_str()) + } + None if policy.requires_pin() => bail!( + "This organization requires --accept-command for plugin installs. Run `cortex plugin {action} {} --json` to review the commands, then pass --accept-command .", + manifest.plugin.id + ), + None => Ok(()), + } +} + +/// Record an accepted command pin. Fail closed: a journal write failure +/// aborts the install so an accepted-command decision is never untracked. +fn audit_command_pin( + manifest: &runtime::PluginManifest, + accepted: &str, + action: &str, + policy: &str, +) -> Result<()> { + let home = cortex_engine::config::find_cortex_home() + .unwrap_or_else(|_| std::path::PathBuf::from(".cortex")); + cortex_engine::audit::record( + &home, + cortex_engine::audit::AuditKind::PluginCommandAccepted, + serde_json::json!({ + "plugin": manifest.plugin.id, + "version": manifest.plugin.version, + "accepted_hash": accepted.to_ascii_lowercase(), + "actual_hash": runtime::command_pin::command_hash(manifest), + "action": action, + "policy": policy, + }), + ) + .map_err(|error| { + anyhow::anyhow!( + "Could not record the accepted command hash in the audit journal: {error}. Install aborted." + ) + })?; + Ok(()) +} + pub(super) async fn install(args: PluginInstallArgs) -> Result<()> { let root = plugins_dir()?; let local = Path::new(&args.name); let id = if local.exists() { - install_local(&root, local, args.force, args.version.as_deref(), None)? + if args.json { + let manifest = inspect_local(local)?; + return review_only(&manifest); + } + install_local( + &root, + local, + args.force, + args.version.as_deref(), + None, + args.accept_command.as_deref(), + "install", + )? } else { runtime::contract::validate_id(&args.name)?; let (entry, bytes) = download(&args.name, args.version.as_deref()).await?; let source = tempfile::NamedTempFile::new()?; std::fs::write(source.path(), bytes)?; + if args.json { + let manifest = inspect_archive(source.path())?; + return review_only(&manifest); + } install_local( &root, source.path(), args.force, Some(&entry.version), Some(&entry.id), + args.accept_command.as_deref(), + "install", )? }; if args.trust_code { @@ -55,12 +175,35 @@ pub(super) async fn install(args: PluginInstallArgs) -> Result<()> { Ok(()) } +/// Read a package manifest without touching the plugin root. +fn inspect_local(source: &Path) -> Result { + let stage = tempfile::tempdir()?; + let package = stage.path().join("package"); + std::fs::create_dir(&package)?; + if source.is_dir() { + copy_package(source, &package)?; + Ok(runtime::package::validate_package(&package)?) + } else { + extract(source, &package)?; + Ok(runtime::package::validate_package(&package_root( + &package, + )?)?) + } +} + +/// Read a downloaded archive manifest without touching the plugin root. +fn inspect_archive(source: &Path) -> Result { + inspect_local(source) +} + pub(super) fn install_local( root: &Path, source: &Path, force: bool, version: Option<&str>, expected_id: Option<&str>, + accept_command: Option<&str>, + action: &str, ) -> Result { let _lock = InstallLock::acquire(root)?; let root = root.canonicalize()?; @@ -82,6 +225,9 @@ pub(super) fn install_local( { bail!("Package identity or version does not match the requested plugin"); } + // The reviewed hash is checked after the package is validated but before + // anything is placed in the plugin root. + enforce_command_pin(&manifest, accept_command, action)?; let destination = runtime::package::destination(&root, &manifest.plugin.id)?; if destination.exists() && !force { bail!("Plugin already installed; use --force"); @@ -339,17 +485,33 @@ pub(super) async fn update(args: PluginUpdateArgs) -> Result<()> { bail!("Plugin is not installed"); } if let Some(source) = args.source { - install_local(&root, &source, true, None, Some(&args.name))?; + if args.json { + return review_only(&inspect_local(&source)?); + } + install_local( + &root, + &source, + true, + None, + Some(&args.name), + args.accept_command.as_deref(), + "update", + )?; } else { let (entry, bytes) = download(&args.name, None).await?; let source = tempfile::NamedTempFile::new()?; std::fs::write(source.path(), bytes)?; + if args.json { + return review_only(&inspect_archive(source.path())?); + } install_local( &root, source.path(), true, Some(&entry.version), Some(&args.name), + args.accept_command.as_deref(), + "update", )?; } println!( @@ -482,404 +644,5 @@ async fn download(id: &str, version: Option<&str>) -> Result<(runtime::PluginInd } #[cfg(test)] -mod tests { - use super::*; - fn fixture(root: &Path, id: &str) { - std::fs::create_dir_all(root).unwrap(); - std::fs::write(root.join("plugin.toml"), format!( - "[plugin]\nid={id:?}\nname={id:?}\nversion=\"0.1.0\"\n[runtime]\nkind=\"node\"\nentrypoint=\"plugin.mjs\"\n")).unwrap(); - std::fs::write(root.join("plugin.mjs"), "export default {};").unwrap(); - } - #[test] - fn failed_update_retains_previous_package() { - let temp = tempfile::tempdir().unwrap(); - let source = temp.path().join("source"); - let installs = temp.path().join("installed"); - fixture(&source, "safe"); - install_local(&installs, &source, false, None, None).unwrap(); - let previous = runtime::package::fingerprint(&installs.join("safe")).unwrap(); - std::fs::write(source.join("plugin.mjs"), "invalid javascript !").unwrap(); - assert!(install_local(&installs, &source, true, None, Some("safe")).is_err()); - assert_eq!( - previous, - runtime::package::fingerprint(&installs.join("safe")).unwrap() - ); - assert!(runtime::package::destination(&installs, "../outside").is_err()); - } - #[test] - fn symlinks_and_archive_links_are_rejected() { - let temp = tempfile::tempdir().unwrap(); - #[cfg(unix)] - { - fixture(temp.path(), "safe"); - std::os::unix::fs::symlink("/etc/passwd", temp.path().join("escape")).unwrap(); - assert!(runtime::package::package_files(temp.path()).is_err()); - } - let tarball = temp.path().join("link.tar.gz"); - let encoder = flate2::write::GzEncoder::new( - std::fs::File::create(&tarball).unwrap(), - flate2::Compression::default(), - ); - let mut builder = tar::Builder::new(encoder); - let mut header = tar::Header::new_gnu(); - header.set_entry_type(tar::EntryType::Symlink); - header.set_size(0); - builder - .append_link(&mut header, "escape", "../outside") - .unwrap(); - builder.into_inner().unwrap().finish().unwrap(); - let stage = tempfile::tempdir().unwrap(); - assert!(extract(&tarball, stage.path()).is_err()); - assert!(!stage.path().join("escape").exists()); - } - - /// tar-rs refuses to build traversing paths, so the raw name field is written - /// directly to reproduce what a hostile archive actually contains. - fn hostile_archive(path: &Path, names: &[&str]) { - let encoder = flate2::write::GzEncoder::new( - std::fs::File::create(path).unwrap(), - flate2::Compression::default(), - ); - let mut builder = tar::Builder::new(encoder); - for name in names { - let mut header = tar::Header::new_ustar(); - header.set_size(1); - header.set_mode(0o644); - header.set_entry_type(tar::EntryType::Regular); - let field = &mut header.as_old_mut().name; - field.fill(0); - field[..name.len()].copy_from_slice(name.as_bytes()); - header.set_cksum(); - builder.append(&header, &b"x"[..]).unwrap(); - } - builder.into_inner().unwrap().finish().unwrap(); - } - - #[test] - fn hostile_archive_paths_are_rejected_before_anything_is_written() { - let temp = tempfile::tempdir().unwrap(); - let deep = (0..17).map(|_| "d").collect::>().join("/"); - let cases: Vec> = vec![ - vec!["../escape"], - vec!["/etc/passwd"], - vec!["a/../../escape"], - vec!["windows\\escape"], - vec!["same", "same"], - vec![deep.as_str()], - ]; - for names in &cases { - let tarball = temp.path().join("hostile.tar.gz"); - let _ = std::fs::remove_file(&tarball); - hostile_archive(&tarball, names); - let stage = tempfile::tempdir().unwrap(); - let error = extract(&tarball, stage.path()).unwrap_err().to_string(); - assert!(error.contains("invalid or duplicate path"), "{names:?}"); - assert!(!temp.path().join("escape").exists()); - assert!(!stage.path().join("d").exists(), "{names:?}"); - } - // A confined relative path in the same archive shape must still extract. - let tarball = temp.path().join("ok.tar.gz"); - hostile_archive(&tarball, &["nested/file"]); - let stage = tempfile::tempdir().unwrap(); - extract(&tarball, stage.path()).unwrap(); - assert_eq!( - std::fs::read(stage.path().join("nested/file")).unwrap(), - b"x" - ); - } - - #[test] - fn archive_entry_count_and_declared_size_limits_are_enforced() { - let temp = tempfile::tempdir().unwrap(); - let names: Vec = (0..=runtime::contract::MAX_PACKAGE_FILES) - .map(|index| format!("f{index}")) - .collect(); - let tarball = temp.path().join("many.tar.gz"); - hostile_archive( - &tarball, - &names.iter().map(String::as_str).collect::>(), - ); - let stage = tempfile::tempdir().unwrap(); - assert!( - extract(&tarball, stage.path()) - .unwrap_err() - .to_string() - .contains("1024 entries") - ); - - // A header that lies about a huge size must be rejected on the declared - // size, before that many bytes are ever written to disk. - let oversized = temp.path().join("huge.tar.gz"); - let encoder = flate2::write::GzEncoder::new( - std::fs::File::create(&oversized).unwrap(), - flate2::Compression::default(), - ); - let mut builder = tar::Builder::new(encoder); - let mut header = tar::Header::new_ustar(); - header.set_path("big").unwrap(); - header.set_size(runtime::contract::MAX_PACKAGE_BYTES + 1); - header.set_mode(0o644); - header.set_entry_type(tar::EntryType::Regular); - header.set_cksum(); - builder.append(&header, std::io::empty()).unwrap(); - builder.into_inner().unwrap().finish().unwrap(); - let stage = tempfile::tempdir().unwrap(); - assert!( - extract(&oversized, stage.path()) - .unwrap_err() - .to_string() - .contains("exceeds 64 MiB") - ); - } - - #[test] - fn archive_root_must_hold_exactly_one_directory_named_after_the_plugin() { - let temp = tempfile::tempdir().unwrap(); - let flat = temp.path().join("flat"); - fixture(&flat, "safe"); - assert_eq!(package_root(&flat).unwrap(), flat); - - let mismatch = temp.path().join("mismatch"); - fixture(&mismatch.join("other"), "safe"); - assert!( - package_root(&mismatch) - .unwrap_err() - .to_string() - .contains("must match the plugin ID") - ); - - let nested = temp.path().join("nested"); - fixture(&nested.join("safe"), "safe"); - assert_eq!(package_root(&nested).unwrap(), nested.join("safe")); - - let two = temp.path().join("two"); - fixture(&two.join("safe"), "safe"); - fixture(&two.join("other"), "other"); - assert!( - package_root(&two) - .unwrap_err() - .to_string() - .contains("one plugin at root") - ); - } - - #[test] - fn install_refuses_to_overwrite_or_accept_a_mismatched_identity() { - let temp = tempfile::tempdir().unwrap(); - let source = temp.path().join("source"); - let installs = temp.path().join("installed"); - fixture(&source, "safe"); - - assert!( - install_local(&installs, &source, false, Some("9.9.9"), None) - .unwrap_err() - .to_string() - .contains("identity or version") - ); - assert!(install_local(&installs, &source, false, None, Some("other")).is_err()); - assert!(!installs.join("safe").exists()); - - // Nested package files must be recreated under the staged package. - std::fs::create_dir(source.join("lib")).unwrap(); - std::fs::write(source.join("lib/helper.mjs"), "export const x = 1;").unwrap(); - install_local(&installs, &source, false, Some("0.1.0"), Some("safe")).unwrap(); - assert!(installs.join("safe/lib/helper.mjs").is_file()); - - assert!( - install_local(&installs, &source, false, None, None) - .unwrap_err() - .to_string() - .contains("--force") - ); - install_local(&installs, &source, true, None, None).unwrap(); - - // Staging directories must never be left behind in the plugin root. - let leftovers: Vec<_> = std::fs::read_dir(&installs) - .unwrap() - .map(|entry| entry.unwrap().file_name()) - .filter(|name| name.to_string_lossy().starts_with(".staging-")) - .collect(); - assert!(leftovers.is_empty(), "{leftovers:?}"); - } - - #[test] - fn an_invalid_manifest_or_artifact_never_reaches_the_plugin_root() { - let temp = tempfile::tempdir().unwrap(); - let installs = temp.path().join("installed"); - let broken = temp.path().join("broken"); - std::fs::create_dir_all(&broken).unwrap(); - std::fs::write(broken.join("plugin.toml"), "this is not toml [[[").unwrap(); - assert!(install_local(&installs, &broken, false, None, None).is_err()); - - let empty = temp.path().join("empty-artifact"); - fixture(&empty, "safe"); - std::fs::write(empty.join("plugin.mjs"), "").unwrap(); - assert!( - install_local(&installs, &empty, false, None, None) - .unwrap_err() - .to_string() - .contains("Artifact") - ); - assert!(!installs.join("safe").exists()); - } - - #[test] - fn a_failed_replacement_rolls_the_previous_package_back() { - let temp = tempfile::tempdir().unwrap(); - let destination = temp.path().join("installed"); - fixture(&destination, "safe"); - let previous = runtime::package::fingerprint(&destination).unwrap(); - let stage = temp.path().join("stage"); - std::fs::create_dir(&stage).unwrap(); - - // A candidate that does not exist makes the second rename fail after the - // previous package has already been moved aside. - assert!(replace(&temp.path().join("missing"), &destination, &stage).is_err()); - assert_eq!( - previous, - runtime::package::fingerprint(&destination).unwrap() - ); - assert!(!stage.join("previous").exists()); - } - - #[test] - fn directory_entries_and_truncated_archive_files_are_handled() { - let temp = tempfile::tempdir().unwrap(); - let tarball = temp.path().join("dirs.tar.gz"); - let encoder = flate2::write::GzEncoder::new( - std::fs::File::create(&tarball).unwrap(), - flate2::Compression::default(), - ); - let mut builder = tar::Builder::new(encoder); - let mut header = tar::Header::new_ustar(); - header.set_path("nested").unwrap(); - header.set_size(0); - header.set_mode(0o755); - header.set_entry_type(tar::EntryType::Directory); - header.set_cksum(); - builder.append(&header, std::io::empty()).unwrap(); - let mut header = tar::Header::new_ustar(); - header.set_path("nested/file").unwrap(); - header.set_size(3); - header.set_mode(0o644); - header.set_entry_type(tar::EntryType::Regular); - header.set_cksum(); - builder.append(&header, &b"abc"[..]).unwrap(); - builder.into_inner().unwrap().finish().unwrap(); - let stage = tempfile::tempdir().unwrap(); - extract(&tarball, stage.path()).unwrap(); - assert!(stage.path().join("nested").is_dir()); - assert_eq!( - std::fs::read(stage.path().join("nested/file")).unwrap(), - b"abc" - ); - - // An archive whose payload stops early must fail, not install a partial file. - let full = std::fs::read(&tarball).unwrap(); - let truncated = temp.path().join("truncated.tar.gz"); - std::fs::write(&truncated, &full[..full.len() / 2]).unwrap(); - let stage = tempfile::tempdir().unwrap(); - assert!(extract(&truncated, stage.path()).is_err()); - } - - #[test] - fn an_oversized_compressed_archive_is_refused_before_it_is_opened() { - let temp = tempfile::tempdir().unwrap(); - let stage = tempfile::tempdir().unwrap(); - // A sparse file keeps this cheap while still exceeding the on-disk limit. - let big = temp.path().join("big.tar.gz"); - std::fs::File::create(&big) - .unwrap() - .set_len(runtime::contract::MAX_PACKAGE_BYTES + 1) - .unwrap(); - assert!( - extract(&big, stage.path()) - .unwrap_err() - .to_string() - .contains("Compressed package exceeds 64 MiB") - ); - } - - #[test] - fn the_install_lock_serializes_mutation_and_is_released_on_drop() { - let temp = tempfile::tempdir().unwrap(); - let root = temp.path().join("plugins"); - let held = InstallLock::acquire(&root).unwrap(); - assert!(root.join(".install.lock").is_file()); - let error = InstallLock::acquire(&root) - .err() - .expect("a second lock must not be granted") - .to_string(); - assert!( - error.contains("Another plugin operation is running"), - "{error}" - ); - drop(held); - assert!(!root.join(".install.lock").exists()); - InstallLock::acquire(&root).unwrap(); - } - - #[test] - fn publish_prepares_an_installable_archive_and_refuses_to_overwrite() { - let temp = tempfile::tempdir().unwrap(); - let source = temp.path().join("safe"); - fixture(&source, "safe"); - let output = temp.path().join("safe-0.1.0.tar.gz"); - let args = || PluginPublishArgs { - path: Some(source.clone()), - dry_run: true, - output: Some(output.clone()), - }; - publish(args()).unwrap(); - assert!(output.is_file()); - // create_new keeps a second run from clobbering an existing archive. - assert!(publish(args()).is_err()); - - assert!( - publish(PluginPublishArgs { - path: Some(source.clone()), - dry_run: false, - output: None, - }) - .unwrap_err() - .to_string() - .contains("not implemented") - ); - assert!( - publish(PluginPublishArgs { - path: Some(source.clone()), - dry_run: true, - output: Some(source.join("inside.tar.gz")), - }) - .unwrap_err() - .to_string() - .contains("outside the source package") - ); - - let installs = temp.path().join("installed"); - assert_eq!( - install_local(&installs, &output, false, Some("0.1.0"), Some("safe")).unwrap(), - "safe" - ); - assert!(installs.join("safe/plugin.mjs").is_file()); - } - - #[tokio::test] - async fn downloads_are_restricted_to_the_registry_origin() { - for url in [ - "http://software.cortex.foundation/plugins/index", - "https://attacker.example/plugins/index", - "https://user@software.cortex.foundation/plugins/index", - "https://user:secret@software.cortex.foundation/plugins/index", - "https://software.cortex.foundation:8443/plugins/index", - "file:///etc/passwd", - ] { - let error = bounded_get(url).await.unwrap_err().to_string(); - assert!( - error.contains("https://software.cortex.foundation"), - "{url}: {error}" - ); - } - assert!(bounded_get("not a url").await.is_err()); - } -} +#[path = "install_tests.rs"] +mod tests; diff --git a/src/cortex-cli/src/plugin_cmd/install_tests.rs b/src/cortex-cli/src/plugin_cmd/install_tests.rs new file mode 100644 index 00000000..5de61455 --- /dev/null +++ b/src/cortex-cli/src/plugin_cmd/install_tests.rs @@ -0,0 +1,650 @@ +//! Plugin install/update tests: staging, hashing, and fail-closed pins. + +use super::*; +fn fixture(root: &Path, id: &str) { + std::fs::create_dir_all(root).unwrap(); + std::fs::write(root.join("plugin.toml"), format!( + "[plugin]\nid={id:?}\nname={id:?}\nversion=\"0.1.0\"\n[runtime]\nkind=\"node\"\nentrypoint=\"plugin.mjs\"\n")).unwrap(); + std::fs::write(root.join("plugin.mjs"), "export default {};").unwrap(); +} +#[test] +fn failed_update_retains_previous_package() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + let installs = temp.path().join("installed"); + fixture(&source, "safe"); + install_local(&installs, &source, false, None, None, None, "install").unwrap(); + let previous = runtime::package::fingerprint(&installs.join("safe")).unwrap(); + std::fs::write(source.join("plugin.mjs"), "invalid javascript !").unwrap(); + assert!( + install_local( + &installs, + &source, + true, + None, + Some("safe"), + None, + "install" + ) + .is_err() + ); + assert_eq!( + previous, + runtime::package::fingerprint(&installs.join("safe")).unwrap() + ); + assert!(runtime::package::destination(&installs, "../outside").is_err()); +} +#[test] +fn symlinks_and_archive_links_are_rejected() { + let temp = tempfile::tempdir().unwrap(); + #[cfg(unix)] + { + fixture(temp.path(), "safe"); + std::os::unix::fs::symlink("/etc/passwd", temp.path().join("escape")).unwrap(); + assert!(runtime::package::package_files(temp.path()).is_err()); + } + let tarball = temp.path().join("link.tar.gz"); + let encoder = flate2::write::GzEncoder::new( + std::fs::File::create(&tarball).unwrap(), + flate2::Compression::default(), + ); + let mut builder = tar::Builder::new(encoder); + let mut header = tar::Header::new_gnu(); + header.set_entry_type(tar::EntryType::Symlink); + header.set_size(0); + builder + .append_link(&mut header, "escape", "../outside") + .unwrap(); + builder.into_inner().unwrap().finish().unwrap(); + let stage = tempfile::tempdir().unwrap(); + assert!(extract(&tarball, stage.path()).is_err()); + assert!(!stage.path().join("escape").exists()); +} + +/// tar-rs refuses to build traversing paths, so the raw name field is written +/// directly to reproduce what a hostile archive actually contains. +fn hostile_archive(path: &Path, names: &[&str]) { + let encoder = flate2::write::GzEncoder::new( + std::fs::File::create(path).unwrap(), + flate2::Compression::default(), + ); + let mut builder = tar::Builder::new(encoder); + for name in names { + let mut header = tar::Header::new_ustar(); + header.set_size(1); + header.set_mode(0o644); + header.set_entry_type(tar::EntryType::Regular); + let field = &mut header.as_old_mut().name; + field.fill(0); + field[..name.len()].copy_from_slice(name.as_bytes()); + header.set_cksum(); + builder.append(&header, &b"x"[..]).unwrap(); + } + builder.into_inner().unwrap().finish().unwrap(); +} + +#[test] +fn hostile_archive_paths_are_rejected_before_anything_is_written() { + let temp = tempfile::tempdir().unwrap(); + let deep = (0..17).map(|_| "d").collect::>().join("/"); + let cases: Vec> = vec![ + vec!["../escape"], + vec!["/etc/passwd"], + vec!["a/../../escape"], + vec!["windows\\escape"], + vec!["same", "same"], + vec![deep.as_str()], + ]; + for names in &cases { + let tarball = temp.path().join("hostile.tar.gz"); + let _ = std::fs::remove_file(&tarball); + hostile_archive(&tarball, names); + let stage = tempfile::tempdir().unwrap(); + let error = extract(&tarball, stage.path()).unwrap_err().to_string(); + assert!(error.contains("invalid or duplicate path"), "{names:?}"); + assert!(!temp.path().join("escape").exists()); + assert!(!stage.path().join("d").exists(), "{names:?}"); + } + // A confined relative path in the same archive shape must still extract. + let tarball = temp.path().join("ok.tar.gz"); + hostile_archive(&tarball, &["nested/file"]); + let stage = tempfile::tempdir().unwrap(); + extract(&tarball, stage.path()).unwrap(); + assert_eq!( + std::fs::read(stage.path().join("nested/file")).unwrap(), + b"x" + ); +} + +#[test] +fn archive_entry_count_and_declared_size_limits_are_enforced() { + let temp = tempfile::tempdir().unwrap(); + let names: Vec = (0..=runtime::contract::MAX_PACKAGE_FILES) + .map(|index| format!("f{index}")) + .collect(); + let tarball = temp.path().join("many.tar.gz"); + hostile_archive( + &tarball, + &names.iter().map(String::as_str).collect::>(), + ); + let stage = tempfile::tempdir().unwrap(); + assert!( + extract(&tarball, stage.path()) + .unwrap_err() + .to_string() + .contains("1024 entries") + ); + + // A header that lies about a huge size must be rejected on the declared + // size, before that many bytes are ever written to disk. + let oversized = temp.path().join("huge.tar.gz"); + let encoder = flate2::write::GzEncoder::new( + std::fs::File::create(&oversized).unwrap(), + flate2::Compression::default(), + ); + let mut builder = tar::Builder::new(encoder); + let mut header = tar::Header::new_ustar(); + header.set_path("big").unwrap(); + header.set_size(runtime::contract::MAX_PACKAGE_BYTES + 1); + header.set_mode(0o644); + header.set_entry_type(tar::EntryType::Regular); + header.set_cksum(); + builder.append(&header, std::io::empty()).unwrap(); + builder.into_inner().unwrap().finish().unwrap(); + let stage = tempfile::tempdir().unwrap(); + assert!( + extract(&oversized, stage.path()) + .unwrap_err() + .to_string() + .contains("exceeds 64 MiB") + ); +} + +#[test] +fn archive_root_must_hold_exactly_one_directory_named_after_the_plugin() { + let temp = tempfile::tempdir().unwrap(); + let flat = temp.path().join("flat"); + fixture(&flat, "safe"); + assert_eq!(package_root(&flat).unwrap(), flat); + + let mismatch = temp.path().join("mismatch"); + fixture(&mismatch.join("other"), "safe"); + assert!( + package_root(&mismatch) + .unwrap_err() + .to_string() + .contains("must match the plugin ID") + ); + + let nested = temp.path().join("nested"); + fixture(&nested.join("safe"), "safe"); + assert_eq!(package_root(&nested).unwrap(), nested.join("safe")); + + let two = temp.path().join("two"); + fixture(&two.join("safe"), "safe"); + fixture(&two.join("other"), "other"); + assert!( + package_root(&two) + .unwrap_err() + .to_string() + .contains("one plugin at root") + ); +} + +#[test] +fn install_refuses_to_overwrite_or_accept_a_mismatched_identity() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + let installs = temp.path().join("installed"); + fixture(&source, "safe"); + + assert!( + install_local( + &installs, + &source, + false, + Some("9.9.9"), + None, + None, + "install" + ) + .unwrap_err() + .to_string() + .contains("identity or version") + ); + assert!( + install_local( + &installs, + &source, + false, + None, + Some("other"), + None, + "install" + ) + .is_err() + ); + assert!(!installs.join("safe").exists()); + + // Nested package files must be recreated under the staged package. + std::fs::create_dir(source.join("lib")).unwrap(); + std::fs::write(source.join("lib/helper.mjs"), "export const x = 1;").unwrap(); + install_local( + &installs, + &source, + false, + Some("0.1.0"), + Some("safe"), + None, + "install", + ) + .unwrap(); + assert!(installs.join("safe/lib/helper.mjs").is_file()); + + assert!( + install_local(&installs, &source, false, None, None, None, "install") + .unwrap_err() + .to_string() + .contains("--force") + ); + install_local(&installs, &source, true, None, None, None, "install").unwrap(); + + // Staging directories must never be left behind in the plugin root. + let leftovers: Vec<_> = std::fs::read_dir(&installs) + .unwrap() + .map(|entry| entry.unwrap().file_name()) + .filter(|name| name.to_string_lossy().starts_with(".staging-")) + .collect(); + assert!(leftovers.is_empty(), "{leftovers:?}"); +} + +#[test] +fn an_invalid_manifest_or_artifact_never_reaches_the_plugin_root() { + let temp = tempfile::tempdir().unwrap(); + let installs = temp.path().join("installed"); + let broken = temp.path().join("broken"); + std::fs::create_dir_all(&broken).unwrap(); + std::fs::write(broken.join("plugin.toml"), "this is not toml [[[").unwrap(); + assert!(install_local(&installs, &broken, false, None, None, None, "install").is_err()); + + let empty = temp.path().join("empty-artifact"); + fixture(&empty, "safe"); + std::fs::write(empty.join("plugin.mjs"), "").unwrap(); + assert!( + install_local(&installs, &empty, false, None, None, None, "install") + .unwrap_err() + .to_string() + .contains("Artifact") + ); + assert!(!installs.join("safe").exists()); +} + +#[test] +fn a_failed_replacement_rolls_the_previous_package_back() { + let temp = tempfile::tempdir().unwrap(); + let destination = temp.path().join("installed"); + fixture(&destination, "safe"); + let previous = runtime::package::fingerprint(&destination).unwrap(); + let stage = temp.path().join("stage"); + std::fs::create_dir(&stage).unwrap(); + + // A candidate that does not exist makes the second rename fail after the + // previous package has already been moved aside. + assert!(replace(&temp.path().join("missing"), &destination, &stage).is_err()); + assert_eq!( + previous, + runtime::package::fingerprint(&destination).unwrap() + ); + assert!(!stage.join("previous").exists()); +} + +#[test] +fn directory_entries_and_truncated_archive_files_are_handled() { + let temp = tempfile::tempdir().unwrap(); + let tarball = temp.path().join("dirs.tar.gz"); + let encoder = flate2::write::GzEncoder::new( + std::fs::File::create(&tarball).unwrap(), + flate2::Compression::default(), + ); + let mut builder = tar::Builder::new(encoder); + let mut header = tar::Header::new_ustar(); + header.set_path("nested").unwrap(); + header.set_size(0); + header.set_mode(0o755); + header.set_entry_type(tar::EntryType::Directory); + header.set_cksum(); + builder.append(&header, std::io::empty()).unwrap(); + let mut header = tar::Header::new_ustar(); + header.set_path("nested/file").unwrap(); + header.set_size(3); + header.set_mode(0o644); + header.set_entry_type(tar::EntryType::Regular); + header.set_cksum(); + builder.append(&header, &b"abc"[..]).unwrap(); + builder.into_inner().unwrap().finish().unwrap(); + let stage = tempfile::tempdir().unwrap(); + extract(&tarball, stage.path()).unwrap(); + assert!(stage.path().join("nested").is_dir()); + assert_eq!( + std::fs::read(stage.path().join("nested/file")).unwrap(), + b"abc" + ); + + // An archive whose payload stops early must fail, not install a partial file. + let full = std::fs::read(&tarball).unwrap(); + let truncated = temp.path().join("truncated.tar.gz"); + std::fs::write(&truncated, &full[..full.len() / 2]).unwrap(); + let stage = tempfile::tempdir().unwrap(); + assert!(extract(&truncated, stage.path()).is_err()); +} + +#[test] +fn an_oversized_compressed_archive_is_refused_before_it_is_opened() { + let temp = tempfile::tempdir().unwrap(); + let stage = tempfile::tempdir().unwrap(); + // A sparse file keeps this cheap while still exceeding the on-disk limit. + let big = temp.path().join("big.tar.gz"); + std::fs::File::create(&big) + .unwrap() + .set_len(runtime::contract::MAX_PACKAGE_BYTES + 1) + .unwrap(); + assert!( + extract(&big, stage.path()) + .unwrap_err() + .to_string() + .contains("Compressed package exceeds 64 MiB") + ); +} + +#[test] +fn the_install_lock_serializes_mutation_and_is_released_on_drop() { + let temp = tempfile::tempdir().unwrap(); + let root = temp.path().join("plugins"); + let held = InstallLock::acquire(&root).unwrap(); + assert!(root.join(".install.lock").is_file()); + let error = InstallLock::acquire(&root) + .err() + .expect("a second lock must not be granted") + .to_string(); + assert!( + error.contains("Another plugin operation is running"), + "{error}" + ); + drop(held); + assert!(!root.join(".install.lock").exists()); + InstallLock::acquire(&root).unwrap(); +} + +#[test] +fn publish_prepares_an_installable_archive_and_refuses_to_overwrite() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("safe"); + fixture(&source, "safe"); + let output = temp.path().join("safe-0.1.0.tar.gz"); + let args = || PluginPublishArgs { + path: Some(source.clone()), + dry_run: true, + output: Some(output.clone()), + }; + publish(args()).unwrap(); + assert!(output.is_file()); + // create_new keeps a second run from clobbering an existing archive. + assert!(publish(args()).is_err()); + + assert!( + publish(PluginPublishArgs { + path: Some(source.clone()), + dry_run: false, + output: None, + }) + .unwrap_err() + .to_string() + .contains("not implemented") + ); + assert!( + publish(PluginPublishArgs { + path: Some(source.clone()), + dry_run: true, + output: Some(source.join("inside.tar.gz")), + }) + .unwrap_err() + .to_string() + .contains("outside the source package") + ); + + let installs = temp.path().join("installed"); + assert_eq!( + install_local( + &installs, + &output, + false, + Some("0.1.0"), + Some("safe"), + None, + "install" + ) + .unwrap(), + "safe" + ); + assert!(installs.join("safe/plugin.mjs").is_file()); +} + +#[tokio::test] +async fn downloads_are_restricted_to_the_registry_origin() { + for url in [ + "http://software.cortex.foundation/plugins/index", + "https://attacker.example/plugins/index", + "https://user@software.cortex.foundation/plugins/index", + "https://user:secret@software.cortex.foundation/plugins/index", + "https://software.cortex.foundation:8443/plugins/index", + "file:///etc/passwd", + ] { + let error = bounded_get(url).await.unwrap_err().to_string(); + assert!( + error.contains("https://software.cortex.foundation"), + "{url}: {error}" + ); + } + assert!(bounded_get("not a url").await.is_err()); +} + +/// Manifest for the accepted-hash fixtures, read through the shipped +/// package validator so the hash covers what an install would register. +fn manifest_of(source: &Path) -> runtime::PluginManifest { + runtime::package::validate_package(source).unwrap() +} + +#[test] +fn an_accepted_hash_installs_and_a_changed_manifest_does_not() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + let installs = temp.path().join("installed"); + fixture(&source, "safe"); + let accepted = runtime::command_pin::command_hash(&manifest_of(&source)); + + // The reviewed hash installs. + assert_eq!( + install_local( + &installs, + &source, + false, + None, + None, + Some(&accepted), + "install" + ) + .unwrap(), + "safe" + ); + + // A manifest that changed after the review must not install, and the + // previous package must survive untouched. + let previous = runtime::package::fingerprint(&installs.join("safe")).unwrap(); + let changed = temp.path().join("changed"); + fixture(&changed, "safe"); + std::fs::write( + changed.join("plugin.toml"), + "[plugin]\nid=\"safe\"\nname=\"safe\"\nversion=\"0.1.0\"\n[runtime]\nkind=\"node\"\nentrypoint=\"plugin.mjs\"\n[[commands]]\nname=\"sneak\"\ndescription=\"added after review\"\n", + ) + .unwrap(); + let error = install_local( + &installs, + &changed, + true, + None, + Some("safe"), + Some(&accepted), + "install", + ) + .unwrap_err() + .to_string(); + assert!( + error.contains("Command hash mismatch. Manifest may have changed."), + "{error}" + ); + assert!(error.contains("Re-run with --json"), "{error}"); + assert_eq!( + previous, + runtime::package::fingerprint(&installs.join("safe")).unwrap(), + "a refused install must leave the installed package alone" + ); +} + +#[test] +fn a_malformed_accepted_hash_fails_before_anything_is_staged() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + let installs = temp.path().join("installed"); + fixture(&source, "safe"); + for bad in ["", "abc", &"z".repeat(64)] { + let error = install_local(&installs, &source, false, None, None, Some(bad), "install") + .unwrap_err() + .to_string(); + assert!(error.contains("SHA-256"), "{bad:?}: {error}"); + } + assert!(!installs.join("safe").exists()); +} + +#[test] +fn the_review_document_matches_the_pinned_hash() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + fixture(&source, "safe"); + let manifest = manifest_of(&source); + let review = command_review(&manifest); + assert_eq!( + review["command_hash"], + runtime::command_pin::command_hash(&manifest) + ); + assert_eq!(review["id"], "safe"); + assert_eq!(review["version"], "0.1.0"); + assert!(review["commands"].is_array()); + // A `--json` review followed by the printed hash installs. + let accepted = review["command_hash"].as_str().unwrap().to_string(); + let installs = temp.path().join("installed"); + install_local( + &installs, + &source, + false, + None, + None, + Some(&accepted), + "install", + ) + .unwrap(); +} + +/// Organization policy that requires a pin refuses an unpinned install. +#[test] +fn organization_policy_requires_the_pin() { + use cortex_engine::org_policy::PluginInstallPolicy; + + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + fixture(&source, "safe"); + let manifest = manifest_of(&source); + + let required = PluginInstallPolicy::RequireAcceptCommand; + assert!(required.requires_pin()); + let unpinned = enforce_command_pin_with(&manifest, None, "install", required) + .unwrap_err() + .to_string(); + assert!(unpinned.contains("requires --accept-command"), "{unpinned}"); + assert!(unpinned.contains("--json"), "{unpinned}"); + + // The reviewed hash is accepted under the same policy. + let accepted = runtime::command_pin::command_hash(&manifest); + assert!(enforce_command_pin_with(&manifest, Some(&accepted), "install", required).is_ok()); + // A changed manifest is still refused even with a pin present. + let changed = temp.path().join("changed"); + fixture(&changed, "safe"); + std::fs::write( + changed.join("plugin.toml"), + "[plugin]\nid=\"safe\"\nname=\"safe\"\nversion=\"0.2.0\"\n[runtime]\nkind=\"node\"\nentrypoint=\"plugin.mjs\"\n", + ) + .unwrap(); + assert!( + enforce_command_pin_with(&manifest_of(&changed), Some(&accepted), "install", required) + .unwrap_err() + .to_string() + .contains("Command hash mismatch") + ); + + // Without the requirement an unpinned install is unchanged. + let optional = PluginInstallPolicy::HostDefault; + assert!(!optional.requires_pin()); + assert!(enforce_command_pin_with(&manifest, None, "install", optional).is_ok()); +} + +/// A refused install leaves nothing behind in the plugin root. +#[test] +fn an_org_required_pin_never_reaches_the_plugin_root() { + use cortex_engine::org_policy::PluginInstallPolicy; + + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + let installs = temp.path().join("installed"); + fixture(&source, "safe"); + let manifest = manifest_of(&source); + // Exercise the shipped decision path with the requirement in place. + let error = enforce_command_pin_with( + &manifest, + None, + "install", + PluginInstallPolicy::RequireAcceptCommand, + ) + .unwrap_err() + .to_string(); + assert!(error.contains("requires --accept-command"), "{error}"); + assert!(!installs.join("safe").exists()); +} + +/// An accepted pin is written to the audit journal as one JSON line. +#[test] +fn an_accepted_pin_is_audited() { + let temp = tempfile::tempdir().unwrap(); + let source = temp.path().join("source"); + fixture(&source, "safe"); + let manifest = manifest_of(&source); + let accepted = runtime::command_pin::command_hash(&manifest); + + let home = temp.path().join("home"); + let journal = cortex_engine::audit::record( + &home, + cortex_engine::audit::AuditKind::PluginCommandAccepted, + serde_json::json!({ + "plugin": manifest.plugin.id, + "version": manifest.plugin.version, + "accepted_hash": accepted, + "actual_hash": runtime::command_pin::command_hash(&manifest), + "action": "install", + "policy": "host_default", + }), + ) + .unwrap(); + let body = std::fs::read_to_string(&journal).unwrap(); + assert!(body.contains("plugin_command_accepted"), "{body}"); + assert!(body.contains(&accepted), "{body}"); + assert!(body.contains("\"action\":\"install\""), "{body}"); + assert_eq!(body.lines().count(), 1); +} diff --git a/src/cortex-cli/tests/cli_schema.rs b/src/cortex-cli/tests/cli_schema.rs index 0581c72a..e1dfed84 100644 --- a/src/cortex-cli/tests/cli_schema.rs +++ b/src/cortex-cli/tests/cli_schema.rs @@ -86,3 +86,57 @@ fn test_plugin_version_and_global_verbosity_are_distinct() { assert_eq!(install.get_one::("version").unwrap(), "1.2.3"); assert!(install.get_flag("verbose")); } + +/// COR-448: the reviewed hash is passed as an explicit value, and a bare +/// `install`/`update` carries none. +#[test] +fn test_plugin_accept_command_flag_is_parsed() { + let hash = "8f4c2a71e0b6d3a5c19f7b204e8a1d6f30c5b9a7e2d4816f0a3c7b5d9e1f2a46"; + let matches = Cli::command() + .try_get_matches_from([ + "cortex", + "plugin", + "install", + "cortex-review", + "--accept-command", + hash, + "--json", + ]) + .expect("plugin install --accept-command must parse"); + let install = matches + .subcommand_matches("plugin") + .unwrap() + .subcommand_matches("install") + .unwrap(); + assert_eq!(install.get_one::("accept_command").unwrap(), hash); + assert!(install.get_flag("json")); + + let matches = Cli::command() + .try_get_matches_from([ + "cortex", + "plugin", + "update", + "cortex-review", + "--accept-command", + hash, + ]) + .expect("plugin update --accept-command must parse"); + let update = matches + .subcommand_matches("plugin") + .unwrap() + .subcommand_matches("update") + .unwrap(); + assert_eq!(update.get_one::("accept_command").unwrap(), hash); + + // Without the flag the value is absent, so policy can still require it. + let matches = Cli::command() + .try_get_matches_from(["cortex", "plugin", "install", "cortex-review"]) + .unwrap(); + let install = matches + .subcommand_matches("plugin") + .unwrap() + .subcommand_matches("install") + .unwrap(); + assert!(install.get_one::("accept_command").is_none()); + assert!(!install.get_flag("json")); +} diff --git a/src/cortex-engine/src/agents.rs b/src/cortex-engine/src/agents.rs index ddfe74cd..ffadf7b5 100644 --- a/src/cortex-engine/src/agents.rs +++ b/src/cortex-engine/src/agents.rs @@ -107,6 +107,9 @@ pub struct AgentMetadata { /// Agents with enabled=false are not registered with the Task tool. #[serde(default = "default_enabled")] pub enabled: bool, + /// Instruction documents this agent skips. Managed is never omitted. + #[serde(default, alias = "omit-instructions")] + pub omit_instructions: Vec, } fn default_can_delegate() -> bool { @@ -134,6 +137,7 @@ impl AgentMetadata { can_delegate: true, max_turns: None, enabled: true, + omit_instructions: Vec::new(), } } @@ -363,6 +367,7 @@ impl AgentRegistry { can_delegate: false, max_turns: Some(10), enabled: true, + omit_instructions: Vec::new(), }, system_prompt: CODE_EXPLORER_PROMPT.to_string(), path: PathBuf::new(), @@ -387,6 +392,7 @@ impl AgentRegistry { can_delegate: false, max_turns: Some(5), enabled: true, + omit_instructions: Vec::new(), }, system_prompt: CODE_REVIEWER_PROMPT.to_string(), path: PathBuf::new(), @@ -412,6 +418,7 @@ impl AgentRegistry { can_delegate: true, max_turns: Some(15), enabled: true, + omit_instructions: Vec::new(), }, system_prompt: ARCHITECT_PROMPT.to_string(), path: PathBuf::new(), @@ -837,6 +844,7 @@ mod tests { can_delegate: true, max_turns: None, enabled: true, + omit_instructions: Vec::new(), }, system_prompt: String::new(), path: PathBuf::new(), diff --git a/src/cortex-engine/src/audit.rs b/src/cortex-engine/src/audit.rs new file mode 100644 index 00000000..6cb90b4e --- /dev/null +++ b/src/cortex-engine/src/audit.rs @@ -0,0 +1,133 @@ +//! Append-only audit journal for fail-closed decisions. +//! +//! One JSON object per line under `{cortex_home}/audit/events.jsonl`. Records +//! name the decision, the affected scope or plugin, and a hash — never a +//! prompt, file body, or secret. A record that cannot be written is reported +//! to the caller so the decision can fail closed instead of proceeding +//! unlogged. + +use std::io::Write; +use std::path::{Path, PathBuf}; + +use serde_json::{Value, json}; + +/// Directory under the Cortex home that holds the journal. +pub const AUDIT_DIR: &str = "audit"; +/// Journal file name. +pub const AUDIT_FILE: &str = "events.jsonl"; +/// Journal schema version, bumped when the record shape changes. +pub const AUDIT_SCHEMA_VERSION: u32 = 1; + +/// Decision kinds written by this build. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum AuditKind { + /// A plugin command hash was pinned by the user and accepted. + PluginCommandAccepted, + /// Instruction documents were omitted from a session or subagent. + InstructionsOmitted, + /// An omission request named managed policy, which is never omitted. + ManagedPolicyNeverOmitted, +} + +impl AuditKind { + /// Stable `kind` value in the journal. + pub const fn as_str(self) -> &'static str { + match self { + Self::PluginCommandAccepted => "plugin_command_accepted", + Self::InstructionsOmitted => "instructions_omitted", + Self::ManagedPolicyNeverOmitted => "managed_policy_never_omitted", + } + } +} + +/// Journal path for a Cortex home. +pub fn audit_path(cortex_home: &Path) -> PathBuf { + cortex_home.join(AUDIT_DIR).join(AUDIT_FILE) +} + +/// Append one record. Creates the directory on first write. +/// +/// The file is opened append-only so a previous journal is never rewritten. +pub fn record(cortex_home: &Path, kind: AuditKind, detail: Value) -> std::io::Result { + let path = audit_path(cortex_home); + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent)?; + } + let line = json!({ + "schema": AUDIT_SCHEMA_VERSION, + "ts": chrono::Utc::now().to_rfc3339(), + "kind": kind.as_str(), + "detail": detail, + }); + let mut file = std::fs::OpenOptions::new() + .create(true) + .append(true) + .open(&path)?; + writeln!(file, "{line}")?; + file.flush()?; + Ok(path) +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + #[test] + fn records_append_as_one_json_object_per_line() { + let temp = tempfile::tempdir().unwrap(); + let first = record( + temp.path(), + AuditKind::InstructionsOmitted, + json!({"source": "subagent", "skipped": ["user", "project"]}), + ) + .unwrap(); + record( + temp.path(), + AuditKind::ManagedPolicyNeverOmitted, + json!({"source": "subagent", "requested": ["managed"]}), + ) + .unwrap(); + assert_eq!(first, audit_path(temp.path())); + let body = std::fs::read_to_string(&first).unwrap(); + let lines: Vec<_> = body.lines().collect(); + assert_eq!(lines.len(), 2); + for line in lines { + let value: Value = serde_json::from_str(line).unwrap(); + assert_eq!(value["schema"], AUDIT_SCHEMA_VERSION); + assert!(value["ts"].as_str().is_some_and(|t| t.contains('T'))); + assert!(value["kind"].as_str().is_some_and(|k| !k.is_empty())); + assert!(value["detail"].is_object()); + } + // No prompt or file body field can appear in a record. + assert!(!body.contains("prompt")); + } + + #[test] + fn kind_strings_are_stable() { + assert_eq!( + AuditKind::PluginCommandAccepted.as_str(), + "plugin_command_accepted" + ); + assert_eq!( + AuditKind::InstructionsOmitted.as_str(), + "instructions_omitted" + ); + assert_eq!( + AuditKind::ManagedPolicyNeverOmitted.as_str(), + "managed_policy_never_omitted" + ); + } + + #[test] + fn an_unwritable_journal_is_reported_not_swallowed() { + let temp = tempfile::tempdir().unwrap(); + let blocked = temp.path().join("audit"); + // A file where the directory must be makes the append impossible. + std::fs::write(&blocked, b"not a directory").unwrap(); + assert!( + record(temp.path(), AuditKind::InstructionsOmitted, json!({})).is_err(), + "an unwritable journal must surface as an error" + ); + } +} diff --git a/src/cortex-engine/src/client/code_agent.rs b/src/cortex-engine/src/client/code_agent.rs index 90530529..b90a8065 100644 --- a/src/cortex-engine/src/client/code_agent.rs +++ b/src/cortex-engine/src/client/code_agent.rs @@ -65,6 +65,8 @@ pub struct CodeTurnContext { pub computer: ComputerKind, pub turn_mode: Option, pub ssh_target: Option, + /// When true, the turn uses the low-latency Fast path (org policy allowing). + pub fast_mode: bool, } /// Fields accepted by `POST /v1/code/sessions`. @@ -301,6 +303,7 @@ impl CodeAgentClient { .ok() .map(|p| p.display().to_string()), turn_mode: None, + fast_mode: false, })), } } @@ -625,12 +628,15 @@ impl CodeAgentClient { })?; let session_id = self.ensure_session().await?; let url = format!("{}/v1/code/sessions/{session_id}/turns", self.base_url); + let fast_mode = self.turn_context().fast_mode; let body = serde_json::json!({ "message": message, "mode": mode.as_str(), // An existing Code conversation keeps its mode. Interaction is the // supported per-turn read-only/planning control in the backend. "interaction": if mode == CodeTurnMode::Chat { "plan" } else { "agent" }, + // Propagate the session Fast setting so remote turns are not UI-only. + "fast_mode": fast_mode, }); let mut req = self diff --git a/src/cortex-engine/src/client/code_agent_tests.rs b/src/cortex-engine/src/client/code_agent_tests.rs index 7f1fc001..9478791e 100644 --- a/src/cortex-engine/src/client/code_agent_tests.rs +++ b/src/cortex-engine/src/client/code_agent_tests.rs @@ -203,6 +203,7 @@ async fn live_guest_code_turn_streams_tokens() { computer: ComputerKind::Cloud, turn_mode: Some(CodeTurnMode::Chat), ssh_target: None, + fast_mode: false, }); let mut stream = client diff --git a/src/cortex-engine/src/client/computer.rs b/src/cortex-engine/src/client/computer.rs index 8008b964..6e169cac 100644 --- a/src/cortex-engine/src/client/computer.rs +++ b/src/cortex-engine/src/client/computer.rs @@ -62,6 +62,12 @@ impl ComputerKind { } } + /// True for the Code Remote runtimes — cloud and self-hosted SSH. These + /// carry the `Remote` status line and the Fast chip. + pub const fn is_remote(self) -> bool { + matches!(self, Self::Cloud | Self::Ssh) + } + pub fn as_str(self) -> &'static str { match self { Self::ThisPc => "this_pc", @@ -94,6 +100,13 @@ mod tests { assert_eq!(CodeTurnContext::default().computer, ComputerKind::Cloud); } + #[test] + fn remote_runtimes_are_cloud_and_ssh() { + assert!(ComputerKind::Cloud.is_remote()); + assert!(ComputerKind::Ssh.is_remote()); + assert!(!ComputerKind::ThisPc.is_remote()); + } + #[test] fn computer_kind_from_env_defaults_to_cloud() { assert_eq!( diff --git a/src/cortex-engine/src/fast_mode.rs b/src/cortex-engine/src/fast_mode.rs new file mode 100644 index 00000000..0d1df4f8 --- /dev/null +++ b/src/cortex-engine/src/fast_mode.rs @@ -0,0 +1,290 @@ +//! Fast-mode availability and organization policy. +//! +//! Fast mode is the low-latency turn path. An organization can disable it for +//! every member. When it is disabled, `/fast` and the CLI never silently fall +//! back to Standard mid-turn: the request is refused with product copy and the +//! session stays on Standard. +//! +//! Policy resolution fails closed and lives in [`crate::org_policy`]: a policy +//! document that exists but cannot be read, or that carries an unknown value, +//! denies fast mode. Only an absent document means the host default applies. + +use crate::error::{CortexError, Result}; +use crate::org_policy; + +pub use crate::org_policy::{FastModePolicy, POLICY_DIR_ENV, POLICY_FILE, policy_dir, policy_path}; + +/// Product copy shown when the organization has disabled fast mode. +pub const ORG_DISABLED_TOAST: &str = + "Fast mode is disabled for your organization. Contact your admin."; +/// Second line of the same refusal: the session does not switch. +pub const ORG_DISABLED_STAYING: &str = "Staying on Standard."; +/// Status-line chip while fast mode is on for a remote session. +pub const FAST_CHIP: &str = "Fast"; +/// Wide status line for a remote session with fast mode on. +pub const REMOTE_FAST_STATUS: &str = "Remote · Fast mode on"; +/// Narrow (40-column) status line for a remote session with fast mode on. +pub const REMOTE_FAST_STATUS_NARROW: &str = "Remote · Fast"; +/// Wide status line for a remote session on the default path. +pub const REMOTE_STATUS: &str = "Remote · Standard"; +/// Narrow (40-column) status line for a remote session on the default path. +pub const REMOTE_STATUS_NARROW: &str = "Remote"; + +/// Resolve fast-mode policy from an explicit directory. +pub fn resolve_policy(dir: Option<&std::path::Path>) -> FastModePolicy { + org_policy::resolve(dir).fast_mode +} + +/// Resolve fast-mode policy from the environment. +pub fn current_policy() -> FastModePolicy { + org_policy::current().fast_mode +} + +/// Requested fast-mode state for a session. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Default)] +pub enum FastMode { + /// Default turn path. + #[default] + Standard, + /// Low-latency turn path, when policy allows it. + Fast, +} + +impl FastMode { + /// True when fast mode is on. + pub const fn is_on(self) -> bool { + matches!(self, Self::Fast) + } + + /// Parse an on/off token. Unknown tokens are an error, never a toggle. + pub fn parse(value: &str) -> Result { + match value.trim().to_ascii_lowercase().as_str() { + "on" | "true" | "enable" | "enabled" => Ok(Self::Fast), + "off" | "false" | "disable" | "disabled" => Ok(Self::Standard), + other => Err(CortexError::InvalidInput(format!( + "Invalid fast mode value: {other}. Use on|off" + ))), + } + } +} + +/// Outcome of a `/fast` request. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum FastModeOutcome { + /// Fast mode is now on. + Enabled, + /// Fast mode is now off. + Disabled, + /// The organization disabled fast mode; the session stays on Standard. + RefusedByOrgPolicy { + /// First toast line. + toast: &'static str, + /// Second toast line, naming the state the session stays in. + staying: &'static str, + }, +} + +impl FastModeOutcome { + /// True when the request changed the session state. + pub const fn applied(&self) -> bool { + matches!(self, Self::Enabled | Self::Disabled) + } + + /// True when the request was refused because of organization policy. + pub const fn refused(&self) -> bool { + matches!(self, Self::RefusedByOrgPolicy { .. }) + } + + /// Toasts to show, in order. + pub fn toasts(&self) -> Vec<&'static str> { + match self { + Self::Enabled => vec!["Fast mode on."], + Self::Disabled => vec!["Fast mode off."], + Self::RefusedByOrgPolicy { toast, staying } => vec![*toast, *staying], + } + } +} + +/// Apply a fast-mode request against a resolved policy. +/// +/// A refusal never mutates the session and never queues a silent re-send. +pub fn apply_fast_mode(requested: FastMode, policy: FastModePolicy) -> FastModeOutcome { + if requested.is_on() && policy.is_disabled() { + return FastModeOutcome::RefusedByOrgPolicy { + toast: ORG_DISABLED_TOAST, + staying: ORG_DISABLED_STAYING, + }; + } + if requested.is_on() { + FastModeOutcome::Enabled + } else { + FastModeOutcome::Disabled + } +} + +/// Status line for a remote session. +/// +/// `remote` is true for cloud and self-hosted Code sessions. Fast mode shows +/// its chip only while the session is actually on fast. +pub fn remote_status_line(remote: bool, fast: bool, narrow: bool) -> Option<&'static str> { + if !remote { + return None; + } + Some(match (fast, narrow) { + (true, false) => REMOTE_FAST_STATUS, + (true, true) => REMOTE_FAST_STATUS_NARROW, + (false, false) => REMOTE_STATUS, + (false, true) => REMOTE_STATUS_NARROW, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write_policy(dir: &std::path::Path, body: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write(policy_path(dir), body).unwrap(); + } + + #[test] + fn absent_policy_document_uses_the_host_default() { + let temp = tempfile::tempdir().unwrap(); + assert_eq!( + resolve_policy(Some(temp.path())), + FastModePolicy::HostDefault + ); + assert_eq!(resolve_policy(None), FastModePolicy::HostDefault); + assert!(resolve_policy(None).allows()); + } + + #[test] + fn a_disabled_organization_refuses_fast_mode() { + let temp = tempfile::tempdir().unwrap(); + write_policy(temp.path(), r#"{"fast_mode": false}"#); + let policy = resolve_policy(Some(temp.path())); + assert_eq!(policy, FastModePolicy::Disabled); + assert!(policy.is_disabled()); + assert!(!policy.allows()); + + let outcome = apply_fast_mode(FastMode::Fast, policy); + assert!(outcome.refused()); + assert!(!outcome.applied()); + assert_eq!( + outcome.toasts(), + vec![ORG_DISABLED_TOAST, ORG_DISABLED_STAYING] + ); + assert!( + outcome.toasts()[0].contains("disabled for your organization"), + "{:?}", + outcome.toasts() + ); + assert!( + outcome.toasts()[1].contains("Staying on Standard"), + "{:?}", + outcome.toasts() + ); + } + + #[test] + fn a_refusal_is_independent_of_the_current_state() { + let temp = tempfile::tempdir().unwrap(); + write_policy(temp.path(), r#"{"fast_mode": false}"#); + let policy = resolve_policy(Some(temp.path())); + // Already on fast: a refused request still reports refusal, so the + // caller cannot treat it as a state change. + let outcome = apply_fast_mode(FastMode::Fast, policy); + assert!(outcome.refused()); + assert!(!outcome.applied()); + // Turning fast off is always allowed. + assert_eq!( + apply_fast_mode(FastMode::Standard, policy), + FastModeOutcome::Disabled + ); + } + + #[test] + fn allowed_organization_applies_the_request() { + let temp = tempfile::tempdir().unwrap(); + write_policy(temp.path(), r#"{"fast_mode": true}"#); + let policy = resolve_policy(Some(temp.path())); + assert_eq!(policy, FastModePolicy::Allowed); + let outcome = apply_fast_mode(FastMode::Fast, policy); + assert_eq!(outcome, FastModeOutcome::Enabled); + assert!(outcome.applied()); + assert_eq!(outcome.toasts(), vec!["Fast mode on."]); + let off = apply_fast_mode(FastMode::Standard, policy); + assert_eq!(off, FastModeOutcome::Disabled); + assert!(off.applied()); + } + + #[test] + fn text_values_are_resolved_and_unknown_values_fail_closed() { + let temp = tempfile::tempdir().unwrap(); + write_policy(temp.path(), r#"{"fast_mode": "on"}"#); + assert_eq!(resolve_policy(Some(temp.path())), FastModePolicy::Allowed); + write_policy(temp.path(), r#"{"fast_mode": "ENABLED"}"#); + assert_eq!(resolve_policy(Some(temp.path())), FastModePolicy::Allowed); + write_policy(temp.path(), r#"{"fast_mode": "off"}"#); + assert_eq!(resolve_policy(Some(temp.path())), FastModePolicy::Disabled); + // An unknown value is not permission. + write_policy(temp.path(), r#"{"fast_mode": "maybe"}"#); + assert_eq!(resolve_policy(Some(temp.path())), FastModePolicy::Disabled); + } + + #[test] + fn an_unparseable_or_unreadable_policy_fails_closed() { + let temp = tempfile::tempdir().unwrap(); + write_policy(temp.path(), "{ not json"); + assert_eq!(resolve_policy(Some(temp.path())), FastModePolicy::Disabled); + + let other = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(policy_path(other.path())).unwrap(); + assert_eq!(resolve_policy(Some(other.path())), FastModePolicy::Disabled); + } + + #[test] + fn a_policy_document_without_the_key_keeps_the_host_default() { + let temp = tempfile::tempdir().unwrap(); + write_policy(temp.path(), r#"{"other_setting": true}"#); + assert_eq!( + resolve_policy(Some(temp.path())), + FastModePolicy::HostDefault + ); + } + + #[test] + fn fast_mode_tokens_reject_unknown_values() { + assert_eq!(FastMode::parse("on").unwrap(), FastMode::Fast); + assert_eq!(FastMode::parse(" OFF ").unwrap(), FastMode::Standard); + assert!(FastMode::parse("sometimes").is_err()); + assert!(!FastMode::default().is_on()); + assert!(FastMode::Fast.is_on()); + } + + #[test] + fn remote_status_line_names_fast_only_when_it_is_on() { + assert_eq!( + remote_status_line(true, true, false), + Some("Remote · Fast mode on") + ); + assert_eq!(remote_status_line(true, true, true), Some("Remote · Fast")); + assert_eq!( + remote_status_line(true, false, false), + Some("Remote · Standard") + ); + assert_eq!(remote_status_line(true, false, true), Some("Remote")); + assert_eq!(remote_status_line(false, true, false), None); + assert_eq!(remote_status_line(false, false, false), None); + assert_eq!(FAST_CHIP, "Fast"); + } + + #[test] + fn policy_names_are_stable() { + assert_eq!(FastModePolicy::HostDefault.as_str(), "host_default"); + assert_eq!(FastModePolicy::Allowed.as_str(), "allowed"); + assert_eq!(FastModePolicy::Disabled.as_str(), "disabled"); + assert_eq!(POLICY_FILE, "policy.json"); + assert_eq!(POLICY_DIR_ENV, "CORTEX_ORG_POLICY_DIR"); + } +} diff --git a/src/cortex-engine/src/instruction_scopes.rs b/src/cortex-engine/src/instruction_scopes.rs new file mode 100644 index 00000000..b732c1f9 --- /dev/null +++ b/src/cortex-engine/src/instruction_scopes.rs @@ -0,0 +1,565 @@ +//! Instruction-document scopes and the opt-in omission plan. +//! +//! Cortex merges instruction Markdown from several places: the personal +//! `~/.cortex` documents, the project root, the directories between the +//! project root and the working directory, and organization-managed policy. +//! A subagent or a headless run may ask to omit the user, project, or local +//! documents. +//! +//! Organization-managed policy is **never** omitted. A request that names it +//! is recorded and the managed document still loads. + +use std::collections::BTreeSet; + +use serde::{Deserialize, Serialize}; + +/// Where an instruction document came from. +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub enum InstructionScope { + /// Personal documents under the Cortex home (`~/.cortex/AGENTS.md`). + User, + /// The repository-root document. + Project, + /// Documents between the repository root and the working directory. + Local, + /// Organization-managed policy. Never omitted. + Managed, +} + +impl InstructionScope { + /// Every scope, in merge order. + pub const ALL: [Self; 4] = [Self::User, Self::Project, Self::Local, Self::Managed]; + + /// Stable name used in JSON, audit records, and diagnostics. + pub const fn as_str(self) -> &'static str { + match self { + Self::User => "user", + Self::Project => "project", + Self::Local => "local", + Self::Managed => "managed", + } + } + + /// Parse a scope name. Unknown names are rejected so a typo cannot + /// silently widen what is omitted. + pub fn parse(value: &str) -> Option { + match value.trim().to_ascii_lowercase().as_str() { + "user" | "personal" => Some(Self::User), + "project" | "repo" => Some(Self::Project), + "local" | "cwd" => Some(Self::Local), + "managed" | "org" | "organization" => Some(Self::Managed), + _ => None, + } + } + + /// True for the scope that no request can remove. + pub const fn is_managed(self) -> bool { + matches!(self, Self::Managed) + } +} + +impl std::fmt::Display for InstructionScope { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +/// Resolved load/skip decision for one run. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +pub struct InstructionPlan { + skipped: BTreeSet, + managed_forced: bool, + requested_managed: bool, +} + +impl InstructionPlan { + /// Nothing omitted: every scope loads. + pub fn load_all() -> Self { + Self::default() + } + + /// Resolve a requested omission list. + /// + /// `Managed` is removed from the skip set. When the request named it, the + /// plan reports [`Self::managed_forced`] and [`Self::requested_managed`] + /// so the caller can audit that managed policy still loaded. + pub fn new(requested: &[InstructionScope]) -> Self { + let mut skipped: BTreeSet = BTreeSet::new(); + let mut requested_managed = false; + for scope in requested { + if scope.is_managed() { + requested_managed = true; + continue; + } + skipped.insert(*scope); + } + Self { + skipped, + managed_forced: requested_managed, + requested_managed, + } + } + + /// True when this scope loads for the run. `Managed` always loads. + pub fn loads(&self, scope: InstructionScope) -> bool { + scope.is_managed() || !self.skipped.contains(&scope) + } + + /// True when any document is actually omitted. + pub fn omits_anything(&self) -> bool { + !self.skipped.is_empty() + } + + /// Scopes that will not load, in a stable order. + pub fn skipped(&self) -> Vec { + self.skipped.iter().copied().collect() + } + + /// True when the request named managed policy and it loaded anyway. + pub fn managed_forced(&self) -> bool { + self.managed_forced + } + + /// True when the request named managed policy, even if it was already + /// covered by another rule. + pub fn requested_managed(&self) -> bool { + self.requested_managed + } + + /// One product-facing line describing what this run loads and skips. + /// + /// Returns `None` when nothing was omitted, so an untouched run prints no + /// extra chrome. + pub fn summary(&self, source: &str) -> Option { + if !self.omits_anything() && !self.requested_managed { + return None; + } + let skipped = if self.skipped.is_empty() { + "nothing".to_string() + } else { + self.skipped() + .iter() + .map(|s| s.as_str()) + .collect::>() + .join(", ") + }; + let managed = if self.requested_managed { + "managed policy still loaded" + } else { + "managed policy loaded" + }; + Some(format!( + "Instructions omitted for {source}: {skipped}. {managed}." + )) + } +} + +/// Parse an `omit_instructions` list, rejecting unknown names. +/// +/// A name that is not a scope is an error rather than a silent no-op: an +/// omission request must never quietly apply to the wrong set of documents. +pub fn parse_scopes<'a, I>(values: I) -> Result, String> +where + I: IntoIterator, +{ + let mut scopes = Vec::new(); + for value in values { + let scope = InstructionScope::parse(value).ok_or_else(|| { + format!("Unknown instruction scope: {value}. Use user, project, local, or managed.") + })?; + if !scopes.contains(&scope) { + scopes.push(scope); + } + } + Ok(scopes) +} + +/// Managed policy file name. Read from the organization policy directory and +/// never omitted. +pub const MANAGED_POLICY_FILE: &str = "AGENTS.md"; + +/// The files that make up each scope for one run. +#[derive(Debug, Clone, Default)] +pub struct InstructionSources { + /// `{cortex_home}/AGENTS.md`. + pub user: Vec, + /// The repository-root `AGENTS.md`. + pub project: Vec, + /// `AGENTS.md` in directories between the repository root and the cwd. + pub local: Vec, + /// Organization-managed policy. Loaded whatever the plan says. + pub managed: Vec, +} + +impl InstructionSources { + /// Paths for one scope. + pub fn for_scope(&self, scope: InstructionScope) -> &[std::path::PathBuf] { + match scope { + InstructionScope::User => &self.user, + InstructionScope::Project => &self.project, + InstructionScope::Local => &self.local, + InstructionScope::Managed => &self.managed, + } + } + + /// Discover user/project/local/managed paths for a working directory. + /// + /// Project is the repository-root `AGENTS.md`. Local is every `AGENTS.md` + /// between the repository root (exclusive) and `cwd` (inclusive). Managed + /// policy comes from the organization policy directory when configured. + pub fn discover(cwd: &std::path::Path, cortex_home: &std::path::Path) -> Self { + let user = vec![cortex_home.join("AGENTS.md")]; + let repo_root = find_git_root(cwd).unwrap_or_else(|| cwd.to_path_buf()); + let project = vec![repo_root.join("AGENTS.md")]; + let mut local = Vec::new(); + if let Ok(relative) = cwd.strip_prefix(&repo_root) { + let mut path = repo_root.clone(); + for component in relative.components() { + path = path.join(component); + if path != repo_root { + local.push(path.join("AGENTS.md")); + } + } + } + let managed = crate::org_policy::policy_dir() + .map(|dir| vec![dir.join(MANAGED_POLICY_FILE)]) + .unwrap_or_default(); + Self { + user, + project, + local, + managed, + } + } +} + +fn find_git_root(start: &std::path::Path) -> Option { + let mut current = start.to_path_buf(); + loop { + if current.join(".git").exists() { + return Some(current); + } + if !current.pop() { + return None; + } + } +} + +/// Which documents a run actually read. +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct InstructionLoad { + /// Scopes whose documents were read. + pub loaded: Vec, + /// Scopes whose documents were skipped without being read. + pub skipped: Vec, + /// Joined document text, empty when nothing loaded. + pub text: String, +} + +/// Load instruction documents under a plan. +/// +/// A skipped scope's files are never opened, so an omitted document cannot +/// reach the prompt by any path. Managed policy is read whenever it exists; +/// a configured managed path that cannot be read fails closed rather than +/// silently omitting organization directives. +pub fn load( + sources: &InstructionSources, + plan: &InstructionPlan, +) -> Result { + let mut result = InstructionLoad::default(); + let mut blocks: Vec = Vec::new(); + for scope in InstructionScope::ALL { + let paths = sources.for_scope(scope); + if !plan.loads(scope) { + result.skipped.push(scope); + continue; + } + let mut read_any = false; + for path in paths { + match std::fs::read_to_string(path) { + Ok(content) => { + blocks.push(content); + read_any = true; + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + // Truly absent is fine. A dangling symlink also reports + // NotFound via read_to_string; for managed policy that + // must fail closed when the configured path is present + // as a link or other metadata entry. + if scope.is_managed() { + match std::fs::symlink_metadata(path) { + Err(meta_err) if meta_err.kind() == std::io::ErrorKind::NotFound => {} + _ => { + return Err(format!( + "Could not read organization-managed instructions at {}: {error}", + path.display() + )); + } + } + } + } + Err(error) if scope.is_managed() => { + return Err(format!( + "Could not read organization-managed instructions at {}: {error}", + path.display() + )); + } + Err(_) => {} + } + } + if read_any { + result.loaded.push(scope); + } + } + result.text = blocks.join("\n\n---\n\n"); + Ok(result) +} + +/// The `omit_instructions` JSON value accepted by `--agents` and subagent +/// frontmatter. +#[derive(Debug, Clone, Deserialize, Serialize, Default)] +pub struct OmitInstructions { + /// Scope names to omit. `managed` is accepted and ignored. + #[serde(default)] + pub omit_instructions: Vec, +} + +impl OmitInstructions { + /// Resolve to a plan, rejecting unknown scope names. + pub fn plan(&self) -> Result { + let names: Vec<&str> = self.omit_instructions.iter().map(String::as_str).collect(); + Ok(InstructionPlan::new(&parse_scopes(names)?)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn managed_policy_is_never_omitted() { + let plan = InstructionPlan::new(&[ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Local, + InstructionScope::Managed, + ]); + assert!(plan.loads(InstructionScope::Managed)); + assert!(plan.managed_forced()); + assert!(plan.requested_managed()); + assert_eq!( + plan.skipped(), + vec![ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Local + ] + ); + assert!(!plan.loads(InstructionScope::User)); + let summary = plan.summary("subagent").expect("summary"); + assert!(summary.contains("user, project, local"), "{summary}"); + assert!(summary.contains("managed policy still loaded"), "{summary}"); + } + + #[test] + fn managed_alone_omits_nothing_and_still_loads() { + let plan = InstructionPlan::new(&[InstructionScope::Managed]); + assert!(!plan.omits_anything()); + assert!(plan.loads(InstructionScope::Managed)); + assert!(plan.loads(InstructionScope::User)); + assert!(plan.managed_forced()); + assert!( + plan.summary("subagent") + .is_some_and(|s| s.contains("nothing") && s.contains("managed policy still loaded")) + ); + } + + #[test] + fn an_empty_request_is_a_no_op() { + let plan = InstructionPlan::new(&[]); + assert_eq!(plan, InstructionPlan::load_all()); + assert!(!plan.omits_anything()); + assert!(plan.summary("subagent").is_none()); + for scope in InstructionScope::ALL { + assert!(plan.loads(scope), "{scope}"); + } + } + + #[test] + fn unknown_scope_names_are_rejected() { + let error = parse_scopes(["user", "projekt"]).unwrap_err(); + assert!(error.contains("projekt"), "{error}"); + assert!( + OmitInstructions { + omit_instructions: vec!["everything".into()], + } + .plan() + .is_err() + ); + assert!( + OmitInstructions { + omit_instructions: vec!["USER".into(), "personal".into()], + } + .plan() + .unwrap() + .omits_anything() + ); + } + + #[test] + fn scope_names_round_trip() { + for scope in InstructionScope::ALL { + assert_eq!(InstructionScope::parse(scope.as_str()), Some(scope)); + assert_eq!(scope.to_string(), scope.as_str()); + } + assert_eq!( + InstructionScope::parse(" org "), + Some(InstructionScope::Managed) + ); + assert_eq!(InstructionScope::parse("nope"), None); + assert!(InstructionScope::Managed.is_managed()); + assert!(!InstructionScope::Local.is_managed()); + } + + fn write(path: &std::path::Path, body: &str) { + if let Some(parent) = path.parent() { + std::fs::create_dir_all(parent).unwrap(); + } + std::fs::write(path, body).unwrap(); + } + + #[test] + fn a_skipped_scope_is_never_read() { + let temp = tempfile::tempdir().unwrap(); + let sources = InstructionSources { + user: vec![temp.path().join("user/AGENTS.md")], + project: vec![temp.path().join("repo/AGENTS.md")], + local: vec![temp.path().join("repo/src/AGENTS.md")], + managed: vec![temp.path().join("org/AGENTS.md")], + }; + write(&sources.user[0], "USER DOC"); + write(&sources.project[0], "PROJECT DOC"); + write(&sources.local[0], "LOCAL DOC"); + write(&sources.managed[0], "MANAGED POLICY"); + + let plan = InstructionPlan::new(&[ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Local, + InstructionScope::Managed, + ]); + let load = load(&sources, &plan).expect("load"); + assert_eq!(load.loaded, vec![InstructionScope::Managed]); + assert_eq!( + load.skipped, + vec![ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Local + ] + ); + assert_eq!(load.text, "MANAGED POLICY"); + for omitted in ["USER DOC", "PROJECT DOC", "LOCAL DOC"] { + assert!( + !load.text.contains(omitted), + "{omitted} leaked: {}", + load.text + ); + } + } + + #[test] + fn managed_policy_loads_even_when_nothing_else_does() { + let temp = tempfile::tempdir().unwrap(); + let sources = InstructionSources { + managed: vec![temp.path().join("org/AGENTS.md")], + ..Default::default() + }; + write(&sources.managed[0], "MANAGED POLICY"); + // A plan built from the most aggressive request still loads managed. + let plan = InstructionPlan::new(&[ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Local, + InstructionScope::Managed, + ]); + assert!(plan.loads(InstructionScope::Managed)); + let load = load(&sources, &plan).expect("load"); + assert_eq!(load.text, "MANAGED POLICY"); + assert!(load.loaded.contains(&InstructionScope::Managed)); + } + + #[test] + fn an_unmanaged_plan_loads_every_present_document() { + let temp = tempfile::tempdir().unwrap(); + let sources = InstructionSources { + user: vec![temp.path().join("user/AGENTS.md")], + project: vec![temp.path().join("repo/AGENTS.md")], + local: vec![temp.path().join("repo/src/AGENTS.md")], + managed: vec![temp.path().join("org/AGENTS.md")], + }; + write(&sources.user[0], "USER DOC"); + write(&sources.project[0], "PROJECT DOC"); + write(&sources.local[0], "LOCAL DOC"); + write(&sources.managed[0], "MANAGED POLICY"); + let load = load(&sources, &InstructionPlan::load_all()).expect("load"); + assert_eq!(load.loaded, InstructionScope::ALL.to_vec()); + assert!(load.skipped.is_empty()); + for text in ["USER DOC", "PROJECT DOC", "LOCAL DOC", "MANAGED POLICY"] { + assert!(load.text.contains(text), "{text} missing: {}", load.text); + } + } + + #[test] + fn an_unreadable_managed_path_fails_closed() { + let temp = tempfile::tempdir().unwrap(); + let managed = temp.path().join("org/AGENTS.md"); + std::fs::create_dir_all(managed.parent().unwrap()).unwrap(); + // Directory where a file is expected — read_to_string fails with IsADirectory + // (or equivalent), which must not silently drop managed instructions. + std::fs::create_dir(&managed).unwrap(); + let sources = InstructionSources { + managed: vec![managed], + ..Default::default() + }; + let err = load(&sources, &InstructionPlan::load_all()).expect_err("managed read"); + assert!(err.contains("organization-managed"), "{err}"); + } + + #[cfg(unix)] + #[test] + fn a_dangling_managed_symlink_fails_closed() { + let temp = tempfile::tempdir().unwrap(); + let missing = temp.path().join("missing-target.md"); + let managed = temp.path().join("org/AGENTS.md"); + std::fs::create_dir_all(managed.parent().unwrap()).unwrap(); + std::os::unix::fs::symlink(&missing, &managed).unwrap(); + let sources = InstructionSources { + managed: vec![managed], + ..Default::default() + }; + let err = load(&sources, &InstructionPlan::load_all()).expect_err("dangling"); + assert!(err.contains("organization-managed"), "{err}"); + } + + #[test] + fn only_omitting_user_keeps_the_project_documents() { + let temp = tempfile::tempdir().unwrap(); + let sources = InstructionSources { + user: vec![temp.path().join("user/AGENTS.md")], + project: vec![temp.path().join("repo/AGENTS.md")], + managed: vec![temp.path().join("org/AGENTS.md")], + ..Default::default() + }; + write(&sources.user[0], "USER DOC"); + write(&sources.project[0], "PROJECT DOC"); + write(&sources.managed[0], "MANAGED POLICY"); + let plan = InstructionPlan::new(&[InstructionScope::User]); + let load = load(&sources, &plan).expect("load"); + assert!(!load.text.contains("USER DOC")); + assert!(load.text.contains("PROJECT DOC")); + assert!(load.text.contains("MANAGED POLICY")); + assert_eq!(load.skipped, vec![InstructionScope::User]); + } +} diff --git a/src/cortex-engine/src/lib.rs b/src/cortex-engine/src/lib.rs index 4c4deac9..77b6e831 100644 --- a/src/cortex-engine/src/lib.rs +++ b/src/cortex-engine/src/lib.rs @@ -183,6 +183,7 @@ pub mod workspace_scripts; // === UTILITIES === pub mod ai_utils; pub mod async_utils; +pub mod audit; pub mod auth; pub mod auth_token; pub mod code_analysis; @@ -192,6 +193,7 @@ pub mod diff; pub mod embeddings; pub mod environment; pub mod events; +pub mod fast_mode; pub mod features; pub mod file_utils; pub mod formatting; @@ -200,12 +202,14 @@ pub mod git_info; pub mod git_ops; pub mod health; pub mod input; +pub mod instruction_scopes; pub mod instructions; pub mod json_utils; pub mod language_utils; pub mod logging; pub mod metrics; pub mod model_family; +pub mod org_policy; pub mod output; pub mod parse_command; diff --git a/src/cortex-engine/src/org_policy.rs b/src/cortex-engine/src/org_policy.rs new file mode 100644 index 00000000..18bbc04c --- /dev/null +++ b/src/cortex-engine/src/org_policy.rs @@ -0,0 +1,330 @@ +//! Organization policy document (`policy.json`). +//! +//! Administrators point `CORTEX_ORG_POLICY_DIR` at a directory holding +//! `policy.json`. Every key is read fail-closed: a document that exists but +//! cannot be read or parsed denies the restricted behavior rather than +//! allowing it, and an unknown value is treated as the restrictive one. + +use std::path::{Path, PathBuf}; + +use serde::Deserialize; + +/// Managed policy file name. +pub const POLICY_FILE: &str = "policy.json"; +/// Environment variable naming the organization policy directory. +pub const POLICY_DIR_ENV: &str = "CORTEX_ORG_POLICY_DIR"; + +/// Whether fast mode is permitted for this organization. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum FastModePolicy { + /// No policy document (or no key): the host setting decides. + HostDefault, + /// The organization allows fast mode. + Allowed, + /// The organization disabled fast mode. Fail closed. + Disabled, +} + +impl FastModePolicy { + /// True only when the organization explicitly disabled fast mode. + pub const fn is_disabled(self) -> bool { + matches!(self, Self::Disabled) + } + + /// True when a request may turn fast mode on. + pub const fn allows(self) -> bool { + !self.is_disabled() + } + + /// Short name for diagnostics and audit records. + pub const fn as_str(self) -> &'static str { + match self { + Self::HostDefault => "host_default", + Self::Allowed => "allowed", + Self::Disabled => "disabled", + } + } +} + +/// Whether a plugin install must pin the reviewed command hash. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PluginInstallPolicy { + /// No policy document (or no key): `--accept-command` is optional. + HostDefault, + /// The organization requires `--accept-command` on install and update. + RequireAcceptCommand, +} + +impl PluginInstallPolicy { + /// True when an install without a pinned hash must be refused. + pub const fn requires_pin(self) -> bool { + matches!(self, Self::RequireAcceptCommand) + } + + /// Short name for diagnostics and audit records. + pub const fn as_str(self) -> &'static str { + match self { + Self::HostDefault => "host_default", + Self::RequireAcceptCommand => "require_accept_command", + } + } +} + +/// Resolved organization policy for this process. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct OrgPolicy { + /// Fast-mode availability. + pub fast_mode: FastModePolicy, + /// Plugin-install pinning requirement. + pub plugin_install: PluginInstallPolicy, +} + +impl Default for OrgPolicy { + fn default() -> Self { + Self { + fast_mode: FastModePolicy::HostDefault, + plugin_install: PluginInstallPolicy::HostDefault, + } + } +} + +impl OrgPolicy { + /// True when the organization governs at least one key. + /// + /// An explicit allow counts: the organization decided, so the value is + /// managed even though it does not restrict anything. + pub const fn is_managed(&self) -> bool { + !matches!(self.fast_mode, FastModePolicy::HostDefault) + || !matches!(self.plugin_install, PluginInstallPolicy::HostDefault) + } +} + +#[derive(Debug, Deserialize)] +struct PolicyDocument { + #[serde(default)] + fast_mode: Option, + #[serde(default)] + plugin_install: Option, +} + +#[derive(Debug, Deserialize)] +#[serde(untagged)] +enum PolicyValue { + Bool(bool), + Text(String), +} + +impl PolicyValue { + /// True only for an explicitly permissive value. Everything else — an + /// unknown word, a wrong type, an empty string — denies. + fn is_permissive(&self) -> bool { + match self { + Self::Bool(value) => *value, + Self::Text(text) => matches!( + text.trim().to_ascii_lowercase().as_str(), + "on" | "true" | "enabled" | "allow" | "allowed" | "optional" + ), + } + } +} + +/// Directory holding the organization policy document, when configured. +pub fn policy_dir() -> Option { + std::env::var(POLICY_DIR_ENV) + .ok() + .map(|value| value.trim().to_string()) + .filter(|value| !value.is_empty()) + .map(PathBuf::from) +} + +/// Policy document path inside `dir`. +pub fn policy_path(dir: &Path) -> PathBuf { + dir.join(POLICY_FILE) +} + +/// Restrictive fallback used when a configured policy path is broken. +const fn restrictive() -> OrgPolicy { + OrgPolicy { + fast_mode: FastModePolicy::Disabled, + plugin_install: PluginInstallPolicy::RequireAcceptCommand, + } +} + +/// Resolve the policy document from an explicit directory. +/// +/// An absent directory or document yields [`OrgPolicy::default`]. A path that +/// exists as a dangling symlink, cannot be read, or cannot be parsed yields +/// the restrictive policy for every key — never host defaults. +pub fn resolve(dir: Option<&Path>) -> OrgPolicy { + let Some(dir) = dir else { + return OrgPolicy::default(); + }; + let path = policy_path(dir); + // Distinguish a truly absent path from a dangling symlink / metadata error. + // `Path::exists` follows links and treats a dangling symlink as absent, + // which would fail open; `symlink_metadata` sees the link itself. + match std::fs::symlink_metadata(&path) { + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return OrgPolicy::default(); + } + Err(_) => return restrictive(), + Ok(_) => {} + } + let text = match std::fs::read_to_string(&path) { + Ok(text) => text, + Err(_) => return restrictive(), + }; + match serde_json::from_str::(&text) { + Ok(document) => OrgPolicy { + fast_mode: match document.fast_mode { + Some(value) if value.is_permissive() => FastModePolicy::Allowed, + Some(_) => FastModePolicy::Disabled, + None => FastModePolicy::HostDefault, + }, + plugin_install: match document.plugin_install { + // Only an explicit permissive value relaxes the pin; anything + // else in a present document requires it. + Some(value) if value.is_permissive() => PluginInstallPolicy::HostDefault, + Some(_) => PluginInstallPolicy::RequireAcceptCommand, + None => PluginInstallPolicy::HostDefault, + }, + }, + Err(_) => restrictive(), + } +} + +/// Resolve the policy for this process from the environment. +pub fn current() -> OrgPolicy { + resolve(policy_dir().as_deref()) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn write(dir: &Path, body: &str) { + std::fs::create_dir_all(dir).unwrap(); + std::fs::write(policy_path(dir), body).unwrap(); + } + + #[test] + fn an_absent_document_leaves_every_key_on_the_host_default() { + let temp = tempfile::tempdir().unwrap(); + for policy in [resolve(None), resolve(Some(temp.path()))] { + assert_eq!(policy, OrgPolicy::default()); + assert!(!policy.is_managed()); + assert_eq!(policy.fast_mode, FastModePolicy::HostDefault); + assert!(!policy.plugin_install.requires_pin()); + } + } + + #[test] + fn both_keys_resolve_from_one_document() { + let temp = tempfile::tempdir().unwrap(); + write( + temp.path(), + r#"{"fast_mode": false, "plugin_install": "require_accept_command"}"#, + ); + let policy = resolve(Some(temp.path())); + assert!(policy.is_managed()); + assert_eq!(policy.fast_mode, FastModePolicy::Disabled); + assert!(policy.plugin_install.requires_pin()); + assert_eq!(policy.plugin_install.as_str(), "require_accept_command"); + } + + #[test] + fn a_present_document_denies_unknown_values() { + let temp = tempfile::tempdir().unwrap(); + write( + temp.path(), + r#"{"fast_mode": "maybe", "plugin_install": "sometimes"}"#, + ); + let policy = resolve(Some(temp.path())); + assert_eq!(policy.fast_mode, FastModePolicy::Disabled); + assert!(policy.plugin_install.requires_pin()); + } + + #[test] + fn an_unreadable_or_unparseable_document_fails_closed() { + let temp = tempfile::tempdir().unwrap(); + write(temp.path(), "{ not json"); + let broken = resolve(Some(temp.path())); + assert_eq!(broken.fast_mode, FastModePolicy::Disabled); + assert!(broken.plugin_install.requires_pin()); + + // A directory where the document must be cannot be read as a file. + let other = tempfile::tempdir().unwrap(); + std::fs::create_dir_all(policy_path(other.path())).unwrap(); + let unreadable = resolve(Some(other.path())); + assert_eq!(unreadable.fast_mode, FastModePolicy::Disabled); + assert!(unreadable.plugin_install.requires_pin()); + } + + #[test] + fn permissive_values_are_recognized_in_either_form() { + let temp = tempfile::tempdir().unwrap(); + write( + temp.path(), + r#"{"fast_mode": true, "plugin_install": "optional"}"#, + ); + let policy = resolve(Some(temp.path())); + assert_eq!(policy.fast_mode, FastModePolicy::Allowed); + assert!(!policy.plugin_install.requires_pin()); + // An explicit allow is still an organization decision. + assert!(policy.is_managed()); + + write(temp.path(), r#"{"fast_mode": "ENABLED"}"#); + assert_eq!( + resolve(Some(temp.path())).fast_mode, + FastModePolicy::Allowed + ); + + write(temp.path(), r#"{"fast_mode": ""}"#); + assert_eq!( + resolve(Some(temp.path())).fast_mode, + FastModePolicy::Disabled + ); + } + + #[test] + fn an_unrelated_document_keeps_the_host_defaults() { + let temp = tempfile::tempdir().unwrap(); + write(temp.path(), r#"{"other_setting": 1}"#); + let policy = resolve(Some(temp.path())); + assert_eq!(policy, OrgPolicy::default()); + assert!(!policy.is_managed()); + } + + #[test] + fn a_dangling_policy_symlink_fails_closed() { + let temp = tempfile::tempdir().unwrap(); + let missing = temp.path().join("missing-target.json"); + let link = policy_path(temp.path()); + #[cfg(unix)] + { + std::os::unix::fs::symlink(&missing, &link).unwrap(); + assert!( + !link.exists(), + "dangling symlink must not exist() as a file" + ); + let policy = resolve(Some(temp.path())); + assert_eq!(policy.fast_mode, FastModePolicy::Disabled); + assert!(policy.plugin_install.requires_pin()); + } + #[cfg(not(unix))] + { + let _ = (missing, link); + } + } + + #[test] + fn names_are_stable() { + assert_eq!(POLICY_FILE, "policy.json"); + assert_eq!(POLICY_DIR_ENV, "CORTEX_ORG_POLICY_DIR"); + assert_eq!(FastModePolicy::HostDefault.as_str(), "host_default"); + assert_eq!(FastModePolicy::Allowed.as_str(), "allowed"); + assert_eq!(FastModePolicy::Disabled.as_str(), "disabled"); + assert_eq!(PluginInstallPolicy::HostDefault.as_str(), "host_default"); + assert!(policy_dir().is_none() || std::env::var(POLICY_DIR_ENV).is_ok()); + } +} diff --git a/src/cortex-engine/src/session/lifecycle.rs b/src/cortex-engine/src/session/lifecycle.rs index 94095762..08102e63 100644 --- a/src/cortex-engine/src/session/lifecycle.rs +++ b/src/cortex-engine/src/session/lifecycle.rs @@ -70,6 +70,7 @@ impl Session { }, ), ssh_target: None, + fast_mode: false, }); let mut tool_router = ToolRouter::new(); @@ -201,6 +202,7 @@ impl Session { }, ), ssh_target: None, + fast_mode: false, }); let mut tool_router = ToolRouter::new(); @@ -341,6 +343,7 @@ impl Session { }, ), ssh_target: None, + fast_mode: false, }); let mut tool_router = ToolRouter::new(); diff --git a/src/cortex-engine/src/tools/handlers/subagent/executor.rs b/src/cortex-engine/src/tools/handlers/subagent/executor.rs index daaf20c7..3a901eee 100644 --- a/src/cortex-engine/src/tools/handlers/subagent/executor.rs +++ b/src/cortex-engine/src/tools/handlers/subagent/executor.rs @@ -8,17 +8,18 @@ use tokio::sync::{RwLock, mpsc}; use tokio::time::timeout; use uuid::Uuid; -use crate::agent::{ - AgentConfig, AgentEvent, Orchestrator, OrchestratorTurnResult, SandboxPolicy, TurnStatus, -}; +use crate::agent::{AgentConfig, Orchestrator, SandboxPolicy}; use crate::agents::{Agent, AgentRegistry}; use crate::client::ModelClient; use crate::error::{CortexError, Result}; use crate::tools::registry::ToolRegistry; +use super::instruction_audit::{managed_policy_sources, record_instruction_audit}; use super::progress::{ProgressEvent, SubagentProgress}; -use super::result::{ - FileChange, FileChangeType, SubagentResult, SubagentResultBuilder, TokenUsageBreakdown, +use super::result::SubagentResult; +use super::run_helpers::{ + TurnOutcome, apply_summary_turn, build_result, effective_max_iterations, effective_model, + needs_summary_turn, spawn_event_forwarder, }; use super::types::{SubagentConfig, SubagentSession, SubagentStatus}; @@ -206,6 +207,80 @@ impl SubagentExecutor { result } + /// Look up the custom agent for this run, if the type names one. + /// + /// A named agent that is missing from the registry is an error, not a + /// silent fallback: the run would otherwise use the wrong prompt. + async fn resolve_custom_agent(&self, config: &SubagentConfig) -> Result> { + let Some(agent_name) = config.agent_type.custom_name() else { + return Ok(None); + }; + match self.agent_registry.get(agent_name).await { + Some(agent) => { + tracing::info!(agent_name = agent_name, "Using custom agent from registry"); + Ok(Some(agent)) + } + None => Err(CortexError::NotFound(format!( + "Custom agent '{}' not found in registry. Available agents: {:?}", + agent_name, + self.agent_registry.list_names().await + ))), + } + } + + /// Task-level omit list wins; otherwise take custom-agent frontmatter scopes. + fn with_frontmatter_omissions( + config: SubagentConfig, + custom_agent: Option<&Agent>, + ) -> SubagentConfig { + if !config.omit_instructions.is_empty() { + return config; + } + let Some(agent) = custom_agent else { + return config; + }; + if agent.metadata.omit_instructions.is_empty() { + return config; + } + config.with_omit_instructions(agent.metadata.omit_instructions.clone()) + } + + /// Append the child's instruction documents to its system prompt. + /// + /// Organization-managed policy always loads. A request that named it is + /// recorded in the audit journal alongside the omission itself, so an + /// omitted run is reviewable after the fact. + fn apply_instruction_plan(&self, config: &SubagentConfig, base: String) -> Result { + let plan = config.instruction_plan(); + let cortex_home = crate::config::find_cortex_home() + .unwrap_or_else(|_| std::path::PathBuf::from(".cortex")); + let mut sources = crate::instruction_scopes::InstructionSources::discover( + &config.working_dir, + &cortex_home, + ); + // Always prefer the explicit managed-policy source when configured. + let managed = managed_policy_sources(); + if !managed.is_empty() { + sources.managed = managed; + } + let load = crate::instruction_scopes::load(&sources, &plan) + .map_err(|error| CortexError::Other(anyhow::anyhow!("{error}")))?; + if plan.omits_anything() || plan.requested_managed() { + record_instruction_audit(&plan, &load); + } + if load.text.is_empty() { + return Ok(base); + } + Ok(format!( + "{base} + +## Project Instructions +{} +", + load.text + )) + } + /// Run a subagent. async fn run_subagent( &self, @@ -228,24 +303,7 @@ impl SubagentExecutor { session.set_status(SubagentStatus::Running); // Look up custom agent from registry if this is a Custom type - let custom_agent = if let Some(agent_name) = config.agent_type.custom_name() { - match self.agent_registry.get(agent_name).await { - Some(agent) => { - tracing::info!(agent_name = agent_name, "Using custom agent from registry"); - Some(agent) - } - None => { - // Agent not found in registry - fail with helpful error - return Err(CortexError::NotFound(format!( - "Custom agent '{}' not found in registry. Available agents: {:?}", - agent_name, - self.agent_registry.list_names().await - ))); - } - } - } else { - None - }; + let custom_agent = self.resolve_custom_agent(&config).await?; // Build system prompt - use base prompt WITHOUT task details // Task details will be sent as user message @@ -256,47 +314,21 @@ impl SubagentExecutor { config.build_base_system_prompt() }; + // Instruction documents for the child: user, project, and local + // documents can be omitted; organization-managed policy always loads. + // Custom-agent frontmatter omissions apply when the Task did not set any; + // a non-empty Task-level list always wins. + let config = Self::with_frontmatter_omissions(config, custom_agent.as_ref()); + let system_prompt = self.apply_instruction_plan(&config, system_prompt)?; + // Build user message containing the task // Tasks are conversational - sent as user messages rather than system config - let user_task_message = if let Some(ref _agent) = custom_agent { - // For custom agents, format task with context - let mut message = format!( - "## Task\n{}\n\n## Instructions\n{}", - config.description, config.prompt - ); - if let Some(ref context) = config.context { - message.push_str("\n\n## Additional Context\n"); - message.push_str(context); - } - message.push_str("\n\nPlease complete this task and provide a clear summary of your findings or actions when done."); - message - } else { - config.build_user_message() - }; - - // Determine model - custom agent can override - let model = if let Some(ref agent) = custom_agent { - agent.effective_model(&self.default_model) - } else { - config - .model - .clone() - .unwrap_or_else(|| self.default_model.clone()) - }; - - // Determine max iterations - custom agent can override - let max_iterations = if let Some(ref agent) = custom_agent { - agent - .metadata - .max_turns - .unwrap_or(config.effective_max_iterations()) - } else { - config.effective_max_iterations() - }; + let user_task_message = config.build_user_message(); + // Determine model and max iterations - custom agent can override let agent_config = AgentConfig { - model, - max_tool_iterations: max_iterations, + model: effective_model(custom_agent.as_ref(), &config, &self.default_model), + max_tool_iterations: effective_max_iterations(custom_agent.as_ref(), &config), max_output_tokens: 16384, tool_timeout: Duration::from_secs(120), sandbox_policy: self.default_sandbox_policy, @@ -311,7 +343,7 @@ impl SubagentExecutor { // This would require modifying the tool registry or orchestrator to filter tools // Create orchestrator for the subagent - let (event_tx, mut event_rx) = mpsc::unbounded_channel(); + let (event_tx, event_rx) = mpsc::unbounded_channel(); let orchestrator = Orchestrator::new( self.client.clone(), self.tools.clone(), @@ -323,97 +355,8 @@ impl SubagentExecutor { orchestrator.initialize(Some(&system_prompt)).await; // Set up event forwarding - let progress_tx_clone = progress_tx.clone(); - let session_id_clone = session_id.clone(); - let event_handler = tokio::spawn(async move { - let mut files_modified = Vec::new(); - let mut turn_number: u32 = 0; // Track actual turn number - while let Some(event) = event_rx.recv().await { - match &event { - AgentEvent::Thinking => { - // Increment turn number each time model is called - turn_number += 1; - let _ = progress_tx_clone.send(ProgressEvent::Thinking { - session_id: session_id_clone.clone(), - turn_number, - }); - } - AgentEvent::TextDelta { content } => { - let _ = progress_tx_clone.send(ProgressEvent::TextOutput { - session_id: session_id_clone.clone(), - content: content.clone(), - is_partial: true, - }); - } - AgentEvent::ToolCallStarted { - id, - name, - arguments, - } => { - let _ = progress_tx_clone.send(ProgressEvent::ToolCallStarted { - session_id: session_id_clone.clone(), - tool_name: name.clone(), - tool_id: id.clone(), - arguments_preview: arguments.chars().take(200).collect(), - }); - } - AgentEvent::ToolCallCompleted { id, name, result } => { - // Track file modifications - if matches!( - name.as_str(), - "Create" | "Edit" | "ApplyPatch" | "MultiEdit" - ) { - // Extract file path from result if possible - if let Some(path) = extract_file_path(&result.output) { - files_modified.push(path); - } - } - - let _ = progress_tx_clone.send(ProgressEvent::ToolCallCompleted { - session_id: session_id_clone.clone(), - tool_name: name.clone(), - tool_id: id.clone(), - success: result.success, - output_preview: result.output.chars().take(200).collect(), - duration_ms: 0, // Not tracked at event level - }); - } - AgentEvent::ToolCallPending { - id, - name, - arguments, - risk_level, - } => { - let _ = progress_tx_clone.send(ProgressEvent::ToolCallPending { - session_id: session_id_clone.clone(), - tool_name: name.clone(), - tool_id: id.clone(), - arguments: arguments.clone(), - risk_level: format!("{:?}", risk_level), - }); - } - AgentEvent::Error { - message, - recoverable, - } => { - if *recoverable { - let _ = progress_tx_clone.send(ProgressEvent::Warning { - session_id: session_id_clone.clone(), - message: message.clone(), - }); - } else { - let _ = progress_tx_clone.send(ProgressEvent::Failed { - session_id: session_id_clone.clone(), - error: message.clone(), - recoverable: false, - }); - } - } - _ => {} - } - } - files_modified - }); + let event_handler = + spawn_event_forwarder(progress_tx.clone(), session_id.clone(), event_rx); // Create turn context with the full user task message // This sends the task as a user message, not embedded in system prompt @@ -443,61 +386,20 @@ impl SubagentExecutor { // MANDATORY: Request explicit summary if the response doesn't contain one // This ensures subagents always provide structured output for the orchestrator - if let Ok(ref result) = turn_result { - if result.status == TurnStatus::Completed && !has_summary_output(&result.response) { - tracing::info!( - session_id = %session_id, - "Subagent output missing summary, requesting explicit summary turn" - ); - - // Request a summary turn - let summary_prompt = SUMMARY_REQUEST_PROMPT.to_string(); - - let summary_turn_id = session.turns_completed as u64 + 2; - let mut summary_turn_ctx = crate::agent::TurnContext::new( - summary_turn_id, - session_id.clone(), - summary_prompt, - config.working_dir.clone(), - ); - - // Execute summary turn with a reasonable timeout - let summary_result = timeout( - Duration::from_secs(60), - orchestrator.process_turn(&mut summary_turn_ctx), - ) - .await; - - // Update turn_result with the summary if successful - if let Ok(Ok(summary_response)) = summary_result { - if summary_response.status == TurnStatus::Completed - && !summary_response.response.is_empty() - { - tracing::info!( - session_id = %session_id, - "Received explicit summary from subagent" - ); - // Combine original response with summary - turn_result = Ok(OrchestratorTurnResult { - turn_id: result.turn_id, - status: TurnStatus::Completed, - response: format!( - "{}\n\n{}", - result.response, summary_response.response - ), - tool_calls: result.tool_calls.clone(), - token_usage: result.token_usage.clone(), - duration: result.duration, - }); - // Update token counts - turn_ctx.tokens.input_tokens += summary_turn_ctx.tokens.input_tokens; - turn_ctx.tokens.output_tokens += summary_turn_ctx.tokens.output_tokens; - turn_ctx.tokens.cached_tokens += summary_turn_ctx.tokens.cached_tokens; - turn_ctx.tokens.reasoning_tokens += - summary_turn_ctx.tokens.reasoning_tokens; - } - } - } + if needs_summary_turn(&turn_result) { + tracing::info!( + session_id = %session_id, + "Subagent output missing summary, requesting explicit summary turn" + ); + apply_summary_turn( + &orchestrator, + &session, + &session_id, + &config, + &mut turn_ctx, + &mut turn_result, + ) + .await; } // CRITICAL: Drop orchestrator to close the event channel @@ -514,18 +416,7 @@ impl SubagentExecutor { // Process result // Extract status for better error messages when success=false but no error - let (success, output, error, status_info) = match &turn_result { - Ok(result) => { - let success = result.status == TurnStatus::Completed; - let status_info = if !success { - Some(format!("{:?}", result.status)) - } else { - None - }; - (success, result.response.clone(), None, status_info) - } - Err(e) => (false, String::new(), Some(e.to_string()), None), - }; + let outcome = TurnOutcome::from_result(&turn_result); // Update session session.record_turn( @@ -535,7 +426,7 @@ impl SubagentExecutor { for path in &files_modified { session.record_file_modified(path); } - session.set_status(if success { + session.set_status(if outcome.success { SubagentStatus::Completed } else { // Any non-success state should be marked as Failed @@ -548,60 +439,22 @@ impl SubagentExecutor { sessions.insert(session_id.clone(), session.clone()); } - // Build token usage breakdown - let mut token_usage = TokenUsageBreakdown::default(); - token_usage.add_turn( - turn_ctx.tokens.input_tokens as u64, - turn_ctx.tokens.output_tokens as u64, - turn_ctx.tokens.cached_tokens as u64, - turn_ctx.tokens.reasoning_tokens as u64, - ); - - // Build file changes - let file_changes: Vec = files_modified - .into_iter() - .map(|path| FileChange::new(path, FileChangeType::Modified)) - .collect(); - // Record completion or failure - if success { - progress.complete(&output); - } else if let Some(ref err) = error { + if outcome.success { + progress.complete(&outcome.output); + } else if let Some(ref err) = outcome.error { progress.fail(err, false); } else { - // Handle non-success without explicit error (interrupted, cancelled, etc.) - // This can happen when turn_result is Ok but status != Completed - let error_msg = if let Some(ref status) = status_info { - format!("Task ended with status: {}", status) - } else { - "Task did not complete successfully".to_string() - }; - progress.fail(&error_msg, true); - } - - // Build result - let mut builder = SubagentResultBuilder::new(session) - .success(success) - .output(&output) - .tokens(token_usage); - - // Add error to result - either from explicit error or from status_info - if let Some(err) = error { - builder = builder.error(err); - } else if let Some(ref status) = status_info { - builder = builder.error(format!("Task ended with status: {}", status)); - } - - for change in file_changes { - builder = builder.file_changed(change); - } - - // Allow continuation if partially complete - if success || turn_ctx.tool_iterations < config.effective_max_iterations() { - builder = builder.continuable(); + progress.fail(&outcome.failure_message(), true); } - Ok(builder.build()) + Ok(build_result( + session, + outcome, + &files_modified, + &turn_ctx, + &config, + )) } /// Get a session by ID. @@ -706,151 +559,10 @@ pub struct SubagentTypeInfo { pub denied_tools: Vec, } -/// Extract file path from tool output. -/// Prompt used to request an explicit summary from a subagent when none was provided. -/// Ensures structured output from agents for orchestrator consumption. -const SUMMARY_REQUEST_PROMPT: &str = r#"You have completed your work but did not provide a summary. Please provide a final summary NOW using EXACTLY this format: - -## Summary for Orchestrator - -### Tasks Completed -- [List each task you completed with brief outcome] - -### Key Findings/Changes -- [Main discoveries or modifications made] - -### Files Modified (if any) -- [List of files with type of change] - -### Recommendations (if applicable) -- [Any follow-up actions or suggestions] - -### Status: COMPLETED - -DO NOT use any tools. Just provide the summary based on the work you have already done."#; - -/// Check if the response contains a proper summary for the orchestrator. -/// Returns true if summary markers are present, false otherwise. -fn has_summary_output(response: &str) -> bool { - // Empty responses definitely don't have a summary - if response.trim().is_empty() { - return false; - } - - // Check for key summary markers that indicate structured output - let summary_markers = [ - "## Summary for Orchestrator", - "### Tasks Completed", - "### Key Findings", - "### Status: COMPLETED", - "Status: COMPLETED", - // Also accept some variations - "## Summary", - "### Summary", - "## Final Summary", - "### Final Summary", - ]; - - let response_lower = response.to_lowercase(); - summary_markers - .iter() - .any(|marker| response_lower.contains(&marker.to_lowercase())) -} - -fn extract_file_path(output: &str) -> Option { - // Try to extract path from common patterns - // "Created file: path/to/file" - // "Edited path/to/file" - // "Wrote N bytes to path/to/file" - - let patterns = [ - "Created file: ", - "Created: ", - "Edited ", - "Modified ", - "Wrote ", - "to ", - ]; - - for pattern in patterns { - if let Some(idx) = output.find(pattern) { - let rest = &output[idx + pattern.len()..]; - // Extract until whitespace or end - let path: String = rest.chars().take_while(|c| !c.is_whitespace()).collect(); - if !path.is_empty() && (path.contains('/') || path.contains('\\') || path.contains('.')) - { - return Some(path); - } - } - } - - None -} - #[cfg(test)] mod tests { use super::*; - #[test] - fn test_extract_file_path() { - assert_eq!( - extract_file_path("Created file: src/main.rs"), - Some("src/main.rs".to_string()) - ); - assert_eq!( - extract_file_path("Wrote 100 bytes to config.json"), - Some("config.json".to_string()) - ); - assert_eq!( - extract_file_path("Successfully edited src/lib.rs"), - None // "edited" doesn't match "Edited " - ); - assert_eq!(extract_file_path("No path here"), None); - } - - #[test] - fn test_has_summary_output_with_proper_summary() { - let response_with_summary = r#" -## Summary for Orchestrator - -### Tasks Completed -- Analyzed the codebase structure - -### Key Findings -- Found 10 modules - -### Status: COMPLETED -"#; - assert!(has_summary_output(response_with_summary)); - } - - #[test] - fn test_has_summary_output_with_variation() { - // Test case-insensitive matching - let response = "## summary\nSome content here"; - assert!(has_summary_output(response)); - - // Test "Status: COMPLETED" alone - let response2 = "Work done.\n\nStatus: COMPLETED"; - assert!(has_summary_output(response2)); - } - - #[test] - fn test_has_summary_output_empty() { - assert!(!has_summary_output("")); - assert!(!has_summary_output(" ")); - assert!(!has_summary_output("\n\n")); - } - - #[test] - fn test_has_summary_output_no_markers() { - let response_without_summary = "I analyzed the code and found some issues."; - assert!(!has_summary_output(response_without_summary)); - - let response_partial = "Here are some findings:\n- Item 1\n- Item 2"; - assert!(!has_summary_output(response_partial)); - } - #[test] fn test_subagent_type_info() { let executor = SubagentExecutor::new( diff --git a/src/cortex-engine/src/tools/handlers/subagent/instruction_audit.rs b/src/cortex-engine/src/tools/handlers/subagent/instruction_audit.rs new file mode 100644 index 00000000..85a1bbd8 --- /dev/null +++ b/src/cortex-engine/src/tools/handlers/subagent/instruction_audit.rs @@ -0,0 +1,51 @@ +//! Managed-policy sources and instruction-omission audit journal helpers. + +use crate::instruction_scopes::{InstructionPlan, InstructionScope}; + +/// Organization-managed instruction documents for this host. +/// +/// Managed policy is read from the organization policy directory. There is no +/// local or project path that can stand in for it, so a child cannot omit it. +pub(super) fn managed_policy_sources() -> Vec { + crate::org_policy::policy_dir() + .map(|dir| vec![dir.join(crate::instruction_scopes::MANAGED_POLICY_FILE)]) + .unwrap_or_default() +} + +/// Record an omission and, when the request named managed policy, the fact +/// that it still loaded. A journal that cannot be written is reported, never +/// silently dropped. +pub(super) fn record_instruction_audit( + plan: &InstructionPlan, + load: &crate::instruction_scopes::InstructionLoad, +) { + let Ok(home) = crate::config::find_cortex_home() else { + return; + }; + let skipped: Vec<&str> = plan.skipped().iter().map(|s| s.as_str()).collect(); + let loaded: Vec<&str> = load.loaded.iter().map(|s| s.as_str()).collect(); + if let Err(error) = crate::audit::record( + &home, + crate::audit::AuditKind::InstructionsOmitted, + serde_json::json!({ + "source": "subagent", + "skipped": skipped, + "loaded": loaded, + }), + ) { + tracing::warn!(%error, "Could not record instruction omission in the audit journal"); + } + if plan.requested_managed() { + if let Err(error) = crate::audit::record( + &home, + crate::audit::AuditKind::ManagedPolicyNeverOmitted, + serde_json::json!({ + "source": "subagent", + "requested": [InstructionScope::Managed.as_str()], + "loaded": load.loaded.contains(&InstructionScope::Managed), + }), + ) { + tracing::warn!(%error, "Could not record managed-policy retention in the audit journal"); + } + } +} diff --git a/src/cortex-engine/src/tools/handlers/subagent/mod.rs b/src/cortex-engine/src/tools/handlers/subagent/mod.rs index 1efc792a..62a4a721 100644 --- a/src/cortex-engine/src/tools/handlers/subagent/mod.rs +++ b/src/cortex-engine/src/tools/handlers/subagent/mod.rs @@ -4,8 +4,10 @@ //! that can execute complex, multi-step tasks autonomously. mod executor; +mod instruction_audit; mod progress; mod result; +mod run_helpers; mod types; pub use executor::SubagentExecutor; diff --git a/src/cortex-engine/src/tools/handlers/subagent/run_helpers.rs b/src/cortex-engine/src/tools/handlers/subagent/run_helpers.rs new file mode 100644 index 00000000..3de79420 --- /dev/null +++ b/src/cortex-engine/src/tools/handlers/subagent/run_helpers.rs @@ -0,0 +1,541 @@ +//! Helpers for one subagent run: event forwarding, turn setup, and result assembly. +//! +//! Kept beside `executor.rs` so the executor stays under the file-size cap while +//! `run_subagent` stays under the complexity cap. + +use std::time::Duration; + +use tokio::sync::mpsc; +use tokio::time::timeout; + +use crate::agent::{AgentEvent, Orchestrator, OrchestratorTurnResult, TurnStatus}; +use crate::agents::Agent; +use crate::error::Result; + +use super::progress::ProgressEvent; +use super::result::{ + FileChange, FileChangeType, SubagentResult, SubagentResultBuilder, TokenUsageBreakdown, +}; +use super::types::{SubagentConfig, SubagentSession}; + +/// Forward agent events to the progress channel, collecting modified files. +/// +/// Returns the modified-file list once the event channel closes. The caller +/// must drop the orchestrator so the channel closes and this task can finish. +pub(super) fn spawn_event_forwarder( + progress_tx: mpsc::UnboundedSender, + session_id: String, + mut event_rx: mpsc::UnboundedReceiver, +) -> tokio::task::JoinHandle> { + tokio::spawn(async move { + let mut files_modified = Vec::new(); + let mut turn_number: u32 = 0; // Track actual turn number + while let Some(event) = event_rx.recv().await { + match &event { + AgentEvent::Thinking => { + // Increment turn number each time model is called + turn_number += 1; + let _ = progress_tx.send(ProgressEvent::Thinking { + session_id: session_id.clone(), + turn_number, + }); + } + AgentEvent::TextDelta { content } => { + let _ = progress_tx.send(ProgressEvent::TextOutput { + session_id: session_id.clone(), + content: content.clone(), + is_partial: true, + }); + } + AgentEvent::ToolCallStarted { + id, + name, + arguments, + } => { + let _ = progress_tx.send(ProgressEvent::ToolCallStarted { + session_id: session_id.clone(), + tool_name: name.clone(), + tool_id: id.clone(), + arguments_preview: arguments.chars().take(200).collect(), + }); + } + AgentEvent::ToolCallCompleted { id, name, result } => { + // Track file modifications + if matches!( + name.as_str(), + "Create" | "Edit" | "ApplyPatch" | "MultiEdit" + ) { + // Extract file path from result if possible + if let Some(path) = extract_file_path(&result.output) { + files_modified.push(path); + } + } + + let _ = progress_tx.send(ProgressEvent::ToolCallCompleted { + session_id: session_id.clone(), + tool_name: name.clone(), + tool_id: id.clone(), + success: result.success, + output_preview: result.output.chars().take(200).collect(), + duration_ms: 0, // Not tracked at event level + }); + } + AgentEvent::ToolCallPending { + id, + name, + arguments, + risk_level, + } => { + let _ = progress_tx.send(ProgressEvent::ToolCallPending { + session_id: session_id.clone(), + tool_name: name.clone(), + tool_id: id.clone(), + arguments: arguments.clone(), + risk_level: format!("{:?}", risk_level), + }); + } + AgentEvent::Error { + message, + recoverable, + } => { + if *recoverable { + let _ = progress_tx.send(ProgressEvent::Warning { + session_id: session_id.clone(), + message: message.clone(), + }); + } else { + let _ = progress_tx.send(ProgressEvent::Failed { + session_id: session_id.clone(), + error: message.clone(), + recoverable: false, + }); + } + } + _ => {} + } + } + files_modified + }) +} + +/// Model for this run: the custom agent's choice wins over the config. +pub(super) fn effective_model( + custom_agent: Option<&Agent>, + config: &SubagentConfig, + default_model: &str, +) -> String { + match custom_agent { + Some(agent) => agent.effective_model(default_model), + None => config + .model + .clone() + .unwrap_or_else(|| default_model.to_string()), + } +} + +/// Iteration cap for this run: the custom agent's `max_turns` wins. +pub(super) fn effective_max_iterations( + custom_agent: Option<&Agent>, + config: &SubagentConfig, +) -> u32 { + match custom_agent { + Some(agent) => agent + .metadata + .max_turns + .unwrap_or_else(|| config.effective_max_iterations()), + None => config.effective_max_iterations(), + } +} + +/// True when a completed turn produced no structured summary. +pub(super) fn needs_summary_turn(turn_result: &Result) -> bool { + match turn_result { + Ok(result) => { + result.status == TurnStatus::Completed && !has_summary_output(&result.response) + } + Err(_) => false, + } +} + +/// Ask the subagent for an explicit summary and fold it into `turn_result`. +pub(super) async fn apply_summary_turn( + orchestrator: &Orchestrator, + session: &SubagentSession, + session_id: &str, + config: &SubagentConfig, + turn_ctx: &mut crate::agent::TurnContext, + turn_result: &mut Result, +) { + let Ok(result) = turn_result else { + return; + }; + let original = result.clone(); + + // Request a summary turn + let summary_turn_id = session.turns_completed as u64 + 2; + let mut summary_turn_ctx = crate::agent::TurnContext::new( + summary_turn_id, + session_id.to_string(), + SUMMARY_REQUEST_PROMPT.to_string(), + config.working_dir.clone(), + ); + + // Execute summary turn with a reasonable timeout + let summary_result = timeout( + Duration::from_secs(60), + orchestrator.process_turn(&mut summary_turn_ctx), + ) + .await; + + // Update turn_result with the summary if successful + let Ok(Ok(summary_response)) = summary_result else { + return; + }; + if summary_response.status != TurnStatus::Completed || summary_response.response.is_empty() { + return; + } + + tracing::info!( + session_id = %session_id, + "Received explicit summary from subagent" + ); + // Combine original response with summary + *turn_result = Ok(OrchestratorTurnResult { + turn_id: original.turn_id, + status: TurnStatus::Completed, + response: format!("{}\n\n{}", original.response, summary_response.response), + tool_calls: original.tool_calls.clone(), + token_usage: original.token_usage.clone(), + duration: original.duration, + }); + // Update token counts + turn_ctx.tokens.input_tokens += summary_turn_ctx.tokens.input_tokens; + turn_ctx.tokens.output_tokens += summary_turn_ctx.tokens.output_tokens; + turn_ctx.tokens.cached_tokens += summary_turn_ctx.tokens.cached_tokens; + turn_ctx.tokens.reasoning_tokens += summary_turn_ctx.tokens.reasoning_tokens; +} + +/// Terminal state of a run, derived from the turn result. +pub(super) struct TurnOutcome { + pub(super) success: bool, + pub(super) output: String, + pub(super) error: Option, + pub(super) status_info: Option, +} + +impl TurnOutcome { + /// Extract status for better error messages when success=false but no error. + pub(super) fn from_result(turn_result: &Result) -> Self { + match turn_result { + Ok(result) => { + let success = result.status == TurnStatus::Completed; + Self { + success, + output: result.response.clone(), + error: None, + status_info: (!success).then(|| format!("{:?}", result.status)), + } + } + Err(e) => Self { + success: false, + output: String::new(), + error: Some(e.to_string()), + status_info: None, + }, + } + } + + /// Message for a failure that carried no explicit error (interrupted, + /// cancelled, or a turn that ended with a non-completed status). + pub(super) fn failure_message(&self) -> String { + match &self.status_info { + Some(status) => format!("Task ended with status: {status}"), + None => "Task did not complete successfully".to_string(), + } + } +} + +/// Assemble the result the orchestrator sees. +pub(super) fn build_result( + session: SubagentSession, + outcome: TurnOutcome, + files_modified: &[String], + turn_ctx: &crate::agent::TurnContext, + config: &SubagentConfig, +) -> SubagentResult { + // Build token usage breakdown + let mut token_usage = TokenUsageBreakdown::default(); + token_usage.add_turn( + turn_ctx.tokens.input_tokens as u64, + turn_ctx.tokens.output_tokens as u64, + turn_ctx.tokens.cached_tokens as u64, + turn_ctx.tokens.reasoning_tokens as u64, + ); + + // Build result + let mut builder = SubagentResultBuilder::new(session) + .success(outcome.success) + .output(&outcome.output) + .tokens(token_usage); + + // Add error to result - either from explicit error or from status_info + if let Some(err) = outcome.error { + builder = builder.error(err); + } else if let Some(ref status) = outcome.status_info { + builder = builder.error(format!("Task ended with status: {}", status)); + } + + for change in files_modified { + builder = builder.file_changed(FileChange::new(change.clone(), FileChangeType::Modified)); + } + + // Allow continuation if partially complete + if outcome.success || turn_ctx.tool_iterations < config.effective_max_iterations() { + builder = builder.continuable(); + } + + builder.build() +} + +/// Prompt used to request an explicit summary from a subagent when none was provided. +/// Ensures structured output from agents for orchestrator consumption. +const SUMMARY_REQUEST_PROMPT: &str = r#"You have completed your work but did not provide a summary. Please provide a final summary NOW using EXACTLY this format: + +## Summary for Orchestrator + +### Tasks Completed +- [List each task you completed with brief outcome] + +### Key Findings/Changes +- [Main discoveries or modifications made] + +### Files Modified (if any) +- [List of files with type of change] + +### Recommendations (if applicable) +- [Any follow-up actions or suggestions] + +### Status: COMPLETED + +DO NOT use any tools. Just provide the summary based on the work you have already done."#; + +/// Check if the response contains a proper summary for the orchestrator. +/// Returns true if summary markers are present, false otherwise. +pub(super) fn has_summary_output(response: &str) -> bool { + // Empty responses definitely don't have a summary + if response.trim().is_empty() { + return false; + } + + // Check for key summary markers that indicate structured output + let summary_markers = [ + "## Summary for Orchestrator", + "### Tasks Completed", + "### Key Findings", + "### Status: COMPLETED", + "Status: COMPLETED", + // Also accept some variations + "## Summary", + "### Summary", + "## Final Summary", + "### Final Summary", + ]; + + let response_lower = response.to_lowercase(); + summary_markers + .iter() + .any(|marker| response_lower.contains(&marker.to_lowercase())) +} + +/// Extract file path from tool output. +pub(super) fn extract_file_path(output: &str) -> Option { + // Try to extract path from common patterns + // "Created file: path/to/file" + // "Edited path/to/file" + // "Wrote N bytes to path/to/file" + + let patterns = [ + "Created file: ", + "Created: ", + "Edited ", + "Modified ", + "Wrote ", + "to ", + ]; + + for pattern in patterns { + if let Some(idx) = output.find(pattern) { + let rest = &output[idx + pattern.len()..]; + // Extract until whitespace or end + let path: String = rest.chars().take_while(|c| !c.is_whitespace()).collect(); + if !path.is_empty() && (path.contains('/') || path.contains('\\') || path.contains('.')) + { + return Some(path); + } + } + } + + None +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_extract_file_path() { + assert_eq!( + extract_file_path("Created file: src/main.rs"), + Some("src/main.rs".to_string()) + ); + assert_eq!( + extract_file_path("Wrote 100 bytes to config.json"), + Some("config.json".to_string()) + ); + assert_eq!( + extract_file_path("Successfully edited src/lib.rs"), + None // "edited" doesn't match "Edited " + ); + assert_eq!(extract_file_path("No path here"), None); + } + + #[test] + fn test_has_summary_output_with_proper_summary() { + let response_with_summary = r#" +## Summary for Orchestrator + +### Tasks Completed +- Analyzed the codebase structure + +### Key Findings +- Found 10 modules + +### Status: COMPLETED +"#; + assert!(has_summary_output(response_with_summary)); + } + + #[test] + fn test_has_summary_output_with_variation() { + // Test case-insensitive matching + let response = "## summary\nSome content here"; + assert!(has_summary_output(response)); + + // Test "Status: COMPLETED" alone + let response2 = "Work done.\n\nStatus: COMPLETED"; + assert!(has_summary_output(response2)); + } + + #[test] + fn test_has_summary_output_empty() { + assert!(!has_summary_output("")); + assert!(!has_summary_output(" ")); + assert!(!has_summary_output("\n\n")); + } + + #[test] + fn test_has_summary_output_no_markers() { + let response_without_summary = "I analyzed the code and found some issues."; + assert!(!has_summary_output(response_without_summary)); + + let response_partial = "Here are some findings:\n- Item 1\n- Item 2"; + assert!(!has_summary_output(response_partial)); + } + + #[test] + fn a_missing_summary_is_requested_only_for_completed_turns() { + use crate::error::CortexError; + + let completed_without_summary = Ok(OrchestratorTurnResult { + turn_id: 1, + response: "did the work".into(), + tool_calls: Vec::new(), + token_usage: Default::default(), + duration: Duration::ZERO, + status: TurnStatus::Completed, + }); + assert!(needs_summary_turn(&completed_without_summary)); + + let completed_with_summary = Ok(OrchestratorTurnResult { + turn_id: 1, + response: "## Summary\n- done".into(), + tool_calls: Vec::new(), + token_usage: Default::default(), + duration: Duration::ZERO, + status: TurnStatus::Completed, + }); + assert!(!needs_summary_turn(&completed_with_summary)); + + let interrupted = Ok(OrchestratorTurnResult { + turn_id: 1, + response: "stopped early".into(), + tool_calls: Vec::new(), + token_usage: Default::default(), + duration: Duration::ZERO, + status: TurnStatus::Interrupted, + }); + assert!(!needs_summary_turn(&interrupted)); + + let failed = Err(CortexError::Timeout); + assert!(!needs_summary_turn(&failed)); + } + + #[test] + fn outcome_reports_explicit_and_derived_failures() { + let ok_completed = Ok(OrchestratorTurnResult { + turn_id: 7, + response: "all good".into(), + tool_calls: Vec::new(), + token_usage: Default::default(), + duration: Duration::ZERO, + status: TurnStatus::Completed, + }); + let outcome = TurnOutcome::from_result(&ok_completed); + assert!(outcome.success); + assert_eq!(outcome.output, "all good"); + assert!(outcome.error.is_none()); + assert!(outcome.status_info.is_none()); + + // A non-completed status is a failure with no explicit error. + let ok_interrupted = Ok(OrchestratorTurnResult { + turn_id: 7, + response: "partial".into(), + tool_calls: Vec::new(), + token_usage: Default::default(), + duration: Duration::ZERO, + status: TurnStatus::Interrupted, + }); + let outcome = TurnOutcome::from_result(&ok_interrupted); + assert!(!outcome.success); + assert!(outcome.error.is_none()); + assert!(outcome.failure_message().contains("Interrupted")); + assert!(outcome.status_info.is_some()); + + // A hard error keeps its own message. + let failed: Result = Err(crate::error::CortexError::Timeout); + let outcome = TurnOutcome::from_result(&failed); + assert!(!outcome.success); + assert!(outcome.output.is_empty()); + assert!(outcome.error.is_some()); + assert!(!outcome.failure_message().is_empty()); + } + + #[test] + fn a_custom_agent_task_message_carries_context() { + let mut config = SubagentConfig::new( + crate::tools::handlers::subagent::SubagentType::Code, + "review the parser", + "check the tokenizer", + std::path::PathBuf::from("/tmp"), + ); + config.context = Some("the parser is in src/parse.rs".into()); + + let message = config.build_user_message(); + assert!(message.contains("check the tokenizer"), "{message}"); + assert!(message.contains("Additional Context"), "{message}"); + assert!( + message.contains("the parser is in src/parse.rs"), + "{message}" + ); + } +} diff --git a/src/cortex-engine/src/tools/handlers/subagent/types.rs b/src/cortex-engine/src/tools/handlers/subagent/types.rs index 94cc6986..cd7b4834 100644 --- a/src/cortex-engine/src/tools/handlers/subagent/types.rs +++ b/src/cortex-engine/src/tools/handlers/subagent/types.rs @@ -309,6 +309,20 @@ pub struct SubagentConfig { /// Optional session ID (if not provided, a new one will be generated). /// Use this to coordinate session_id between UI and executor. pub session_id: Option, + /// Instruction documents this subagent skips. Organization-managed policy + /// is never omitted, whatever this list says. + #[serde(default)] + pub omit_instructions: Vec, +} + +impl SubagentConfig { + /// Resolved instruction plan for this subagent. + /// + /// Managed policy always loads; a request that named it is reported so the + /// caller can audit it. + pub fn instruction_plan(&self) -> crate::instruction_scopes::InstructionPlan { + crate::instruction_scopes::InstructionPlan::new(&self.omit_instructions) + } } impl SubagentConfig { @@ -334,9 +348,21 @@ impl SubagentConfig { context: None, custom_agent_name: None, session_id: None, + omit_instructions: Vec::new(), } } + /// Omit the named instruction scopes for this subagent. + /// + /// `Managed` is accepted and ignored: organization policy always loads. + pub fn with_omit_instructions( + mut self, + scopes: impl IntoIterator, + ) -> Self { + self.omit_instructions = scopes.into_iter().collect(); + self + } + /// Set the model. pub fn with_model(mut self, model: impl Into) -> Self { self.model = Some(model.into()); @@ -623,4 +649,78 @@ mod tests { assert_eq!(session.turns_completed, 1); assert_eq!(session.tool_calls_made, 3); } + + /// COR-449: omission is opt-in and managed policy always loads. + #[test] + fn subagent_instruction_plan_defaults_to_loading_everything() { + let config = SubagentConfig::new( + SubagentType::Code, + "review", + "review src/auth", + PathBuf::from("/project"), + ); + assert!(config.omit_instructions.is_empty()); + let plan = config.instruction_plan(); + assert!(!plan.omits_anything()); + assert!(plan.summary("subagent").is_none()); + for scope in crate::instruction_scopes::InstructionScope::ALL { + assert!(plan.loads(scope), "{scope}"); + } + } + + #[test] + fn subagent_omission_never_drops_managed_policy() { + use crate::instruction_scopes::InstructionScope; + let config = SubagentConfig::new( + SubagentType::Code, + "review", + "review src/auth", + PathBuf::from("/project"), + ) + .with_omit_instructions([ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Local, + InstructionScope::Managed, + ]); + let plan = config.instruction_plan(); + assert!(plan.loads(InstructionScope::Managed)); + assert!(plan.managed_forced()); + assert!(plan.requested_managed()); + assert!(!plan.loads(InstructionScope::User)); + assert!(!plan.loads(InstructionScope::Project)); + assert!(!plan.loads(InstructionScope::Local)); + assert_eq!( + plan.skipped(), + vec![ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Local + ] + ); + let summary = plan.summary("subagent").expect("summary"); + assert!(summary.contains("managed policy still loaded"), "{summary}"); + } + + #[test] + fn subagent_config_round_trips_the_omit_list() { + use crate::instruction_scopes::InstructionScope; + let config = SubagentConfig::new( + SubagentType::Code, + "review", + "review src/auth", + PathBuf::from("/project"), + ) + .with_omit_instructions([InstructionScope::User, InstructionScope::Project]); + let encoded = serde_json::to_value(&config).unwrap(); + assert_eq!(encoded["omit_instructions"][0], "user"); + assert_eq!(encoded["omit_instructions"][1], "project"); + let decoded: SubagentConfig = serde_json::from_value(encoded).unwrap(); + assert_eq!(decoded.omit_instructions, config.omit_instructions); + // A config without the field still deserializes. + let mut bare = serde_json::to_value(&config).unwrap(); + bare.as_object_mut().unwrap().remove("omit_instructions"); + let decoded: SubagentConfig = serde_json::from_value(bare).unwrap(); + assert!(decoded.omit_instructions.is_empty()); + } } diff --git a/src/cortex-engine/src/tools/handlers/task.rs b/src/cortex-engine/src/tools/handlers/task.rs index 04edef20..8787f987 100644 --- a/src/cortex-engine/src/tools/handlers/task.rs +++ b/src/cortex-engine/src/tools/handlers/task.rs @@ -115,6 +115,11 @@ Children cannot spawn nested Task tools and cannot use AskUser, Questions, or se "type": "boolean", "description": "Wait for the child (true) or return after task_started (false).", "default": true + }, + "omit_instructions": { + "type": "array", + "items": {"type": "string", "enum": ["user", "project", "local", "managed"]}, + "description": "Instruction documents this child skips. Organization-managed policy is never omitted." } }, "required": [], @@ -159,6 +164,30 @@ Children cannot spawn nested Task tools and cannot use AskUser, Questions, or se .and_then(|a| a.as_bool()) .unwrap_or(true); + // Instruction omission is opt-in and validated: an unknown scope name + // is an error, and `managed` is accepted but never skipped. + let omit_instructions = match arguments.get("omit_instructions") { + None | Some(Value::Null) => Vec::new(), + Some(Value::Array(values)) => { + let names: Vec<&str> = values + .iter() + .map(|value| { + value.as_str().ok_or_else(|| { + CortexError::InvalidInput( + "omit_instructions entries must be scope names".into(), + ) + }) + }) + .collect::>()?; + crate::instruction_scopes::parse_scopes(names).map_err(CortexError::InvalidInput)? + } + Some(_) => { + return Err(CortexError::InvalidInput( + "omit_instructions must be an array of scope names".into(), + )); + } + }; + Ok(TaskParams { agent, task, @@ -166,6 +195,7 @@ Children cannot spawn nested Task tools and cannot use AskUser, Questions, or se await_result, mode, description, + omit_instructions, }) } @@ -187,6 +217,7 @@ Children cannot spawn nested Task tools and cannot use AskUser, Questions, or se config.env.insert("CORTEX_SPEC_MODE".into(), "1".into()); } config.prompt = format!("{}\n\n{}", params.mode.system_prompt(), config.prompt); + config = config.with_omit_instructions(params.omit_instructions); config } } @@ -321,6 +352,8 @@ struct TaskParams { await_result: bool, mode: crate::harness::TaskRole, description: String, + /// Instruction scopes this child skips. Managed policy always loads. + omit_instructions: Vec, } /// Create a standalone task handler with minimal dependencies (for registry integration). @@ -568,6 +601,76 @@ mod tests { assert!(params.await_result); } + /// COR-449: `omit_instructions` is validated and never omits managed policy. + #[test] + fn test_task_params_omit_instructions() { + use crate::instruction_scopes::InstructionScope; + + let handler = TaskHandler::with_executor( + Arc::new(SubagentExecutor::new( + Arc::new(MockClient::new()), + Arc::new(ToolRegistry::new()), + Arc::new(AgentRegistry::new(&PathBuf::from("/tmp"), None)), + "gpt-4o", + )), + PathBuf::from("/project"), + ); + + // Absent and explicit-null both mean "load everything". + for args in [ + json!({"prompt": "review src/auth"}), + json!({"prompt": "review src/auth", "omit_instructions": null}), + ] { + let params = handler.parse_params(args).unwrap(); + assert!(params.omit_instructions.is_empty()); + } + + // Named scopes are parsed, and managed is kept in the list only to be + // ignored by the plan. + let params = handler + .parse_params(json!({ + "prompt": "review src/auth", + "omit_instructions": ["user", "project", "managed"], + })) + .unwrap(); + assert_eq!( + params.omit_instructions, + vec![ + InstructionScope::User, + InstructionScope::Project, + InstructionScope::Managed + ] + ); + let plan = handler.build_config(params).instruction_plan(); + assert!(!plan.loads(InstructionScope::User)); + assert!(!plan.loads(InstructionScope::Project)); + assert!(plan.loads(InstructionScope::Local)); + assert!(plan.loads(InstructionScope::Managed)); + assert!(plan.managed_forced()); + + // An unknown scope is an error, never a silent no-op. + let error = handler + .parse_params(json!({ + "prompt": "review src/auth", + "omit_instructions": ["projekt"], + })) + .unwrap_err() + .to_string(); + assert!(error.contains("projekt"), "{error}"); + + // Wrong shape is an error too. + assert!( + handler + .parse_params(json!({"prompt": "x", "omit_instructions": "user"})) + .is_err() + ); + assert!( + handler + .parse_params(json!({"prompt": "x", "omit_instructions": [1]})) + .is_err() + ); + } + #[test] fn test_task_params_with_context() { let handler = TaskHandler::with_executor( diff --git a/src/cortex-plugins/src/command_pin.rs b/src/cortex-plugins/src/command_pin.rs new file mode 100644 index 00000000..6f2a1a5f --- /dev/null +++ b/src/cortex-plugins/src/command_pin.rs @@ -0,0 +1,294 @@ +//! Command-hash pinning for plugin installs. +//! +//! `plugin install` and `plugin update` can print the exact commands a package +//! would register. `--accept-command ` pins that review: the install +//! proceeds only when the package under install hashes to the same value. +//! +//! The hash covers the plugin id, version, and every declared command with its +//! aliases, arguments, and flags, so a manifest that changed in any way that a +//! user could notice produces a different value. A mismatch fails closed; there +//! is no `-y` shortcut and no automatic re-acceptance. + +use sha2::{Digest, Sha256}; + +use crate::{PluginError, PluginManifest, Result}; + +/// Product copy for a hash that does not match the reviewed package. +pub const HASH_MISMATCH: &str = + "Command hash mismatch. Manifest may have changed. Re-run with --json and accept the new hash."; +/// Product copy for a hash that is not a SHA-256 value. +pub const HASH_INVALID: &str = + "Accept-command hash must be a 64-character SHA-256 value from --json."; + +/// Length-prefix a free-text field so unrestricted values cannot collide +/// across `|` (or other) separators in the review digest. +fn field(label: &str, value: &str) -> String { + format!("{label}:{}\n{value}\n", value.len()) +} + +/// Canonical, stable description of the commands a manifest declares. +/// +/// One entry per command, in declaration order, with every user-visible field +/// that a review would show. Free text is length-prefixed so distinct metadata +/// cannot hash-collide. Hooks include type, priority, pattern, and function. +pub fn command_review(manifest: &PluginManifest) -> String { + let mut out = format!( + "plugin {} {}\n", + manifest.plugin.id, manifest.plugin.version + ); + for command in &manifest.commands { + out.push_str("command\n"); + out.push_str(&field("name", &command.name)); + out.push_str(&field("description", &command.description)); + out.push_str(&field("usage", command.usage.as_deref().unwrap_or(""))); + out.push_str(&format!("hidden={}\n", command.hidden)); + for alias in &command.aliases { + out.push_str(&field("alias", alias)); + } + for arg in &command.args { + out.push_str("arg\n"); + out.push_str(&field("name", &arg.name)); + out.push_str(&format!("required={}\n", arg.required)); + out.push_str(&field("default", arg.default.as_deref().unwrap_or(""))); + } + } + for hook in &manifest.hooks { + out.push_str("hook\n"); + out.push_str(&field("type", &hook.hook_type.to_string())); + out.push_str(&format!("priority={}\n", hook.priority)); + out.push_str(&field("pattern", hook.pattern.as_deref().unwrap_or(""))); + out.push_str(&field("function", hook.function.as_deref().unwrap_or(""))); + } + for tool in &manifest.tools { + out.push_str(&field("tool", &tool.name)); + } + out +} + +/// SHA-256 of the canonical command review. +pub fn command_hash(manifest: &PluginManifest) -> String { + hex::encode(Sha256::digest(command_review(manifest).as_bytes())) +} + +/// True when `value` is a well-formed SHA-256 hex string. +pub fn is_sha256(value: &str) -> bool { + value.len() == 64 && value.bytes().all(|b| b.is_ascii_hexdigit()) +} + +/// Verify a reviewed hash against the package that is about to install. +/// +/// Fails closed: an invalid hash and a mismatched hash are both errors, and the +/// error names the exact recovery step. +pub fn verify_command_hash(manifest: &PluginManifest, accepted: &str) -> Result<()> { + if !is_sha256(accepted) { + return Err(PluginError::validation_error( + "accept-command", + HASH_INVALID, + )); + } + let actual = command_hash(manifest); + if actual != accepted.to_ascii_lowercase() { + return Err(PluginError::validation_error( + "accept-command", + HASH_MISMATCH, + )); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + const BASE: &str = r#" +[plugin] +id = "review" +name = "Review" +version = "1.2.0" + +[runtime] +kind = "node" +entrypoint = "plugin.mjs" + +[[commands]] +name = "review" +description = "Review the working tree" +usage = "/review [path]" +ALIASES + +[[commands.args]] +name = "path" +required = false +"#; + + fn manifest(body: &str) -> PluginManifest { + PluginManifest::parse(body).expect("fixture parses") + } + + /// Fill the fixture's command-table marker with extra command fields. + fn with_command_fields(fields: &str) -> PluginManifest { + manifest(&BASE.replace("ALIASES", fields)) + } + + #[test] + fn the_hash_is_stable_and_hex_sha256() { + let hash = command_hash(&with_command_fields("")); + assert!(is_sha256(&hash), "{hash}"); + assert_eq!(hash, command_hash(&with_command_fields(""))); + assert_eq!(hash, hash.to_ascii_lowercase()); + } + + #[test] + fn any_reviewed_change_moves_the_hash() { + let base = command_hash(&with_command_fields("")); + let renamed = command_hash(&manifest( + &BASE + .replace("ALIASES", "") + .replace("name = \"review\"", "name = \"check\""), + )); + assert_ne!(base, renamed, "a renamed command must change the hash"); + let described = command_hash(&manifest(&BASE.replace("ALIASES", "").replace( + "description = \"Review the working tree\"", + "description = \"Review a diff\"", + ))); + assert_ne!(base, described); + let versioned = command_hash(&manifest( + &BASE + .replace("ALIASES", "") + .replace("version = \"1.2.0\"", "version = \"1.2.1\""), + )); + assert_ne!(base, versioned, "a version bump must change the hash"); + let argued = command_hash(&manifest( + &BASE + .replace("ALIASES", "") + .replace("required = false", "required = true"), + )); + assert_ne!(base, argued); + let hidden = command_hash(&with_command_fields("hidden = true")); + assert_ne!(base, hidden); + } + + #[test] + fn an_alias_only_change_still_moves_the_hash() { + let base = command_hash(&with_command_fields("")); + let aliased = command_hash(&with_command_fields("aliases = [\"rev\"]")); + assert_ne!(base, aliased); + } + #[test] + fn a_matching_hash_is_accepted_in_either_case() { + let review = with_command_fields(""); + let hash = command_hash(&review); + assert!(verify_command_hash(&review, &hash).is_ok()); + assert!(verify_command_hash(&review, &hash.to_ascii_uppercase()).is_ok()); + } + + #[test] + fn a_mismatch_fails_closed_with_the_review_copy() { + let review = with_command_fields(""); + let other = manifest( + &BASE + .replace("ALIASES", "") + .replace("version = \"1.2.0\"", "version = \"2.0.0\""), + ); + let error = verify_command_hash(&other, &command_hash(&review)) + .expect_err("a changed manifest must not install") + .to_string(); + assert!(error.contains(HASH_MISMATCH), "{error}"); + assert!(error.contains("Re-run with --json"), "{error}"); + } + + #[test] + fn a_malformed_hash_is_rejected_before_any_work() { + let review = with_command_fields(""); + for bad in ["", "abc", &"z".repeat(64), &"a".repeat(63)] { + let error = verify_command_hash(&review, bad) + .expect_err("a malformed hash must not install") + .to_string(); + assert!(error.contains("SHA-256"), "{bad:?}: {error}"); + } + } + + #[test] + fn the_review_names_every_declared_surface() { + let review = command_review(&with_command_fields("")); + assert!(review.contains("plugin review 1.2.0"), "{review}"); + assert!(review.contains("name:6\nreview\n"), "{review}"); + assert!(review.contains("/review [path]"), "{review}"); + assert!(review.contains("name:4\npath\n"), "{review}"); + } + + #[test] + fn free_text_cannot_collide_across_fields() { + // Distinct description/usage pairs that would collide under raw `|` + // joins must produce different hashes with length-prefixed fields. + let a = manifest( + r#" +[plugin] +id = "review" +name = "Review" +version = "1.0.0" + +[runtime] +kind = "node" +entrypoint = "plugin.mjs" + +[[commands]] +name = "x" +description = "ab|c" +usage = "d" +"#, + ); + let b = manifest( + r#" +[plugin] +id = "review" +name = "Review" +version = "1.0.0" + +[runtime] +kind = "node" +entrypoint = "plugin.mjs" + +[[commands]] +name = "x" +description = "ab" +usage = "c|d" +"#, + ); + assert_ne!(command_hash(&a), command_hash(&b)); + assert_ne!(command_review(&a), command_review(&b)); + } + + #[test] + fn hook_executable_fields_move_the_hash() { + let base_body = r#" +[plugin] +id = "review" +name = "Review" +version = "1.0.0" + +[runtime] +kind = "node" +entrypoint = "plugin.mjs" + +[[hooks]] +hook_type = "session_start" +priority = 10 +pattern = "*.rs" +function = "on_start" +"#; + let base = command_hash(&manifest(base_body)); + let changed_fn = command_hash(&manifest( + &base_body.replace("function = \"on_start\"", "function = \"other\""), + )); + assert_ne!(base, changed_fn, "hook function must be pinned"); + let changed_pri = command_hash(&manifest( + &base_body.replace("priority = 10", "priority = 99"), + )); + assert_ne!(base, changed_pri, "hook priority must be pinned"); + let changed_pat = command_hash(&manifest( + &base_body.replace("pattern = \"*.rs\"", "pattern = \"*.toml\""), + )); + assert_ne!(base, changed_pat, "hook pattern must be pinned"); + } +} diff --git a/src/cortex-plugins/src/lib.rs b/src/cortex-plugins/src/lib.rs index ac26887f..47fd2f5d 100644 --- a/src/cortex-plugins/src/lib.rs +++ b/src/cortex-plugins/src/lib.rs @@ -49,6 +49,7 @@ mod abi; pub mod activation; pub mod api; +pub mod command_pin; pub mod commands; pub mod config; pub mod contract; diff --git a/src/cortex-tui/src/app/state.rs b/src/cortex-tui/src/app/state.rs index 342e5323..6e16b15f 100644 --- a/src/cortex-tui/src/app/state.rs +++ b/src/cortex-tui/src/app/state.rs @@ -212,6 +212,12 @@ pub struct AppState { pub show_computer_default: bool, /// Live read-only share link minted by `/share`; `None` once `/unshare` clears it. pub share_link: Option, + /// Remote (cloud / self-hosted) Code session — shows the Remote status line. + pub remote_session: bool, + /// Fast mode is on for this session, as resolved against organization policy. + pub fast_mode: cortex_engine::fast_mode::FastMode, + /// Organization policy resolved for this session. + pub fast_mode_policy: cortex_engine::fast_mode::FastModePolicy, /// Launched via the `agent` binary / alias. pub agent_entrypoint: bool, /// Token counter used / window. @@ -365,6 +371,9 @@ impl AppState { computer_held: false, show_computer_default: false, share_link: None, + remote_session: false, + fast_mode: cortex_engine::fast_mode::FastMode::Standard, + fast_mode_policy: cortex_engine::fast_mode::FastModePolicy::HostDefault, agent_entrypoint: false, tokens_used: 0, context_window: 500_000, @@ -976,20 +985,5 @@ impl AppState { } #[cfg(test)] -mod tests { - use super::*; - use cortex_core::widgets::Message; - - #[test] - fn first_user_turn_drops_the_launch_splash() { - let mut state = AppState::default(); - assert!(state.show_launch_splash); - state.add_message(Message::user("hello")); - assert!(!state.show_launch_splash); - state.clear_messages(); - assert!( - !state.show_launch_splash, - "/clear must not restore the splash" - ); - } -} +#[path = "state_tests.rs"] +mod tests; diff --git a/src/cortex-tui/src/app/state_tests.rs b/src/cortex-tui/src/app/state_tests.rs new file mode 100644 index 00000000..d459fdbe --- /dev/null +++ b/src/cortex-tui/src/app/state_tests.rs @@ -0,0 +1,16 @@ +//! App state tests: splash, clear, and message bookkeeping. +use super::*; +use cortex_core::widgets::Message; + +#[test] +fn first_user_turn_drops_the_launch_splash() { + let mut state = AppState::default(); + assert!(state.show_launch_splash); + state.add_message(Message::user("hello")); + assert!(!state.show_launch_splash); + state.clear_messages(); + assert!( + !state.show_launch_splash, + "/clear must not restore the splash" + ); +} diff --git a/src/cortex-tui/src/bin/generate_tui_lock_screenshots.rs b/src/cortex-tui/src/bin/generate_tui_lock_screenshots.rs index 00cf799a..beddc0c3 100644 --- a/src/cortex-tui/src/bin/generate_tui_lock_screenshots.rs +++ b/src/cortex-tui/src/bin/generate_tui_lock_screenshots.rs @@ -11,6 +11,7 @@ use std::process; use cortex_tui::lock_proof::write_lock_frames; use cortex_tui::lock_v2::{validate_lock_v2_only_ids, write_lock_v2_frames}; +use cortex_tui::lock_v2_batch56::{BATCH56_IDS, validate_batch56_only_ids, write_batch56_frames}; fn print_help() { println!( @@ -26,8 +27,15 @@ OPTIONS: -h, --height Terminal height (default: 40) --v2 Capture lock v2 scenes (real session chrome) --only Comma-separated lock v2 scene ids (with --v2) + --batch56 Capture the Batch 56 boards (COR-447/448/449) instead + of the SPEC §7 pack; combine with --only for a subset --help Show this help -" + +Batch 56 ids ({}): + {} +", + BATCH56_IDS.len(), + BATCH56_IDS.join(", ") ); } @@ -37,6 +45,7 @@ fn main() { let mut width: u16 = 120; let mut height: u16 = 40; let mut v2 = false; + let mut batch56 = false; let mut only_flag = false; let mut only: Vec = Vec::new(); @@ -71,6 +80,9 @@ fn main() { "--v2" => { v2 = true; } + "--batch56" => { + batch56 = true; + } "--only" => { only_flag = true; i += 1; @@ -94,12 +106,27 @@ fn main() { i += 1; } - if only_flag && !v2 { - eprintln!("--only requires --v2"); + if only_flag && !v2 && !batch56 { + eprintln!("--only requires --v2 or --batch56"); + process::exit(1); + } + if batch56 && v2 { + eprintln!("--batch56 and --v2 are separate packs; pick one"); process::exit(1); } - let result = if v2 { + let result = if batch56 { + let ids: Vec<&str> = if only_flag { + only.iter().map(String::as_str).collect() + } else { + BATCH56_IDS.to_vec() + }; + if let Err(err) = validate_batch56_only_ids(&ids) { + eprintln!("{err:#}"); + process::exit(1); + } + write_batch56_frames(&ids, width, height, &output) + } else if v2 { if only_flag { let ids: Vec<&str> = only.iter().map(String::as_str).collect(); if let Err(err) = validate_lock_v2_only_ids(&ids, width) { @@ -112,7 +139,7 @@ fn main() { } } else { if only_flag { - eprintln!("--only requires --v2"); + eprintln!("--only requires --v2 or --batch56"); process::exit(1); } write_lock_frames(width, height, &output) diff --git a/src/cortex-tui/src/commands/executor/dispatch.rs b/src/cortex-tui/src/commands/executor/dispatch.rs index 583c78f9..0df4d06f 100644 --- a/src/cortex-tui/src/commands/executor/dispatch.rs +++ b/src/cortex-tui/src/commands/executor/dispatch.rs @@ -94,6 +94,7 @@ impl CommandExecutor { // ============ MODEL ============ "model" | "models" | "m" | "lm" | "list-models" => self.cmd_models(cmd), "approval" | "approve" => self.cmd_approval(cmd), + "fast" => self.cmd_fast(cmd), "sandbox" | "sb" => self.cmd_sandbox(cmd), "auto" | "autopilot" => self.cmd_auto(cmd), "provider" | "prov" => self.cmd_provider(cmd), diff --git a/src/cortex-tui/src/commands/executor/model.rs b/src/cortex-tui/src/commands/executor/model.rs index 118cd27a..a6bd5699 100644 --- a/src/cortex-tui/src/commands/executor/model.rs +++ b/src/cortex-tui/src/commands/executor/model.rs @@ -41,6 +41,21 @@ impl CommandExecutor { } } + /// `/fast [on|off]` — the organization-policy check happens where the + /// session state lives, so this only parses and rejects unknown tokens. + pub(super) fn cmd_fast(&self, cmd: &ParsedCommand) -> CommandResult { + match cmd.first_arg() { + Some(value) => match cortex_engine::fast_mode::FastMode::parse(value) { + Ok(mode) => CommandResult::SetValue( + "fast".to_string(), + if mode.is_on() { "on" } else { "off" }.to_string(), + ), + Err(error) => CommandResult::Error(error.to_string()), + }, + None => CommandResult::Toggle("fast".to_string()), + } + } + pub(super) fn cmd_sandbox(&self, cmd: &ParsedCommand) -> CommandResult { match cmd.first_arg() { Some("on") | Some("true") => { diff --git a/src/cortex-tui/src/commands/executor/tests.rs b/src/cortex-tui/src/commands/executor/tests.rs index 50182dc4..e37a7e46 100644 --- a/src/cortex-tui/src/commands/executor/tests.rs +++ b/src/cortex-tui/src/commands/executor/tests.rs @@ -602,3 +602,28 @@ fn plugins_command_is_registered() { "got {result:?}" ); } + +#[test] +fn fast_command_toggles_and_parses_on_off() { + let executor = CommandExecutor::new(); + // Bare `/fast` toggles; the policy check happens in the session. + assert!( + matches!( + executor.execute_str("/fast"), + CommandResult::Toggle(ref feature) if feature == "fast" + ), + "bare /fast must toggle" + ); + for (token, expected) in [("on", "on"), ("ON", "on"), ("off", "off"), ("false", "off")] { + let result = executor.execute_str(&format!("/fast {token}")); + assert!( + matches!(result, CommandResult::SetValue(ref key, ref value) + if key == "fast" && value == expected), + "/fast {token} → {result:?}" + ); + } + // An unknown token is an error, never a silent toggle. + let invalid = executor.execute_str("/fast maybe"); + assert!(invalid.is_error(), "{invalid:?}"); + assert!(format!("{invalid:?}").contains("on|off"), "{invalid:?}"); +} diff --git a/src/cortex-tui/src/commands/registry/builtin.rs b/src/cortex-tui/src/commands/registry/builtin.rs index 4b6756b8..dc0e6791 100644 --- a/src/cortex-tui/src/commands/registry/builtin.rs +++ b/src/cortex-tui/src/commands/registry/builtin.rs @@ -111,6 +111,15 @@ pub fn register_builtin_commands(registry: &mut CommandRegistry) { false, )); + registry.register(CommandDef::new( + "fast", + &[], + "Toggle fast mode for remote sessions", + "/fast [on|off]", + CommandCategory::Model, + true, + )); + registry.register(CommandDef::new( "btw", &[], diff --git a/src/cortex-tui/src/lib.rs b/src/cortex-tui/src/lib.rs index 6a979543..24989783 100644 --- a/src/cortex-tui/src/lib.rs +++ b/src/cortex-tui/src/lib.rs @@ -107,6 +107,7 @@ pub mod lock_boards; pub mod lock_palette; pub mod lock_proof; pub mod lock_v2; +pub mod lock_v2_batch56; mod lock_v2_boards; mod lock_v2_computer; mod lock_v2_cor35; @@ -118,6 +119,7 @@ mod lock_v2_network; mod lock_v2_parity; mod lock_v2_residual; mod lock_v2_scenes; +mod lock_v2_settings; mod lock_v2_share; pub mod plugin_marketplace; pub mod readme_hero; diff --git a/src/cortex-tui/src/lock_v2_batch56.rs b/src/cortex-tui/src/lock_v2_batch56.rs new file mode 100644 index 00000000..42247f13 --- /dev/null +++ b/src/cortex-tui/src/lock_v2_batch56.rs @@ -0,0 +1,472 @@ +//! Batch 56 runtime lock boards (COR-447 / COR-448 / COR-449). +//! +//! Six real `MockTerminal` scenes captured through the same raster pipeline as +//! the rest of lock v2, without joining the SPEC §7 id lists: the boards here +//! document shipped Code/CLI behavior for one batch, so they are captured by +//! id through [`write_batch56_frames`] and land next to the existing runtime +//! boards. +//! +//! Copy stays Cortex-only: no competitor or provider names. +//! +//! - COR-447: `remote-fast`, `remote-standard`, `org-disabled-fast` +//! - COR-448: `plugin-accept-command`, `command-hash-mismatch` +//! - COR-449: `omit-instructions` + +use std::path::{Path, PathBuf}; + +use anyhow::{Context, Result}; +use cortex_core::widgets::Message; +use cortex_engine::fast_mode::{ + FastMode, FastModePolicy, ORG_DISABLED_STAYING, ORG_DISABLED_TOAST, +}; + +use crate::app::AppState; +use crate::lock_v2::render_lock_v2_scene; +use crate::lock_v2_scenes::{conversation, resumed}; + +/// The batch's board ids. Each one is also its capture file name. +pub const BATCH56_IDS: &[&str] = &[ + "remote-fast", + "remote-standard", + "org-disabled-fast", + "plugin-accept-command", + "command-hash-mismatch", + "omit-instructions", +]; + +/// True when `id` belongs to this batch. +pub fn is_batch56_id(id: &str) -> bool { + BATCH56_IDS.contains(&id) +} + +/// Reject empty, unknown, or repeated ids before any frame is written. +pub fn validate_batch56_only_ids(ids: &[&str]) -> Result<()> { + if ids.is_empty() { + anyhow::bail!("--only requires at least one scene id"); + } + let mut seen = std::collections::HashSet::new(); + for id in ids { + if !is_batch56_id(id) { + anyhow::bail!("unknown batch 56 scene id `{id}`"); + } + if !seen.insert(*id) { + anyhow::bail!("repeated batch 56 scene id `{id}`"); + } + } + Ok(()) +} + +/// Write the requested batch frames as ANSI plus a manifest, in the same shape +/// [`crate::lock_v2::write_lock_v2_id_frames`] uses so the existing +/// `ansi-frames-to-gif.py --png-only` step consumes them unchanged. +pub fn write_batch56_frames( + ids: &[&str], + width: u16, + height: u16, + output_dir: &Path, +) -> Result { + validate_batch56_only_ids(ids)?; + std::fs::create_dir_all(output_dir) + .with_context(|| format!("create {}", output_dir.display()))?; + let mut frames = Vec::new(); + for id in ids { + let frame = render_lock_v2_scene(id, width, height)?; + let file = format!("{id}.ans"); + std::fs::write(output_dir.join(&file), &frame.ansi) + .with_context(|| format!("write {file}"))?; + frames.push(serde_json::json!({ + "file": file, + "label": (*id).to_string(), + "hold": 1, + })); + } + let manifest = output_dir.join("manifest.json"); + std::fs::write( + &manifest, + serde_json::to_string_pretty(&serde_json::json!({ + "width": width, + "height": height, + "fps": 1, + "frames": frames, + }))?, + )?; + Ok(manifest) +} + +/// Apply a Batch 56 scene. Returns `false` when `id` is not one of ours. +pub fn apply_batch56_scene(id: &str, state: &mut AppState, width: u16) -> bool { + if !is_batch56_id(id) { + return false; + } + let narrow = width <= 40; + match id { + "remote-fast" => apply_remote_fast(state), + "remote-standard" => apply_remote_standard(state), + "org-disabled-fast" => apply_org_disabled_fast(state, narrow), + "plugin-accept-command" => apply_plugin_accept_command(state, narrow), + "command-hash-mismatch" => apply_command_hash_mismatch(state, narrow), + "omit-instructions" => apply_omit_instructions(state, narrow), + _ => return false, + } + true +} + +/// COR-447: a remote session on fast mode. The status line and the composer +/// Fast chip are painted by the real view, not by this scene. +fn apply_remote_fast(state: &mut AppState) { + conversation(state); + state.remote_session = true; + state.fast_mode = FastMode::Fast; + state.fast_mode_policy = FastModePolicy::Allowed; +} + +/// COR-447: a remote session on the default path. No Fast chip. +fn apply_remote_standard(state: &mut AppState) { + conversation(state); + state.remote_session = true; + state.fast_mode = FastMode::Standard; + state.fast_mode_policy = FastModePolicy::Allowed; +} + +/// COR-447: the organization refused `/fast on`. Fail-closed, stays Standard. +fn apply_org_disabled_fast(state: &mut AppState, narrow: bool) { + resumed(state); + state.remote_session = true; + state.fast_mode = FastMode::Standard; + state.fast_mode_policy = FastModePolicy::Disabled; + state.add_message(Message::user("/fast on").with_timestamp("09:26 AM")); + state.add_message(Message::system(format!("! {ORG_DISABLED_TOAST}"))); + let staying = if narrow { + ORG_DISABLED_STAYING.to_string() + } else { + format!("{ORG_DISABLED_STAYING} Nothing was re-sent, and there is no client override.") + }; + state.add_message(Message::system(staying)); +} + +/// The fixture package the install review describes. Parsed through the +/// shipped manifest type so the hash printed in the frame is a real one. +const REVIEW_FIXTURE: &str = r#" +[plugin] +id = "cortex-review" +name = "Cortex Review" +version = "1.2.0" + +[runtime] +kind = "node" +entrypoint = "plugin.mjs" + +[[commands]] +name = "review" +description = "Review the working tree" +usage = "/review [path]" + +[[commands.args]] +name = "path" +required = false +"#; + +/// The reviewed command hash for the fixture, as the CLI would print it. +fn review_fixture_hash() -> String { + let manifest = cortex_engine::plugin::runtime::PluginManifest::parse(REVIEW_FIXTURE) + .expect("review fixture parses"); + cortex_engine::plugin::runtime::command_pin::command_hash(&manifest) +} + +/// COR-448: the `--accept-command` review surface with its real command hash. +fn apply_plugin_accept_command(state: &mut AppState, narrow: bool) { + resumed(state); + let hash = review_fixture_hash(); + state.input.set_text("/plugins install cortex-review"); + state.add_message(Message::system(if narrow { + "Install review — accept the pinned command" + } else { + "Install review — read every command before you accept it." + })); + state.add_message(Message::system(if narrow { + "cortex-review 1.2.0 · 1 command" + } else { + "cortex-review 1.2.0 — registers /review. No hooks, no tools." + })); + // The hash is long; the narrow board keeps the field name and the fact + // that it is a sha256, the wide board prints the whole value. + // The hash is long; the narrow board keeps the field name plus a real + // prefix/suffix, the wide board prints the whole value. + state.add_message(Message::system(if narrow { + format!("command_hash {}…{}", &hash[..4], &hash[hash.len() - 4..]) + } else { + // The value the user pastes back into --accept-command. + format!("command_hash {hash}") + })); + state.add_message(Message::system(if narrow { + "accept-command pins it" + } else { + "Pass it back with --accept-command to pin exactly these commands." + })); +} + +/// COR-448: a manifest that changed after review. Fail-closed. +fn apply_command_hash_mismatch(state: &mut AppState, narrow: bool) { + resumed(state); + state + .input + .set_text("/plugins update cortex-review --accept-command 8f4c2a71"); + state.add_message(Message::user( + "cortex plugin update cortex-review --accept-command 8f4c2a71", + )); + state.add_message(Message::system( + "× Command hash mismatch. Manifest may have changed. Re-run with --json and accept the new hash.", + )); + state.add_message(Message::system(if narrow { + "Nothing was installed." + } else { + "Nothing was installed. The installed cortex-review 1.2.0 stays as it is, and no trust was renewed." + })); +} + +/// COR-449: `omit_instructions` loads, skips, and records — managed policy is +/// never omitted. +fn apply_omit_instructions(state: &mut AppState, narrow: bool) { + resumed(state); + state + .input + .set_text("/agents review --omit-instructions user,project,local,managed"); + state.add_message(Message::system( + "Subagent instructions — managed policy is never omitted.", + )); + state.add_message(Message::system(if narrow { + "loads managed · skips user, project, local" + } else { + "Requested: user, project, local, managed. Loading: managed policy. Skipping: user, project, local." + })); + state.add_message(Message::system(if narrow { + "audit: managed_policy_never_omitted" + } else { + "Audit: managed_policy_never_omitted {source: subagent, loaded: true}" + })); +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::ui::consts::FAST_CHIP_SUFFIX; + + const SIZES: [(u16, u16); 2] = [(120, 40), (40, 12)]; + + /// Collapse wrapped rows and box drawing so copy can be matched. + fn squeezed(plain: &str) -> String { + plain + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c.is_ascii_punctuation() || c.is_whitespace() { + c + } else { + ' ' + } + }) + .collect::() + .split_whitespace() + .collect::>() + .join(" ") + } + + #[test] + fn batch56_ids_are_known_and_validated() { + assert_eq!(BATCH56_IDS.len(), 6); + let mut seen = std::collections::HashSet::new(); + for id in BATCH56_IDS { + assert!(seen.insert(*id), "duplicate {id}"); + assert!(is_batch56_id(id)); + } + assert!(!is_batch56_id("welcome-cortex")); + assert!(validate_batch56_only_ids(&["remote-fast"]).is_ok()); + assert!( + validate_batch56_only_ids(&["remote-fast", "remote-fast"]) + .unwrap_err() + .to_string() + .contains("repeated") + ); + assert!( + validate_batch56_only_ids(&["nope"]) + .unwrap_err() + .to_string() + .contains("unknown") + ); + assert!( + validate_batch56_only_ids(&[]) + .unwrap_err() + .to_string() + .contains("at least one") + ); + } + + #[test] + fn remote_fast_shows_the_status_line_and_the_chip() { + for (width, height) in SIZES { + let frame = render_lock_v2_scene("remote-fast", width, height) + .unwrap_or_else(|e| panic!("remote-fast {width}x{height}: {e}")); + assert!(frame.plain.contains("Remote"), "{width}x{height}"); + assert!( + frame.plain.contains(FAST_CHIP_SUFFIX), + "composer Fast chip at {width}x{height}:\n{}", + frame.plain + ); + if width >= 120 { + assert!( + frame + .plain + .contains(cortex_engine::fast_mode::REMOTE_FAST_STATUS), + "{width}x{height}:\n{}", + frame.plain + ); + } else { + assert!( + frame + .plain + .contains(cortex_engine::fast_mode::REMOTE_FAST_STATUS_NARROW), + "{width}x{height}:\n{}", + frame.plain + ); + } + } + } + + #[test] + fn remote_standard_has_no_fast_chip() { + for (width, height) in SIZES { + let frame = render_lock_v2_scene("remote-standard", width, height) + .unwrap_or_else(|e| panic!("remote-standard {width}x{height}: {e}")); + assert!(frame.plain.contains("Remote"), "{width}x{height}"); + assert!( + !frame.plain.contains(FAST_CHIP_SUFFIX), + "Standard must not paint the chip at {width}x{height}:\n{}", + frame.plain + ); + } + } + + #[test] + fn org_disabled_fast_uses_the_refusal_copy_and_no_chip() { + for (width, height) in SIZES { + let frame = render_lock_v2_scene("org-disabled-fast", width, height) + .unwrap_or_else(|e| panic!("org-disabled-fast {width}x{height}: {e}")); + let plain = squeezed(&frame.plain); + assert!( + plain.contains(ORG_DISABLED_TOAST), + "{width}x{height}:\n{plain}" + ); + assert!( + plain.contains(ORG_DISABLED_STAYING), + "{width}x{height}:\n{plain}" + ); + // The refusal copy names fast mode, so the chip check is the + // composer suffix, not the bare word. + assert!( + !frame.plain.contains(FAST_CHIP_SUFFIX), + "a refused session must not paint the Fast chip:\n{plain}" + ); + assert!( + !plain.contains(cortex_engine::fast_mode::REMOTE_FAST_STATUS), + "a refused session must not paint the fast status line:\n{plain}" + ); + assert!( + plain.contains(cortex_engine::fast_mode::REMOTE_STATUS) + || plain.contains(cortex_engine::fast_mode::REMOTE_STATUS_NARROW), + "a refused session stays on the Standard remote status line:\n{plain}" + ); + } + } + + #[test] + fn plugin_accept_command_carries_a_real_hash_and_the_pin() { + let hash = review_fixture_hash(); + assert_eq!(hash.len(), 64); + let frame = render_lock_v2_scene("plugin-accept-command", 120, 40).unwrap(); + assert!(frame.plain.contains("command_hash"), "{}", frame.plain); + assert!(frame.plain.contains(&hash), "{}", frame.plain); + assert!(frame.plain.contains("accept-command"), "{}", frame.plain); + let narrow = render_lock_v2_scene("plugin-accept-command", 40, 12).unwrap(); + assert!(narrow.plain.contains("command_hash"), "{}", narrow.plain); + // The narrow board keeps a real prefix and suffix of the same hash. + assert!( + narrow.plain.contains(&hash[..4]) && narrow.plain.contains(&hash[hash.len() - 4..]), + "{}", + narrow.plain + ); + assert!(narrow.plain.contains("accept-command"), "{}", narrow.plain); + } + + #[test] + fn command_hash_mismatch_is_the_fail_closed_copy() { + for (width, height) in SIZES { + let frame = render_lock_v2_scene("command-hash-mismatch", width, height) + .unwrap_or_else(|e| panic!("command-hash-mismatch {width}x{height}: {e}")); + let plain = squeezed(&frame.plain); + assert!( + plain.contains( + "Command hash mismatch. Manifest may have changed. Re-run with --json and accept the new hash." + ), + "{width}x{height}:\n{plain}" + ); + assert!( + plain.contains("Nothing was installed"), + "{width}x{height}:\n{plain}" + ); + } + } + + #[test] + fn omit_instructions_keeps_managed_policy_loading() { + for (width, height) in SIZES { + let frame = render_lock_v2_scene("omit-instructions", width, height) + .unwrap_or_else(|e| panic!("omit-instructions {width}x{height}: {e}")); + let plain = squeezed(&frame.plain); + assert!(plain.contains("managed"), "{width}x{height}:\n{plain}"); + assert!( + plain.contains("user") && plain.contains("project") && plain.contains("local"), + "{width}x{height}:\n{plain}" + ); + assert!( + plain.contains("managed_policy_never_omitted") || plain.contains("never omitted"), + "{width}x{height}:\n{plain}" + ); + assert!( + !plain.contains("loads nothing"), + "managed policy always loads:\n{plain}" + ); + } + } + + #[test] + fn every_batch_frame_is_distinct() { + for (width, height) in SIZES { + let mut seen: std::collections::HashMap = + std::collections::HashMap::new(); + for id in BATCH56_IDS { + let frame = render_lock_v2_scene(id, width, height) + .unwrap_or_else(|e| panic!("{id} {width}x{height}: {e}")); + if let Some(prev) = seen.insert(frame.ansi.clone(), id) { + panic!("{id} is identical to {prev} at {width}x{height}"); + } + } + assert_eq!(seen.len(), BATCH56_IDS.len()); + } + } + + #[test] + fn unknown_ids_are_not_applied() { + let mut state = AppState::default(); + assert!(!apply_batch56_scene("welcome-cortex", &mut state, 120)); + assert!(!apply_batch56_scene("session-empty", &mut state, 120)); + } + + #[test] + fn writing_an_invalid_id_set_writes_nothing() { + let dir = std::env::temp_dir().join(format!("cortex-batch56-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let err = write_batch56_frames(&["remote-fast", "nope"], 120, 40, &dir).unwrap_err(); + assert!(err.to_string().contains("unknown"), "{err}"); + assert!(!dir.join("remote-fast.ans").exists()); + assert!(!dir.join("manifest.json").exists()); + } +} diff --git a/src/cortex-tui/src/lock_v2_boards.rs b/src/cortex-tui/src/lock_v2_boards.rs index 1f88c2dc..8d045325 100644 --- a/src/cortex-tui/src/lock_v2_boards.rs +++ b/src/cortex-tui/src/lock_v2_boards.rs @@ -12,6 +12,7 @@ use crate::interactive::builders::{ build_permissions_picker, build_plan_confirm, build_question_prompt, build_sandbox_deny_prompt, }; use crate::lock_v2::PRODUCT_ERROR; +use crate::lock_v2_batch56::apply_batch56_scene; use crate::lock_v2_computer::apply_computer_scene; use crate::lock_v2_cor35::apply_cor35_scene; use crate::lock_v2_designed::apply_designed_scene; @@ -19,12 +20,12 @@ use crate::lock_v2_goal::{apply_goal_chip_scene, show_goal_in_narrow_palette}; use crate::lock_v2_network::apply_offline_rate_limit_scene; use crate::lock_v2_parity::apply_parity_scene; use crate::lock_v2_scenes::*; +use crate::lock_v2_settings::apply_settings_scene; use crate::lock_v2_share::apply_share_scene; use crate::modal::mcp_manager::{McpServerInfo, McpStatus}; use crate::session::SessionSummary; use crate::ui::consts::SERVICE_UNAVAILABLE_NEXT_STEP; use crate::views::tool_call::ToolStatus; -use crate::widgets::settings_modal::SettingsRowKind; pub(crate) fn scene_state(id: &str, width: u16, height: u16) -> AppState { let mut state = lock_app(); @@ -266,47 +267,6 @@ Tell me what you'd like to do.", "model-effort-hover" => { effort_picker(&mut state, crate::interactive::EffortLevel::Medium, true) } - "settings-appearance" => open_settings(&mut state, |_| {}), - "settings-mouse" => open_settings(&mut state, |modal| { - if let Some(i) = modal - .visible_rows() - .iter() - .position(|r| r.id == "mouse_capture") - { - modal.selected = i; - modal.scroll = modal - .visible_rows() - .iter() - .position(|r| r.id == "mouse" || r.label == "Mouse") - .unwrap_or(i.saturating_sub(1)); - } - }), - "settings-row-hover" => open_settings(&mut state, |modal| { - modal.selected = 1; // Compact mode - if let Some(i) = modal - .visible_rows() - .iter() - .position(|r| r.id == "timestamps") - { - modal.hovered = Some(i); - } - }), - "settings-search" => open_settings(&mut state, |modal| { - modal.search = "scro".into(); - modal.search_focused = true; - modal.selected = 0; - if let Some(i) = modal - .visible_rows() - .iter() - .position(|r| r.kind != SettingsRowKind::Category) - { - modal.selected = i; - } - }), - "settings-theme-submenu" => open_settings(&mut state, |modal| { - modal.theme_open = true; - modal.theme_selected = 0; - }), "mode-agent" => { resumed(&mut state); state.agent_mode_label = "Agent".into(); @@ -985,6 +945,7 @@ Tell me what you'd like to do.", None, )); } + id if apply_settings_scene(id, &mut state) => {} id if apply_btw_scene(id, &mut state) => {} id if apply_offline_rate_limit_scene(id, &mut state) => {} id if apply_parity_scene(id, &mut state, width) => {} @@ -993,6 +954,7 @@ Tell me what you'd like to do.", id if apply_goal_chip_scene(id, &mut state) => {} id if apply_computer_scene(id, &mut state, width) => {} id if apply_cor35_scene(id, &mut state, width) => {} + id if apply_batch56_scene(id, &mut state, width) => {} other => panic!("unknown lock v2 scene {other}"), } state diff --git a/src/cortex-tui/src/lock_v2_settings.rs b/src/cortex-tui/src/lock_v2_settings.rs new file mode 100644 index 00000000..5252c96d --- /dev/null +++ b/src/cortex-tui/src/lock_v2_settings.rs @@ -0,0 +1,88 @@ +//! Settings-modal lock v2 scenes. +//! +//! Split out of [`crate::lock_v2_boards`] so that file stays under the +//! source-policy line-count target. Each scene opens the real settings modal +//! and tunes the row the board documents. + +use crate::app::AppState; +use crate::lock_v2_scenes::open_settings; +use crate::widgets::settings_modal::SettingsRowKind; + +/// Apply a settings-modal lock scene. Returns `false` when `id` is not one. +pub(crate) fn apply_settings_scene(id: &str, state: &mut AppState) -> bool { + match id { + "settings-appearance" => open_settings(state, |_| {}), + "settings-mouse" => open_settings(state, |modal| { + if let Some(i) = modal + .visible_rows() + .iter() + .position(|r| r.id == "mouse_capture") + { + modal.selected = i; + modal.scroll = modal + .visible_rows() + .iter() + .position(|r| r.id == "mouse" || r.label == "Mouse") + .unwrap_or(i.saturating_sub(1)); + } + }), + "settings-row-hover" => open_settings(state, |modal| { + modal.selected = 1; // Compact mode + if let Some(i) = modal + .visible_rows() + .iter() + .position(|r| r.id == "timestamps") + { + modal.hovered = Some(i); + } + }), + "settings-search" => open_settings(state, |modal| { + modal.search = "scro".into(); + modal.search_focused = true; + modal.selected = 0; + if let Some(i) = modal + .visible_rows() + .iter() + .position(|r| r.kind != SettingsRowKind::Category) + { + modal.selected = i; + } + }), + "settings-theme-submenu" => open_settings(state, |modal| { + modal.theme_open = true; + modal.theme_selected = 0; + }), + _ => return false, + } + true +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn only_settings_ids_are_claimed() { + for id in [ + "settings-appearance", + "settings-mouse", + "settings-row-hover", + "settings-search", + "settings-theme-submenu", + ] { + let mut state = crate::lock_v2_scenes::lock_app(); + assert!(apply_settings_scene(id, &mut state), "{id}"); + } + let mut state = crate::lock_v2_scenes::lock_app(); + assert!(!apply_settings_scene("welcome-cortex", &mut state)); + } + + #[test] + fn the_search_scene_focuses_the_search_field() { + let mut state = crate::lock_v2_scenes::lock_app(); + assert!(apply_settings_scene("settings-search", &mut state)); + let modal = state.settings_modal.as_ref().expect("settings modal"); + assert_eq!(modal.search, "scro"); + assert!(modal.search_focused); + } +} diff --git a/src/cortex-tui/src/runner/event_loop/commands.rs b/src/cortex-tui/src/runner/event_loop/commands.rs index 3f39f63e..85acb9bd 100644 --- a/src/cortex-tui/src/runner/event_loop/commands.rs +++ b/src/cortex-tui/src/runner/event_loop/commands.rs @@ -145,6 +145,9 @@ impl EventLoop { "shortcuts" => { self.app_state.toggle_shortcuts_sheet(); } + "fast" => { + self.toggle_fast_mode(); + } "auto" => { let is_yolo = matches!( self.app_state.permission_mode, @@ -741,6 +744,7 @@ impl EventLoop { .toasts .info(format!("Permissions: {}", value)); } + "fast" => self.set_fast_mode(value), _ => { self.add_system_message(&format!( "Setting '{key}' is unsupported in this session. No setting was changed." diff --git a/src/cortex-tui/src/runner/event_loop/core.rs b/src/cortex-tui/src/runner/event_loop/core.rs index e1c51f08..6d4044e7 100644 --- a/src/cortex-tui/src/runner/event_loop/core.rs +++ b/src/cortex-tui/src/runner/event_loop/core.rs @@ -198,6 +198,13 @@ impl EventLoop { let (width, height) = app_state.terminal_size; let tui_capture = TuiCapture::new(width, height); + let mut app_state = app_state; + // Remote Code runtimes (cloud, self-hosted SSH) carry the Remote status + // line and the Fast chip. Organization policy decides whether fast mode + // may be turned on; a disabled organization starts on Standard. + app_state.remote_session = cortex_engine::client::ComputerKind::detect().is_remote(); + app_state.fast_mode_policy = cortex_engine::fast_mode::current_policy(); + Self { app_state, session_bridge: None, diff --git a/src/cortex-tui/src/runner/event_loop/fast_mode.rs b/src/cortex-tui/src/runner/event_loop/fast_mode.rs new file mode 100644 index 00000000..4b3c2369 --- /dev/null +++ b/src/cortex-tui/src/runner/event_loop/fast_mode.rs @@ -0,0 +1,55 @@ +//! Fast-mode command handling on [`EventLoop`]. +//! +//! Split out of `event_loop/commands.rs` so the dispatch tables there stay +//! under the source-policy line-count target. The policy check happens here, +//! where the session state lives, and fails closed: when the organization +//! disabled fast mode the session stays on Standard and nothing is re-sent. + +use super::core::EventLoop; +use cortex_engine::fast_mode::FastMode; + +impl EventLoop { + /// Apply a fast-mode request against organization policy. + /// + /// Fail-closed: when the organization disabled fast mode the session stays + /// on Standard, both product toasts are shown, and nothing is re-sent. + pub(super) fn apply_fast_mode(&mut self, requested: FastMode) { + let policy = cortex_engine::fast_mode::current_policy(); + self.app_state.fast_mode_policy = policy; + let outcome = cortex_engine::fast_mode::apply_fast_mode(requested, policy); + if outcome.refused() { + // Fail-closed: the session stays on Standard and nothing is re-sent. + self.app_state.fast_mode = FastMode::Standard; + for toast in outcome.toasts() { + self.app_state.toasts.warning(toast); + } + return; + } + self.app_state.fast_mode = requested; + // Subsequent turns read `app_state.fast_mode` in + // `handle_submit_with_provider` and pass it through `CodeTurnContext`. + for toast in outcome.toasts() { + self.app_state.toasts.info(toast); + } + } + + /// Flip fast mode for the `/fast` toggle form. + pub(super) fn toggle_fast_mode(&mut self) { + let requested = if self.app_state.fast_mode.is_on() { + FastMode::Standard + } else { + FastMode::Fast + }; + self.apply_fast_mode(requested); + } + + /// `/fast ` — reject an unknown token instead of guessing. + pub(super) fn set_fast_mode(&mut self, value: &str) { + match FastMode::parse(value) { + Ok(mode) => self.apply_fast_mode(mode), + Err(error) => { + self.app_state.toasts.error(error.to_string()); + } + } + } +} diff --git a/src/cortex-tui/src/runner/event_loop/mod.rs b/src/cortex-tui/src/runner/event_loop/mod.rs index 39087095..e9f7dce0 100644 --- a/src/cortex-tui/src/runner/event_loop/mod.rs +++ b/src/cortex-tui/src/runner/event_loop/mod.rs @@ -31,6 +31,7 @@ mod auth; mod commands; mod cor35; mod core; +mod fast_mode; mod handoff; mod input; mod local_workflows; diff --git a/src/cortex-tui/src/runner/event_loop/runtime_contract_streaming_tests.rs b/src/cortex-tui/src/runner/event_loop/runtime_contract_streaming_tests.rs index 39e3fd3e..514b962b 100644 --- a/src/cortex-tui/src/runner/event_loop/runtime_contract_streaming_tests.rs +++ b/src/cortex-tui/src/runner/event_loop/runtime_contract_streaming_tests.rs @@ -208,7 +208,7 @@ async fn runtime_contract_forwarder_cancels_silent_initial_request() { #[test] fn first_submit_uses_cloud_as_the_shipped_tui_default() { - let ctx = tui_code_turn_context(false); + let ctx = tui_code_turn_context(false, false); assert_eq!(ctx.computer, ComputerKind::detect()); assert_eq!(ctx.turn_mode, Some(CodeTurnMode::Code)); assert_eq!( @@ -221,7 +221,7 @@ fn first_submit_uses_cloud_as_the_shipped_tui_default() { ComputerKind::ThisPc ); assert_eq!( - tui_code_turn_context(true).turn_mode, + tui_code_turn_context(true, false).turn_mode, Some(CodeTurnMode::Chat) ); } diff --git a/src/cortex-tui/src/runner/event_loop/streaming.rs b/src/cortex-tui/src/runner/event_loop/streaming.rs index 1f5d6c6e..185bafb9 100644 --- a/src/cortex-tui/src/runner/event_loop/streaming.rs +++ b/src/cortex-tui/src/runner/event_loop/streaming.rs @@ -96,7 +96,7 @@ pub(super) fn classify_stream_error(error: &str) -> StreamErrorKind { } /// Shared TUI + exec product rule: Cloud unless This PC/SSH is explicit. -fn tui_code_turn_context(plan_or_spec: bool) -> CodeTurnContext { +fn tui_code_turn_context(plan_or_spec: bool, fast_mode: bool) -> CodeTurnContext { CodeTurnContext { workspace: std::env::current_dir() .ok() @@ -115,6 +115,7 @@ fn tui_code_turn_context(plan_or_spec: bool) -> CodeTurnContext { .ok() .filter(|s| !s.is_empty()) }), + fast_mode, } } @@ -254,7 +255,10 @@ impl EventLoop { if plan_or_spec { cortex_engine::harness::enter_spec_mode(); } - c.configure_code_turn(tui_code_turn_context(plan_or_spec)); + c.configure_code_turn(tui_code_turn_context( + plan_or_spec, + self.app_state.fast_mode.is_on(), + )); } // Create channel for streaming events @@ -761,7 +765,10 @@ impl EventLoop { if let Some(ref c) = client { let plan_or_spec = self.app_state.is_plan_mode() || self.app_state.is_spec_mode(); - c.configure_code_turn(tui_code_turn_context(plan_or_spec)); + c.configure_code_turn(tui_code_turn_context( + plan_or_spec, + self.app_state.fast_mode.is_on(), + )); } // Create channel for streaming events diff --git a/src/cortex-tui/src/ui/consts.rs b/src/cortex-tui/src/ui/consts.rs index e0695595..6aa416c2 100644 --- a/src/cortex-tui/src/ui/consts.rs +++ b/src/cortex-tui/src/ui/consts.rs @@ -72,6 +72,12 @@ pub const SHARE_MARKER: &str = "Shared · read-only"; /// Narrow (40-column) form of the share marker. Keeps the word `Shared`. pub const SHARE_MARKER_NARROW: &str = "Shared"; +/// Fast-mode chip painted next to the model chip while a remote session runs +/// on the low-latency path. +pub const FAST_CHIP: &str = "Fast"; +/// Fast-mode chip text appended to the composer model chip. +pub const FAST_CHIP_SUFFIX: &str = " · Fast"; + /// Sandbox deny title painted in error red. pub const SANDBOX_DENIED_TITLE: &str = "Sandbox denied"; diff --git a/src/cortex-tui/src/views/minimal_session/tests.rs b/src/cortex-tui/src/views/minimal_session/tests.rs index eecb8512..79831fbe 100644 --- a/src/cortex-tui/src/views/minimal_session/tests.rs +++ b/src/cortex-tui/src/views/minimal_session/tests.rs @@ -458,6 +458,95 @@ mod harness_snapshots { ); assert!(!text.to_lowercase().contains("grok")); } + + /// COR-447: a remote session on fast mode carries both the status line and + /// the Fast chip in the composer border. + #[test] + fn snapshot_remote_fast_chip_and_status_line() { + let mut state = AppState::default(); + state.remote_session = true; + state.fast_mode = cortex_engine::fast_mode::FastMode::Fast; + state.fast_mode_policy = cortex_engine::fast_mode::FastModePolicy::Allowed; + + let wide = render(&state, 120, 40); + dump_snapshot("remote-fast-wide", &wide); + assert!( + wide.contains(cortex_engine::fast_mode::REMOTE_FAST_STATUS), + "wide status line missing:\n{wide}" + ); + assert!( + wide.contains(crate::ui::consts::FAST_CHIP), + "wide Fast chip missing:\n{wide}" + ); + + let narrow = render(&state, 40, 12); + dump_snapshot("remote-fast-narrow", &narrow); + assert!( + narrow.contains(cortex_engine::fast_mode::REMOTE_FAST_STATUS_NARROW), + "narrow status line missing:\n{narrow}" + ); + assert!(narrow.contains("Remote"), "narrow:\n{narrow}"); + } + + /// COR-447: fast mode off — or a local session — never paints the chip. + #[test] + fn snapshot_fast_chip_is_absent_off_and_locally() { + let mut standard = AppState::default(); + standard.remote_session = true; + standard.fast_mode = cortex_engine::fast_mode::FastMode::Standard; + let text = render(&standard, 120, 40); + assert!( + text.contains(cortex_engine::fast_mode::REMOTE_STATUS), + "a remote session on Standard still shows Remote:\n{text}" + ); + assert!( + !text.contains(crate::ui::consts::FAST_CHIP_SUFFIX), + "Standard must not paint the Fast chip:\n{text}" + ); + + let mut local = AppState::default(); + local.fast_mode = cortex_engine::fast_mode::FastMode::Fast; + let text = render(&local, 120, 40); + assert!( + !text.contains("Remote"), + "a local session must not show a Remote status line:\n{text}" + ); + assert!( + !text.contains(crate::ui::consts::FAST_CHIP_SUFFIX), + "a local session must not paint the Fast chip:\n{text}" + ); + } + + /// COR-447: the organization-disabled refusal uses the exact product copy + /// and leaves the session on Standard. + #[test] + fn snapshot_org_disabled_fast_mode_copy() { + let mut state = AppState::default(); + state.remote_session = true; + state.fast_mode_policy = cortex_engine::fast_mode::FastModePolicy::Disabled; + state.add_message(cortex_core::widgets::Message::user("/fast on")); + state.add_message(cortex_core::widgets::Message::system( + cortex_engine::fast_mode::ORG_DISABLED_TOAST, + )); + state.add_message(cortex_core::widgets::Message::system( + cortex_engine::fast_mode::ORG_DISABLED_STAYING, + )); + let text = render(&state, 120, 40); + dump_snapshot("fast-org-disabled", &text); + assert!( + text.contains(cortex_engine::fast_mode::ORG_DISABLED_TOAST) + || text.contains("disabled for your organization"), + "refusal copy missing:\n{text}" + ); + assert!( + text.contains("Staying on Standard"), + "the session must stay on Standard:\n{text}" + ); + assert!( + !text.contains(crate::ui::consts::FAST_CHIP_SUFFIX), + "a refused session must not paint the Fast chip:\n{text}" + ); + } } #[cfg(test)] diff --git a/src/cortex-tui/src/views/minimal_session/view.rs b/src/cortex-tui/src/views/minimal_session/view.rs index 9c80f061..1d7a58be 100644 --- a/src/cortex-tui/src/views/minimal_session/view.rs +++ b/src/cortex-tui/src/views/minimal_session/view.rs @@ -160,6 +160,18 @@ impl<'a> MinimalSessionView<'a> { } } + /// Status line for a remote Code session, or `None` for a local session. + /// + /// The Fast chip appears only while fast mode is actually on: an + /// organization-disabled session stays on `Remote · Standard`. + fn remote_status_line(&self) -> Option<&'static str> { + cortex_engine::fast_mode::remote_status_line( + self.app_state.remote_session, + self.app_state.fast_mode.is_on(), + area_is_narrow(self.app_state.terminal_size.0), + ) + } + /// All scrollable content (welcome header + messages) as unified lines. fn content_lines(&self, width: u16, height: u16) -> Vec> { let mut all_lines: Vec> = Vec::new(); @@ -275,6 +287,12 @@ impl<'a> MinimalSessionView<'a> { .as_deref() .unwrap_or("medium"); let chip = model_chip(&self.app_state.model, Some(effort)); + // Fast chip: remote sessions only, and only while fast mode is on. + let chip = if self.app_state.remote_session && self.app_state.fast_mode.is_on() { + format!("{chip}{}", crate::ui::consts::FAST_CHIP_SUFFIX) + } else { + chip + }; let goal_chip = self.app_state.goal.as_ref().map(|g| g.chip()); paint_composer_box( area, @@ -621,6 +639,12 @@ impl<'a> Widget for MinimalSessionView<'a> { .chars() .count() as u16; paint_status_marker(area, area.y, buf, label, counter_cols); + } else if let Some(status) = self.remote_status_line() { + let counter_cols = + format_token_counter(self.app_state.tokens_used, self.app_state.context_window) + .chars() + .count() as u16; + paint_status_marker(area, area.y, buf, status, counter_cols); } let autocomplete_visible = self.app_state.autocomplete.visible; @@ -953,27 +977,5 @@ fn composer_display_text(state: &AppState) -> String { } #[cfg(test)] -mod file_chip_footer_tests { - use super::*; - - #[test] - fn completed_at_path_is_a_file_chip() { - assert!(composer_has_completed_file_chip( - "explain @src/cortex-tui/src/composer.rs " - )); - assert!(composer_has_completed_file_chip( - "explain @src/composer.rs " - )); - assert!(composer_has_completed_file_chip("@src/foo.rs")); - } - - #[test] - fn email_and_bare_at_are_not_file_chips() { - assert!(!composer_has_completed_file_chip("contact me@example.com")); - assert!(!composer_has_completed_file_chip("ada@example.com")); - assert!(!composer_has_completed_file_chip("please inspect @")); - assert!(!composer_has_completed_file_chip("please inspect @src/")); - assert!(!composer_has_completed_file_chip("@")); - assert!(!composer_has_completed_file_chip("hello world")); - } -} +#[path = "view_tests.rs"] +mod file_chip_footer_tests; diff --git a/src/cortex-tui/src/views/minimal_session/view_tests.rs b/src/cortex-tui/src/views/minimal_session/view_tests.rs new file mode 100644 index 00000000..776b4f42 --- /dev/null +++ b/src/cortex-tui/src/views/minimal_session/view_tests.rs @@ -0,0 +1,23 @@ +//! Composer file-chip footer tests. +use super::*; + +#[test] +fn completed_at_path_is_a_file_chip() { + assert!(composer_has_completed_file_chip( + "explain @src/cortex-tui/src/composer.rs " + )); + assert!(composer_has_completed_file_chip( + "explain @src/composer.rs " + )); + assert!(composer_has_completed_file_chip("@src/foo.rs")); +} + +#[test] +fn email_and_bare_at_are_not_file_chips() { + assert!(!composer_has_completed_file_chip("contact me@example.com")); + assert!(!composer_has_completed_file_chip("ada@example.com")); + assert!(!composer_has_completed_file_chip("please inspect @")); + assert!(!composer_has_completed_file_chip("please inspect @src/")); + assert!(!composer_has_completed_file_chip("@")); + assert!(!composer_has_completed_file_chip("hello world")); +}