diff --git a/Logue.xcodeproj/project.pbxproj b/Logue.xcodeproj/project.pbxproj index 5904594..1fbdc6c 100644 --- a/Logue.xcodeproj/project.pbxproj +++ b/Logue.xcodeproj/project.pbxproj @@ -148,6 +148,7 @@ 2912A07E0CA39107DCE54499 /* DocumentPersistenceSplitTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 0E1246768E9657B7146D7062 /* DocumentPersistenceSplitTests.swift */; }; 2944C84805092418E45892EA /* MCPCatalog.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A0EC77C2B3E1C387145AC15 /* MCPCatalog.swift */; }; 294DE2CCA2CF984F9088FA89 /* SpaceFile.swift in Sources */ = {isa = PBXBuildFile; fileRef = B612ADDBC4E7FB7917F6D72A /* SpaceFile.swift */; }; + 2973EEEA0E69B58153B61738 /* SkillInvocation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 32016A19FB7B3A19A78DF890 /* SkillInvocation.swift */; }; 2AADC5238C76ED6A02544A34 /* AgentConversationListView.swift in Sources */ = {isa = PBXBuildFile; fileRef = 92DB0F899AF6F08FEAF8AA46 /* AgentConversationListView.swift */; }; 2AF2DBAEFCA1E332ECBA2A1D /* DeviceLossPolicyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 17F31B485FED361E8702AA2D /* DeviceLossPolicyTests.swift */; }; 2AFCB1DEDD52C307A5201BC7 /* UpcomingEventCard.swift in Sources */ = {isa = PBXBuildFile; fileRef = 510929670B711C38C5D0F08F /* UpcomingEventCard.swift */; }; @@ -444,6 +445,7 @@ 8E93C898597CE89098CAA75E /* MessageActions.swift in Sources */ = {isa = PBXBuildFile; fileRef = B55FA53C32F4D2968C07CEE5 /* MessageActions.swift */; }; 8F052589F2A1FE3183EC5F56 /* TranscriptSentenceMerge.swift in Sources */ = {isa = PBXBuildFile; fileRef = 52B96B5DBA01C6D2AC2AFD19 /* TranscriptSentenceMerge.swift */; }; 900D62D26E62CC1BD86F6152 /* AudioTimelineMixer.swift in Sources */ = {isa = PBXBuildFile; fileRef = 2CCEAA072E5CE2EDA0828E14 /* AudioTimelineMixer.swift */; }; + 90A49AA4762ECCB5320A9EBF /* SkillInvocationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = A7BC3FCFA1A0116B83C06470 /* SkillInvocationTests.swift */; }; 90CE4DBE3D79C429616DB00A /* AgentConversation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 160A71994367176C72C11277 /* AgentConversation.swift */; }; 91504F26FF724A765B016AF2 /* whatsnew-wikilinks.png in Resources */ = {isa = PBXBuildFile; fileRef = F7E1089CE3E964A5BC0545A8 /* whatsnew-wikilinks.png */; }; 9179F7B5B38B3E68AB712CC5 /* MCPServersSection.swift in Sources */ = {isa = PBXBuildFile; fileRef = 407A90F8E61C4E0576B95ADD /* MCPServersSection.swift */; }; @@ -914,6 +916,7 @@ 31ED31FAD045668E64BA99B1 /* LLMTestEvalExtensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LLMTestEvalExtensions.swift; sourceTree = ""; }; 31F21D2EC731B88B30A3B044 /* TimelineAudioConversionTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TimelineAudioConversionTests.swift; sourceTree = ""; }; 31F56338AE354AB4607F3597 /* JavaScriptTool.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = JavaScriptTool.swift; sourceTree = ""; }; + 32016A19FB7B3A19A78DF890 /* SkillInvocation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SkillInvocation.swift; sourceTree = ""; }; 3224E9FBD3E9D86A60DAA79C /* LLMEngineStatus.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LLMEngineStatus.swift; sourceTree = ""; }; 33AE1CBE332B50E515AAA7EE /* RecordingSessionManager+Recovery.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "RecordingSessionManager+Recovery.swift"; sourceTree = ""; }; 3423F242140F7A2AE832B0FD /* DocumentFilenameTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentFilenameTests.swift; sourceTree = ""; }; @@ -1268,6 +1271,7 @@ A6440E04631318411820648E /* MCPWireFormat.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MCPWireFormat.swift; sourceTree = ""; }; A6BFFF957D5FFFEB3DD160BF /* MeetingListView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MeetingListView.swift; sourceTree = ""; }; A7BB6836738E8815F0F59EBE /* WriteDocumentTools.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = WriteDocumentTools.swift; sourceTree = ""; }; + A7BC3FCFA1A0116B83C06470 /* SkillInvocationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SkillInvocationTests.swift; sourceTree = ""; }; A81080B89B0F0AC01017D6D7 /* ModelManager+HuggingFace.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "ModelManager+HuggingFace.swift"; sourceTree = ""; }; A81121A106491181334F7EFE /* TranscriptReplacement.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = TranscriptReplacement.swift; sourceTree = ""; }; A864D7C5937FF77CB9835242 /* DocumentListContentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DocumentListContentView.swift; sourceTree = ""; }; @@ -1712,6 +1716,7 @@ 398EDF0A083883200C30D278 /* SidebarSelectionMigrationTests.swift */, 5EC0BFF05793C2090F8A90C1 /* SidebarWidthLimitTests.swift */, 4C9B5A0EF0F9DF7AB61FB25C /* SkillExecutionTests.swift */, + A7BC3FCFA1A0116B83C06470 /* SkillInvocationTests.swift */, 3B10ECDC2A200BF826E8AF38 /* SkillModelTests.swift */, DA05DBB227EC6FF0BA790263 /* SkillStoreTests.swift */, 20A9DCED765C659BAD6BA11C /* SortformerTimelineTests.swift */, @@ -2456,6 +2461,7 @@ 23BA15F0B350AB9FBBC3AC7E /* AgentSkill.swift */, FAFDB80BDB51BE96C075692A /* SkillCatalog.swift */, 5EA9FF274B62CED370A907AD /* SkillFile.swift */, + 32016A19FB7B3A19A78DF890 /* SkillInvocation.swift */, F5FA7A75D8EF4325E4699083 /* SkillLayering.swift */, 965FEFAD362D49C3EE00089C /* SkillName.swift */, B33D38585FFCB1764C45F7DF /* SkillStore.swift */, @@ -2997,6 +3003,7 @@ D5DE12695588CC6B29787019 /* SidebarSelectionMigrationTests.swift in Sources */, F5591335FF5786E244D72E2E /* SidebarWidthLimitTests.swift in Sources */, 8B9B5EE514FCA092639CC439 /* SkillExecutionTests.swift in Sources */, + 90A49AA4762ECCB5320A9EBF /* SkillInvocationTests.swift in Sources */, 595BE98FA71A505D0DB5DCB8 /* SkillModelTests.swift in Sources */, 2752CE3A9D504B735C0F4333 /* SkillStoreTests.swift in Sources */, 480E2F2ECBABB70296373573 /* SortformerTimelineTests.swift in Sources */, @@ -3490,6 +3497,7 @@ 2E44F9F45806D57C7D99BC21 /* SkeletonView.swift in Sources */, A449E25873E30D531486C971 /* SkillCatalog.swift in Sources */, 7C21B5457EDE7D56A9E81F41 /* SkillFile.swift in Sources */, + 2973EEEA0E69B58153B61738 /* SkillInvocation.swift in Sources */, A55E8790B32F7788B503FFFC /* SkillLayering.swift in Sources */, AF02C63201E3C3ED826BF85B /* SkillName.swift in Sources */, 23A3C37522CE6FD3D4B231AC /* SkillStore.swift in Sources */, diff --git a/Logue/Agent/AgentCoordinator.swift b/Logue/Agent/AgentCoordinator.swift index 962b033..7181657 100644 --- a/Logue/Agent/AgentCoordinator.swift +++ b/Logue/Agent/AgentCoordinator.swift @@ -250,20 +250,26 @@ final class AgentCoordinator { } } - func sendWithoutAppendingUser(conversationID: UUID, oneShotWebSearch: Bool = false) { + func sendWithoutAppendingUser( + conversationID: UUID, + oneShotWebSearch: Bool = false, + skill: AgentSkill? = nil + ) { guard !isProcessingAnyConversation else { return } run.dismissError() processingTask?.cancel() if oneShotWebSearch { setOneShotIncludeWebTools(true) } + let skillRun = setActiveSkill(skill) processingTask = Task { [weak self] in guard let self else { return } defer { - if oneShotWebSearch { - Task { @MainActor [weak self] in + Task { @MainActor [weak self] in + if oneShotWebSearch { self?.setOneShotIncludeWebTools(false) } + self?.clearActiveSkill(generation: skillRun) } } await runGraph(conversationID: conversationID) diff --git a/Logue/Agent/Skills/SkillInvocation.swift b/Logue/Agent/Skills/SkillInvocation.swift new file mode 100644 index 0000000..b07914f --- /dev/null +++ b/Logue/Agent/Skills/SkillInvocation.swift @@ -0,0 +1,131 @@ +import Foundation + +/// Reading a skill invocation out of what the user typed. +/// +/// #64 says a skill is invoked **by name**, so typing it has to work — a picker alone means +/// the fastest way to run a saved instruction is to reach for the mouse. `/weekly-review do +/// the last week` runs that skill on "do the last week". +/// +/// Pure, and not a method on either composer, for the reason `AskRouter` is: a decision made +/// inside a `View` is one the other surface cannot reach, and this one has to be identical +/// on both. `AskSurface` is deliberately not an input. +enum SkillInvocation { + /// The character that starts an invocation. + static let marker: Character = "/" + + /// What the text turned out to be. + enum Outcome: Equatable { + /// No invocation was attempted. The text is the message. + case none(text: String) + /// A skill was named and found. The text is what remains after the name. + case invoked(skill: AgentSkill, text: String) + /// A skill was named and not found. + /// + /// Deliberately its own case rather than falling back to `.none`. Someone who typed + /// `/weekly-reveiw` meant to run something; sending it to the model as an ordinary + /// message produces a confident answer to a question they did not ask, and nothing + /// anywhere says the skill did not run. The composer refuses and names what it tried. + case unknown(name: String) + } + + /// Reads `text` as a possible invocation. + /// + /// - Parameter skills: everything invocable, in the order the store lists it. + static func resolve(_ text: String, against skills: [AgentSkill]) -> Outcome { + let trimmed = text.trimmingCharacters(in: .whitespacesAndNewlines) + guard trimmed.first == marker else { return .none(text: text) } + + let afterMarker = trimmed.dropFirst() + // The name ends at the first whitespace; everything after it is the message. + let split = afterMarker.firstIndex(where: { $0.isWhitespace }) + let rawName = String(afterMarker[afterMarker.startIndex ..< (split ?? afterMarker.endIndex)]) + let remainder = split.map { String(afterMarker[afterMarker.index(after: $0)...]) } ?? "" + + // A bare "/" is someone who has started typing, not a failed invocation. Treating it + // as unknown would put an error under the cursor before they had finished the word. + guard !rawName.isEmpty else { return .none(text: text) } + + let wanted = SkillName.invocation(from: rawName) + guard let skill = skills.first(where: { $0.invocation == wanted }) else { + return .unknown(name: rawName) + } + return .invoked(skill: skill, text: remainder.trimmingCharacters(in: .whitespacesAndNewlines)) + } + + /// What a composer should actually do with what was typed. + /// + /// The whole decision in one place: read the name, pick between a typed name and an + /// armed chip, and settle whether a skill applies to this route at all. Both composers + /// call this and neither re-derives any of it — the precedence rule written twice is the + /// per-surface drift the rest of #61 exists to stop. + enum Turn: Equatable { + /// Send it. `skill` is nil when none applies. + case send(skill: AgentSkill?, message: String) + /// Do not send. Show this. + case refuse(reason: String) + } + + /// - Parameters: + /// - armed: the skill armed by the chip, if any. + /// - route: where this send is going. A skill only applies to the agent loop. + static func turn( + for text: String, + armed: AgentSkill?, + route: AskRoute, + in skills: [AgentSkill] + ) -> Turn { + let message: String + let skill: AgentSkill? + + switch resolve(text, against: skills) { + case let .unknown(name): + return .refuse(reason: unknownMessage(name: name)) + case let .invoked(found, remainder): + // A typed name wins over an armed chip: it is the more specific instruction, and + // the one under the cursor as Return is pressed. + (skill, message) = (found, remainder) + case let .none(plain): + (skill, message) = (armed, plain) + } + + // A skill layers onto the agent's system prompt and narrows the agent's tools — + // neither of which a Deep Research run or an image generation has. Dropping it + // silently would be the same failure as answering an unknown name as a plain + // message: the user asked for something and got something else, with nothing saying + // so. Refused, and named. + if let skill, route != .agentLoop { + return .refuse(reason: doesNotApplyMessage(skill: skill, route: route)) + } + return .send(skill: skill, message: message) + } + + /// Why a skill was not run for this send. + static func doesNotApplyMessage(skill: AgentSkill, route: AskRoute) -> String { + let what = switch route { + case .deepResearch: "Deep Research" + case .imagePlayground: "image generation" + case .agentLoop: "this" + } + return "“\(SkillName.title(from: skill.title))” can't be used with \(what). " + + "Turn one of them off and send again." + } + + /// What to say when a name matched nothing. + /// + /// Names the thing that was tried. "No such skill" leaves the user checking whether they + /// mistyped the name or never made the skill. + static func unknownMessage(name: String) -> String { + "No skill called “\(DisplayText.clamp(DisplayText.singleLine(name), to: 40))”. " + + "Type / to see the ones you have." + } + + /// Skills whose invocation begins with a partly-typed name, for a completion list. + /// + /// Ordered by the store, not by closeness of match: the list must not reshuffle under + /// the user as they type, because they are aiming at a position in it. + static func completions(for partial: String, in skills: [AgentSkill]) -> [AgentSkill] { + let wanted = SkillName.invocation(from: partial) + guard !wanted.isEmpty else { return skills } + return skills.filter { $0.invocation.hasPrefix(wanted) } + } +} diff --git a/Logue/UI/UICopy.swift b/Logue/UI/UICopy.swift index 43d3e5f..a023321 100644 --- a/Logue/UI/UICopy.swift +++ b/Logue/UI/UICopy.swift @@ -42,6 +42,7 @@ enum UICopy { static let addFiles = "Add photos & files" static let searchTheWeb = "Search the web" static let deepResearchMenu = "Deep research" + static let skills = "Skills" static let toolSettings = "Tool settings…" static let composerMenuHelp = "Attach, search, deep research…" static let composerMenuLabel = "More actions" diff --git a/Logue/Views/Agent/AgentChatView+Input.swift b/Logue/Views/Agent/AgentChatView+Input.swift index 105d8db..860af8d 100644 --- a/Logue/Views/Agent/AgentChatView+Input.swift +++ b/Logue/Views/Agent/AgentChatView+Input.swift @@ -36,6 +36,10 @@ extension AgentChatView { var isDeepResearch: Bool = false let isProcessing: Bool let isBusy: Bool + /// The skill armed for the next send. The parent owns it, because the parent is what + /// hands it to the coordinator — and because the island holds its own, so this must + /// not be shared state. + @Binding var armedSkill: AgentSkill? var onSend: () -> Void var onCancel: () -> Void @@ -148,13 +152,24 @@ extension AgentChatView { /// True when any per-send mode is on. Drives whether the chip row /// renders (turning the pill into a 2-row card). private var hasActiveModes: Bool { - isWebSearchOnce || isDeepResearch + isWebSearchOnce || isDeepResearch || armedSkill != nil } /// Horizontal row of chips for the modes currently on. Each chip has /// an `×` to turn that mode off without opening the + menu. private var activeModeChips: some View { HStack(spacing: 6) { + // Same category as a mode: what the send is about to *do*, so it is visible + // before it happens rather than discovered afterwards. + if let armedSkill { + ModeChip( + title: armedSkill.title, + systemImage: "wand.and.stars", + tint: AppThemeConstants.brandPrimary + ) { + self.armedSkill = nil + } + } if isWebSearchOnce { ModeChip( title: UICopy.Input.webSearch, @@ -207,7 +222,8 @@ extension AgentChatView { ComposerPlusMenu( surface: .mainWindow, isDisabled: isProcessing || isBusy, - onAttach: { openFilePicker() } + onAttach: { openFilePicker() }, + onPickSkill: { armedSkill = $0 } ) } diff --git a/Logue/Views/Agent/AgentChatView.swift b/Logue/Views/Agent/AgentChatView.swift index 6a3a40a..bf49023 100644 --- a/Logue/Views/Agent/AgentChatView.swift +++ b/Logue/Views/Agent/AgentChatView.swift @@ -13,6 +13,20 @@ struct AgentChatView: View { /// Incremented to pull focus into the input after a card fills it. @State private var focusRequest = 0 + /// The skill armed for the next send in this window. + /// + /// The main window's own — the island holds a separate one, so choosing a skill in one + /// does not arm it in the other, exactly as the one-shot modes behave. + @State private var armedSkill: AgentSkill? + + /// An error this window raised itself, as opposed to one a run left behind. + /// + /// The same shape the island uses, and for the same reason: a refused send never reaches + /// a coordinator, so there is no run to own the message and `lastError(in:)` — which is + /// scoped to a conversation — can only answer nil. Both surfaces show a refusal the same + /// way rather than one getting a banner and the other a toast. + @State private var localError: String? + // Injected by `MainWindowView`. Both are per-window state rather than a global rule, // which is why they stay in the environment: `insights` is derived and each surface // owns its own, and `modelManager` is read for what this window is showing. @@ -294,7 +308,7 @@ struct AgentChatView: View { } ) - if let error = coordinator.lastError(in: conversation.id) { + if let error = localError ?? coordinator.lastError(in: conversation.id) { errorBanner(error) .transition(.move(edge: .bottom).combined(with: .opacity)) } @@ -319,6 +333,7 @@ struct AgentChatView: View { attachments: $inputAttachments, isProcessing: isThisConversationProcessing || isThisConversationResearching, isBusy: isBusy, + armedSkill: $armedSkill, onSend: { let text = inputText.trimmingCharacters(in: .whitespacesAndNewlines) let attachments = inputAttachments @@ -339,22 +354,47 @@ struct AgentChatView: View { ) guard let route else { return } + // The same decision the island makes, from the same pure place — including + // which of a typed name and an armed chip wins, and whether a skill applies + // to this route at all. + let skillThisTurn: AgentSkill? + let messageThisTurn: String + switch SkillInvocation.turn( + for: text, + armed: armedSkill, + route: route, + in: SkillStore.shared.skills + ) { + case let .refuse(reason): + localError = reason + return + case let .send(skill, message): + (skillThisTurn, messageThisTurn) = (skill, message) + } + + localError = nil inputText = "" inputAttachments = [] // Reset the per-send AppStorage flags so the next turn starts // clean. These mirror the chip state in the input pill. isDeepResearch = false isWebSearchOnce = false + armedSkill = nil switch route { case .deepResearch: - startDeepResearch(text, oneShotWebSearch: oneShotWeb) + startDeepResearch(messageThisTurn, oneShotWebSearch: oneShotWeb) case let .imagePlayground(concept): HapticFeedback.send() imagePlaygroundConcept = concept showImagePlayground = true case .agentLoop: - sendMessage(text, attachments: attachments, oneShotWebSearch: oneShotWeb) + sendMessage( + messageThisTurn, + attachments: attachments, + oneShotWebSearch: oneShotWeb, + skill: skillThisTurn + ) } }, onCancel: { @@ -478,6 +518,9 @@ struct AgentChatView: View { Button { withAnimation { + // Both, because the banner shows either — dismissing one and leaving the + // other means the banner reappears the moment the view redraws. + localError = nil coordinator.dismissError() } } label: { @@ -511,7 +554,8 @@ struct AgentChatView: View { private func sendMessage( _ text: String, attachments: [TempAttachment] = [], - oneShotWebSearch: Bool = false + oneShotWebSearch: Bool = false, + skill: AgentSkill? = nil ) { // Routing happens at the send site, in `AskRouter`. This used to re-ask the // ImagePlayground question here, which meant two places could answer it @@ -535,7 +579,8 @@ struct AgentChatView: View { // `runGraph` so we don't need to pass them again here. coordinator.sendWithoutAppendingUser( conversationID: conversationID, - oneShotWebSearch: oneShotWebSearch + oneShotWebSearch: oneShotWebSearch, + skill: skill ) } diff --git a/Logue/Views/Agent/ComposerPlusMenu.swift b/Logue/Views/Agent/ComposerPlusMenu.swift index 6336ec7..9acb42d 100644 --- a/Logue/Views/Agent/ComposerPlusMenu.swift +++ b/Logue/Views/Agent/ComposerPlusMenu.swift @@ -63,6 +63,16 @@ struct ComposerPlusMenu: View { let onAttach: () -> Void let style: Style + /// Arms a skill for the next send. + /// + /// A closure rather than a binding to the store, because the *armed* skill is per + /// composer — the island and the main window each hold their own, the same way they hold + /// their own one-shot modes. Sharing it would arm a skill in one window by choosing it in + /// the other. + let onPickSkill: (AgentSkill) -> Void + + @State private var skills = SkillStore.shared + @AppStorage private var isWebSearchOnce: Bool @AppStorage private var isDeepResearchOnce: Bool @@ -70,11 +80,13 @@ struct ComposerPlusMenu: View { surface: Surface, isDisabled: Bool, style: Style = .mainWindow, - onAttach: @escaping () -> Void + onAttach: @escaping () -> Void, + onPickSkill: @escaping (AgentSkill) -> Void = { _ in } ) { self.isDisabled = isDisabled self.style = style self.onAttach = onAttach + self.onPickSkill = onPickSkill let keys = Self.keys(for: surface) _isWebSearchOnce = AppStorage(wrappedValue: false, keys.webSearch) _isDeepResearchOnce = AppStorage(wrappedValue: false, keys.deepResearch) @@ -99,6 +111,23 @@ struct ComposerPlusMenu: View { Divider() + // Mounted here rather than drawn per surface, which is the whole of #61's rule + // and the reason the island gets skills for free. The names double as what you + // can type: the menu is the discoverable half, `/name` the fast half. + Menu(UICopy.Input.skills) { + ForEach(skills.skills) { skill in + Button { + onPickSkill(skill) + } label: { + Text("\(skill.title) /\(skill.invocation)") + } + .help(skill.summary) + } + } + .disabled(skills.skills.isEmpty) + + Divider() + Button { AppDelegate.openToolSettings() } label: { diff --git a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift index 5c54f41..2891a38 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView+Composer.swift @@ -30,7 +30,8 @@ extension CommandCenterChatView { let picked = await AttachmentIntake.pickFiles() attachments = AttachmentIntake.merging(picked, into: attachments) } - } + }, + onPickSkill: { armedSkill = $0 } ) // Input field @@ -168,6 +169,15 @@ extension CommandCenterChatView { ) } + /// Whether anything is staged for the next send. + /// + /// Extension-visible: read by the view's `body`, which is in the core file. It lives here + /// because everything it asks about — the attachments, the modes, the armed skill — is + /// drawn by `stagedChips` just below. + var hasStagedChips: Bool { + !attachments.isEmpty || isWebSearchOnce || isDeepResearchOnce || armedSkill != nil + } + /// What is staged for the next send, with a way to take each one back off. /// /// Part of the island's layout rather than an overlay floating above the pill. As an @@ -244,6 +254,19 @@ extension CommandCenterChatView { /// booleans the row has to remember to add up. private var activeModes: [ComposerMode] { var modes: [ComposerMode] = [] + // A skill is a mode, not an attachment, so `ComposerChipRow` never hides it. What the + // send is about to *do* has to be visible; a hidden skill is a send the user did not + // know they were making, which is the same argument web search and Deep Research get. + if let armedSkill { + modes.append( + ComposerMode( + id: "skill", + title: armedSkill.title, + systemImage: "wand.and.stars", + tint: AppThemeConstants.brandPrimary + ) { self.armedSkill = nil } + ) + } if isDeepResearchOnce { modes.append( ComposerMode( diff --git a/Logue/Views/CrossApp/CommandCenterChatView.swift b/Logue/Views/CrossApp/CommandCenterChatView.swift index 0e0d748..d184885 100644 --- a/Logue/Views/CrossApp/CommandCenterChatView.swift +++ b/Logue/Views/CrossApp/CommandCenterChatView.swift @@ -52,6 +52,12 @@ struct CommandCenterChatView: View { @AppStorage(AppConstants.UserDefaultsKeys.islandOneShotDeepResearch) var isDeepResearchOnce: Bool = false // Extension-visible: +Composer + /// The skill armed for the next send, if any. + /// + /// The island's own, not the store's and not the main window's — arming a skill here must + /// not arm it over there, exactly as the one-shot modes are per surface. + @State var armedSkill: AgentSkill? + @State var deepResearch = DeepResearchCoordinator.shared // Extension-visible: +Composer /// Built the way `MainWindowView` builds its own, off the same singletons. The island is @@ -111,11 +117,6 @@ struct CommandCenterChatView: View { !rows.isEmpty } - /// Whether anything is staged for the next send. - private var hasStagedChips: Bool { - !attachments.isEmpty || isWebSearchOnce || isDeepResearchOnce - } - /// Whether this island owns a thread — drawn or not. /// /// The close button lives in `messagesPanel`, and the rules that refuse to dismiss the @@ -387,6 +388,8 @@ struct CommandCenterChatView: View { ) guard let route, !isGenerating else { return } + guard let turn = resolvedTurn(for: text, route: route) else { return } + // A new attempt supersedes the last refusal. Without this the "busy elsewhere" // banner outlives the condition that raised it and sits over a send that worked. localError = nil @@ -404,6 +407,9 @@ struct CommandCenterChatView: View { return } + let skillThisTurn = turn.skill + let messageThisTurn = turn.message + let conversationID = ensureConversation() let staged = attachments let searchThisTurn = isWebSearchOnce @@ -414,6 +420,7 @@ struct CommandCenterChatView: View { // for why they are not the main window's — and are cleared again in `.onAppear`. isWebSearchOnce = false isDeepResearchOnce = false + armedSkill = nil // Re-focus input after state update Task { @@ -424,10 +431,11 @@ struct CommandCenterChatView: View { switch route { case .agentLoop: coordinator.send( - message: text, + message: messageThisTurn, conversationID: conversationID, attachments: staged, - oneShotWebSearch: searchThisTurn + oneShotWebSearch: searchThisTurn, + skill: skillThisTurn ) case .deepResearch: // The same launch the main window uses, on the island's own thread — so the @@ -439,7 +447,7 @@ struct CommandCenterChatView: View { // dropped a file in gets it back on the pill rather than silently losing it. attachments = staged let started = deepResearch.start( - prompt: text, + prompt: messageThisTurn, in: conversationID, oneShotWebSearch: searchThisTurn ) @@ -458,7 +466,8 @@ struct CommandCenterChatView: View { message: concept, conversationID: conversationID, attachments: staged, - oneShotWebSearch: searchThisTurn + oneShotWebSearch: searchThisTurn, + skill: skillThisTurn ) } } @@ -675,6 +684,27 @@ struct CommandCenterChatView: View { isInputFocused = true } + /// What this send is actually asking, once a typed `/name` has been read out of it. + /// + /// Returns `nil` when the send must not go ahead. A name that matched nothing stops here: + /// someone who typed `/weekly-reveiw` meant to run something, and passing it on as an + /// ordinary message answers a question they did not ask while nothing anywhere says the + /// skill never ran. + private func resolvedTurn(for text: String, route: AskRoute) -> (skill: AgentSkill?, message: String)? { + switch SkillInvocation.turn( + for: text, + armed: armedSkill, + route: route, + in: SkillStore.shared.skills + ) { + case let .refuse(reason): + localError = reason + return nil + case let .send(skill, message): + return (skill, message) + } + } + /// Puts a refused send back exactly as it was. /// /// `sendMessage` clears the composer before it knows whether the send was accepted, so diff --git a/LogueTests/SkillInvocationTests.swift b/LogueTests/SkillInvocationTests.swift new file mode 100644 index 0000000..19c62da --- /dev/null +++ b/LogueTests/SkillInvocationTests.swift @@ -0,0 +1,197 @@ +import Foundation +import Testing +@testable import Logue + +/// Reading a skill invocation out of what someone typed. +@Suite("Skill invocation") +struct SkillInvocationTests { + private let review = AgentSkill(title: "Weekly Review", instructions: "i") + private let tighten = AgentSkill(title: "Tighten this", instructions: "i") + + private var skills: [AgentSkill] { [review, tighten] } + + private func resolve(_ text: String) -> SkillInvocation.Outcome { + SkillInvocation.resolve(text, against: skills) + } + + // MARK: - Reading a name + + @Test("A named skill runs on the rest of the message") + func namedSkillTakesTheRemainder() { + #expect(resolve("/weekly-review cover the last week") == .invoked(skill: review, text: "cover the last week")) + } + + @Test("A name with no message still invokes") + func nameAloneInvokes() { + #expect(resolve("/weekly-review") == .invoked(skill: review, text: "")) + } + + @Test("The name is matched the way it is derived, not literally") + func nameIsMatchedByDerivation() { + // A near miss in punctuation or case still lands, because both fold to the same + // invocation. This is the whole reason the derived name exists. + #expect(resolve("/weekly_review go") == .invoked(skill: review, text: "go")) + #expect(resolve("/WEEKLY-REVIEW go") == .invoked(skill: review, text: "go")) + #expect(resolve("/weekly.review go") == .invoked(skill: review, text: "go")) + } + + @Test("The name is one token, so typing the title with its space does not invoke") + func titleWithSpaceIsNotTheName() { + // `/Weekly Review` reads "Weekly" as the name and "Review" as the message — the + // space is what separates the two, and it has to be, or there is no way to tell + // where a name ends and a question begins. + // + // This is exactly why the menu shows `/weekly-review` beside the title: what you + // read and what you type are different strings, and the menu says so. + #expect(resolve("/Weekly Review") == .unknown(name: "Weekly")) + } + + @Test("Text with no marker is just a message") + func plainTextIsAMessage() { + #expect(resolve("what happened last week") == .none(text: "what happened last week")) + } + + @Test("A marker in the middle is not an invocation") + func markerMustLead() { + // Otherwise "what is 3/4 of this" would try to run a skill called "4". + #expect(resolve("what is 3/4 of this") == .none(text: "what is 3/4 of this")) + } + + @Test("A bare marker is someone still typing, not a failure") + func bareMarkerIsNotAnError() { + // Putting an error under the cursor before the word is finished is worse than saying + // nothing yet. + #expect(resolve("/") == .none(text: "/")) + #expect(resolve("/ ") == .none(text: "/ ")) + } + + // MARK: - A name that matched nothing + + @Test("An unknown name does not silently become an ordinary message") + func unknownNameIsItsOwnAnswer() { + // The failure this rules out: a typo answers a question the user did not ask, with + // nothing anywhere saying the skill never ran. + #expect(resolve("/weekly-reveiw do the week") == .unknown(name: "weekly-reveiw")) + } + + @Test("The message names what was tried") + func unknownMessageNamesTheAttempt() { + // "No such skill" leaves the user checking whether they mistyped or never made it. + #expect(SkillInvocation.unknownMessage(name: "weekly-reveiw").contains("weekly-reveiw")) + } + + @Test("A hostile name cannot break the message it is quoted in") + func unknownMessageIsSafe() { + let message = SkillInvocation.unknownMessage(name: "a\u{202E}b") + #expect(message.unicodeScalars.contains { $0.value == 0x202E } == false) + } + + @Test("The named thing is bounded in the message") + func unknownMessageIsBounded() { + let message = SkillInvocation.unknownMessage(name: String(repeating: "x", count: 500)) + #expect(message.count < 200) + } + + // MARK: - Completions + + @Test("A partial name lists what it could become") + func completionsFilter() { + #expect(SkillInvocation.completions(for: "week", in: skills).map(\.id) == [review.id]) + } + + @Test("An empty partial lists everything") + func emptyPartialListsAll() { + #expect(SkillInvocation.completions(for: "", in: skills).count == 2) + } + + @Test("The list keeps the store's order, so it does not reshuffle while typing") + func completionsKeepStoreOrder() { + // The user is aiming at a position in the list; reordering by closeness of match + // moves the target as they type. + #expect(SkillInvocation.completions(for: "t", in: skills).map(\.id) == [tighten.id]) + #expect(SkillInvocation.completions(for: "", in: skills).map(\.id) == [review.id, tighten.id]) + } + + @Test("Nothing matching is an empty list, not everything") + func noMatchesIsEmpty() { + #expect(SkillInvocation.completions(for: "zzz", in: skills).isEmpty) + } +} + +/// The whole composer decision, in the one place both surfaces call. +@Suite("Skill turn") +struct SkillTurnTests { + private let review = AgentSkill(title: "Weekly Review", instructions: "i") + private var skills: [AgentSkill] { [review] } + + private func turn(_ text: String, armed: AgentSkill? = nil, route: AskRoute = .agentLoop) -> SkillInvocation.Turn { + SkillInvocation.turn(for: text, armed: armed, route: route, in: skills) + } + + @Test("A typed name beats an armed chip") + func typedNameWins() { + let other = AgentSkill(title: "Tighten this", instructions: "i") + #expect(turn("/weekly-review go", armed: other) == .send(skill: review, message: "go")) + } + + @Test("An armed chip is used when nothing is typed") + func armedChipIsUsed() { + #expect(turn("do the week", armed: review) == .send(skill: review, message: "do the week")) + } + + @Test("No skill anywhere sends the text unchanged") + func plainSend() { + #expect(turn("do the week") == .send(skill: nil, message: "do the week")) + } + + @Test("An unknown name refuses rather than sending") + func unknownRefuses() { + guard case let .refuse(reason) = turn("/nope go") else { + Issue.record("an unknown name was sent anyway") + return + } + #expect(reason.contains("nope")) + } + + @Test("A skill is refused rather than silently dropped on Deep Research") + func skillAndDeepResearchConflict() { + // A skill layers onto the agent's prompt and narrows the agent's tools, and a Deep + // Research run has neither. Dropping it quietly is the same failure as answering an + // unknown name as a plain message: the user asked for one thing and got another. + guard case let .refuse(reason) = turn("do the week", armed: review, route: .deepResearch) else { + Issue.record("the skill was silently dropped") + return + } + #expect(reason.contains("Weekly Review")) + #expect(reason.contains("Deep Research")) + } + + @Test("A typed name is refused on Deep Research too") + func typedNameAndDeepResearchConflict() { + guard case .refuse = turn("/weekly-review go", route: .deepResearch) else { + Issue.record("the skill was silently dropped") + return + } + } + + @Test("Image generation is the same answer") + func skillAndImageConflict() { + guard case .refuse = turn("a cat", armed: review, route: .imagePlayground(concept: "a cat")) else { + Issue.record("the skill was silently dropped") + return + } + } + + @Test("Without a skill, another route sends normally") + func otherRoutesAreFineWithoutASkill() { + #expect(turn("research this", route: .deepResearch) == .send(skill: nil, message: "research this")) + } + + @Test("The marker is stripped even when the skill does not apply to the route") + func markerNeverLeaksIntoAnotherRoute() { + // The refusal is what the user sees, but the reason this matters is the case above + // it: a `/name` that reaches a research prompt is a research run on a question with + // a stray token glued to the front of it. + #expect(turn("/weekly-review go") == .send(skill: review, message: "go")) + } +}