From da0830aefafc266869cc0ac974d89bf308980d7c Mon Sep 17 00:00:00 2001 From: xavix-yo Date: Sun, 2 Aug 2026 22:20:52 +0100 Subject: [PATCH] feat: an hour-precision clock that says so, and a cron tool that names the real timezone MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The datetime block claimed second precision it never had. It is built once per request and a turn can run for minutes, so `17:54:31` is a lie by the time the model reads it — and the model, having no way to know, wrote cron expressions from it. Rounding was already there but configurable (`round_minutes`, shipped at 60) and justified by the prompt cache. That justification was false: the block is the LAST system message, after the whole conversation, so the cached prefix is identical from turn to turn whatever the timestamp says. Rounding buys nothing for caching today. So the knob goes and the granularity becomes part of the contract: always truncated to the hour, stated in words, with a pointer to `date` for the cases that need the minute. `DatetimeConfig` keeps only `enabled`. - truncation happens in the DISPLAYED zone, not on the UTC epoch: +05:30 zones would otherwise render 20:30 — an hour off and not on an hour boundary, which reads as precise again. - the weekday is spelled out. "next Tuesday" is a far more common ask than the minute, and weekday-from-date is exactly the arithmetic models get wrong. Also fixes a real bug found on the way: `execute_task` told the model, twice, that cron expressions are evaluated in Europe/London — hardcoded, while TaskManager uses the configured timezone. On a non-UK box every scheduled job was written against the wrong clock. The description now names the zone the scheduler actually uses (`TaskManager::timezone_name`), and the assistant's AGENT.md stops repeating the literal. CLAUDE.md: record that the instance is in production. The greenfield licence has expired — schema changes need a versioning mechanism, and per-user SQLCipher files mean it cannot be a boot-time sweep. --- CLAUDE.md | 4 +- agents/assistant/AGENT.md | 2 +- crates/skald-core/src/config.rs | 8 +- crates/skald-core/src/cron/mod.rs | 11 +++ crates/skald-core/src/loop_adapters/system.rs | 94 +++++++++++-------- .../skald-core/src/loop_adapters/testkit.rs | 2 +- crates/skald-core/src/tools/cron_jobs.rs | 32 ++++--- default.config.yaml | 15 ++- 8 files changed, 106 insertions(+), 62 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8d07b13..149c034 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -16,7 +16,7 @@ The design lives in **`blueprint/project-family.md`**. Read it before any archit Load-bearing decisions from that document: - **Not upstreamable.** Nothing here needs to preserve Skald's schema or be portable back to it. -- **Greenfield.** No users in production ⇒ **no migrations, no backwards compatibility**. Tables get restructured, renamed and moved freely; the schema collapses into a single clean baseline v1. +- **~~Greenfield~~ — no longer true. The instance is in production.** There are live users with data we cannot recreate, so the greenfield licence (restructure, rename, wipe, recreate) has expired: **every schema change now needs a versioning mechanism**, and "drop the box and re-run setup" stopped being an acceptable answer. Until that mechanism exists, the only safe change is an additive one through `db::ensure_column` (see the DB section); anything that renames, drops, retypes or moves a column or table is **blocked** on building schema versioning first, not something to do carefully by hand. A user's `{userid}.db` is SQLCipher-encrypted and readable **only while they are logged in**, so a migration cannot be a boot-time sweep over every file — it has to run per user, at unlock, and be idempotent. Design for that when the time comes. - **Dual memory**: a private per-user pool plus a shared pool. A user's private space is encrypted so that nobody else — the admin included — can read it *through normal use of the system*. Never claim "mathematically impossible": the honest promise is transparency plus verifiability (§3). - **Threat model** (§2): the adversary is the **tempted admin**, who owns the box but does not recompile the binary or dump RAM. Do not design against a forensic attacker. - **Roles are data, not enums** (§0.1): a `roles` table binds permission-group, run-context and data-handling attributes. "Children" is a seeded preset row, never a hardcoded type. @@ -132,7 +132,7 @@ The schema is split into two buckets (§5.1), and the split is the point: - **`create_registry_tables`** — instance-wide, readable without any user key: `users`, `roles`, `llm_providers`, `llm_models`, `transcribe_models`, `tts_models`, `image_generate_models`, `plugins`, `plugin_access` + `plugin_user_configs`, `approval_rules`, `tool_permission_groups`, `config`, `known_tools`, `llm_requests`, `mcp_catalog`, `mcp_global_servers` + `mcp_global_access`, `oauth_providers`, `role_capabilities`, `shared_folders` + `shared_folder_members`, `projects` + `project_members`, `supervision`, `system_agent_coverage`. The MCP tables back the Connectors model (§7/§14/§15 — see its own section); `oauth_providers` (accessor `db/oauth_providers.rs`) holds one row per identity provider (Google…) — endpoints + `client_id`/`client_secret` + `redirect_uri`, admin-owned household secrets (§4/§15b), never a per-user token. The last two pairs are junction-backed membership: `shared_folder_members` (accessor `db/shared_folders.rs`) for the on-disk shared folders (§6), `project_members` (accessor `db/project_members.rs`) for projects (see the Projects section) — both let a member be read-only (`can_write`) and both drive the container mount topology + the fs routing. Their FKs are registry→registry (same file), which is allowed — unlike an owner→registry key. - **`create_owner_tables`** — one owner's content, **identical schema in every file that has it**: `chat_sessions`, `chat_sessions_stack`, `chat_history`, `chat_llm_tools`, `chat_summaries`, `session_scratchpad`, `session_mcp_grants`, `stack_mcp_grants`, `scheduled_jobs`, `job_runs`, `system_agent_runs`, `system_agent_state`, `mcp_user_servers`, `mcp_events`, `sources`, `secrets`, `llm_request_payloads`, `memory_docs` (+ FTS5 `memory_docs_fts`), `reports`. `mcp_user_servers` (a user's activated per-user connectors) carries `catalog_name` as a **bare `TEXT` snapshot** of `mcp_catalog.name`, never a FK — an owner→registry key would fail every INSERT; for an OAuth connector it also snapshots `oauth_provider` + `deliver_json`, and its `api_key` column holds the refresh token (in the SQLCipher-encrypted file, so no column crypto). Because `memory_docs` is an owner table, one definition backs **private** memory in each `{userid}.db` and **shared** memory in `system.db` (the household owner) — see the memory namespace note below. (`projects`/`project_tickets` were owner tables in the single-user past: projects are shareable now, so `projects` + `project_members` are registry tables and `project_tickets` is gone.) -Schema is greenfield (no migrations, §0), but a purely **additive** column lands on an existing DB in place: `db::ensure_column` runs `ALTER TABLE … ADD COLUMN` and swallows the "duplicate column" error, a no-op on a fresh DB where the `CREATE TABLE` already has the column. Used for the OAuth columns on `mcp_catalog` / `mcp_user_servers` so a dev box need not be wiped for an additive change (a full recreate is still valid). +**The schema is no longer greenfield** (see the production note at the top): a full recreate is not an option anymore. `db::ensure_column` — `ALTER TABLE … ADD COLUMN` swallowing the "duplicate column" error, a no-op on a fresh DB where the `CREATE TABLE` already carries it — is therefore not a convenience for dev boxes anymore but the **only** change shape that is currently safe, and additive-with-a-default is the shape to design towards. Used for the OAuth columns on `mcp_catalog` / `mcp_user_servers`. Anything destructive waits for real versioning. **No foreign key in the owner bucket may point at a registry table.** SQLite cannot enforce a key across files, not even through `ATTACH`, and sqlx turns on `PRAGMA foreign_keys`: the `CREATE TABLE` succeeds and every `INSERT` fails. `db::tests::owner_tables_stand_alone_with_foreign_keys_on` enforces this by running the owner schema against a database holding nothing else, then inserting a row into each table. One key crossed and was fixed: `chat_history.model_db_id` (dropped — write-only, and `llm_requests.model_name` already records the model). diff --git a/agents/assistant/AGENT.md b/agents/assistant/AGENT.md index cbe75bf..1c5a15c 100644 --- a/agents/assistant/AGENT.md +++ b/agents/assistant/AGENT.md @@ -58,7 +58,7 @@ Rules of thumb: - **`mode=async`** — **the default for anything non-trivial.** It launches without blocking you, so you keep talking to the user while it runs. When it finishes, the system injects the result as a synthetic `task_completed` tool call — react to it and relay the outcome. After launching, tell the user it is running, then **do not poll** — the result arrives on its own. - **`mode=sync`** — run now and block for the answer. Only for **short** sub-tasks whose result you need immediately to finish composing your current reply. -- **`mode=cron`** — schedule a recurring or one-shot task (7-field cron expression, `Europe/London`). The result arrives as a notification. +- **`mode=cron`** — schedule a recurring or one-shot task (7-field cron expression; the tool description names the timezone it is evaluated in). The result arrives as a notification. ## Notifications diff --git a/crates/skald-core/src/config.rs b/crates/skald-core/src/config.rs index 7774a74..26a4f32 100644 --- a/crates/skald-core/src/config.rs +++ b/crates/skald-core/src/config.rs @@ -39,13 +39,15 @@ pub struct LlmConfig { } /// Controls date/time injection in the dynamic tail of each LLM request. +/// +/// The injected time is **always** truncated to the hour, and the block says so: +/// see [`crate::loop_adapters::system`]. There is deliberately no rounding knob — +/// the granularity is part of what the model is told, not an instance setting. #[derive(Debug, Clone, Deserialize)] pub struct DatetimeConfig { /// Inject the current date/time into the LLM context. Default: true. #[serde(default = "default_true")] pub enabled: bool, - /// When set, round the injected time down to the nearest N-minute boundary. - pub round_minutes: Option, /// IANA timezone name to use when formatting the injected timestamp. /// Populated at startup from the global `timezone` config field. #[serde(skip)] @@ -54,7 +56,7 @@ pub struct DatetimeConfig { impl Default for DatetimeConfig { fn default() -> Self { - Self { enabled: true, round_minutes: None, timezone: None } + Self { enabled: true, timezone: None } } } diff --git a/crates/skald-core/src/cron/mod.rs b/crates/skald-core/src/cron/mod.rs index 0049f4b..732e0f8 100644 --- a/crates/skald-core/src/cron/mod.rs +++ b/crates/skald-core/src/cron/mod.rs @@ -57,6 +57,17 @@ impl TaskManager { }) } + /// The zone cron expressions are evaluated in — the configured `timezone`, + /// else the system's. Exists so the `execute_task` tool description can name + /// it instead of hardcoding one: a model told the wrong zone writes a + /// correct-looking expression that fires at the wrong hour. + pub fn timezone_name(&self) -> String { + self.tz + .map(|tz| tz.name().to_string()) + .or_else(|| iana_time_zone::get_timezone().ok()) + .unwrap_or_else(|| "the server's local timezone".to_string()) + } + /// Called once after ChatSessionManager is built, breaking the circular dep. pub fn set_session(&self, session: Arc) { let _ = self.session.set(session); diff --git a/crates/skald-core/src/loop_adapters/system.rs b/crates/skald-core/src/loop_adapters/system.rs index 2028254..3ac20df 100644 --- a/crates/skald-core/src/loop_adapters/system.rs +++ b/crates/skald-core/src/loop_adapters/system.rs @@ -143,6 +143,18 @@ fn os_description() -> &'static str { OS.get_or_init(|| os_info::get().to_string()) } +/// Formats an instant to hour precision: `Sunday 2026-08-02 17:00 +02:00`. +/// +/// Minutes and seconds are dropped by the format string itself, so the +/// truncation always happens in the zone being displayed. The weekday is part +/// of the format on purpose — see [`AgentSystemContext::datetime_block`]. +fn render_hour(dt: chrono::DateTime) -> String +where + Tz::Offset: std::fmt::Display, +{ + dt.format("%A %Y-%m-%d %H:00 %:z").to_string() +} + /// System IANA timezone name, computed once. fn system_timezone() -> Option<&'static str> { static TZ: std::sync::OnceLock> = std::sync::OnceLock::new(); @@ -169,23 +181,25 @@ impl AgentSystemContext { /// The current date/time + OS + cwd block (`None` when disabled). /// - /// Rounding exists for the prompt cache: a timestamp that changes every - /// second would invalidate any cached suffix, so the instance can quantize - /// it (this block is in the dynamic tail, after the cached prefix, but the - /// rounding still helps providers that cache further). + /// The time is **truncated to the hour**, always, and the block says so in + /// words. Two reasons, neither of which is the prompt cache — this block is + /// the last system message, after the whole conversation, so the cached + /// prefix is identical from one turn to the next whatever the timestamp says: + /// + /// 1. **Honesty.** A second-precision timestamp reads as exact to the model + /// long after it stopped being true (it is built once per request, and a + /// turn can run for minutes). An hour-precision one that announces itself + /// as such lets the model know what it does *not* know — which matters + /// when it is about to write a cron expression from "in ten minutes". + /// 2. It keeps the block cache-safe if it ever moves into the prefix. + /// + /// The weekday is spelled out: "next Tuesday" is a far more common ask than + /// the minute, and deriving it from a date is exactly the arithmetic models + /// get wrong. fn datetime_block(&self) -> Option { if !self.datetime.enabled { return None; } - let secs = chrono::Utc::now().timestamp(); - let secs = match self.datetime.round_minutes { - Some(m) if m > 0 => { - let bucket = (m as i64) * 60; - (secs / bucket) * bucket - } - _ => secs, - }; - let tz = self .datetime .timezone @@ -193,30 +207,15 @@ impl AgentSystemContext { .and_then(|s| s.parse::().ok()) .or_else(|| system_timezone().and_then(|s| s.parse::().ok())); + // Truncate in the *displayed* zone, not on the UTC epoch: a zone at a + // 30- or 45-minute offset (Asia/Kolkata, Asia/Kathmandu) would otherwise + // render as `17:30`, which is not an hour boundary and reads as precise. let (formatted, tz_name) = match tz { - Some(tz) => { - use chrono::TimeZone as _; - let f = tz - .timestamp_opt(secs, 0) - .single() - .map(|dt| dt.format("%Y-%m-%dT%H:%M:%S%:z").to_string()) - .unwrap_or_else(|| { - chrono::Local::now().format("%Y-%m-%dT%H:%M:%S%:z").to_string() - }); - (f, Some(tz.name().to_string())) - } - None => { - let f = chrono::DateTime::from_timestamp(secs, 0) - .map(|utc| { - utc.with_timezone(&chrono::Local) - .format("%Y-%m-%dT%H:%M:%S%:z") - .to_string() - }) - .unwrap_or_else(|| { - chrono::Local::now().format("%Y-%m-%dT%H:%M:%S%:z").to_string() - }); - (f, None) - } + Some(tz) => ( + render_hour(chrono::Utc::now().with_timezone(&tz)), + Some(tz.name().to_string()), + ), + None => (render_hour(chrono::Local::now()), None), }; let date_line = match tz_name { Some(name) => format!("Current date and time: {formatted} ({name})"), @@ -227,7 +226,11 @@ impl AgentSystemContext { let cwd = "~"; Some(format!( - "{date_line}\nOperating system: {}\nWorking directory: {cwd}\n\ + "{date_line}\n\ + The time above is truncated to the hour — you do not know the current minute. \ + If you need it exactly (for instance to schedule something within the hour), \ + run `date` with execute_cmd first.\n\ + Operating system: {}\nWorking directory: {cwd}\n\ Filesystem tools and execute_cmd resolve relative paths against your home directory.", os_description() )) @@ -505,6 +508,23 @@ mod tests { assert_eq!(out, input); } + #[test] + fn hour_render_names_the_day_and_drops_the_minutes() { + let instant = chrono::DateTime::parse_from_rfc3339("2026-08-02T15:54:31Z").unwrap(); + let rome = instant.with_timezone(&chrono_tz::Europe::Rome); + assert_eq!(render_hour(rome), "Sunday 2026-08-02 17:00 +02:00"); + } + + #[test] + fn hour_render_truncates_in_the_displayed_zone() { + // Asia/Kolkata is +05:30: truncating the UTC epoch instead would render + // 20:30 — an hour off AND not on an hour boundary, so it would read as + // a precise time. Truncation must happen after the zone conversion. + let instant = chrono::DateTime::parse_from_rfc3339("2026-08-02T15:54:31Z").unwrap(); + let kolkata = instant.with_timezone(&chrono_tz::Asia::Kolkata); + assert_eq!(render_hour(kolkata), "Sunday 2026-08-02 21:00 +05:30"); + } + #[test] fn shared_folders_table_renders_access_and_description() { use crate::db::shared_folders::SharedFolderAccess; diff --git a/crates/skald-core/src/loop_adapters/testkit.rs b/crates/skald-core/src/loop_adapters/testkit.rs index 96c0761..7f25969 100644 --- a/crates/skald-core/src/loop_adapters/testkit.rs +++ b/crates/skald-core/src/loop_adapters/testkit.rs @@ -181,7 +181,7 @@ pub fn mcp() -> Arc { /// The datetime block is disabled: it embeds `now()`, which no snapshot can /// pin down. pub fn datetime() -> DatetimeConfig { - DatetimeConfig { enabled: false, round_minutes: None, timezone: None } + DatetimeConfig { enabled: false, timezone: None } } /// The base tool definitions the projection is handed. diff --git a/crates/skald-core/src/tools/cron_jobs.rs b/crates/skald-core/src/tools/cron_jobs.rs index 5088972..df8e482 100644 --- a/crates/skald-core/src/tools/cron_jobs.rs +++ b/crates/skald-core/src/tools/cron_jobs.rs @@ -18,18 +18,23 @@ use crate::tools::{SimpleExecution, Tool, ToolContext, ToolDescriptionLength, To pub struct ExecuteTask(pub Arc); impl ExecuteTask { - fn description_text() -> &'static str { - "Create and run a task. Three modes:\n\ - • mode=cron — scheduled by a 7-field cron expression (sec min hour dom month dow year, \ - Europe/London timezone). Returns task_id and next scheduled run. Recurring unless the \ - expression can only fire once.\n\ - • mode=sync — run immediately, block until the agent finishes, and return the result inline. \ - Best for short tasks (a few seconds to a few minutes).\n\ - • mode=async — start the task in the background and return the task_id immediately. \ - When the task completes its result will be delivered back to this chat automatically." + /// `tz` is the zone the scheduler actually evaluates expressions in + /// (`TaskManager::timezone_name`), never a literal: the description is what + /// the model reasons from, so a wrong zone here is an hours-off cron job. + fn description_text(tz: &str) -> String { + format!( + "Create and run a task. Three modes:\n\ + • mode=cron — scheduled by a 7-field cron expression (sec min hour dom month dow year, \ + {tz} timezone). Returns task_id and next scheduled run. Recurring unless the \ + expression can only fire once.\n\ + • mode=sync — run immediately, block until the agent finishes, and return the result inline. \ + Best for short tasks (a few seconds to a few minutes).\n\ + • mode=async — start the task in the background and return the task_id immediately. \ + When the task completes its result will be delivered back to this chat automatically." + ) } - fn schema() -> Value { + fn schema(tz: &str) -> Value { json!({ "type": "object", "required": ["mode", "title", "prompt", "agent_id"], @@ -41,7 +46,7 @@ impl ExecuteTask { }, "title": { "type": "string", "description": "Short name for this task" }, "description": { "type": "string", "description": "What this task does" }, - "cron": { "type": "string", "description": "7-field cron expression — required when mode=cron (times in Europe/London). E.g. '0 0 9 * * * *' = every day at 09:00" }, + "cron": { "type": "string", "description": format!("7-field cron expression — required when mode=cron (times in {tz}). E.g. '0 0 9 * * * *' = every day at 09:00") }, "prompt": { "type": "string", "description": "Prompt sent to the agent at each run" }, "agent_id": { "type": "string", "description": "Task agent to run (required; e.g. software-engineer, researcher, generalist). Must be a `task` agent — chat/system agents are rejected." } } @@ -107,6 +112,7 @@ pub fn build_execute_task_interface_tool( ) -> crate::session::handler::InterfaceTool { use crate::session::handler::{InterfaceTool, ToolFuture}; + let tz = task_mgr.timezone_name(); let tool = Arc::new(ExecuteTask(task_mgr)); InterfaceTool { @@ -114,8 +120,8 @@ pub fn build_execute_task_interface_tool( "type": "function", "function": { "name": "execute_task", - "description": ExecuteTask::description_text(), - "parameters": ExecuteTask::schema(), + "description": ExecuteTask::description_text(&tz), + "parameters": ExecuteTask::schema(&tz), } }), handler: Arc::new(move |args: Value| -> ToolFuture { diff --git a/default.config.yaml b/default.config.yaml index b923c17..87384e9 100644 --- a/default.config.yaml +++ b/default.config.yaml @@ -60,9 +60,12 @@ llm: # execute_subtask) in a single response. Bounds fan-out to avoid provider # rate-limit storms. Omit for the default (4); set to 1 to force sequential. max_parallel_subagents: 4 + # Injects "Current date and time: Sunday 2026-08-02 17:00 +02:00 (Europe/Rome)" + # as the last system message of each request. The time is always truncated to + # the hour and the block tells the model so, pointing it at `date` when it + # needs the exact minute — there is no rounding setting. datetime: enabled: true - round_minutes: 60 # Help with KV cache (instead of 10:54, it will pass 10:50 to the LLM) # ── Tool result size limit ────────────────────────────────────────────────── # When set, tool results from *previous* turns that exceed this character @@ -109,11 +112,13 @@ llm: # ─────────────────────────────────────────────────────────────────────────── # ── Date/time injection ───────────────────────────────────────────────────── - # Configured above as `datetime`. Rounding keeps the injected timestamp stable - # for up to N minutes, so the dynamic tail can be KV-cached across requests - # instead of changing every second. + # Configured above as `datetime`. The only setting is `enabled`. + # The injected time is always truncated to the hour: not for the prompt cache + # (the block is the LAST system message, so the cached prefix never changes + # whatever the timestamp says) but because a second-precision stamp reads as + # exact to the model long after it stopped being true. The block states the + # granularity and tells the agent to run `date` when it needs the minute. # enabled: true # set to false to disable injection entirely - # round_minutes: 10 # round down to nearest N minutes (e.g. 10:56 → 10:50) # ─────────────────────────────────────────────────────────────────────────── # ── LLM request/response log ────────────────────────────────────────────────