feat(skill): K-1 code-mode domain globals and K-2 skill auto-arming - #62
Merged
its-janghoon merged 3 commits intoOct 2, 2026
Merged
Conversation
A skill document is cheap only because the objects it drives already exist. Code Mode could previously only be written as `tools.<namespace>.<tool>(...)`, so a skill had to describe a tool call instead of naming a method. Mechanism, in @redrob-code/codemode: `globals` on ExecuteOptions names top-level tool namespaces that are ALSO bound as bare identifiers in the interpreter's global scope. A global is an alias for the same tool path, so there is one implementation and one authorization point, not two. Code Mode stays host-neutral: it never learns what a given name means. The builtin global seeding moves into seedBuiltinGlobals(), and BUILTIN_GLOBAL_NAMES is derived from it, so a builtin added later cannot silently become available as a host global name. assertValidGlobals refuses a name that is not a namespace of the tool tree, one that shadows a builtin, a duplicate, a non-identifier, and a tool rather than a namespace. The generated instructions gain a "Domain globals" section stating the bare and tools forms are the same call. Objects, in packages/redrob/src/tool/domain.ts: Page and Channel as real TypeScript interfaces with a single implementation each, injected under the names `page` and `channel`. - Channel is implemented: `channel.send` appends a text part to the assistant message that owns the execution, so every surface the session is attached to renders it without the engine knowing which surface that is. `channel.id` returns the session id. - Page is NOT implemented, and says so. Nothing in the engine process controls a browser page today, so its single implementation is unavailablePage, whose every method fails with the capability named: "the engine process has no browser-page control surface". It returns no plausible value, so a skill written against the interface fails loudly rather than reading an empty string. The domain namespaces are spread after the MCP catalog deliberately: a connected MCP server named `page` must not shadow the object a skill is written against.
Two optional frontmatter keys beside name/description/slash:
icon: string # a URL
autoInject:
keywords: [string] # matched against the prompt
url: [string] # globs, e.g. docs.google.com/document/**
Both lists are optional and an empty list matches nothing, so `autoInject: {}`
is a skill that still never arms rather than one that arms on everything.
The arming decision is a pure function in packages/core/src/skill/arming.ts:
SkillArming.arm(input: Input): ReadonlyArray<Armed>
SkillArming.armedNames(input: Input): ReadonlyArray<string>
It takes the loaded skills, the current prompt text and the current tab URL,
and returns which skills arm, with the patterns that armed each one. It reads
nothing and caches nothing, so the decision is identical in the prompt builder,
in a UI preview and in a test. Keyword and URL matching are both
case-insensitive; keywords match on word boundaries so `ai` does not match
inside `said`; `*` and `?` stop at a `/` while `**` crosses segments; a `.` in
a glob is a literal dot and not a wildcard. The scheme is ignored on both
sides, so globs are written without one.
A skill with NO autoInject block never auto-arms. It stays explicitly loadable,
which is the point: a skill that arms on everything is always in the prompt.
Loading decodes one document at a time, so a malformed frontmatter costs that
one document and not the set. The decoders are the Result-returning form rather
than the Option form, because the Option form answers only "no" and makes the
cause unfindable; every rejection is logged at warning level with the file and
the decoder's own reason. A malformed autoInject is narrower still: it is
ignored with its own warning and the skill loads without it, since that leaves
the skill exactly where a skill with no block already sits. The reasons are in
the log MESSAGE rather than in annotations, because the default logger renders
an annotation object as [object Object] and loses precisely that detail.
Tests in packages/core/test/skill/arming.test.ts: 13 pass, 0 fail. They cover
a keyword hit, a URL-glob hit, three glob near-misses that must not arm, a
skill with no block, an empty block, case-insensitivity in both directions, and
a malformed autoInject that is ignored rather than fatal while a sibling bad
document is dropped with a logged reason. Each of five restored defects
(explicit-only guard removed, `*` crossing `/`, glob compiled as a regex,
malformed autoInject made fatal, reason text dropped from the log) was confirmed
to fail the suite.
Note: packages/schema has 2 failing tests in test/event-manifest.test.ts. They
fail identically on a clean tree (git stash push -u, 13 pass / 2 fail both
ways) and are unrelated to this change.
`packages/client` commits its generated SDK and gates it with
`check:generated` (bun run generate && git diff --exit-code), so adding
`icon` and `autoInject` to `SkillV2.Info` in packages/schema left the
committed artifact behind and that gate red on CI - the core (linux) job
passed its tests and then failed on this step, and unit (linux) is only
the aggregation job reporting it.
Regenerated with the repo's own generator (`bun run generate` in
packages/client), not hand-edited, because the gate rejects a hand edit by
construction. The whole diff is the two new optional fields on
SkillsListOutput, which is exactly the propagation of the schema change:
readonly icon?: string
readonly autoInject?: { readonly keywords?: ReadonlyArray<string>
readonly url?: ReadonlyArray<string> }
Measured after the change: packages/client typecheck passes; its suite is
15 pass / 1 fail, and that one failure ("exposes every standard HTTP API
group") also fails on a clean origin/develop worktree, so it is not from
this change.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Phase 6 items K-1 and K-2 from the delivery loop.
50ba34c723, already on the branch) — typed domain objects as code-mode globals.0dfe0fbe0b) — skill auto-arming from frontmatter keywords and URL globs.K-1: typed domain objects, not a tool list
A skill document is cheap only because the objects it drives already exist. Code Mode could
previously only be written as
tools.<namespace>.<tool>(...), so a skill had to describe atool call rather than name a method. Now a skill body reads:
The mechanism, in
@redrob-code/codemode:globalsonExecuteOptionsnames top-leveltool namespaces that are also bound as bare identifiers in the interpreter's global scope.
A global is an alias for the same tool path, so there is one implementation and one
authorization point, not two. Code Mode stays host-neutral — it never learns what a given
name means. Builtin global seeding moved into
seedBuiltinGlobals()andBUILTIN_GLOBAL_NAMESis derived from it, so a builtin added later cannot silently becomeavailable as a host global name.
assertValidGlobalsrefuses a name that is not a namespaceof the tool tree, one that shadows a builtin, a duplicate, a non-identifier, and a tool
rather than a namespace.
The objects, in
packages/redrob/src/tool/domain.ts— real TypeScript interfaces with asingle implementation each:
Pageurl/text/query/click/type/navigateChannelid/sendPageNodepage.queryDomainUnavailableErrorobject+missingunavailablePagePageimplementation (see below)sessionChannelChannelimplementationunavailableChannelChannelfor the catalog preview, where nothing may be postedpageTools/channelTools/domainToolsDOMAIN_GLOBALS["channel", "page"]— the names bound as bare globalsInjected in
packages/redrob/src/tool/code-mode.tsviatools: { …, ...domainTools({ page, channel }) }withglobals: DOMAIN_GLOBALS. The domainnamespaces are spread after the MCP catalog deliberately: a connected MCP server named
pagemust not shadow the object a skill is written against.pageis NOT implemented, and says soNothing in the engine process controls a browser page today. There is no page-control seam
from the browser to the engine, so there is nothing for a real
Pageto call.Its single implementation is therefore
unavailablePage, whose every method fails withDomainUnavailableErrornaming the missing capability:It returns no plausible value — not an empty string, not an empty node list — so a skill
written against the interface fails loudly instead of silently reading nothing. The
interface, the tool schemas and the generated instructions all exist and are exercised now,
so skills can be written and reviewed before the browser side lands; only the act fails.
channelis implemented:channel.sendappends a text part to the assistant messagethat owns the execution, so every surface the session is attached to renders it without the
engine knowing which surface that is.
channel.idreturns the session id.unavailableChannelis a second deliberate refusal rather than a gap: it backs the catalogpreview path, so the generated instructions list the same globals a live execution binds
without a preview being able to post into a conversation.
K-2: frontmatter
Two optional keys beside the existing
name/description/slash:Both lists inside
autoInjectare optional, and an empty list matches nothing — soautoInject: {}is a skill that still never arms, rather than one that arms oneverything. A skill that arms on everything is a skill that is always in the prompt.
K-2: the arming decision
A pure function, in
packages/core/src/skill/arming.ts:It reads nothing, caches nothing and logs nothing, so the decision is identical in the
prompt builder, in a UI preview and in a test.
armreturns the patterns that armed eachskill, which is what a "why is this skill loaded?" surface needs;
armedNamesis forcallers that only want the set.
Matching rules, both case-insensitive:
aidoes not match insidesaid, while akeyword like
.pdfordocs.google.comstill matches in running text.*and?stop at a/;**crosses path segments; every other character isliteral, so a
.in a hostname is a dot and not a wildcard. The scheme is ignored on bothsides, so globs are written
docs.google.com/document/**, not with anhttps://prefix.A skill with no
autoInjectblock never auto-arms. It stays explicitly loadable.K-2: one document at a time, and the reason is kept
Loading decodes each skill document on its own, so a malformed frontmatter costs that one
document and not the whole set.
The decoders are
Schema.decodeUnknownResult, notdecodeUnknownOption: the Option formanswers only "no", which makes a malformed document unfindable among the ones that loaded.
Every rejection is logged at warning level with the file and the decoder's own reason:
A malformed
autoInjectis narrower still — it is ignored with its own warning and theskill loads without it, which leaves the skill exactly where a skill with no block already
sits, instead of losing the skill to a typo in an optional block.
The reasons go in the log message, not in annotations. The default logger renders an
annotation object as
[object Object], which was dropping precisely the detail that makesa drop findable — caught by the test that asserts on captured log output.
Tests
packages/core/test/skill/arming.test.ts— 13 pass, 0 fail, 27 expect() calls.Covering: a keyword hit; which keyword armed; case-insensitivity in both directions; a
keyword not matching inside a longer word; a URL-glob hit; scheme-ignoring and
case-insensitive URL matching; three glob near-misses that must not arm (
documentsvsdocument, a single*refusing to cross/, a literal dot); a skill with no block;an empty block; no current URL; keyword and URL hits on one skill; result ordering; and a
malformed
autoInjectthat is ignored rather than fatal while a sibling undecodabledocument is dropped with a logged reason and count.
Reverse-verified — each of five restored defects was confirmed to fail the suite:
*crosses/autoInjectmade fatalGates
bun turbo typecheck— 18/18 packages successful (also run by the pre-push hook).bun run lint(oxlint) — 0 errors.packages/core:bun test— 987 pass, 0 fail, 119 files.packages/schema: 13 pass, 2 fail intest/event-manifest.test.ts. Pre-existing andunrelated: measured with
git stash push -u, which gives an identical 13 pass / 2 fail onthe clean tree. Not touched here.
Not in this change
pagecannot act. Its interface and its refusal ship; the capability does not, because thebrowser-to-engine page-control seam does not exist yet. Detailed above.
page, screen, mail, vault. Onlypageandchannelarehere, which is the scope the item was given: the two nothing else depends on.
screen,mailandvaultare not stubbed, not declared, and not silently half-present.armyet — K-2 is the mechanism and the frontmatter, and thecall site in the session prompt builder is a separate item.
iconis decoded and carried onSkill.Infofor surfaces that list skills; no surfacerenders it yet.
Follow-up commit: the generated client artifact
CI's
core (linux)job was red and it was a real defect in this branch, not flake. The jobruns the core suite (which passed) and then
bun run check:generatedinpackages/client,which is
bun run generate && git diff --exit-code -- src/generated src/generated-effect.packages/clientcommits its generated SDK, so addingiconandautoInjecttoSkillV2.Infoleft the committed artifact stale and that gate failed.unit (linux)is onlythe aggregation job reporting the same failure, not a second one.
Fixed in
feat(client): K-2 regenerate the client types for the new skill keysby running therepo's own generator rather than editing the file, since the gate rejects a hand edit by
construction. The entire diff is the propagation of the schema change onto
SkillsListOutput:check:generatedexits 0 after the commit.Gate measurements re-run on the final tree
Every number below was measured on this branch's tip, not carried over:
bun turbo typecheck --force(codemode, core, redrob, schema)bun turbo typecheck(pre-push hook)packages/codemodebun testpackages/corebun testpackages/redrobbun testpackages/clientbun testpackages/schemabun testNew test files, measured individually:
packages/core/test/skill/arming.test.tspackages/codemode/test/globals.test.tspackages/redrob/test/tool/domain.test.tsBoth pre-existing failures were measured against a control rather than inferred, by checking
out
origin/developinto a detached worktree and running the same suite there:packages/schematest/event-manifest.test.ts— 13 pass / 2 fail on this branch and13 pass / 2 fail on clean
origin/develop. Identical.packages/client"exposes every standard HTTP API group" — fails on cleanorigin/developas well. The control tree in fact shows 14 pass / 2 fail, one failuremore than this branch, so this change does not add a client failure.