Skip to content

skills: OpenCode's and Goose's folders are read too (on top of #1410) - #1409

Closed
AbirAbbas wants to merge 852 commits into
devfrom
feat/1277-skills
Closed

AbirAbbas wants to merge 852 commits into
devfrom
feat/1277-skills

Conversation

@AbirAbbas

@AbirAbbas AbirAbbas commented Sep 23, 2026 •

Copy link
Copy Markdown
Collaborator

Skills (#1277), on top of #1410. It is based on santos/dev2, which already has Santosh's skills work: skills from other tools read in place, /skill, use_skill, and skills chosen per message. #1396 is still landing there. When #1410 merges and santos/dev2 is deleted, GitHub retargets this pull request to dev.

Draft. Do not merge before #1410.

What is in it now

  • .opencode/skills and .goose/skills are read too, under the project and the home directory. Use skills from other agent harnesses #1277's day-one list names both. They rank after the other six folders, so an earlier folder's copy of a name wins and the later copy stays in the result, marked shadowed.
  • A change entry.

Checked

  • make pr-ready BASE=origin/santos/dev2: exit 0.

  • Real binary, real model (deepseek-v4-flash), throwaway home, driven in tmux. Four skills were placed:

    All four were listed in the /skill picker. Each was used by the model on its own, and each answer carried that skill's unique marker line. The global one went through use_skill get.

  • On santos/dev2 as it stands, attaching from /skill does not work in the real binary. Every row says memory is off, so this cannot be attached even with memory.enabled on. The picker asserts SkillFacts on the surface's memory seam, but the launch hands it the v3Brain adapter (cmd/codeaf/chatv3.go), which does not forward that method. The picker test passes only because its fake memory has the method. Skills from Claude Code plugins and Codex, with memory off, chosen by what they are for #1396 moves that read onto the session. I built Skills from Claude Code plugins and Codex, with memory off, chosen by what they are for #1396's head with this pull request's commit cherry-picked (it applies cleanly and internal/skills tests pass). There the warning is gone, enter attaches, and the turn shows · skills · goose-honk, open-sesame, house-greeting.

Still to do here

🤖 Generated with Claude Code

santoshkumarradha and others added 30 commits September 18, 2026 15:03
…s the dot row

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ail's held row keeps its title

PlanTasks compared a task's id against the word `root`, but a run the
conversation opens is rooted at the task's own number, so no real run's row
carried progress and the dot row never drew. It asks the store for its root.
On a rail under thirty cells a held row's tail took the line and left the
title one letter; the title is laid first there, and yields only where it
still reads beside the whole tail.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s own edge

The rail's projection kept the indent of a conversation row it never draws,
so the run's root stood one level in and the page's one-level budget at that
width drew a grandchild at its parent's indent. The projection draws no
conversation row, affords two levels, and keeps the rail's one-cell edge.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…dots, and the narrow rail draws the dot row under the run's title

The live line under a task and the task under a task each left a blank in the
family's column, so the tree read as loose pieces; the walk now carries a rule
through every ancestor that still has a sibling to come. A run with a failure
was drawn in words alone at the widest tier, the one run most worth a picture.
On a rail under 40 columns the dot row stands on the line under the title.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…unning cell keeps the frontier

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ailed is stopped

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…own done is never cancelled

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…for it (#1207)

Under CODEAF_TASK_BELT=bash a review check resolved to the mastermind tier, so the crew's checker was never asked and the review round was billed at the thinker's price. SeatFor now seats a check on the careful work tier. The door resolves the check seat in order: --check-model, CODEAF_CHECK_MODEL, the plan seat when the person pinned it by flag or by environment, else the profile's careful row. A pinned two-model run stays two-model. The check seat stays read-only.
…en questions

The stamp and the rows covered every task the conversation's filter admitted,
and the held questions were read from a store kind nothing writes, so they were
always empty. The family is the root and what reaches it through its parents;
the questions are the session's open questions whose subject works one of those
tasks, by their heads.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
santoshkumarradha and others added 8 commits September 22, 2026 00:33
#1373)

* tui3: the stopped frame is compared against a clock in the test's hand

The head draws the time of day on every frame, so a test comparing two whole
frames was comparing the wall clock too, and failed whenever its two captures
straddled a minute boundary. One failure in eighty one runs, read once on a
pull request as a regression in a change that could not reach the path.

stoppingApp now holds its own clock, the way stopbound_test.go's boundedStopApp
already does. The seam existed: app.now reads a.clock and falls back to
time.Now. It is pinned there and not in newTestApp, whose calling files are in
the hundreds and many of which assert on elapsed durations.

Two repairs produce the same green here and only one of them is right, so the
comparison has a name now and a guard on it: narrowing stoppedFrame to drop the
head leaves the original test passing and turns the guard red.

* docs: change entry for the stopped frame and its held clock

---------

Co-authored-by: codeaf <agentfield-bot@users.noreply.github.com>
…#1374)

The guard is a guard only because it calls the same function the real test
calls. The comment said the helper must stay the whole frame and that narrowing
it turns the guard red. It did not say why the helper exists at all, so inlining
it back into its callers reads like removing a pointless indirection, leaves
every test green, and detaches the guard from the comparison it guards in the
same stroke.

Comment and change entry only. No code changes.

Co-authored-by: santoshkumarradha <eng@agentfield.ai>
… unbuilt (#1382)

A person with eighty-one SKILL.md folders on disk asked whether the chat could
use skills and was told codeaf has no such mechanism. The binary carried the
wave. memory.enabled was off.

Every part behaved as designed. The launch import returns when memory is nil,
the catalog renders zero bytes for an empty shelf, and use_skill is withheld
rather than present and refusing. The model was left with no shelf, no verb and
no sentence about either, so it reasoned from the silence and denied a shipped
feature: correctly from the inside, wrongly about the world.

Absent-not-broken stops a model planning a reply around a call that can only
refuse. It does not stop a model reasoning from the absence. So it gets a
companion clause rather than an exception: when a capability is withheld by a
setting, something has to say so.

The catalog renders one line when the machine holds skills this session cannot
reach, naming the setting and saying they exist. The door measures that once at
launch, because a prompt prefix may not walk six folders per render. A machine
with no skills still pays nothing, which is what the emptiness law is about.

The /skill picker reads the folders rather than the shelf, so it listed skills
that could not be attached. Each row now carries the reason beside the folder's
own warning rather than instead of it.

Both tests were run red against the unfixed tree before they were kept.

Co-authored-by: santoshkumarradha <eng@agentfield.ai>
dev carries #1332, #1108, #1333, #1107, #1208, #1071, #1376, #1377, #1381,
#1385, #1386, #1387 and #1384. #1108 is the santos/dev slice held at
d39a4e3, which dev2 already descends from, so the tree was merged against
d39a4e3 as its base: the slice counts as already here, and only what dev
did beyond it is brought in (the nested-module walk, the image guard's
duplicate, and everything from #1333 on). The commit still records origin/dev
as its second parent, so a dev2 to dev pull request sees dev as merged.

Four files conflicted:
- tools_image.go: both sides restored the empty-prompt guard; dev2's
  comment kept, and dev's second copy of the guard's test dropped because
  #1372's test pins the same refusal.
- surfacedoors_law_test.go: dev2 added one ledger door and #1071 crossed
  workingNowAgent; the ratchet counts the merged ledger, 22.
- tui_e2e_test.go, tuiwords_test.go: the home that #1071 draws is the one
  in the tree (dev2 never redrew home after the slice), so the suite waits
  for #1071's words.
…ests do not read the shell's telemetry switch

A parked parent was woken by launchWaits as soon as its children's rows
read done, before their returns were absorbed. The woken parent then wrote
its ending ahead of the review the child's return seats beneath it, the
store refused that ending over the open check, and the run answered
incomplete. review_order_test.go's parked root failed this way under a
loaded box on dev2 before this merge. The wake now waits for every settled
task it names to have landed, which is the rule landed() already states.

Since dev's #1387 the telemetry off switch quiets the Model Pool to `read`,
so a shell exporting CODEAF_TELEMETRY=off turned every pool test that means
the default into a failure. cmd/codeaf's TestMain clears the switch (a test
binary never reports, and the pool's submit address is already pinned), and
the config row test pins it for itself.
TestAStandingFiringReachesTheHostedConversation (from dev's #1377) opened
the lane through WatchTaskUpdates, whose ask is made off the loop, and then
repeated the ask synchronously as its receipt. When the door's own ask
reached the engine after the firing, it replaced the far subscription the
firing had gone down and the row was lost. Under parallel load this failed
on dev and on this branch alike. The test now installs the client lane and
makes the one ask itself, answered before the firing.
# Conflicts:
#	internal/manual/chat/services.md
The wrapper names the lock after the suite starts, so the suite's pid on
the pipe says nothing about the file. The fixture's 50 ms sleep stood in
for that wait, and on a loaded box the test read an empty lock file. It
now reads the file until the line is there, under a ten second deadline.
…1395)

Twelve review findings on the task page, the tab mark, the pool judge sweep and the manual. Full make check on Spark at 635c029: EXIT 0.
… and through every door (#1393)

Nine review findings on the chat manager, the digest, config keys and the commit trailer. Full make check on Spark at 449e792: EXIT 0.
…it (#1411)

Ten review findings on the run lifecycle: no new request adopts an earlier run, every ending is written on the root, the dollar limit ends peers, close lands nothing, steers on ended runs are refused, interrupted rows are honest. Full make check on Spark at 888911c: EXIT 0.
@santoshkumarradha

Copy link
Copy Markdown
Member

Heads-up on overlap with santos/dev2 → dev (#1410, going ready shortly).

Two findings from our testing to carry into your steps:

  • With memory.enabled off, dev2's skills are dead, so it's good that the settled design doesn't need the store.
  • Automatic pickup by shared words missed reworded requests. Giving the model each turned-on skill's name and description (your step 3) is the right shape. It's worth a small check with reworded prompts.

+1 on ~/.agents/skills for #1399. ~/.codeaf/skills holds the resident's bin/ shelf.

@santoshkumarradha

Copy link
Copy Markdown
Member

Proposal so skills land on dev once, in your design:

  1. We remove our side. Right after santos/dev2 lands on dev: the worker harness as default, runs as work, skills from other harnesses, the chat as manager #1410 lands, we open a PR to dev that deletes dev2's shelf-based skills: use_skill, the store shelf, per-message automatic choice, /skill attach, and their three manual pages. internal/skills discovery stays as the shared base. Merge dev after that and you're on a clean tree.
  2. Skills from Claude Code plugins and Codex, with memory off, chosen by what they are for #1396 closes unmerged as the reference. Its evidence, for your steps 3 and 6:
    • Real installs: skills from npx skills add, claude plugin install example-skills@anthropic-agent-skills and a hand-made ~/.codex/skills skill, with memory off and on. Exactly the 12 skills claude plugin details lists, and 7 folders belonging to other plugins correctly left out.
    • Reworded-request check: deepseek-v4-flash, 12 reworded requests plus 4 that need no skill. Offering every skill's name and description (160-character clip, 12 KiB budget) reached the right skill in 16 of 24 with memory on and 15 of 24 with memory off. None of the no-skill requests got one.
    • Misses: the model answers directly or searches the workspace instead of opening the skill first. Weakest were PDF merge and extract, slides for investors, release notes and packaging a service. It needs one instruction line: when a listed skill fits, open it before doing anything else.
    • Don't carry the word-match pre-pass. It attaches unrelated skills to any message containing "skill".
    • Not handled yet: Claude Code's disable-model-invocation and user-invocable frontmatter (a natural fit for your turned-on subset), nested SKILL.md, CLAUDE_CONFIG_DIR, and the picker over --host reading the local disk.
  3. We'll port the proof as tests onto feat/1277-skills once step 3 is in: the tmux end-to-end check, the reworded-request check and the real-install check, so the PR's acceptance is "these pass".

Sound right?

santoshkumarradha and others added 3 commits September 23, 2026 13:31
…s not one machine, and the dual lock is seen and taken back (#1392)

The one-machine retry loop waits as its verdict says and never wraps to zero; only a request with no pool is one machine; the suite lock is seen by old checkouts and a dead directory lock is taken back under the flock. Full make check on Spark at e097553: EXIT 0.
Issue #1277's day-one list names .opencode/skills and .goose/skills beside
the six folders discovery already read. They are appended after .gemini, so
a name held in any of the first six still owns it and the later copy is kept
in the result marked shadowed, in both the project and the home scope.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@AbirAbbas
AbirAbbas changed the base branch from dev to santos/dev2 September 23, 2026 17:52
@AbirAbbas AbirAbbas changed the title Skills: use SKILL.md folders from every harness, by name or on their own skills: OpenCode's and Goose's folders are read too (on top of #1410) Sep 23, 2026
Base automatically changed from santos/dev2 to dev September 25, 2026 02:20
@santoshkumarradha santoshkumarradha added feature Work that adds a capability; developers break it into tasks area:tools The tool layer: built-in tools, registries, belts labels Sep 27, 2026
@santoshkumarradha santoshkumarradha added this to the Reliable agent milestone Sep 27, 2026
@AbirAbbas

Copy link
Copy Markdown
Collaborator Author

Superseded by #1691: the same two roots re-cut onto current dev (this branch was stacked on #1410 and can't merge now that it's squashed), plus OpenCode's and Goose's real global folders under ~/.config and the manual page.

@AbirAbbas AbirAbbas closed this Sep 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tools The tool layer: built-in tools, registries, belts feature Work that adds a capability; developers break it into tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants