feat(skills): always_inject belongs beside the skill, not in one database
Migration 0083 added the column for a measured failure — under the `index` arm, `workspace-repo-commit-protocol` scored Trigger=FAIL while its boundary check passed, because a rule that applies to everyone who writes reads to each agent as nobody's in particular. The column shipped and was never set: prod ran 0 of 53 skills flagged, and the post-v0.8.5 validation mission made 76 tool calls with ZERO ReadMcpResourceTool among them. Not plumbing — the door answered 200 from inside that container, and the agents used ToolSearch four times to reach for other tools they did not have. Setting it by hand fixes one database. A rebuilt one comes up un-flagged, with nothing in the repo recording that the skill was ever meant to be injected — the same shape as every silent-success defect in this project. So the frontmatter carries it, the loader parses it, and the upsert writes it. The file wins on conflict: builtins are code-managed, and a setting that exists only in one database is a setting nobody can find. Guarded both ways. `always_inject` defaults FALSE, because defaulting true would quietly abolish the index arm rather than fix it; and a test asserts the shipped skill still carries the flag, verified by flipping it to false and watching the test fail. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_01WZb5A2kfVfjpdwSochkuHz
This commit is contained in:
co-authored by
Claude Opus 5
parent
bda6bef4db
commit
42c24de6a9
@@ -80,6 +80,13 @@ pub struct UpsertBuiltinSkill<'a> {
|
||||
pub when_to_use: Option<&'a str>,
|
||||
pub tags: Vec<String>,
|
||||
pub body: &'a str,
|
||||
/// Deliver this skill's full body even under the `index` arm.
|
||||
///
|
||||
/// Carried from frontmatter so the decision lives beside the skill it is
|
||||
/// about. It was a bare DB column first, which meant a rebuilt database
|
||||
/// came up with every skill un-flagged and nothing in the repo recording
|
||||
/// that any of them were meant to be injected.
|
||||
pub always_inject: bool,
|
||||
}
|
||||
|
||||
/// Idempotent upsert for builtin skills. Bumps `current_version` +
|
||||
@@ -116,8 +123,8 @@ pub async fn upsert_builtin(pool: &PgPool, b: UpsertBuiltinSkill<'_>) -> Result<
|
||||
sqlx::query(
|
||||
"INSERT INTO skills
|
||||
(id, name, title, author, description, when_to_use, tags,
|
||||
source_kind, workspace_id, current_version, body)
|
||||
VALUES ($1,$2,$2,'system',$3,$4,$5,'builtin',NULL,$6,$7)
|
||||
source_kind, workspace_id, current_version, body, always_inject)
|
||||
VALUES ($1,$2,$2,'system',$3,$4,$5,'builtin',NULL,$6,$7,$8)
|
||||
ON CONFLICT (id) DO UPDATE SET
|
||||
name = EXCLUDED.name,
|
||||
title = EXCLUDED.title,
|
||||
@@ -126,6 +133,11 @@ pub async fn upsert_builtin(pool: &PgPool, b: UpsertBuiltinSkill<'_>) -> Result<
|
||||
tags = EXCLUDED.tags,
|
||||
current_version = EXCLUDED.current_version,
|
||||
body = EXCLUDED.body,
|
||||
-- The FILE wins. An operator who flips this column by hand gets
|
||||
-- it restored to what the frontmatter says on the next boot,
|
||||
-- which is the point: builtins are code-managed, and a setting
|
||||
-- that only exists in one database is a setting nobody can find.
|
||||
always_inject = EXCLUDED.always_inject,
|
||||
updated_at = now()",
|
||||
)
|
||||
.bind(b.id)
|
||||
@@ -135,6 +147,7 @@ pub async fn upsert_builtin(pool: &PgPool, b: UpsertBuiltinSkill<'_>) -> Result<
|
||||
.bind(&b.tags)
|
||||
.bind(next_version)
|
||||
.bind(b.body)
|
||||
.bind(b.always_inject)
|
||||
.execute(&mut *tx)
|
||||
.await?;
|
||||
|
||||
|
||||
Reference in New Issue
Block a user