From c5dbce1c7444aca2f52c39e7d0194689c346844e Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:33:29 +0530 Subject: [PATCH 1/2] feat(skills): edit skills in Settings, with the built-ins readable as examples MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Part of #56. Closes #87 — the last box of #64, which this closes too. ## The built-ins are the documentation #64 asks that they be "readable as examples", so they are listed first, opening one shows exactly what it says to the model, and each is written the way someone's own skill should be: a short summary, instructions that say what to *do* rather than what to be, and a tool list that is narrow — because narrowing is the normal case, not an advanced option. **Editing one copies it.** That is `SkillStore`'s decision rather than this screen's, and the form says so before you save. "Restore" appears only on the ones that have been changed, because a button that undoes nothing is a button that teaches you not to trust it. ## What the form has to get right - **The invocation is shown as you type the name**, because what you read and what you type are different strings and this is where that becomes obvious. - **"Limit which tools" is a switch, not an empty field.** Absent and empty mean different things — see `AgentSkill.allowedToolNames` — and inferring "narrows to nothing" from an empty text field would take every tool from a skill the moment someone cleared it to retype. - **The tool list splits on commas or whitespace**, because someone will use either and a list that accepts one silently drops half of what was typed. - **Validation is the store's.** The form shows `SkillName.Rejection`'s message and keeps what was typed; a second copy of the name rules is a second copy that can disagree. - The three tool states read as words in the list — "Any tool the agent has", "No tools", "Only: …" — because they are easy to confuse and the difference matters. ## Export and import `SkillTransfer` presents a panel rather than inventing a path: this is the user's filesystem, and a silent write into Documents is how a feature becomes something people cannot find again. `MessageActions.exportMarkdown` already makes that argument. A file that cannot be read costs that file and nothing else — picking six and losing all of them to one bad encoding would be the worst possible reading of "import". The result is said out loud **including when it is zero**, because an import that quietly does nothing is indistinguishable from one that worked. The wording is pure and tested at every count, since "1 skills" is the kind of thing nobody notices until it ships. The log names the *file*, never the path — the project rule, and the filename is what the user recognises anyway. ## Two size caps, split rather than suppressed `AISettingsTab` went one line past the 450-line type-body cap, so the per-tool switches and their static group list moved to `AISettingsTab+Tools.swift` — static data rather than logic, which is exactly what the extension-file rule is for. Three members widened to internal to make that work, each carrying the project's `// Extension-visible: +Tools` marker so the wider access reads as deliberate. Verified: build succeeds, 1877 tests in 170 suites pass, SwiftFormat --lint clean over 573 files, SwiftLint --strict 0 violations in 732 files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01M3Wpnj9ZmWPKYdFPB1AVY3 --- Logue.xcodeproj/project.pbxproj | 16 ++ Logue/Agent/Skills/SkillTransfer.swift | 112 ++++++++ Logue/Views/Settings/SkillsSection.swift | 255 ++++++++++++++++++ .../Settings/Tabs/AISettingsTab+Tools.swift | 120 +++++++++ Logue/Views/Settings/Tabs/AISettingsTab.swift | 123 +-------- LogueTests/SkillTransferTests.swift | 62 +++++ 6 files changed, 575 insertions(+), 113 deletions(-) create mode 100644 Logue/Agent/Skills/SkillTransfer.swift create mode 100644 Logue/Views/Settings/SkillsSection.swift create mode 100644 Logue/Views/Settings/Tabs/AISettingsTab+Tools.swift create mode 100644 LogueTests/SkillTransferTests.swift diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index 1fbdc6c..617ad3a 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -35,6 +35,7 @@ 067D657E0BB41B6E92B7E7B2 /* LLMEngine.swift in Sources */ = {isa = PBXBuildFile; fileRef = 5DB8083A539C38A17D6FF132 /* LLMEngine.swift */; }; 06AA8A863594D0566529704C /* TranscriptRealignmentTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1396470F3C9D5EA09515233B /* TranscriptRealignmentTests.swift */; }; 06C84580BE43350D339A4824 /* ModelSettingsRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = AD07F2A2082486F313E370B3 /* ModelSettingsRow.swift */; }; + 06EFD8B83F839B605E725106 /* SkillTransferTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4C7C24753EEA8C5C7C9883FA /* SkillTransferTests.swift */; }; 0718AE38C6B1FB211513F3B1 /* HTTPMessage.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6022EE09B190183A46D41F97 /* HTTPMessage.swift */; }; 073A26468426AA1BDB4714DA /* LogueMark.svg in Resources */ = {isa = PBXBuildFile; fileRef = 307669D269B6660BBFAF3CF2 /* LogueMark.svg */; }; 073F4389E29E0F214606228C /* VerifyPanelLLMTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 806C1E95DE7824AC8E6B8CF2 /* VerifyPanelLLMTests.swift */; }; @@ -181,6 +182,7 @@ 330E0913F834295E3AF08E40 /* SpaceTreeRow.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6908921C4944C7BF243B8509 /* SpaceTreeRow.swift */; }; 3323132CAC627887271FF6F6 /* RetryHelper.swift in Sources */ = {isa = PBXBuildFile; fileRef = 70684260B4C198545C4A6AC2 /* RetryHelper.swift */; }; 35592A04877BCC129AE90A1E /* AgentDictationService.swift in Sources */ = {isa = PBXBuildFile; fileRef = CA29CBB4724CFEE0030E5FE3 /* AgentDictationService.swift */; }; + 357F114D7B3F70239EDC81FB /* AISettingsTab+Tools.swift in Sources */ = {isa = PBXBuildFile; fileRef = BD4D514B909C53B1A57A1EC2 /* AISettingsTab+Tools.swift */; }; 3599049B9B46D8BBC567D6F7 /* ActionItemsRingCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = FABEE67955D09AB40C79784C /* ActionItemsRingCard.swift */; }; 35CBDE38DC02F798AE51C76F /* DocumentWidthModeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = AE8030D02974A04A80B48065 /* DocumentWidthModeTests.swift */; }; 35FC159E499EA14E07D6304B /* PreRollBufferTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DA91866185B2C46BE60A76CB /* PreRollBufferTests.swift */; }; @@ -529,6 +531,7 @@ AD07D0DCE8536510D087A05C /* TroubleshootingActions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 15DD5F0F004960A0D0B66E8B /* TroubleshootingActions.swift */; }; AE20DB2F67CD986E3DEEAC19 /* MCPNamespaceClashTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = F475CCA42BE819BF19DD4FA4 /* MCPNamespaceClashTests.swift */; }; AE7BCB76D77404A8B71D119E /* PIIModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3B9978579DD7A8D5743C9DB6 /* PIIModels.swift */; }; + AE7DD298DE7E7CA96EA9CE14 /* SkillsSection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 130CDB6A8FF6641CDFDF90EF /* SkillsSection.swift */; }; AF02C63201E3C3ED826BF85B /* SkillName.swift in Sources */ = {isa = PBXBuildFile; fileRef = 965FEFAD362D49C3EE00089C /* SkillName.swift */; }; AF35E4BCA1244DFE4B983329 /* UndoToastView.swift in Sources */ = {isa = PBXBuildFile; fileRef = AFBC89D908FFD586D1C38ABF /* UndoToastView.swift */; }; AFCA039A58319E9F99A6A763 /* VerifyModels.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4F31BC63173139D2C8BB0D1D /* VerifyModels.swift */; }; @@ -607,6 +610,7 @@ CF7EF95C9DA5C2830CE64C33 /* MeetingStore+Transcript.swift in Sources */ = {isa = PBXBuildFile; fileRef = 709692D702A29C9D5A0C4C9D /* MeetingStore+Transcript.swift */; }; CF82951A70FC5F643DD8F1CD /* whatsnew-privacy.png in Resources */ = {isa = PBXBuildFile; fileRef = 9C6A8A41EE02367332FFCEEE /* whatsnew-privacy.png */; }; CFC1E2D409AE2B588FE6A9AD /* OverviewQuickActionsCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = ED35C64E018F0B223D5FE487 /* OverviewQuickActionsCard.swift */; }; + D016605BB7129C9E7657608C /* SkillTransfer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3D772C19829F4A087866BE7C /* SkillTransfer.swift */; }; D0430E32BB8F114084B8555D /* MeetingSummaryPanelView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 079936683006158188BD417B /* MeetingSummaryPanelView.swift */; }; D04366207D6EDB7BB6134314 /* SystemAudioArmingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 6405B6178237C69E2674D9E9 /* SystemAudioArmingTests.swift */; }; D0FFBA721311ED3F007A7D49 /* ImportFileReadingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0062B04D59734428AFC49682 /* ImportFileReadingTests.swift */; }; @@ -836,6 +840,7 @@ 126A669ACEBF8395600EC8EE /* SuggestionPanelView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SuggestionPanelView.swift; sourceTree = ""; }; 12A71C8E402261DE0D93DA2A /* ModelAction.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ModelAction.swift; sourceTree = ""; }; 12C9A281A8D252F78714C155 /* DashboardActionItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DashboardActionItem.swift; sourceTree = ""; }; + 130CDB6A8FF6641CDFDF90EF /* SkillsSection.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SkillsSection.swift; sourceTree = ""; }; 132AE87705CAF5786CEA7086 /* StatusFilterChip.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StatusFilterChip.swift; sourceTree = ""; }; 132C503C3139368F66A16B8C /* MeetingStore+SeedData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MeetingStore+SeedData.swift"; sourceTree = ""; }; 133717C9C61937FC843612D1 /* MultiBlockKeyHandler.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MultiBlockKeyHandler.swift; sourceTree = ""; }; @@ -958,6 +963,7 @@ 3C9DEE7B3D28180F51C678C6 /* DocumentFilename.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentFilename.swift; sourceTree = ""; }; 3CE2A98EFCCC6AF746E2E734 /* TaskFolderStoreTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TaskFolderStoreTests.swift; sourceTree = ""; }; 3D36503D92C1D2637BBC06BF /* MeetingStore+Metadata.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "MeetingStore+Metadata.swift"; sourceTree = ""; }; + 3D772C19829F4A087866BE7C /* SkillTransfer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SkillTransfer.swift; sourceTree = ""; }; 3E2383DBE96586FC70FE52CB /* Motion.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Motion.swift; sourceTree = ""; }; 3E3BFD10323B282DEB7E0A9A /* whatsnew-smart-minutes.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "whatsnew-smart-minutes.png"; sourceTree = ""; }; 3E86EBBF6643966395A7DE8B /* FileAccessGate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FileAccessGate.swift; sourceTree = ""; }; @@ -997,6 +1003,7 @@ 4B4D41D14C5F7B6352AC692F /* CanvasPaneView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CanvasPaneView.swift; sourceTree = ""; }; 4B73B624BC6F40990495B96C /* SpeakerShortLabelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SpeakerShortLabelTests.swift; sourceTree = ""; }; 4C618174E127839238F1ABD6 /* AboutSettingsTab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AboutSettingsTab.swift; sourceTree = ""; }; + 4C7C24753EEA8C5C7C9883FA /* SkillTransferTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SkillTransferTests.swift; sourceTree = ""; }; 4C9B5A0EF0F9DF7AB61FB25C /* SkillExecutionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SkillExecutionTests.swift; sourceTree = ""; }; 4E9727B7A91126D27FA68CA0 /* NetworkEgressSummaryTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NetworkEgressSummaryTests.swift; sourceTree = ""; }; 4F21C87791C6ECF443D14823 /* AppVersionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AppVersionTests.swift; sourceTree = ""; }; @@ -1340,6 +1347,7 @@ BC51048570778AAA190A1A3C /* BatchTranscriptFilter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = BatchTranscriptFilter.swift; sourceTree = ""; }; BCB33CA2F7C185927E16825F /* whatsnew-chrome-extension.png */ = {isa = PBXFileReference; lastKnownFileType = image.png; path = "whatsnew-chrome-extension.png"; sourceTree = ""; }; BCC95C3045D2301DCC960849 /* AIContentScorer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AIContentScorer.swift; sourceTree = ""; }; + BD4D514B909C53B1A57A1EC2 /* AISettingsTab+Tools.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "AISettingsTab+Tools.swift"; sourceTree = ""; }; BDA92F8718851010A4B75DD2 /* TaskFolderLocatorTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TaskFolderLocatorTests.swift; sourceTree = ""; }; BE170B6B814BD80DC8505A4C /* PrivacySettingsTab.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PrivacySettingsTab.swift; sourceTree = ""; }; BE62E705043D78A5D42C1700 /* WikiLinkURL.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WikiLinkURL.swift; sourceTree = ""; }; @@ -1566,6 +1574,7 @@ children = ( 4C618174E127839238F1ABD6 /* AboutSettingsTab.swift */, 93368B5F8A112B6802BF4001 /* AISettingsTab.swift */, + BD4D514B909C53B1A57A1EC2 /* AISettingsTab+Tools.swift */, 3669C3260F81F837792FCFE9 /* AutomationSettingsTab.swift */, 431C845CF5A3DC729ED17CC0 /* BackupSettingsTab.swift */, 19E3F885C9304AB29EE2748E /* GeneralSettingsTab.swift */, @@ -1719,6 +1728,7 @@ A7BC3FCFA1A0116B83C06470 /* SkillInvocationTests.swift */, 3B10ECDC2A200BF826E8AF38 /* SkillModelTests.swift */, DA05DBB227EC6FF0BA790263 /* SkillStoreTests.swift */, + 4C7C24753EEA8C5C7C9883FA /* SkillTransferTests.swift */, 20A9DCED765C659BAD6BA11C /* SortformerTimelineTests.swift */, 7AE731B3E0B169C32E1D10C9 /* SourcesPanelContentTests.swift */, 382510F54B865B60B16FDE73 /* SpaceFileTests.swift */, @@ -1922,6 +1932,7 @@ 407A90F8E61C4E0576B95ADD /* MCPServersSection.swift */, 38B7C47637B8FBFF7CC83C4F /* NetworkEgressSection.swift */, B57D98C9B0D1E6A69681C8C1 /* SettingsView.swift */, + 130CDB6A8FF6641CDFDF90EF /* SkillsSection.swift */, ); path = Settings; sourceTree = ""; @@ -2466,6 +2477,7 @@ 965FEFAD362D49C3EE00089C /* SkillName.swift */, B33D38585FFCB1764C45F7DF /* SkillStore.swift */, C14D1B22AAD10C3180BC5AD8 /* SkillToolScope.swift */, + 3D772C19829F4A087866BE7C /* SkillTransfer.swift */, ); path = Skills; sourceTree = ""; @@ -3006,6 +3018,7 @@ 90A49AA4762ECCB5320A9EBF /* SkillInvocationTests.swift in Sources */, 595BE98FA71A505D0DB5DCB8 /* SkillModelTests.swift in Sources */, 2752CE3A9D504B735C0F4333 /* SkillStoreTests.swift in Sources */, + 06EFD8B83F839B605E725106 /* SkillTransferTests.swift in Sources */, 480E2F2ECBABB70296373573 /* SortformerTimelineTests.swift in Sources */, 6DAA46367DD76A4D62BC2B67 /* SourcesPanelContentTests.swift in Sources */, AA1456FCC75FBB7DFEFC1BE0 /* SpaceFileTests.swift in Sources */, @@ -3067,6 +3080,7 @@ 421E11A102014201A63058EB /* AIChatPanelView.swift in Sources */, 6D86DD0D26811B19EEE00299 /* AIContentScorer.swift in Sources */, 88EEF097CAC3DE11B876A929 /* AIContentTools.swift in Sources */, + 357F114D7B3F70239EDC81FB /* AISettingsTab+Tools.swift in Sources */, 12538CC57937C8F7BD7D9433 /* AISettingsTab.swift in Sources */, 5516345F1ECA8AD048BB0889 /* AITitleGenerator.swift in Sources */, 09BD207321C61F236BB38D30 /* AVAudioPCMBuffer+Detached.swift in Sources */, @@ -3502,6 +3516,8 @@ AF02C63201E3C3ED826BF85B /* SkillName.swift in Sources */, 23A3C37522CE6FD3D4B231AC /* SkillStore.swift in Sources */, D5CE09C89E00EEF7F9F0CE67 /* SkillToolScope.swift in Sources */, + D016605BB7129C9E7657608C /* SkillTransfer.swift in Sources */, + AE7DD298DE7E7CA96EA9CE14 /* SkillsSection.swift in Sources */, A3BB9CABAD3F8815F31B297F /* SlashCommandView.swift in Sources */, FFC8FD3443FBDCF45F791FCF /* SlideDeckBuilder.swift in Sources */, 662398037CDC210681F909C7 /* SlideTools.swift in Sources */, diff --git a/Logue/Agent/Skills/SkillTransfer.swift b/Logue/Agent/Skills/SkillTransfer.swift new file mode 100644 index 0000000..e27ae91 --- /dev/null +++ b/Logue/Agent/Skills/SkillTransfer.swift @@ -0,0 +1,112 @@ +import AppKit +import Foundation +import os.log +import UniformTypeIdentifiers + +/// Moving skills between this Mac and a file. +/// +/// #64 asks that skills "export and import as plain files", which is a claim about the +/// filesystem rather than about a format — the format is `SkillFile`'s. This is the half that +/// presents a panel, reads bytes, and says what happened. +/// +/// The panel is presented rather than a path invented: this is the user's filesystem, and a +/// silent write into Documents is how a feature becomes something people cannot find again. +/// `MessageActions.exportMarkdown` makes the same argument for the same reason. +@MainActor +enum SkillTransfer { + private static let logger = Logger(subsystem: AppConstants.bundleID, category: "Skills") + + /// Files we will read a skill out of. + /// + /// Markdown and plain text: a skill file is frontmatter plus prose, and the extension a + /// person gives it is not something to be strict about. + static var acceptedTypes: [UTType] { + [UTType("net.daringfireball.markdown"), .plainText, .text].compactMap(\.self) + } + + /// Writes one skill to a file the user picks. + static func export(_ skill: AgentSkill) { + let panel = NSSavePanel() + panel.allowedContentTypes = acceptedTypes + panel.nameFieldStringValue = "\(skill.invocation).md" + panel.canCreateDirectories = true + panel.message = "Save this skill as a file you can share." + + let text = SkillFile.render(skill) + panel.begin { response in + guard response == .OK, let url = panel.url else { return } + do { + try text.write(to: url, atomically: true, encoding: .utf8) + } catch { + // Surfaced, not only logged: the user asked for a file and there is now no + // file, which is not a thing to find out about later. + Task { @MainActor in + ToastCenter.shared.show("Could not export the skill: \(error.localizedDescription)") + } + Self.logger.error("Skill export failed: \(error.localizedDescription, privacy: .public)") + } + } + } + + /// Reads skills out of files the user picks, and adds them. + /// + /// Returns how many landed. A file that cannot be read costs that file and nothing else — + /// picking six and losing all of them because one had the wrong encoding would be the + /// worst possible reading of "import". + @discardableResult + static func importSkills(into store: SkillStore) async -> Int { + let panel = NSOpenPanel() + panel.allowsMultipleSelection = true + panel.canChooseFiles = true + panel.canChooseDirectories = false + panel.allowedContentTypes = acceptedTypes + panel.prompt = "Import" + panel.message = "Choose skill files to import." + + let urls: [URL] = await withCheckedContinuation { continuation in + panel.begin { response in + continuation.resume(returning: response == .OK ? panel.urls : []) + } + } + guard !urls.isEmpty else { return 0 } + + var texts: [String] = [] + for url in urls { + do { + try texts.append(String(contentsOf: url, encoding: .utf8)) + } catch { + // The name, never the path — the project rule is that a full path does not go + // into a log, and a filename is what the user recognises anyway. + logger.error( + "Could not read a skill file named \(url.lastPathComponent, privacy: .public): \(error.localizedDescription, privacy: .public)" + ) + } + } + + let taken = store.importSkills(from: texts) + // Said out loud, including when it is zero. An import that quietly does nothing is + // indistinguishable from one that worked. + ToastCenter.shared.show(summary(taken: taken, chosen: urls.count)) + return taken + } + + /// What to say after an import. + /// + /// Pure and separate so the wording is testable without a panel — the counts are the part + /// that is easy to get wrong, and "1 skills" is the kind of thing nobody notices until it + /// ships. + static func summary(taken: Int, chosen: Int) -> String { + switch (taken, chosen) { + case (0, _): + "Nothing could be imported from \(count(chosen, "file"))." + case let (taken, chosen) where taken == chosen: + "Imported \(count(taken, "skill"))." + case let (taken, chosen): + "Imported \(count(taken, "skill")) of \(chosen)." + } + } + + private static func count(_ number: Int, _ noun: String) -> String { + "\(number) \(noun)\(number == 1 ? "" : "s")" + } +} diff --git a/Logue/Views/Settings/SkillsSection.swift b/Logue/Views/Settings/SkillsSection.swift new file mode 100644 index 0000000..cad9412 --- /dev/null +++ b/Logue/Views/Settings/SkillsSection.swift @@ -0,0 +1,255 @@ +import SwiftUI + +/// Reading, writing and sharing skills. +/// +/// The built-ins are listed first and are **readable**, which #64 asks for in as many words: +/// someone writing their first skill should have a worked one in front of them rather than an +/// empty box. Opening one shows exactly what it says to the model. +/// +/// Editing a built-in copies it. That is decided in `SkillStore` rather than here — the +/// built-ins are documentation as much as they are features, and someone who overwrote the +/// only worked example has no way back. "Restore" is the way back, and it appears only on the +/// ones that have been changed. +struct SkillsSection: View { + @State private var store = SkillStore.shared + + @State private var editingID: UUID? + @State private var isAdding = false + @State private var draft = Draft() + @State private var rejection: SkillName.Rejection? + + /// What the form is holding, before it is a skill. + private struct Draft { + var title = "" + var summary = "" + var instructions = "" + /// The tool list as typed. Parsed on save so a half-finished line is not an error. + var tools = "" + /// Whether this skill narrows at all. Absent and empty mean different things — + /// see `AgentSkill.allowedToolNames` — so the switch is real state, not `tools.isEmpty`. + var narrowsTools = false + var editingBuiltIn = false + } + + var body: some View { + VStack(alignment: .leading, spacing: 10) { + header + + ForEach(store.skills) { skill in + if editingID == skill.id { + form + } else { + row(for: skill) + } + } + + if isAdding { + form + } else if editingID == nil { + HStack(spacing: 12) { + Button("Add a skill…") { beginAdding() } + .buttonStyle(.link) + Button("Import…") { importSkills() } + .buttonStyle(.link) + } + .font(.callout) + } + } + } + + private var header: some View { + VStack(alignment: .leading, spacing: 2) { + Text("Skills").font(.headline) + Text( + "A skill is a set of instructions you can invoke by name from either composer. " + + "Logue's own rules still apply; a skill is added on top of them, and can only " + + "narrow which tools the agent may use." + ) + .font(.caption) + .foregroundStyle(.secondary) + } + } + + // MARK: - A skill + + private func row(for skill: AgentSkill) -> some View { + HStack(alignment: .firstTextBaseline, spacing: 10) { + VStack(alignment: .leading, spacing: 2) { + HStack(spacing: 6) { + Text(skill.title).font(.callout) + Text("/\(skill.invocation)") + .font(.system(size: 10, design: .monospaced)) + .foregroundStyle(.tertiary) + if skill.isBuiltIn { + Text("built-in") + .font(.system(size: 9)) + .padding(.horizontal, 5) + .padding(.vertical, 1) + .background(Capsule().fill(Color.secondary.opacity(0.15))) + .foregroundStyle(.secondary) + } + } + if !skill.summary.isEmpty { + Text(skill.summary).font(.caption).foregroundStyle(.secondary) + } + Text(toolSummary(for: skill)) + .font(.caption2) + .foregroundStyle(.tertiary) + } + + Spacer() + + Button("Edit") { beginEditing(skill) } + .buttonStyle(.link) + .font(.caption) + .accessibilityLabel("Edit \(skill.title)") + Button("Export") { SkillTransfer.export(skill) } + .buttonStyle(.link) + .font(.caption) + .accessibilityLabel("Export \(skill.title)") + if store.overriddenBuiltInIDs.contains(skill.id) { + Button("Restore") { store.restore(id: skill.id) } + .buttonStyle(.link) + .font(.caption) + .accessibilityLabel("Restore the built-in \(skill.title)") + } else if !skill.isBuiltIn { + Button("Remove") { store.remove(id: skill.id) } + .buttonStyle(.link) + .font(.caption) + .foregroundStyle(AppThemeConstants.error) + .accessibilityLabel("Remove \(skill.title)") + } + } + .padding(.vertical, 4) + } + + /// What a skill's tool list means, in words. + /// + /// The three states read very differently and are easy to confuse in a list: not + /// narrowing at all, narrowing to some, and narrowing to none. + private func toolSummary(for skill: AgentSkill) -> String { + guard let allowed = skill.allowedToolNames else { return "Any tool the agent has" } + return allowed.isEmpty ? "No tools" : "Only: \(allowed.joined(separator: ", "))" + } + + // MARK: - The form + + private var form: some View { + VStack(alignment: .leading, spacing: 8) { + if draft.editingBuiltIn { + Text("Saving keeps your version and hides the built-in. Restore brings it back.") + .font(.caption) + .foregroundStyle(.secondary) + } + + TextField("Name", text: $draft.title) + .textFieldStyle(.roundedBorder) + Text("Invoked as /\(SkillName.invocation(from: draft.title))") + .font(.system(size: 10, design: .monospaced)) + .foregroundStyle(.tertiary) + + TextField("What it is for", text: $draft.summary) + .textFieldStyle(.roundedBorder) + + TextEditor(text: $draft.instructions) + .font(.body) + .frame(minHeight: 120) + .overlay( + RoundedRectangle(cornerRadius: 6) + .stroke(Color.secondary.opacity(0.25), lineWidth: 0.5) + ) + .accessibilityLabel("Instructions") + + Toggle("Limit which tools this skill may use", isOn: $draft.narrowsTools) + .font(.callout) + if draft.narrowsTools { + TextField("get_document, rephrase_text", text: $draft.tools) + .textFieldStyle(.roundedBorder) + Text( + "Only these, and only if they are already available. A skill can take tools " + + "away; it can never add one you have turned off." + ) + .font(.caption) + .foregroundStyle(.secondary) + } + + if let rejection { + Text(rejection.message) + .font(.caption) + .foregroundStyle(AppThemeConstants.error) + } + + HStack { + Button("Cancel") { cancel() } + Button(editingID == nil ? "Add" : "Save") { commit() } + .buttonStyle(.borderedProminent) + .disabled(draft.title.trimmingCharacters(in: .whitespaces).isEmpty) + } + } + .padding(.vertical, 4) + } + + // MARK: - Doing things + + private func beginAdding() { + editingID = nil + rejection = nil + draft = Draft() + isAdding = true + } + + private func beginEditing(_ skill: AgentSkill) { + isAdding = false + rejection = nil + draft = Draft( + title: skill.title, + summary: skill.summary, + instructions: skill.instructions, + tools: skill.allowedToolNames?.joined(separator: ", ") ?? "", + narrowsTools: skill.allowedToolNames != nil, + editingBuiltIn: skill.isBuiltIn + ) + editingID = skill.id + } + + private func cancel() { + isAdding = false + editingID = nil + rejection = nil + draft = Draft() + } + + private func commit() { + let skill = AgentSkill( + id: editingID ?? UUID(), + title: draft.title, + summary: draft.summary, + instructions: draft.instructions, + allowedToolNames: draft.narrowsTools ? parsedTools : nil + ) + let result = editingID == nil ? store.add(skill) : store.update(skill) + switch result { + case .success: + cancel() + case let .failure(reason): + // Kept open with what was typed still in it. The name rules are the store's, and + // this reports them rather than keeping a second copy that can disagree. + rejection = reason + } + } + + /// The tool list as typed, split and cleaned. + /// + /// Commas or whitespace, because someone will use either, and a list that only accepts + /// one of them is a list that silently drops half of what was typed. + private var parsedTools: [String] { + draft.tools + .split(whereSeparator: { $0 == "," || $0.isWhitespace }) + .map { String($0) } + .filter { !$0.isEmpty } + } + + private func importSkills() { + Task { await SkillTransfer.importSkills(into: store) } + } +} diff --git a/Logue/Views/Settings/Tabs/AISettingsTab+Tools.swift b/Logue/Views/Settings/Tabs/AISettingsTab+Tools.swift new file mode 100644 index 0000000..06c0e67 --- /dev/null +++ b/Logue/Views/Settings/Tabs/AISettingsTab+Tools.swift @@ -0,0 +1,120 @@ +import SwiftUI + +/// The per-tool switches, and the list of what can be switched. +/// +/// Split out of `AISettingsTab` to keep its body inside the 450-line cap — the group list is +/// static data rather than logic, which is exactly the kind of thing the project's +/// extension-file rule exists to move. Nothing about the switches changed with the move. +/// +/// Extension-visible: `toolsSection` is mounted by the tab's `body` in the core file. +extension AISettingsTab { + static let toolGroups: [(title: String, items: [(String, String)])] = [ + ("Read — meetings & documents", [ + ("list_meetings", "Browse meetings"), + ("search_meetings", "Keyword search meetings"), + ("semantic_search_meetings", "Concept search meetings"), + ("get_meeting_details", "Read a meeting"), + ("get_transcript", "Read full transcript"), + ("get_action_items", "Read action items"), + ("get_daily_digest", "Daily activity digest"), + ("list_documents", "Browse documents"), + ("search_documents", "Keyword search documents"), + ("semantic_search_documents", "Concept search documents"), + ("get_document", "Read a document"), + ]), + ("Write — content", [ + ("create_document", "Create a document"), + ("update_document", "Edit a document"), + ("delete_document", "Delete a document"), + ("move_document", "Move a document"), + ("add_document_tag", "Tag a document"), + ("create_space", "Create a space"), + ("rename_space", "Rename a space"), + ("delete_space", "Delete a space"), + ("export_document_pdf", "Export a document as PDF"), + ]), + ("Calendar & Reminders", [ + ("get_upcoming_events", "List upcoming events"), + ("create_calendar_event", "Create event"), + ("update_calendar_event", "Update event"), + ("delete_calendar_event", "Delete event"), + ("get_reminders", "List reminders"), + ("add_reminder", "Add reminder"), + ("update_reminder", "Update reminder"), + ("delete_reminder", "Delete reminder"), + ]), + ("AI helpers", [ + ("summarize_document", "Summarize"), + ("rephrase_text", "Rephrase"), + ("check_grammar", "Grammar check"), + ("check_clarity", "Clarity check"), + ("detect_tone", "Tone detect"), + ("fact_check_document", "Fact-check"), + ("detect_pii", "PII detect"), + ("render_diagram", "Render diagram"), + ("generate_slides", "Generate slide deck"), + ]), + ("Apple-native", [ + ("draft_email", "Draft email in Mail"), + ("fetch_contacts", "Look up contacts"), + ("get_location", "Get current location"), + ]), + ("Compute & dialogs", [ + ("run_javascript", "Run JavaScript"), + ("get_confirmation", "Yes/no dialog"), + ("get_text_input", "Text input dialog"), + ("get_user_selection", "Pick-one dialog"), + ]), + ("Files (Phase G)", [ + ("list_directory", "List directory contents"), + ("read_file_at_path", "Read file at path"), + ("write_text_to_file", "Write text to file"), + ("delete_file_at_path", "Delete file at path"), + ]), + ] + + var toolsSection: some View { + VStack(alignment: .leading, spacing: 8) { + sectionHeader("Tools", subtitle: "Turn off any tool you don't want the agent to call. Toggling takes effect on the next message.") + ForEach(Self.toolGroups, id: \.title) { group in + DisclosureGroup(group.title) { + VStack(alignment: .leading, spacing: 4) { + ForEach(group.items, id: \.0) { item in + toolRow(name: item.0, label: item.1) + } + } + .padding(.leading, 8) + .padding(.top, 4) + } + .font(.callout.weight(.medium)) + } + } + } + + private func toolRow(name: String, label: String) -> some View { + let isEnabled = !disabledTools.contains(name) + return Toggle(isOn: Binding( + get: { isEnabled }, + set: { newValue in setTool(name, enabled: newValue) } + )) { + HStack { + Text(label).font(.callout) + Spacer() + Text(name) + .font(.system(size: 10, design: .monospaced)) + .foregroundStyle(.tertiary) + } + } + .toggleStyle(.switch) + } + + private func setTool(_ name: String, enabled: Bool) { + var current = disabledTools + if enabled { + current.remove(name) + } else { + current.insert(name) + } + disabledToolsRaw = current.sorted().joined(separator: ",") + } +} diff --git a/Logue/Views/Settings/Tabs/AISettingsTab.swift b/Logue/Views/Settings/Tabs/AISettingsTab.swift index a5f07bb..357c577 100644 --- a/Logue/Views/Settings/Tabs/AISettingsTab.swift +++ b/Logue/Views/Settings/Tabs/AISettingsTab.swift @@ -21,6 +21,10 @@ struct AISettingsTab: View { // tab would suggest they are a different kind of thing. MCPServersSection() Divider() + // Next to the tools, because a skill's other half is which tools it narrows + // to — and the list above is where those names come from. + SkillsSection() + Divider() inferenceSection Divider() memorySection @@ -72,124 +76,16 @@ struct AISettingsTab: View { // MARK: - Per-tool enable/disable @AppStorage(AppConstants.UserDefaultsKeys.disabledAgentTools) - private var disabledToolsRaw: String = "" + // Extension-visible: +Tools + var disabledToolsRaw: String = "" - private var disabledTools: Set { + // Extension-visible: +Tools + var disabledTools: Set { Set(disabledToolsRaw.split(separator: ",") .map { $0.trimmingCharacters(in: .whitespacesAndNewlines) } .filter { !$0.isEmpty }) } - private static let toolGroups: [(title: String, items: [(String, String)])] = [ - ("Read — meetings & documents", [ - ("list_meetings", "Browse meetings"), - ("search_meetings", "Keyword search meetings"), - ("semantic_search_meetings", "Concept search meetings"), - ("get_meeting_details", "Read a meeting"), - ("get_transcript", "Read full transcript"), - ("get_action_items", "Read action items"), - ("get_daily_digest", "Daily activity digest"), - ("list_documents", "Browse documents"), - ("search_documents", "Keyword search documents"), - ("semantic_search_documents", "Concept search documents"), - ("get_document", "Read a document"), - ]), - ("Write — content", [ - ("create_document", "Create a document"), - ("update_document", "Edit a document"), - ("delete_document", "Delete a document"), - ("move_document", "Move a document"), - ("add_document_tag", "Tag a document"), - ("create_space", "Create a space"), - ("rename_space", "Rename a space"), - ("delete_space", "Delete a space"), - ("export_document_pdf", "Export a document as PDF"), - ]), - ("Calendar & Reminders", [ - ("get_upcoming_events", "List upcoming events"), - ("create_calendar_event", "Create event"), - ("update_calendar_event", "Update event"), - ("delete_calendar_event", "Delete event"), - ("get_reminders", "List reminders"), - ("add_reminder", "Add reminder"), - ("update_reminder", "Update reminder"), - ("delete_reminder", "Delete reminder"), - ]), - ("AI helpers", [ - ("summarize_document", "Summarize"), - ("rephrase_text", "Rephrase"), - ("check_grammar", "Grammar check"), - ("check_clarity", "Clarity check"), - ("detect_tone", "Tone detect"), - ("fact_check_document", "Fact-check"), - ("detect_pii", "PII detect"), - ("render_diagram", "Render diagram"), - ("generate_slides", "Generate slide deck"), - ]), - ("Apple-native", [ - ("draft_email", "Draft email in Mail"), - ("fetch_contacts", "Look up contacts"), - ("get_location", "Get current location"), - ]), - ("Compute & dialogs", [ - ("run_javascript", "Run JavaScript"), - ("get_confirmation", "Yes/no dialog"), - ("get_text_input", "Text input dialog"), - ("get_user_selection", "Pick-one dialog"), - ]), - ("Files (Phase G)", [ - ("list_directory", "List directory contents"), - ("read_file_at_path", "Read file at path"), - ("write_text_to_file", "Write text to file"), - ("delete_file_at_path", "Delete file at path"), - ]), - ] - - private var toolsSection: some View { - VStack(alignment: .leading, spacing: 8) { - sectionHeader("Tools", subtitle: "Turn off any tool you don't want the agent to call. Toggling takes effect on the next message.") - ForEach(Self.toolGroups, id: \.title) { group in - DisclosureGroup(group.title) { - VStack(alignment: .leading, spacing: 4) { - ForEach(group.items, id: \.0) { item in - toolRow(name: item.0, label: item.1) - } - } - .padding(.leading, 8) - .padding(.top, 4) - } - .font(.callout.weight(.medium)) - } - } - } - - private func toolRow(name: String, label: String) -> some View { - let isEnabled = !disabledTools.contains(name) - return Toggle(isOn: Binding( - get: { isEnabled }, - set: { newValue in setTool(name, enabled: newValue) } - )) { - HStack { - Text(label).font(.callout) - Spacer() - Text(name) - .font(.system(size: 10, design: .monospaced)) - .foregroundStyle(.tertiary) - } - } - .toggleStyle(.switch) - } - - private func setTool(_ name: String, enabled: Bool) { - var current = disabledTools - if enabled { - current.remove(name) - } else { - current.insert(name) - } - disabledToolsRaw = current.sorted().joined(separator: ",") - } - // MARK: - Inference params @AppStorage(AppConstants.UserDefaultsKeys.inferenceTemperature) @@ -521,7 +417,8 @@ struct AISettingsTab: View { // MARK: - Helpers - private func sectionHeader(_ title: String, subtitle: String? = nil) -> some View { + // Extension-visible: +Tools + func sectionHeader(_ title: String, subtitle: String? = nil) -> some View { VStack(alignment: .leading, spacing: 2) { Text(title).font(.headline) if let subtitle { diff --git a/LogueTests/SkillTransferTests.swift b/LogueTests/SkillTransferTests.swift new file mode 100644 index 0000000..027e18a --- /dev/null +++ b/LogueTests/SkillTransferTests.swift @@ -0,0 +1,62 @@ +import Testing +@testable import Logue + +/// What an import says happened. +/// +/// The counts are the easy thing to get wrong here — "1 skills" is the kind of detail that +/// nobody notices until it ships, and "imported 0" reading the same as "imported nothing at +/// all" is the kind that makes a user press the button again. +@Suite("Skill transfer") +@MainActor +struct SkillTransferTests { + @Test("Everything landing says so plainly") + func allImported() { + #expect(SkillTransfer.summary(taken: 3, chosen: 3) == "Imported 3 skills.") + } + + @Test("One is singular") + func singular() { + #expect(SkillTransfer.summary(taken: 1, chosen: 1) == "Imported 1 skill.") + #expect(SkillTransfer.summary(taken: 0, chosen: 1) == "Nothing could be imported from 1 file.") + } + + @Test("A partial import says how many of how many") + func partialImport() { + // The number that matters is what did *not* arrive — a bare "imported 2" beside a + // selection of five reads as success. + #expect(SkillTransfer.summary(taken: 2, chosen: 5) == "Imported 2 skills of 5.") + } + + @Test("Nothing landing is said out loud") + func nothingImported() { + // An import that quietly does nothing is indistinguishable from one that worked. + #expect(SkillTransfer.summary(taken: 0, chosen: 4) == "Nothing could be imported from 4 files.") + } + + @Test("Every count reads as a sentence, at every size") + func everyCountIsWellFormed() { + for chosen in 0 ... 12 { + for taken in 0 ... chosen { + let text = SkillTransfer.summary(taken: taken, chosen: chosen) + #expect(text.hasSuffix("."), "not a sentence: \(text)") + #expect(text.contains(" 1 skills") == false, "bad plural: \(text)") + #expect(text.contains(" 1 files") == false, "bad plural: \(text)") + } + } + } + + @Test("A skill file can be read back out of what export writes") + func exportRoundTrips() throws { + // `export` writes `SkillFile.render`; this is the pairing that makes sharing work, + // asserted without touching a panel or the filesystem. + let skill = AgentSkill( + title: "Weekly Review", + summary: "s", + instructions: "Body.", + allowedToolNames: ["get_document"] + ) + let parsed = try #require(SkillFile.parse(SkillFile.render(skill))) + #expect(parsed.title == skill.title) + #expect(parsed.allowedToolNames == skill.allowedToolNames) + } +} From f625a0588e6d6e0e1fd90ffee99f4dfa38292012 Mon Sep 17 00:00:00 2001 From: shanforge <299346208+shanforge@users.noreply.github.com> Date: Fri, 4 Sep 2026 13:35:12 +0530 Subject: [PATCH 2/2] fix: say when a skill is longer than what gets used MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **Instructions past the prompt budget were stored and never sent, with nothing saying so.** Only `AgentSkill.maxInstructionCharacters` of a skill ever reaches the model — it is layered on top of the system prompt and everything else the turn needs, so it spends context the conversation would otherwise have. Paste a long document into a skill and it saves, appears complete, and the agent quietly acts on the first part of it. The way you find out is an answer that ignored half of what you wrote. The count is now said while it is still editable, and the two limits are distinguished because they mean different things: past the prompt budget the extra is saved but never used; past the file limit it will not save at all — refused rather than silently cut, since losing what someone typed on save is the one failure here that cannot be undone. Verified: build succeeds, 1877 tests in 170 suites pass, SwiftFormat --lint clean over 573 files, SwiftLint --strict 0 violations in 732 files. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01M3Wpnj9ZmWPKYdFPB1AVY3 --- Logue/Views/Settings/SkillsSection.swift | 40 +++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) diff --git a/Logue/Views/Settings/SkillsSection.swift b/Logue/Views/Settings/SkillsSection.swift index cad9412..e89b979 100644 --- a/Logue/Views/Settings/SkillsSection.swift +++ b/Logue/Views/Settings/SkillsSection.swift @@ -160,6 +160,18 @@ struct SkillsSection: View { ) .accessibilityLabel("Instructions") + // Said while it is still editable, not discovered later. Only a bounded amount + // of a skill ever reaches the model — the rest is stored and never used, which + // is the kind of thing you would otherwise find out from an answer that ignored + // half of what you wrote. + if let notice = lengthNotice { + Text(notice) + .font(.caption) + .foregroundStyle(draft.instructions.count > SkillFile.maxBodyCharacters + ? AppThemeConstants.error + : .secondary) + } + Toggle("Limit which tools this skill may use", isOn: $draft.narrowsTools) .font(.callout) if draft.narrowsTools { @@ -183,14 +195,40 @@ struct SkillsSection: View { Button("Cancel") { cancel() } Button(editingID == nil ? "Add" : "Save") { commit() } .buttonStyle(.borderedProminent) - .disabled(draft.title.trimmingCharacters(in: .whitespaces).isEmpty) + .disabled(!canCommit) } } .padding(.vertical, 4) } + /// What to say about how long the instructions are. + /// + /// Two different limits, and they mean different things. Past + /// `AgentSkill.maxInstructionCharacters` the extra is *stored but never sent* — a skill + /// is layered on top of the system prompt and everything else the turn needs, so it + /// spends context the conversation would otherwise have. Past + /// `SkillFile.maxBodyCharacters` it will not save at all. + private var lengthNotice: String? { + let count = draft.instructions.count + if count > SkillFile.maxBodyCharacters { + return "Too long to save — \(count) characters, and the limit is \(SkillFile.maxBodyCharacters)." + } + if count > AgentSkill.maxInstructionCharacters { + return "Only the first \(AgentSkill.maxInstructionCharacters) characters are sent to the model. " + + "The rest is saved but never used." + } + return nil + } + // MARK: - Doing things + /// Refuses rather than silently cutting: losing what someone typed on save is the one + /// failure here that cannot be undone. + private var canCommit: Bool { + !draft.title.trimmingCharacters(in: .whitespaces).isEmpty + && draft.instructions.count <= SkillFile.maxBodyCharacters + } + private func beginAdding() { editingID = nil rejection = nil