Repository navigation
fix: publish the commands when the skills change - #643
Open
nikita-ashihmin wants to merge 2 commits into
Open
nikita-ashihmin wants to merge 2 commits into
nikita-ashihmin wants to merge 2 commits into
Conversation
A skill that appears while a session is open — a new file, a new symlink, or a link whose target changed — became a slash command only after the next completed turn, because the `skills/changed` notification was ignored. The server now lists the skills again on that notification and publishes the commands, debounced so one burst of file changes lists once.
A publish now carries a revision, so a slower earlier `skills/list` no longer overwrites the newer list a later one already sent — the very command the change is about would stay missing until the next turn. The pending publish holds the full `sessionPublishIsCurrent` guard, including the close fence, instead of a bare identity check, and `closeSession` drops the publish a closing session was still waiting out. The debounce keeps a cap, so a writer that never pauses — a long checkout, a watcher of a skill root — no longer holds the publish back forever. The two publishes a turn triggers now use the same guard. The tests go through the wire: `skills-changed.test.ts` emits the real notification and asserts the commands the client receives, which the unit tests on the private handler could not. Removing either call site now fails the suite.
This branch has not been deployed
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
1 participant
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.
Problem
A skill that appears while a session is open — a new
SKILL.md, a new symlink into.agents/skills, or a link whose target changed — did not show up in the slash-command completion until the next completed turn.The app-server sends
skills/changedfor exactly this, and documents it as "Treat this as an invalidation signal and re-runskills/list". The adapter listed it among the ignored notifications, so nothing re-ran. The only re-list was the post-turn publish, which is why the skill eventually appeared — one turn late.Change
CodexAcpServerhandlesskills/changedon both the baseline turn tracker and the prompt subscription, and asks the command registry to publish for that session.CodexCommands.publishOnSkillsChangeddebounces the publish by 300 ms, so one burst of file changes lists the skills once. The timer isunref'd, and a session that closed in the meantime publishes nothing.CodexEventHandlergivesskills/changedits own case instead of leaving it in the ignored block — the change itself is no session update.Tests
Two cases in
CodexAcpClient.test.ts: a burst of skill changes publishes once after it settles and publishes again when the list actually differs; a session that closed while its change settled never lists.npx tsc --noEmitis clean. The three failures in a fullvitest run(CodexHomeWatcher) reproduce on untouchedmainon this machine.