Old custom skills no longer break the upgrade: Core boots, incompatible skills are cleaned up, missing ones are named - #420
Merged
Conversation
…ill it cannot find
…issing ones get an endpoint
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.
Found while migrating a real user's 2.1.1 configs (Xul: 9 configs, 18 Wingmen, 479 commands, three custom skills) to 3.2.3 and then booting Core with his actual
sc_log_readerv4.9.0.0 installed.Core did not start at all
One Wingman carried a skill setting the installed skill version no longer defines:
A Wingman stores only
{id, value}per skill property;nameandproperty_typecome from the skill'sdefault_config.yaml. When a skill update drops a property, nothing can complete the stored entry, the merged Wingman fails validation,parse_configre-raises and the process ends. One stale key in one of 18 files is enough. This has nothing to do with the migration — it hits anyone who updates a custom skill that lost an option.ConfigManager._drop_orphaned_skill_propertiesnow drops such a value while building the merged config. The YAML on disk is untouched, and an entry that carries the whole property itself still survives, so skills that ship their metadata inside the Wingman config keep working.Incompatible skills were only cleaned in the config that happened to be open
disable_ineligible_skillswalkedcurrent_config_dironly. With nine configs,SC_LogReaderstayed switched on in the other eight until the user opened them — and the switch did nothing. It now sweeps every config, and the skill's stored settings go with it: a skill that has to be rewritten for the v3 API gets new properties anyway.Skills that are not installed at all were invisible
SC_AccountantandSC_Navigatorare switched on for two Wingmen but their folders are not on this machine. Core cannot describe them, so they never appeared in/wingman-skills— no row, no badge, no hint. NewGET /wingman-skills/missingreturns them by name; the client renders a disabled row (ShipBit/wingman-client#51). These are deliberately not removed: the folder may simply live on another machine.Migration
migration_211_to_300pinnedparakeet.execution_providertocpu.hardware_scan_performedis alreadytruefor every 2.x user (2.0.0 set it for FasterWhisper), so the boot-time scan never runs again and every GPU owner would transcribe on the CPU forever. It now asks the machine, the same way 1.8.2 → 2.0.0 did for FasterWhisper, and logs the result..migrationmarker was written at the end of the last chain step, so the custom-skill warnings and the closing line only ever reached the terminal — not the file users send us. The full log is flushed at the end now.audit_skill_availability()names every skill a migrated Wingman references but that is not installed, for bothdiscoverable_skills(names, what the UI shows as on) andskills(module paths, the settings).Migrating Wingman …andCopied file: …lines carry the config dir now. Xul haszRandom NPCthree times; a bare file name made the log unreadable for exactly the people with the most to lose.Verified
Migration re-run from scratch four times against the untouched 2.1.1 folder (checksums over all 150 files unchanged): 9 configs, 18 Wingmen, 479 commands, backstories, avatars and PTT keys identical, one default config instead of two, no duplicates. Boot with the real legacy skill installed now reaches
readyand cleans up as described. 319 local tests pass, including two new files for the GPU detection and the orphaned properties (/tests/is gitignored by house convention, so they stay local).Skill verdicts can be asked for now
legacy_v2,quarantinedandfailedonly existed as a WebSocket broadcast duringinitialize_tower. Reloading the client while Core keeps running lost the badges, and an incompatible skill sat there switched off with nothing saying why.GET /skill-verdictsreturns the same records as a snapshot — scan verdicts with this boot's runtime failures folded in — and the client fetches them with the skill list.Core no longer dies on the joystick thread (macOS)
start_joysticksranpygame.init(), which starts every pygame module: the mixer, which opens an output device while Core's audio stack already holds one, and the video subsystem, which on macOS reaches into Cocoa and sets the application's main menu — only allowed on the main thread:Any Wingman with a joystick PTT binding killed Core on macOS. The loop only reads joystick events and never draws, so it now initializes display and joystick only, with the dummy video driver on macOS.
pygame.event.get()refuses to work without a video driver, which is why display is initialized but left without a window. Windows and Linux keep their driver; the joystick path there needs a check on Windows, I could not test it.Verified on macOS with the Wingman that used to kill Core (
RIGHT VPC Stick WarBRDbutton 28): the tower initializes fully and Core stays up.The system prompt is reset again in 3.2.3
The same hard reset 3.1.2 did. It is not user content: it carries the tool-calling contract, the output format and the placeholders the backend fills in, and every release moves it. Someone who edited it once keeps an old contract forever and gets worse answers from a backend that has moved on.
defaults.yamlis overwritten with the prompt shipped in this build.prompts.system_promptis deleted, not overwritten, so it inherits fromdefaults.yamlagain. Overwriting would pin today's text into the file and the next release would have to chase it a second time.Verified end to end, not only in unit tests: a 3.2.2 install was planted with an edited
defaults.yamlprompt (# MY OWN RULES / Ignore every tool) and a Wingman carrying its own (You are a pirate. Never call a function.), then migrated for real. The log:Afterwards
defaults.yamlmatchestemplates/configs/defaults.yamlbyte for byte, none of the 18 Wingmen carries asystem_prompt, and all 18 backstories are still there.