Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion clients/subc-client/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@cortexkit/subc-client",
"version": "0.5.0",
"version": "0.6.0",
"description": "TypeScript client for the subc daemon. Wire-compatible (byte-for-byte) with the Rust subc-transport handshake and subc-protocol envelope.",
"type": "module",
"exports": {
Expand Down
5 changes: 0 additions & 5 deletions clients/subc-client/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,19 +70,16 @@ export {
type BindDecision,
type Principal,
type BindingsInput,
type CircuitBreakerInput,
type Concurrency,
type ConsumerRoleInput,
type ExecutionMode,
type IdentityBindingInput,
type IdentityScope,
type InternalTransport,
type LeaseScope,
type ManagementOperationInput,
type ManagementOperationKind,
type ManagementSurfaceManifestOptions,
type ManifestInput,
type ModelPolicyInput,
type ModuleHelloAckBody,
type ObservabilityKind,
type ObservabilitySurfaceInput,
Expand All @@ -95,12 +92,10 @@ export {
type ProviderHealthHandler,
type ProviderRoleInput,
type RouteBindRequest,
type ScheduledTaskInput,
type StorageBindingInput,
type StorageKind,
type StorageScope,
type SubcProviderConnectOptions,
type TaskEligibilityInput,
type ToolInput,
type TrustTier,
type VaultGrantInput,
Expand Down
49 changes: 0 additions & 49 deletions clients/subc-client/src/provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export type ObservabilityKind = "snapshot" | "stream";
export type InternalTransport = "bulk";
export type StorageKind = "sqlite";
export type StorageScope = "project";
export type LeaseScope = "project";
export type HealthStatus = "ok" | "degraded" | "failing";

export interface HealthReport {
Expand All @@ -70,7 +69,6 @@ export interface ManifestInput {
trust_tier: TrustTier;
provides: ProviderRoleInput[];
consumes: ConsumerRoleInput[];
scheduled_tasks: ScheduledTaskInput[];
bindings: BindingsInput;
}

Expand Down Expand Up @@ -134,31 +132,6 @@ export type ConsumerRoleInput =
| { role: "llm_client"; via: string; auth: string }
| { role: "service_client"; of: string[] };

export interface ScheduledTaskInput {
task_id: string;
eligibility: TaskEligibilityInput;
lease_scope: LeaseScope;
renews_during_calls: boolean;
toolset: string[];
model_policy: ModelPolicyInput;
step_cap: number;
circuit_breaker: CircuitBreakerInput;
}

export interface TaskEligibilityInput {
cooldown: string;
window: string;
}

export interface ModelPolicyInput {
tier: string;
fallback_chain: string[];
}

export interface CircuitBreakerInput {
identical_failures: number;
}

export interface BindingsInput {
storage: StorageBindingInput;
vault_grants: VaultGrantInput[];
Expand Down Expand Up @@ -402,7 +375,6 @@ export function managementSurfaceManifest(opts: ManagementSurfaceManifestOptions
},
],
consumes: [],
scheduled_tasks: [],
bindings: {
storage: {
kind: "sqlite",
Expand Down Expand Up @@ -1403,7 +1375,6 @@ function normalizeManifest(manifest: ManifestInput): ManifestInput {
trust_tier: manifest.trust_tier,
provides: manifest.provides.map(normalizeProviderRole),
consumes: manifest.consumes.map(normalizeConsumerRole),
scheduled_tasks: manifest.scheduled_tasks.map(normalizeScheduledTask),
bindings: {
storage: {
kind: manifest.bindings.storage.kind,
Expand Down Expand Up @@ -1487,23 +1458,3 @@ function normalizeConsumerRole(role: ConsumerRoleInput): ConsumerRoleInput {
}
}

function normalizeScheduledTask(task: ScheduledTaskInput): ScheduledTaskInput {
return {
task_id: task.task_id,
eligibility: {
cooldown: task.eligibility.cooldown,
window: task.eligibility.window,
},
lease_scope: task.lease_scope,
renews_during_calls: task.renews_during_calls,
toolset: [...task.toolset],
model_policy: {
tier: task.model_policy.tier,
fallback_chain: [...task.model_policy.fallback_chain],
},
step_cap: task.step_cap,
circuit_breaker: {
identical_failures: task.circuit_breaker.identical_failures,
},
};
}
1 change: 0 additions & 1 deletion clients/subc-client/tests/provider.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ describe("managementSurfaceManifest", () => {
},
],
consumes: [],
scheduled_tasks: [],
bindings: {
storage: { kind: "sqlite", scope: "project", owns_schema: false },
vault_grants: [],
Expand Down
2 changes: 1 addition & 1 deletion crates/subc-client-rs/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ async-trait = "0.1"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
subc-control = { path = "../subc-control", version = "0.1" }
subc-protocol = { path = "../subc-protocol", version = "0.10" }
subc-protocol = { path = "../subc-protocol", version = "0.11" }
subc-transport = { path = "../subc-transport", version = "0.5" }
tokio = { version = "1", features = ["io-util", "macros", "net", "rt", "sync", "time"] }
tokio-util = { version = "0.7", features = ["rt"] }
Expand Down
1 change: 0 additions & 1 deletion crates/subc-client-rs/examples/echo-module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,6 @@ fn manifest(module_id: &str) -> subc_protocol::manifest::ModuleManifest {
sub_supervises: true,
}],
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
1 change: 0 additions & 1 deletion crates/subc-client-rs/tests/real_daemon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1296,7 +1296,6 @@ fn inline_module_manifest(module_id: &str, tool_names: &[&str]) -> ModuleManifes
trust_tier: TrustTier::FirstParty,
provides: vec![tool_provider_role(tool_names)],
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
2 changes: 1 addition & 1 deletion crates/subc-control/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ repository = "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/cortexkit/subconscious"
[dependencies]
serde = { version = "1", features = ["derive"] }
serde_json = "1"
subc-protocol = { path = "../subc-protocol", version = "0.10" }
subc-protocol = { path = "../subc-protocol", version = "0.11" }
2 changes: 1 addition & 1 deletion crates/subc-core/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "subc-core"
version = "0.3.0"
version = "0.3.1"
edition = "2021"
publish = false
description = "subc daemon core: loopback TCP transport and opaque-byte splice router."
Expand Down
1 change: 0 additions & 1 deletion crates/subc-core/src/bench_harness.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ pub fn bench_tool_provider_manifest(module_id: &str) -> ModuleManifest {
sub_supervises: false,
}],
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
1 change: 0 additions & 1 deletion crates/subc-core/src/bin/fake-aft-stub.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,6 @@ fn manifest(
trust_tier: TrustTier::FirstParty,
provides: vec![provider_role(role, concurrency, tools)],
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
23 changes: 2 additions & 21 deletions crates/subc-core/src/control.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2761,9 +2761,8 @@ mod tests {
use serde_json::{json, Value};
use subc_protocol::{
manifest::{
Bindings, CircuitBreaker, Concurrency, ExecutionMode, IdentityBinding, IdentityScope,
ModelPolicy, ProviderRole, ScheduledTask, StorageBinding, StorageKind, StorageScope,
TaskEligibility, Tool,
Bindings, Concurrency, ExecutionMode, IdentityBinding, IdentityScope, ProviderRole,
StorageBinding, StorageKind, StorageScope, Tool,
},
session::HealthStatus,
FrameType,
Expand Down Expand Up @@ -2963,24 +2962,6 @@ mod tests {
sub_supervises: true,
}],
consumes: Vec::new(),
scheduled_tasks: vec![ScheduledTask {
task_id: "aft.dreamer".to_string(),
eligibility: TaskEligibility {
cooldown: "1h".to_string(),
window: "always".to_string(),
},
lease_scope: subc_protocol::manifest::LeaseScope::Project,
renews_during_calls: true,
toolset: vec!["read".to_string()],
model_policy: ModelPolicy {
tier: "cheap".to_string(),
fallback_chain: vec!["fallback".to_string()],
},
step_cap: 10,
circuit_breaker: CircuitBreaker {
identical_failures: 3,
},
}],
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
1 change: 0 additions & 1 deletion crates/subc-core/tests/catalog_update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,7 +417,6 @@ fn supervision_only_manifest(module_id: &str) -> ModuleManifest {
trust_tier: TrustTier::FirstParty,
provides: Vec::new(),
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
1 change: 0 additions & 1 deletion crates/subc-core/tests/forwarding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4540,7 +4540,6 @@ fn consumer_manifest(module_id: &str) -> ModuleManifest {
trust_tier: TrustTier::FirstParty,
provides: Vec::new(),
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
1 change: 0 additions & 1 deletion crates/subc-core/tests/reverse_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1028,7 +1028,6 @@ fn tool_provider_manifest(module_id: &str, concurrency: Concurrency) -> ModuleMa
sub_supervises: true,
}],
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
1 change: 0 additions & 1 deletion crates/subc-mcp/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1921,7 +1921,6 @@ fn supervision_manifest(module_id: String) -> ModuleManifest {
trust_tier: TrustTier::FirstParty,
provides: Vec::new(),
consumes: vec![ConsumerRole::ToolClient { of: Vec::new() }],
scheduled_tasks: Vec::new(),
bindings: supervision_bindings(),
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/subc-mcp/tests/phase1_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4597,7 +4597,6 @@ fn raw_provider_manifest(module_id: &str, tool_name: &str) -> ModuleManifest {
sub_supervises: true,
}],
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
2 changes: 1 addition & 1 deletion crates/subc-protocol/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "subc-protocol"
version = "0.10.0"
version = "0.11.0"
edition = "2021"
publish = true
description = "Shared wire contract for subc <-> modules: the 17-byte envelope, the Frame (header + opaque body), channel-0 control bodies, route.bind/RouteTarget session shapes, and the capability manifest. Single source of truth, depended on by subc-core and AFT."
Expand Down
56 changes: 0 additions & 56 deletions crates/subc-protocol/src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ pub struct ModuleManifest {
pub trust_tier: TrustTier,
pub provides: Vec<ProviderRole>,
pub consumes: Vec<ConsumerRole>,
pub scheduled_tasks: Vec<ScheduledTask>,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1: When a new module sends HELLO to an older daemon, registration fails because the older required scheduled_tasks field is now omitted. Coordinate this wire-shape change with a protocol/versioned rollout, or continue emitting the field until older daemons are retired; deny_unknown_fields does not provide backwards compatibility for missing required fields.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At crates/subc-protocol/src/manifest.rs, line 21:

<comment>When a new module sends HELLO to an older daemon, registration fails because the older required `scheduled_tasks` field is now omitted. Coordinate this wire-shape change with a protocol/versioned rollout, or continue emitting the field until older daemons are retired; `deny_unknown_fields` does not provide backwards compatibility for missing required fields.</comment>

<file context>
@@ -18,7 +18,6 @@ pub struct ModuleManifest {
     pub provides: Vec<ProviderRole>,
     pub consumes: Vec<ConsumerRole>,
-    pub scheduled_tasks: Vec<ScheduledTask>,
     pub bindings: Bindings,
 }
 
</file context>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Valid, and my compatibility claim in the PR body is wrong. I asserted "compatible both directions" and only tested one of them.

Reproduced the failing direction rather than reasoning about it further:

#[derive(serde::Deserialize)]
struct OldManifest { module_id: String, scheduled_tasks: Vec<String> }

serde_json::from_str::<OldManifest>(r#"{"module_id":"aft"}"#)
// → Error: missing field `scheduled_tasks` at line 1 column 19

ModuleManifest at master carries no #[serde(default)] on that field, so it is genuinely required on deserialize. deny_unknown_fields governs extra keys and says nothing about missing ones. I conflated the two.

The directions are not symmetric:

old daemon new daemon
old module sends the field parses parses — extra key ignored
new module omits it FAILS — missing required field parses

The safe direction is old-module→new-daemon, which is the one I checked and the one my deny_unknown_fields reasoning actually covers. The unsafe one is new-module→old-daemon — exactly the deployment order that happens in practice, since clients upgrade independently of the daemon.

This box demonstrates the hazard: my daemon ran a build 623 commits stale until this afternoon while modules were rebuilt against a much newer tree. A skew window measured in days.

Options, and the call is yours

Cubic's suggestion — keep emitting the field — defeats the purpose, since authors would still have to declare it.

I also can't fix this from the new side. The old daemon is the one that needs the key present, and no change to a new module makes an already-shipped binary tolerate its absence. So:

  1. Land as-is with a hard ordering constraint — daemon upgrades before any module rebuilt against the new manifest. ck daemon's build_git_sha makes that checkable rather than hoped-for, and it's a one-time cost on a fleet you control.
  2. Add #[serde(default)] to the field on master first, ship it, then excise in a later release. The intermediate daemon tolerates both shapes and the ordering constraint disappears.

Option 2 is the correct protocol move and costs one release cycle. Option 1 is defensible only because every consumer is in your fleet.

If you want option 2, this PR should probably become the #[serde(default)] commit and the excision waits behind it. Say which and I'll reshape it.

Worth naming the general shape, since it bit me twice today: I checked the direction that confirmed my conclusion and stopped. Same error as my MASKED verdict on #11 — one explanation enumerated, the other never asked about.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parent comment still applies: your reproduction confirms that a new module omitting required scheduled_tasks cannot register with an old daemon, while the reverse direction remains safe. I’d choose option 2—ship a #[serde(default)] compatibility release before excising the field—because the demonstrated multi-day skew makes daemon-first ordering fragile. deny_unknown_fields only addresses extra keys, and continuing to emit the field would not achieve the intended removal.

Thanks for the feedback! I've saved this as a new learning.

pub bindings: Bindings,
}

Expand Down Expand Up @@ -185,60 +184,6 @@ pub enum ConsumerRole {
ServiceClient { of: Vec<String> },
}

/// Scheduler-owned task declaration. The runner module executes the loop; subc
/// owns eligibility checks and the lease.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct ScheduledTask {
pub task_id: String,
pub eligibility: TaskEligibility,
pub lease_scope: LeaseScope,
pub renews_during_calls: bool,
pub toolset: Vec<String>,
pub model_policy: ModelPolicy,
pub step_cap: u32,
pub circuit_breaker: CircuitBreaker,
}

/// Time/window gates for a scheduled task. Values are serialized policy strings
/// (for example, durations or cron/window expressions) owned by the scheduler.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct TaskEligibility {
pub cooldown: String,
pub window: String,
}

/// Scope at which subc enforces one active scheduler lease.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
#[serde(rename_all = "snake_case")]
pub enum LeaseScope {
Project,
}

/// Model selection policy for the LLM-runner that executes a scheduled task.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct ModelPolicy {
pub tier: String,
pub fallback_chain: Vec<String>,
}

/// Declared trip threshold for a scheduled task's circuit breaker: stop after this
/// many IDENTICAL consecutive failures.
///
/// SCOPE, because the name invites a wider reading than the field supports. The
/// alarm condition here is "this failure looks like the last one", so it detects a
/// task stuck failing the SAME way and is silent on a task failing MANY DIFFERENT
/// ways -- and it goes quieter the more varied the failures become, which is often
/// the more alarming case. A module treating this as its only stop condition will
/// find it mutest during the messiest outage. Pair it with a signal that counts
/// failures regardless of their kind.
///
/// The daemon carries this field and does not act on it: enforcement belongs to the
/// module running the task, since only it can compare two failures for identity.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct CircuitBreaker {
pub identical_failures: u32,
}

/// External storage, vault, and identity bindings supplied through subc.
#[derive(Serialize, Deserialize, Debug, Clone, PartialEq)]
pub struct Bindings {
Expand Down Expand Up @@ -343,7 +288,6 @@ mod tests {
consumes: vec![ConsumerRole::ServiceClient {
of: vec!["embedding.v2".to_string()],
}],
scheduled_tasks: vec![],
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
1 change: 0 additions & 1 deletion crates/subc-protocol/tests/golden/module_hello_body.json
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
]
}
],
"scheduled_tasks": [],
"trust_tier": "first_party"
},
"protocol_ver": 2
Expand Down
1 change: 0 additions & 1 deletion crates/subc-protocol/tests/golden_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,6 @@ fn module_manifest(module_id: &str) -> ModuleManifest {
trust_tier: TrustTier::FirstParty,
provides: provider_roles(),
consumes: Vec::new(),
scheduled_tasks: Vec::new(),
bindings: Bindings {
storage: StorageBinding {
kind: StorageKind::Sqlite,
Expand Down
2 changes: 1 addition & 1 deletion crates/subc-transport/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ hmac = "0.12"
serde = { version = "1", features = ["derive"] }
serde_json = "1"
sha2 = "0.10"
subc-protocol = { path = "../subc-protocol", version = "0.10" }
subc-protocol = { path = "../subc-protocol", version = "0.11" }
subtle = "2"
tokio = { version = "1", features = ["io-util", "time"] }

Expand Down