diff --git a/CLAUDE.md b/CLAUDE.md index dc91036..db8bfad 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -394,7 +394,9 @@ Same economics, other end of the request. `AgentSystemContext::system_context` i The refresh rule is the only one that is free: **rebuild once the conversation has been idle longer than a provider's cache could survive** (`PREFIX_TTL`, 20 min). The clock is therefore *idle time of this conversation*, not time since a file changed, and reading restarts it — every `get` is a request about to go out. The asymmetry that sets the constant: below a provider's window you pay misses that buy nothing, above it you only pay freshness. -**Writes are deliberately not reacted to, and there is no bus variant for this.** When the agent itself edits an injected file the content is already in the context — its tool call and result sit two messages downstream — so refreshing would repeat what the model just said. A write from *elsewhere* (the same user's Telegram session, a cron job, another member editing `shared-memory/`) is genuinely invisible until the TTL: that is the case where an immediate rebuild costs the most, since a conversation that would notice is by definition a warm one, and the cheaper freshness path already exists — the agent can `read_file`, and a tool result *appends*, which invalidates nothing. The injection header says so in words. Cross-user invalidation would need a `SystemEventBus` variant plus a subscriber per user (the writer lives in a different `UserContext`); it is future work, and this type's key is the seam for it. Note `base` is frozen **whole**: freezing the memory files while letting `__USER_PROFILE__` move would invalidate just as much. The cost is that an `AGENT.md` edit lands at the next rebuild rather than the next round. +**Writes are deliberately not reacted to, and there is no bus variant for this.** When the agent itself edits an injected file the content is already in the context — its tool call and result sit two messages downstream — so refreshing would repeat what the model just said. A write from *elsewhere* (the same user's Telegram session, a cron job, another member editing `shared-memory/`) is genuinely invisible until the TTL: that is the case where an immediate rebuild costs the most, since a conversation that would notice is by definition a warm one, and the cheaper freshness path already exists — the agent can `read_file`, and a tool result *appends*, which invalidates nothing. The injection header says so in words. Cross-user invalidation of a *file* write would need a `SystemEventBus` variant plus a subscriber per user (the writer lives in a different `UserContext`); it is future work, and this type's key is the seam for it. Note `base` is frozen **whole**: freezing the memory files while letting `__USER_PROFILE__` move would invalidate just as much. The cost is that an `AGENT.md` edit lands at the next rebuild rather than the next round. + +**What *is* invalidated eagerly: the two generated lists, because a stale one makes the model deny a tool it has.** The TTL is right for injected content the agent can re-read on demand and wrong for an inventory — a model that reads "no such connector" in the `## MCP servers` table does not go looking, it answers the question. So `Skald::invalidate_prompt_prefix` (the skills door, called straight from `skill_register`/`skill_delete`) has two MCP siblings, both looping the `all_live()` they already had: `refresh_global_mcp_access` — the admin enabling or re-granting a global connector, where refreshing the access snapshot alone fixed what `mcp.tools()` *offers* while leaving the table describing the world before it — and `refresh_connector_after_reinstall`, where a reinstall's new `llm_short_description` reached the runtime but not the prompt. **Order is load-bearing and opposite to the intuition**: `render_mcp_list` renders the live runtime's in-RAM state, not the DB, so the invalidation goes **last**, after the snapshot refresh and after the servers restart — rebuild the prefix first and it is repopulated from the very descriptions being replaced, with nothing left to invalidate it again. In the reinstall that means waiting out a global dependency install that can take minutes; correct anyway, since those users were already reading a stale table and an early rebuild would only freeze the stale one in place. The price is a provider cache miss on the next turn of every open conversation of every live user — cross-user by nature, since one admin is changing something for other people, and there is no cheaper direct path the way there is for a user editing their own memory. It buys back the failure the skills doc-comment already describes word for word. ## Approval gate diff --git a/agents/project-coordinator/AGENT.md b/agents/project-coordinator/AGENT.md index 4c550f1..8d0d428 100644 --- a/agents/project-coordinator/AGENT.md +++ b/agents/project-coordinator/AGENT.md @@ -90,6 +90,16 @@ Then add a clear `## TASK` section describing exactly what you want done. You ca --- +## Suggest keeping a project history + +Any project can grow worth keeping a **history** of — seeing what changed, or undoing a wrong turn. Offer this early on, in **plain, non-technical words** adapted to the project's nature ("I can keep a history of this project, so we can always look back at what changed or return to an earlier version — want me to?"). Propose it once; if the user declines, don't push. + +The mechanism is **git** (available in the sandbox), but keep the jargon out of the conversation. Initialize only after an **explicit yes**: run `git init` in the project folder via `execute_cmd` and make a first commit (set a repo-local identity if asked, e.g. `git config user.name "Skald"`). Then note it in `SKALD.md` ("Versioned with git since … — commit at meaningful milestones") so future sessions know. + +From then on, **commit at meaningful milestones** — a draft finished, a plan agreed, a feature done — with a short message, and mention it casually ("I've saved a snapshot of this stage"). The initial yes is your standing consent; don't re-ask each time. + +--- + ## Keep `SKALD.md` up to date `SKALD.md` (project root) is this project's living diary — the equivalent of personal memory, but scoped to this project. Keep it current so a future conversation resumes with full context. Record there: the goal and scope, key decisions made, current status, useful references (paths to research reports, drafts, specs), and the next steps. Update it with `write_file` / `edit_file` whenever something durable changes — don't let it go stale. If it doesn't exist yet, create it the first time the project has state worth remembering. diff --git a/crates/skald-core/src/skald/accessors.rs b/crates/skald-core/src/skald/accessors.rs index 0f311ba..321ee71 100644 --- a/crates/skald-core/src/skald/accessors.rs +++ b/crates/skald-core/src/skald/accessors.rs @@ -235,6 +235,16 @@ impl Skald { if let Err(e) = ctx.refresh_global_access().await { tracing::warn!(user = %ctx.user_id, error = %e, "failed to refresh global MCP access"); } + // Then rebuild the frozen prompt prefix, for the reason spelled out in + // `invalidate_prompt_prefix`: refreshing the snapshot fixes what `mcp.tools()` + // *offers*, while the `## MCP servers` table the model reads lives inside + // `base`, which `PrefixCache` holds for twenty idle minutes. Without this the + // admin enables a connector, asks for it in an open conversation, and is told + // in good faith that it does not exist — with the tools sitting right there. + // + // After the refresh, never before: the table is rendered from the access + // snapshot we just replaced. + ctx.sessions.loop_runtime().invalidate_prefixes(); } } @@ -252,6 +262,8 @@ impl Skald { /// re-copy its files/deps into the container (`prepare_local_connector` — a hash /// no-op when the source is unchanged) and restart that one server. The rebuilt /// spec now carries the fresh catalog description (see `user_row_spec_resolved`). + /// - **Prompt prefix**: last, invalidate it for every live user, so the + /// `## MCP servers` table stops describing the version that was just replaced. /// /// Best-effort: the catalog write already committed, so a Docker/MCP hiccup here /// must not fail the reinstall — anything not refreshed settles at the user's next @@ -329,6 +341,26 @@ impl Skald { tracing::warn!(user = %ctx.user_id, connector = %catalog_name, error = %e, "reinstall refresh: failed to restart per-user connector"); } } + + // 3. Rebuild the frozen prompt prefix, for everyone — a reinstall changes the + // connector's `llm_short_description`, which the model reads from the + // `## MCP servers` table inside `base` rather than from the runtime it just + // reconnected to. Restarting the servers alone left the prompt describing the + // old version for up to twenty idle minutes. + // + // **Last, deliberately.** `render_mcp_list` renders the live runtime's in-RAM + // state, so a prefix rebuilt before the restarts above would be repopulated + // from the descriptions we are in the middle of replacing — and nothing would + // invalidate it a second time. That the global dependency install can take + // minutes is not a reason to move this earlier: those users were already + // reading a stale table, and rebuilding it early would only freeze the stale + // one in place. + // + // Everyone, not just the users who run this connector per-user: an enabled + // global connector is in every granted user's table. + for ctx in self.rt_user_contexts().all_live().await { + ctx.sessions.loop_runtime().invalidate_prefixes(); + } } pub fn sessions(&self) -> &Arc { &self.rt.sessions } pub fn config(&self) -> &Arc { &self.rt.config } diff --git a/src/frontend/api/marketplace.rs b/src/frontend/api/marketplace.rs index 681ee12..26d3bf1 100644 --- a/src/frontend/api/marketplace.rs +++ b/src/frontend/api/marketplace.rs @@ -393,6 +393,19 @@ fn card_of(h: &Hydrated, installed: bool, installed_version: Option) -> Mar let source = norm_source(&h.entry, &h.manifest); let doc = h.manifest.docs.first().cloned().unwrap_or_default(); // Prefer the manifest's version trio, falling back to the index entry's. + // A disagreement between the two is a malformed feed and is otherwise completely + // invisible: the manifest silently wins, `installed_version` keeps whatever the + // install snapshotted, and if the feed's index is the lower of the two the strict + // `feed > have` below is false forever — the connector never offers an Update and + // nothing anywhere says why. Say it once, in the log. + if let (Some(m), Some(e)) = (h.manifest.version, h.entry.version) { + if m != e { + tracing::warn!( + connector = %h.entry.id, manifest_version = m, index_version = e, + "marketplace feed version desync: connector.json and the index disagree; the manifest wins", + ); + } + } let version = h.manifest.version.or(h.entry.version); let version_string = h.manifest.version_string.clone().or_else(|| h.entry.version_string.clone()); let version_release_date = h.manifest.version_release_date.clone().or_else(|| h.entry.version_release_date.clone());