diff --git a/.kandev/review-notes.md b/.kandev/review-notes.md new file mode 100644 index 0000000..f310c45 --- /dev/null +++ b/.kandev/review-notes.md @@ -0,0 +1,54 @@ +# Review notes + +These notes cover **both** PRs on `feature/per-workspace-notes-ac1`: this plugin +repo (`yattdev/kandev-plugin-notes`) and the host repo (`yattdev/kandev`). They +live here because `kandev-source` gitignores `/.kandev/` by design, so the file +cannot be committed there — fold the entry below into the host PR description +as well as this one. + +## Known issue found during review (out of scope for these PRs) + +- **A pre-existing authentication weakness in the host's plugin-webhook + routing.** Confirmed live during QA. Location, mechanism, reproduction, blame, + and three suggested remedies are recorded in task + `51781b28-0580-48e7-ac31-a732b07e3ddb`. + + The specifics are deliberately withheld here. `kdlbs/kandev`, + `yattdev/kandev` and this repo are all public and the issue is unpatched + upstream, so a PR description is both the wrong place to publish it and the + wrong channel to notify the blamed author through. Route it upstream + privately — a GitHub security advisory on `kdlbs/kandev`, or a direct message + to the maintainers — rather than by @-mentioning anyone in this PR. + + Pre-existing and **not introduced by this branch**; the code it lives in is + untouched by either PR. Observed on `feature/per-workspace-notes-ac1` at + plugin `777992c` / host `0f8c8f4cb`. The author reviewed it this cycle and + decided to ship as-is and track it separately, so nothing on either branch + changes for it. + +## Action required by author + +- **Confirm the webhook finding's tracking task is the one you want to keep.** + It now lives in task `51781b28-0580-48e7-ac31-a732b07e3ddb`, which carries the + full repro, the cause, and three suggested remedies. An earlier task for the + same finding was opened and then deleted mid-cycle. Because the detail is now + deliberately kept out of this PR text, that task is the **only** record of it + — if it is closed without action, nothing preserves the finding. + +- **Veto QA's redaction if you disagree with it.** QA removed the file, line, + mechanism and blame attribution from the entry above because all three repos + are public and the issue is unpatched upstream. That was QA's call, not a + decision you made — if you would rather the PR carry the full description, + revert that hunk; nothing else in the entry changed. + +- **Release trigger matters for this branch.** `manifest.yaml` and `Makefile` + are hand-set to `0.3.0` and `CHANGELOG.md` carries a hand-written + `## [0.3.0]` section, which is correct for the tag-push path only: push + `v0.3.0` and `.github/workflows/release.yml` skips its `prepare` job and + publishes 0.3.0 straight from this metadata. Dispatching the same workflow + manually instead computes `max(manifest, latest tag)` and then bumps past it, + so the default `patch` choice would publish **0.3.1**, rewrite the version in + `manifest.yaml`, `Makefile` and the `README.md` tarball name, and prepend a + generated `## [0.3.1]` section above the hand-written + `## [0.3.0]` one, leaving two changelog entries for one set of changes and no + 0.3.0 release. Push the tag. diff --git a/CHANGELOG.md b/CHANGELOG.md index 9943278..d8287ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,39 @@ # Changelog +## [0.3.0] - 2026-08-11 + +### Added + +- feat: per-workspace notes — a sidebar button (registered for the host's + `sidebar-workspace-actions` slot, inert on hosts without it) opens the same + note editor/modal scoped to the active workspace instead of a task +- feat: Enhance with AI now returns a stable, machine-readable failure code + (unset/missing/disabled/unconfigured-profile/unavailable) and a guided-setup + action button that + jumps to the correct settings page instead of one message for every cause + +### Changed + +- `createNoteStore` and the card-indicator cache are now scope-generic + (`scope`/`scopeId` instead of a hardcoded "task"); existing task callers are + unaffected (`taskId` remains a working alias) + +### Fixed + +- Enhance with AI now recognizes a fifth state: an agent that is selected and + enabled but has **no model or agent profile bound** — the state every + built-in utility agent ships in, and therefore what most people hit right + after following the documented two-step setup. It gets its own + `agent_unconfigured_profile` code and a "Finish setting up the agent" button + pointing at Settings > Utility Agents. Previously it fell through to + `agent_unavailable`, whose message sent the user back to Settings > Plugins > + Notes — the step they had just completed correctly +- `agent_unavailable` now means only "a cause this plugin could not identify". + It names no settings page (any page would be a guess) and quotes the host's + own wording instead +- The workspace note editor no longer shows the task-scoped placeholder + ("Jot a note about this task…") under a modal titled "Workspace notes" + ## [0.2.3] - 2026-08-11 ### Changed diff --git a/Makefile b/Makefile index f6e2b23..4ec178b 100644 --- a/Makefile +++ b/Makefile @@ -1,7 +1,7 @@ .PHONY: build run test fmt vet package package-host clean BIN := bin/kandev-plugin-notes -VERSION := 0.2.3 +VERSION := 0.3.0 STAGE := .build/stage PKG_OUT := kandev-plugin-notes-$(VERSION).tar.gz diff --git a/README.md b/README.md index 72967d5..565b2d6 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,16 @@ editor and optional AI-assisted proofreading. overwritten automatically. See "Notes are private to you" below for the privacy trade-off this makes. - **Card indicator** — a small glyph on cards that have a note. +- **Workspace notes** — a small book icon beside Quick Terminal and Quick Chat + in the sidebar's New Task row (on a host build that carries the + `sidebar-workspace-actions` slot) opens the same editor, modal, toolbar, + Preview, and Enhance with AI included, scoped to the active **workspace** + instead of a task. Use it for a half-formed idea or a reminder that isn't + worth creating a task to hold. The icon is muted when the workspace has no + note and full-contrast once it does, and flips live (no reload) as the note + is written, emptied, or edited from another tab. A task's note and its + workspace's note are stored and shown independently; the icon is not + rendered without an active workspace. - **Cross-tab sync** — an edit in one tab shows up in another without a reload. ## If a note won't load @@ -81,24 +91,69 @@ otherwise save over an existing note the read never actually saw. ## Notes are private to you — except when you ask AI to enhance one -Each note is stored per **user**, per **task**, under the plugin's own key -(`("task", , "note")`) via Kandev's per-user plugin storage -(`capabilities.user_state`). Two people looking at the same task each see their -own note; nobody else can read yours, and the agent working the task cannot +Each note is stored per **user**, per **task or workspace** (whichever you +opened), under the plugin's own key (`(scope, id, "note")`, `scope` being +`"task"` or `"workspace"`) via Kandev's per-user plugin storage +(`capabilities.user_state`). Two people looking at the same task or workspace +each see their own note; nobody else can read yours, and no task's agent can read or write it. **The one exception is the "Enhance with AI" button.** Clicking it sends the note's current markdown to the utility agent configured for this plugin (**Settings > Plugins > Notes**) via a one-shot completion (`capabilities.agent_invoke` / `Host.InvokeUtilityAgent`) — that content -leaves the "nobody else can read it" boundary for that one request. If no -utility agent is configured, the button shows a clear, non-fatal message -instead of failing silently. Skip the button entirely to keep a note fully -private. +leaves the "nobody else can read it" boundary for that one request. See +"Setting up Enhance with AI" below for what has to be configured first, and +what each failure message means. Skip the button entirely to keep a note +fully private. -If you want the task's own agent to see something, put it in the task +If you want a task's own agent to see something, put it in the task description or say it in chat. This is a scratchpad, not a shared field. +## Setting up Enhance with AI + +"Enhance with AI" needs **two separate settings**, both satisfied, before it +can run: + +1. **Select an agent for this plugin** — Settings > Plugins > Notes, + `config_schema.utility_agent`. This is what tells the plugin which + utility agent to ask. +2. **Enable that agent, with a model** — Settings > Utility Agents. Selecting + an agent in step 1 does not enable it; a newly-added utility agent starts + disabled with no model chosen. + +Both steps are required because **a disabled utility agent is usable by +kandev's own built-in features (e.g. task-create prompt enhancement) but not +by any plugin**, including this one. Kandev's own prompt-enhancement path +does not check `Enabled`; this plugin's request goes through +`Host.InvokeUtilityAgent`, which does. That asymmetry is host behavior this +plugin cannot change — clicking Enhance with an agent selected-but-disabled +fails exactly like having no agent selected at all, and the two failures now +say so explicitly rather than both pointing back at Settings > Plugins > Notes: + +| Situation | Message points you to | +| --- | --- | +| No agent ever selected | Settings > Plugins > Notes | +| Selected agent was since deleted | Settings > Plugins > Notes | +| Selected agent exists but is disabled | **Settings > Utility Agents** ("Enable the agent") | +| Selected and enabled, but no model / agent profile bound | **Settings > Utility Agents** ("Finish setting up the agent") | +| Any other setup problem the plugin can't identify | no page named; the host's own wording is quoted instead | +| A real execution failure (the agent ran and failed) | no settings link — try again | + +For the first four, **Dismiss** is joined by a second action button that +jumps straight to the right page for that cause, so there's no need to guess +which setting is missing. + +The fourth row is the one most people hit, because **every built-in utility +agent ships with no model bound**: completing steps 1 and 2 above still leaves +it unconfigured. It is called out separately from "disabled" on purpose — +both are fixed on the same page but by different controls, and being told to +enable an agent you just enabled is the dead end this plugin exists to avoid. + +The fifth row is the honest fallback: a `FailedPrecondition` this plugin does +not recognize (for example after a host rephrase). It names no page, because +any page it named would be a guess, and quotes the host's own wording instead. + ## Install Until the first release is published, install by sideload — build a package and @@ -113,9 +168,7 @@ curl -F "package=@kandev-plugin-notes-.tar.gz" \ Sideloaded plugins register disabled/unverified; enable it in **Settings > Plugins**. Reinstalling the same version returns 409 — bump the version in `manifest.yaml` (and `Makefile`) first. To use "Enhance with AI", -also pick a utility agent for this plugin under **Settings > Plugins > Notes** -(`config_schema.utility_agent`) — without one, the button surfaces a -not-configured message rather than failing. +see "Setting up Enhance with AI" above — it's a two-step setup, not one. ## Development diff --git a/go.mod b/go.mod index 90bc0f0..cf22dc8 100644 --- a/go.mod +++ b/go.mod @@ -20,6 +20,7 @@ require ( github.com/mattn/go-isatty v0.0.19 // indirect github.com/oklog/run v1.1.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect + github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 // indirect golang.org/x/net v0.56.0 // indirect golang.org/x/sys v0.46.0 // indirect golang.org/x/text v0.39.0 // indirect @@ -33,3 +34,5 @@ require ( // layout this path assumes, and what changes once pkg/pluginsdk ships as its // own versioned module. replace github.com/kandev/kandev => ../kandev/apps/backend + +tool github.com/kandev/kandev/cmd/plugin-pack diff --git a/go.sum b/go.sum index 9cdccd9..a2c2211 100644 --- a/go.sum +++ b/go.sum @@ -5,6 +5,8 @@ github.com/cespare/xxhash/v2 v2.3.0/go.mod h1:VGX0DQ3Q6kWi7AoAeZDth3/j3BFtOZR5XL github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dlclark/regexp2 v1.11.0 h1:G/nrcoOa7ZXlpoa/91N3X7mM3r8eIlMBBJZvsz/mxKI= +github.com/dlclark/regexp2 v1.11.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/fatih/color v1.13.0 h1:8LOYc1KYPPmyKMuN8QV2DNRWNbLo6LZ0iLs8+mlH53w= github.com/fatih/color v1.13.0/go.mod h1:kLAiJbzzSOZDVNGyDpeOxJ47H46qBXwg5ILebYFFOfk= github.com/go-logr/logr v1.4.3 h1:CjnDlHq8ikf6E492q6eKboGOC0T8CDaOvkHCIg8idEI= @@ -36,6 +38,8 @@ github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA= github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2 h1:KRzFb2m7YtdldCEkzs6KqmJw4nqEVZGK7IN2kJkjTuQ= +github.com/santhosh-tekuri/jsonschema/v6 v6.0.2/go.mod h1:JXeL+ps8p7/KNMjDQk3TCwPpBy0wYklyWTfbkIzdIFU= github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= github.com/stretchr/testify v1.7.2/go.mod h1:R6va5+xMeoiuVRoj+gSkQ7d3FALtqAAGI1FQKckRals= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= diff --git a/manifest.yaml b/manifest.yaml index 7c92934..8207806 100644 --- a/manifest.yaml +++ b/manifest.yaml @@ -5,9 +5,9 @@ # (/api/plugins//...). Keep the three in sync when bumping version. id: "kandev-plugin-notes" api_version: 1 -version: "0.2.3" +version: "0.3.0" display_name: "Notes" -description: "A private, per-user scratchpad note on any task: a dockview/mobile panel, markdown editing with a formatting toolbar, AI-assisted proofreading, and a kanban card shortcut." +description: "A private, per-user scratchpad note on any task or workspace: a dockview/mobile panel, a sidebar workspace-notes button, markdown editing with a formatting toolbar, AI-assisted proofreading with guided setup, and a kanban card shortcut." author: "yattdev" categories: ["tools"] repo_url: "https://github.com/yattdev/kandev-plugin-notes" diff --git a/server/plugin.go b/server/plugin.go index d7a1ea1..97f35d5 100644 --- a/server/plugin.go +++ b/server/plugin.go @@ -16,6 +16,7 @@ import ( "encoding/json" "fmt" "net/http" + "strings" "github.com/kandev/kandev/pkg/pluginsdk" "google.golang.org/grpc/codes" @@ -49,11 +50,103 @@ type enhanceResponseBody struct { Content string `json:"content"` } +// enhanceErrorCode is the enhance webhook's stable, machine-readable 412 +// classification (C1) — the UI maps it to a specific settings page (C2) +// instead of guessing from prose. Every other failure branch (400/404/405/ +// 502/503) omits Code/Detail and keeps the plain {error} shape it always +// had, via omitempty below. +type enhanceErrorCode string + +const ( + // enhanceErrorCodeAgentUnset: the plugin has no utility agent selected + // at all (Settings > Plugins > Notes was never used). + enhanceErrorCodeAgentUnset enhanceErrorCode = "agent_unset" + // enhanceErrorCodeAgentMissing: the selected agent id no longer exists + // (deleted after selection). + enhanceErrorCodeAgentMissing enhanceErrorCode = "agent_missing" + // enhanceErrorCodeAgentDisabled: the selected agent exists but is + // disabled — a different fix (Settings > Utility Agents), not a + // reselection, per the ADR 0048 Enabled asymmetry this plugin cannot + // change (see README's setup section). + enhanceErrorCodeAgentDisabled enhanceErrorCode = "agent_disabled" + // enhanceErrorCodeAgentUnconfiguredProfile: the selected agent exists and + // is enabled, but has no model / agent profile bound. Distinct from + // disabled: the remedy is the same page (Settings > Utility Agents) but a + // different control, and telling this user the agent is "disabled" when + // they just enabled it is the same dead end as pointing them back at + // Settings > Plugins > Notes. It earns its own code because it is the + // state every built-in utility agent ships in, so it is what a user hits + // immediately after completing the README's two documented steps. + enhanceErrorCodeAgentUnconfiguredProfile enhanceErrorCode = "agent_unconfigured_profile" + // enhanceErrorCodeAgentUnavailable: a FailedPrecondition whose wording + // matched none of the above — the host may have rephrased its message. + // Detail still carries that raw wording verbatim so the user sees real + // information rather than a guessed instruction. + enhanceErrorCodeAgentUnavailable enhanceErrorCode = "agent_unavailable" +) + +// enhanceErrorMessages pairs each code with the one correct remedy. Keep +// this the single source of truth for that mapping — HandleWebhook never +// composes an error message inline, so unset/missing and disabled can never +// be swapped at a call site. +var enhanceErrorMessages = map[enhanceErrorCode]string{ + enhanceErrorCodeAgentUnset: "No utility agent is configured for this plugin — configure one in Settings > Plugins > Notes.", + enhanceErrorCodeAgentMissing: "The utility agent configured for this plugin no longer exists — choose another one in Settings > Plugins > Notes.", + enhanceErrorCodeAgentDisabled: "The utility agent configured for this plugin is disabled — enable it (with a model) in Settings > Utility Agents.", + enhanceErrorCodeAgentUnconfiguredProfile: "The utility agent configured for this plugin has no model or agent profile bound — finish setting it up in Settings > Utility Agents.", + enhanceErrorCodeAgentUnavailable: "The configured utility agent is unavailable.", +} + +// enhanceErrorMessage resolves the user-facing message for a code. A +// classified cause names the one page that fixes it. agent_unavailable is by +// definition a FailedPrecondition this plugin could NOT classify, so there is +// no page it can name without guessing — naming one anyway is how a user who +// has already done that step gets sent back to it. It therefore surfaces the +// host's own wording instead of a remedy that may not apply, which is the +// degradation this classifier was designed for: a missing button, not a wrong +// instruction. +func enhanceErrorMessage(code enhanceErrorCode, detail string) string { + message := enhanceErrorMessages[code] + if code == enhanceErrorCodeAgentUnavailable && detail != "" { + return message + " The host reported: " + detail + } + return message +} + +// classifyUtilityAgentError maps each of host_utility.go's four +// FailedPrecondition wordings ("no utility agent configured for this plugin", +// "configured utility agent %q not found", "configured utility agent %q is +// disabled", "configured utility agent %q has no usable agent profile") to a +// stable code, kept as its own function (rather than inlined at the call +// site) so the mapping is unit-testable in isolation and has exactly one +// home. Substring matching is coupled to the host's current wording — a +// rephrase degrades to enhanceErrorCodeAgentUnavailable rather than +// misclassifying, since Detail (the raw message) is always preserved +// alongside it. +func classifyUtilityAgentError(message string) enhanceErrorCode { + switch { + case strings.Contains(message, "no utility agent configured"): + return enhanceErrorCodeAgentUnset + case strings.Contains(message, "not found"): + return enhanceErrorCodeAgentMissing + case strings.Contains(message, "is disabled"): + return enhanceErrorCodeAgentDisabled + case strings.Contains(message, "no usable agent profile"): + return enhanceErrorCodeAgentUnconfiguredProfile + default: + return enhanceErrorCodeAgentUnavailable + } +} + // enhanceErrorBody is the JSON body returned on a handled failure (missing // utility agent, bad input) — a stable {error} shape the UI can surface -// without parsing prose out of a plain-text body. +// without parsing prose out of a plain-text body. Code/Detail are only ever +// populated on the 412 (utility-agent) branch; every other branch keeps +// the bare {error} shape it always had. type enhanceErrorBody struct { - Error string `json:"error"` + Error string `json:"error"` + Code enhanceErrorCode `json:"code,omitempty"` + Detail string `json:"detail,omitempty"` } // notesPlugin implements pluginsdk.Plugin via UnimplementedPlugin's no-op @@ -94,10 +187,19 @@ func (p *notesPlugin) HandleWebhook(ctx context.Context, req *pluginsdk.WebhookR improved, err := host.InvokeUtilityAgent(ctx, fmt.Sprintf(enhancePromptTemplate, body.Content)) if err != nil { if status.Code(err) == codes.FailedPrecondition { - // No utility agent configured (or the configured one was - // deleted/disabled) — a distinguishable, non-fatal condition - // per ADR 0048, not an internal error. - return jsonErrorResponse(http.StatusPreconditionFailed, "no utility agent is configured for this plugin — configure one in Settings > Plugins > Notes") + // No utility agent configured, or the configured one was + // deleted/disabled — a distinguishable, non-fatal condition per + // ADR 0048, not an internal error. classifyUtilityAgentError + // turns the host's raw gRPC message into a stable code (C1) so + // the UI can point at the correct settings page (C2) instead of + // this one message covering unset/missing/disabled alike. + rawMessage := status.Convert(err).Message() + code := classifyUtilityAgentError(rawMessage) + return jsonResponse(http.StatusPreconditionFailed, enhanceErrorBody{ + Error: enhanceErrorMessage(code, rawMessage), + Code: code, + Detail: rawMessage, + }) } return jsonErrorResponse(http.StatusBadGateway, "AI enhancement failed") } diff --git a/server/plugin_test.go b/server/plugin_test.go index a87876b..121c095 100644 --- a/server/plugin_test.go +++ b/server/plugin_test.go @@ -190,6 +190,8 @@ func TestHandleWebhook_Enhance_NoUtilityAgentConfigured_ReturnsPreconditionFaile var out enhanceErrorBody require.NoError(t, json.Unmarshal(resp.Body, &out)) require.NotEmpty(t, out.Error) + require.Equal(t, enhanceErrorCodeAgentUnset, out.Code) + require.Equal(t, "no utility agent configured for this plugin", out.Detail) } func TestHandleWebhook_Enhance_OtherAgentError_ReturnsBadGateway(t *testing.T) { @@ -208,4 +210,130 @@ func TestHandleWebhook_Enhance_OtherAgentError_ReturnsBadGateway(t *testing.T) { }) require.NoError(t, err) require.Equal(t, int32(502), resp.Status) + + // C3: a real execution failure must never turn into a configuration + // message — no code/detail leak onto this generic branch. + var out enhanceErrorBody + require.NoError(t, json.Unmarshal(resp.Body, &out)) + require.Empty(t, out.Code) + require.Empty(t, out.Detail) + require.Equal(t, "AI enhancement failed", out.Error) +} + +// TestHandleWebhook_Enhance_ClassifiesEachFailedPreconditionWording is C1/C7: +// each of host_utility.go's classified wordings, plus unclassified ones, maps +// to a code with the raw message preserved verbatim as Detail. +func TestHandleWebhook_Enhance_ClassifiesEachFailedPreconditionWording(t *testing.T) { + tests := []struct { + name string + hostMessage string + wantCode enhanceErrorCode + }{ + { + name: "unset", + hostMessage: "no utility agent configured for this plugin", + wantCode: enhanceErrorCodeAgentUnset, + }, + { + name: "missing", + hostMessage: `configured utility agent "builtin-enhance-prompt" not found`, + wantCode: enhanceErrorCodeAgentMissing, + }, + { + name: "disabled", + hostMessage: `configured utility agent "builtin-enhance-prompt" is disabled`, + wantCode: enhanceErrorCodeAgentDisabled, + }, + { + name: "unrecognized wording degrades to unavailable, not a wrong instruction", + hostMessage: "utility agent invocation is temporarily throttled", + wantCode: enhanceErrorCodeAgentUnavailable, + }, + { + // host_utility.go:84 — reachable by following the README's own + // two-step setup: agent selected AND enabled, but the profile + // binding left at its shipped default (every builtin utility + // agent starts with an empty agent_profile_id). Confirmed live + // against a real host during QA. + name: "enabled agent with no bound profile", + hostMessage: `configured utility agent "builtin-enhance-prompt" has no usable agent profile`, + wantCode: enhanceErrorCodeAgentUnconfiguredProfile, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := ¬esPlugin{} + p.SetHost(&fakeHost{invokeUtilityAgent: func(context.Context, string) (string, error) { + return "", status.Error(codes.FailedPrecondition, tt.hostMessage) + }}) + + body, err := json.Marshal(map[string]string{"content": "hello"}) + require.NoError(t, err) + + resp, err := p.HandleWebhook(context.Background(), &pluginsdk.WebhookRequest{ + WebhookKey: "enhance", + Method: "POST", + Body: body, + }) + require.NoError(t, err) + require.Equal(t, int32(412), resp.Status) + + var out enhanceErrorBody + require.NoError(t, json.Unmarshal(resp.Body, &out)) + require.Equal(t, tt.wantCode, out.Code) + require.Equal(t, tt.hostMessage, out.Detail) + require.Equal(t, enhanceErrorMessage(tt.wantCode, tt.hostMessage), out.Error) + + // An unclassified cause must not prescribe a settings page: the + // user may have already completed the step it would name. It + // carries the host's own wording instead. + if tt.wantCode == enhanceErrorCodeAgentUnavailable { + require.NotContains(t, out.Error, "Settings >") + require.Contains(t, out.Error, tt.hostMessage) + } + }) + } +} + +// TestEnhanceErrorMessage_UnavailableNamesNoPage pins the rule directly: every +// classified code names exactly one remedy page, and agent_unavailable names +// none. Without this, a later edit could quietly reintroduce a wrong-page +// instruction for a cause the plugin cannot identify. +func TestEnhanceErrorMessage_UnavailableNamesNoPage(t *testing.T) { + for _, code := range []enhanceErrorCode{ + enhanceErrorCodeAgentUnset, + enhanceErrorCodeAgentMissing, + enhanceErrorCodeAgentDisabled, + enhanceErrorCodeAgentUnconfiguredProfile, + } { + require.Contains(t, enhanceErrorMessage(code, "raw detail"), "Settings >", + "classified code %q must name its remedy page", code) + } + + unavailable := enhanceErrorMessage(enhanceErrorCodeAgentUnavailable, "raw detail") + require.NotContains(t, unavailable, "Settings >") + require.Contains(t, unavailable, "raw detail") + + // No detail to pass through: still no invented page. + require.NotContains(t, enhanceErrorMessage(enhanceErrorCodeAgentUnavailable, ""), "Settings >") +} + +// TestClassifyUtilityAgentError_TableDriven exercises classifyUtilityAgentError +// directly, isolated from HandleWebhook and the gRPC status plumbing. +func TestClassifyUtilityAgentError_TableDriven(t *testing.T) { + tests := []struct { + message string + want enhanceErrorCode + }{ + {"no utility agent configured for this plugin", enhanceErrorCodeAgentUnset}, + {`configured utility agent "x" not found`, enhanceErrorCodeAgentMissing}, + {`configured utility agent "x" is disabled`, enhanceErrorCodeAgentDisabled}, + {`configured utility agent "x" has no usable agent profile`, enhanceErrorCodeAgentUnconfiguredProfile}, + {"", enhanceErrorCodeAgentUnavailable}, + {"something else entirely", enhanceErrorCodeAgentUnavailable}, + } + for _, tt := range tests { + require.Equal(t, tt.want, classifyUtilityAgentError(tt.message), "message: %q", tt.message) + } } diff --git a/ui/bundle.js b/ui/bundle.js index f3faebd..fd7e1c8 100644 --- a/ui/bundle.js +++ b/ui/bundle.js @@ -48,7 +48,11 @@ // the operator-configured utility agent (README's Privacy section explains // this trade-off). -const NOTE_SCOPE = "task"; +// DEFAULT_SCOPE is the store/cache default when no scope is given, keeping +// every pre-existing task-scoped caller (panel, mobile panel, kanban modal, +// card indicator) working unchanged. A workspace note passes scope: +// "workspace" explicitly instead. +const DEFAULT_SCOPE = "task"; const NOTE_KEY = "note"; const WRITE_DEBOUNCE_MS = 150; const ENHANCE_WEBHOOK_PATH = "webhooks/enhance"; @@ -289,7 +293,10 @@ export async function enhanceNote(host, content) { // 412 is this webhook's distinguishable "no utility agent configured" // signal (server/plugin.go, mapped from gRPC FailedPrecondition per // ADR 0048) — surfaced as a clear, non-fatal message rather than a - // generic failure. + // generic failure. `code`/`detail` (C1) let the UI point at the right + // settings page instead of one message covering unset/missing/disabled + // alike; an older server that omits them (C5) leaves both undefined and + // the caller falls back to the plain message with no action button. const notConfigured = response.status === 412; const message = notConfigured ? (data && data.error) || "No utility agent is configured for this plugin yet." @@ -297,6 +304,8 @@ export async function enhanceNote(host, content) { const error = new Error(message); error.notConfigured = notConfigured; error.status = response.status; + error.code = data && typeof data.code === "string" ? data.code : undefined; + error.detail = data && typeof data.detail === "string" ? data.detail : undefined; throw error; } @@ -305,6 +314,11 @@ export async function enhanceNote(host, content) { error.notConfigured = false; throw error; } + if (data.content.trim() === "") { + const error = new Error("The AI enhance service returned an empty result."); + error.notConfigured = false; + throw error; + } return data.content; } @@ -328,7 +342,12 @@ export function enhancePreviewReducer(state, action) { case "success": return { status: "preview", preview: action.content }; case "failure": - return { status: "error", message: action.message, notConfigured: Boolean(action.notConfigured) }; + return { + status: "error", + message: action.message, + notConfigured: Boolean(action.notConfigured), + code: action.code, + }; case "discard": case "accept": case "dismiss": @@ -338,6 +357,35 @@ export function enhancePreviewReducer(state, action) { } } +// enhanceErrorAction (C2/C4) maps an enhance failure's `code` to the guided +// setup action NotesEditor's error branch renders beside Dismiss: which +// settings page fixes *this* cause, in its own words. "unset"/"missing" both +// land on the Notes plugin page (pick or re-pick an agent); "disabled" and +// "unconfigured_profile" land on Utility Agents instead — a different page, +// because picking an agent there again would not fix either one (see +// server/plugin.go's classifyUtilityAgentError comment for the host-side half +// of this split). The two Utility Agents causes keep separate labels because +// they are separate controls on that page: flipping Enabled, versus binding a +// model/profile. Telling someone to "enable" an agent they just enabled is +// the dead end this whole mapping exists to remove. +// A pure function (no host, no React) so C7's code -> action mapping is +// testable directly; returns null for an absent/unrecognized code (C5: an +// older server that omits `code`, or "agent_unavailable" — a cause the plugin +// could not identify, where the message alone is what's known). +export function enhanceErrorAction(code) { + switch (code) { + case "agent_unset": + case "agent_missing": + return { label: "Choose an agent", href: "/settings/plugins/kandev-plugin-notes" }; + case "agent_disabled": + return { label: "Enable the agent", href: "/settings/utility-agents" }; + case "agent_unconfigured_profile": + return { label: "Finish setting up the agent", href: "/settings/utility-agents" }; + default: + return null; + } +} + // --------------------------------------------------------------------------- // describeReadError — classifies a rejected host.storage.get() into a // snapshot-safe shape ({ status, message, retryable, detail }) so the read @@ -417,15 +465,22 @@ export function describeReadError(error) { // - a PluginStorageConflictError (409) stops the write queue, preserves // the caller's in-flight edit, and only refreshes the authoritative // updatedAt — it never silently discards the edit. -// - setTaskId() clears value/updatedAt synchronously (before the new -// task's read resolves) so a write in flight for the old task can never -// be sent under the new task's id with a stale ifUnmodifiedSince. +// - setScopeId() (alias: setTaskId()) clears value/updatedAt synchronously +// (before the new scopeId's read resolves) so a write in flight for the +// old scopeId can never be sent under the new one's id with a stale +// ifUnmodifiedSince. +// +// scope defaults to "task" (and scopeId falls back to the legacy `taskId` +// option) so every pre-existing caller is unaffected; a workspace note store +// passes { scope: "workspace", scopeId: workspaceId } instead. scope itself +// is fixed for a store's lifetime — only scopeId changes via setScopeId. // --------------------------------------------------------------------------- -export function createNoteStore(host, { taskId, surfaceId, onCommit }) { - let currentTaskId = taskId; +export function createNoteStore(host, { scope = DEFAULT_SCOPE, scopeId, taskId, surfaceId, onCommit }) { + const currentScope = scope; + let currentScopeId = scopeId ?? taskId; let value = ""; let updatedAt; - let loadedTaskId = null; + let loadedScopeId = null; let readError = null; let dirty = false; let conflict = false; @@ -458,14 +513,14 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { } } - function scheduleAutoRetry(forTaskId) { + function scheduleAutoRetry(forScopeId) { if (!readError || !readError.retryable) return; if (autoRetryCount >= AUTO_RETRY_LIMIT) return; autoRetryCount += 1; const delay = AUTO_RETRY_BASE_MS * 2 ** (autoRetryCount - 1); autoRetryTimer = setTimeout(() => { autoRetryTimer = undefined; - if (disposed || forTaskId !== currentTaskId) return; + if (disposed || forScopeId !== currentScopeId) return; refresh(); }, delay); } @@ -476,14 +531,14 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { // logical read as a raw host.api.fetch so the snapshot can carry the real // response.status and any JSON error body instead of a regex guess. Fires // at most once per rejected refresh(); a probe for a superseded - // generation/taskId is dropped by the same guard refresh() itself uses, + // generation/scopeId is dropped by the same guard refresh() itself uses, // and a probe that itself fails just leaves the describeReadError // classification in place. - function issueReadErrorProbe(rawError, generation, forTaskId) { + function issueReadErrorProbe(rawError, generation, forScopeId) { const fetchApi = host.api && host.api.fetch; const probe = typeof fetchApi === "function" - ? Promise.resolve(fetchApi(`user-state/${NOTE_SCOPE}/${forTaskId}/${NOTE_KEY}`)).then( + ? Promise.resolve(fetchApi(`user-state/${currentScope}/${forScopeId}/${NOTE_KEY}`)).then( async (response) => { let body = null; try { @@ -502,7 +557,7 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { : Promise.resolve(undefined); probe.then((probeInfo) => { - if (!disposed && generation === refreshGeneration && forTaskId === currentTaskId && probeInfo) { + if (!disposed && generation === refreshGeneration && forScopeId === currentScopeId && probeInfo) { readError = { ...readError, status: probeInfo.status, detail: probeInfo.detail }; notify(); } @@ -522,9 +577,11 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { function getSnapshot() { return { - taskId: currentTaskId, + scope: currentScope, + scopeId: currentScopeId, + taskId: currentScope === "task" ? currentScopeId : null, value, - loaded: loadedTaskId === currentTaskId, + loaded: loadedScopeId === currentScopeId, readError, conflict, writeError, @@ -537,13 +594,13 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { function refresh(options = {}) { const preserveValue = options.preserveValue ?? dirty; const generation = ++refreshGeneration; - const forTaskId = currentTaskId; - return host.storage.get(NOTE_SCOPE, forTaskId, NOTE_KEY).then( + const forScopeId = currentScopeId; + return host.storage.get(currentScope, forScopeId, NOTE_KEY).then( (entry) => { - if (disposed || generation !== refreshGeneration || forTaskId !== currentTaskId) return false; + if (disposed || generation !== refreshGeneration || forScopeId !== currentScopeId) return false; if (!preserveValue && !dirty) value = entry ? entry.value : ""; updatedAt = entry ? entry.updatedAt : undefined; - loadedTaskId = forTaskId; + loadedScopeId = forScopeId; readError = null; autoRetryCount = 0; clearAutoRetryTimer(); @@ -551,14 +608,14 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { return true; }, (rawError) => { - // Do not mark the task loaded after a rejected read: an empty + // Do not mark this scopeId loaded after a rejected read: an empty // editor here could omit ifUnmodifiedSince on its first save and // silently overwrite an existing note. Stay in a retry state. - if (disposed || generation !== refreshGeneration || forTaskId !== currentTaskId) return false; + if (disposed || generation !== refreshGeneration || forScopeId !== currentScopeId) return false; readError = describeReadError(rawError); notify(); - scheduleAutoRetry(forTaskId); - issueReadErrorProbe(rawError, generation, forTaskId); + scheduleAutoRetry(forScopeId); + issueReadErrorProbe(rawError, generation, forScopeId); return false; }, ); @@ -566,9 +623,9 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { function subscribeStorage() { if (unsubscribeStorage) unsubscribeStorage(); - const forTaskId = currentTaskId; + const forScopeId = currentScopeId; unsubscribeStorage = host.storage.subscribe( - { scope: NOTE_SCOPE, scopeId: forTaskId, key: NOTE_KEY, writerId: surfaceId }, + { scope: currentScope, scopeId: forScopeId, key: NOTE_KEY, writerId: surfaceId }, () => refresh(), ); } @@ -586,12 +643,12 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { } } - function setTaskId(nextTaskId) { - if (nextTaskId === currentTaskId) return; - currentTaskId = nextTaskId; + function setScopeId(nextScopeId) { + if (nextScopeId === currentScopeId) return; + currentScopeId = nextScopeId; value = ""; updatedAt = undefined; - loadedTaskId = null; + loadedScopeId = null; readError = null; autoRetryCount = 0; clearAutoRetryTimer(); @@ -621,12 +678,12 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { // existing note (see refresh()'s reject handler above). if (writeBlocked || writeInFlight || pendingValue === undefined || readError) return; const generation = writeGeneration; - const forTaskId = currentTaskId; + const forScopeId = currentScopeId; const next = pendingValue; pendingValue = undefined; writeInFlight = true; host.storage - .set(NOTE_SCOPE, forTaskId, NOTE_KEY, next, { writerId: surfaceId, ifUnmodifiedSince: updatedAt }) + .set(currentScope, forScopeId, NOTE_KEY, next, { writerId: surfaceId, ifUnmodifiedSince: updatedAt }) .then((result) => { if (writeGeneration !== generation) return; updatedAt = result.updatedAt; @@ -700,7 +757,11 @@ export function createNoteStore(host, { taskId, surfaceId, onCommit }) { }, getSnapshot, setValue, - setTaskId, + setScopeId, + // setTaskId: alias for setScopeId, kept for the "task" scope's existing + // callers (useNoteStore's own effect, and any external caller written + // before scope existed) — same synchronous clear-before-read guarantee. + setTaskId: setScopeId, retryRead, retryWrite, dispose, @@ -720,12 +781,17 @@ const activeStores = new Set(); // --------------------------------------------------------------------------- // Card-indicator cache — module-level so it survives route navigation and is // shared by every rendered card, per AC17: at most one host.storage.get per -// taskId per page session. A second render serves the cache; a cross-tab -// subscribe notification updates it from the notification's `deleted` flag -// alone (no refetch — the payload carries no value); and a successful local -// write updates it directly, since own-tab echoes of that write are -// suppressed by the writerId-scoped subscription above and would never -// reach this module-level listener otherwise. +// (scope, scopeId) per page session. A second render serves the cache; a +// cross-tab subscribe notification updates it from the notification's +// `deleted` flag alone (no refetch — the payload carries no value); and a +// successful local write updates it directly, since own-tab echoes of that +// write are suppressed by the writerId-scoped subscription above and would +// never reach this module-level listener otherwise. +// +// Keyed by `${scope}:${scopeId}` (cacheKey below) so a task and a workspace +// that happen to share a raw id never collide — every exported helper below +// takes a `scope` argument defaulting to "task", the pre-existing (and only, +// before workspace notes) caller. // // A cross-tab *non-delete* notification is optimistically treated as "has a // note" without inspecting content, since PluginUserStateChange carries no @@ -738,63 +804,73 @@ const pendingGets = new Map(); const cacheListeners = new Map(); let indicatorUnsubscribe = null; -function notifyCacheListeners(taskId) { - const listeners = cacheListeners.get(taskId); +function cacheKey(scope, scopeId) { + return `${scope}:${scopeId}`; +} + +function notifyCacheListeners(key) { + const listeners = cacheListeners.get(key); if (!listeners) return; - const hasNote = noteCache.get(taskId) ?? false; + const hasNote = noteCache.get(key) ?? false; listeners.forEach((listener) => listener(hasNote)); } -export function markNote(taskId, hasNote) { - noteCache.set(taskId, hasNote); - notifyCacheListeners(taskId); +export function markNote(scopeId, hasNote, scope = DEFAULT_SCOPE) { + const key = cacheKey(scope, scopeId); + noteCache.set(key, hasNote); + notifyCacheListeners(key); } -export function getCachedHasNote(host, taskId) { - if (noteCache.has(taskId)) return Promise.resolve(noteCache.get(taskId)); - const pending = pendingGets.get(taskId); +export function getCachedHasNote(host, scopeId, scope = DEFAULT_SCOPE) { + const key = cacheKey(scope, scopeId); + if (noteCache.has(key)) return Promise.resolve(noteCache.get(key)); + const pending = pendingGets.get(key); if (pending) return pending; - const request = host.storage.get(NOTE_SCOPE, taskId, NOTE_KEY).then( + const request = host.storage.get(scope, scopeId, NOTE_KEY).then( (entry) => { - pendingGets.delete(taskId); + pendingGets.delete(key); const hasNote = Boolean(entry && typeof entry.value === "string" && entry.value !== ""); - noteCache.set(taskId, hasNote); - notifyCacheListeners(taskId); + noteCache.set(key, hasNote); + notifyCacheListeners(key); return hasNote; }, () => { - // Leave this taskId uncached on a failed read so a later render can + // Leave this key uncached on a failed read so a later render can // retry, instead of pinning it to a possibly-wrong false forever. - pendingGets.delete(taskId); + pendingGets.delete(key); return false; }, ); - pendingGets.set(taskId, request); + pendingGets.set(key, request); return request; } -export function subscribeCache(taskId, listener) { - let listeners = cacheListeners.get(taskId); +export function subscribeCache(scopeId, listener, scope = DEFAULT_SCOPE) { + const key = cacheKey(scope, scopeId); + let listeners = cacheListeners.get(key); if (!listeners) { listeners = new Set(); - cacheListeners.set(taskId, listeners); + cacheListeners.set(key, listeners); } listeners.add(listener); return () => { listeners.delete(listener); - if (listeners.size === 0) cacheListeners.delete(taskId); + if (listeners.size === 0) cacheListeners.delete(key); }; } // initNoteIndicatorSubscription is called from initialize() every time the // plugin is (re-)enabled. It tears down any prior subscription first so // calling initialize() twice in one tab (disable -> re-enable) still leaves -// exactly one module-level subscription, never two. +// exactly one module-level subscription, never two. No `scope` filter here +// (deliberately, unlike a single store's own subscribeStorage): one +// subscription must see every scope's note changes, task and workspace +// alike, so a workspace note write also flips its sidebar-button cache entry. export function initNoteIndicatorSubscription(host) { if (indicatorUnsubscribe) indicatorUnsubscribe(); - indicatorUnsubscribe = host.storage.subscribe({ scope: NOTE_SCOPE, key: NOTE_KEY }, (change) => { - markNote(change.scopeId, !change.deleted); + indicatorUnsubscribe = host.storage.subscribe({ key: NOTE_KEY }, (change) => { + markNote(change.scopeId, !change.deleted, change.scope); }); } @@ -828,15 +904,25 @@ export function disposeNoteIndicatorSubscription() { // not render one — confirmed live (empty, uneditable modal body). That is // host-platform code this repo cannot change, so the modal keeps the // textarea+toolbar fallback until the host wraps plugin modals in one. +// +// { scope, scopeId } generalizes this factory beyond the task modal — the +// workspace sidebar button (openWorkspaceNoteModal) reuses it unchanged with +// scope: "workspace". `close`, when given, is the owning PluginModalHandle's +// close() (see openScopedNoteModal below) — NotesEditor's enhance-error +// action (C4) closes the modal before navigating away from it. // --------------------------------------------------------------------------- -export function makeNoteModalContent(host, taskId) { +export function makeNoteModalContent(host, { scope = DEFAULT_SCOPE, scopeId, taskId } = {}, close) { + const resolvedScopeId = scopeId ?? taskId; return function NoteModalContent() { const { jsx: h } = host; return h(NotesEditor, { host, - taskId, + scope, + scopeId: resolvedScopeId, + taskId: scope === DEFAULT_SCOPE ? resolvedScopeId : undefined, surfaceId: "note-modal", presentation: "modal", + onCloseModal: close, }); }; } @@ -845,16 +931,21 @@ export function makeNoteModalContent(host, taskId) { // React layer. Deliberately thin: all the guard logic above is framework- // free and unit-tested directly; these components only subscribe to it. // --------------------------------------------------------------------------- -function useNoteStore(host, { taskId, surfaceId }) { +function useNoteStore(host, { scope = DEFAULT_SCOPE, scopeId, taskId, surfaceId }) { const React = host.React; + const resolvedScopeId = scopeId ?? taskId; const storeRef = React.useRef(null); const [snapshot, setSnapshot] = React.useState(null); React.useEffect(() => { const store = createNoteStore(host, { - taskId, + scope, + scopeId: resolvedScopeId, surfaceId, - onCommit: (hasNote) => markNote(store.getSnapshot().taskId, hasNote), + onCommit: (hasNote) => { + const snap = store.getSnapshot(); + markNote(snap.scopeId, hasNote, snap.scope); + }, }); storeRef.current = store; setSnapshot(store.getSnapshot()); @@ -864,15 +955,15 @@ function useNoteStore(host, { taskId, surfaceId }) { store.dispose(); storeRef.current = null; }; - // surfaceId identifies the store; a taskId change while the same + // surfaceId/scope identify the store; a scopeId change while the same // surface stays mounted is handled by the effect below via - // store.setTaskId(), not by recreating the store. + // store.setScopeId(), not by recreating the store. // eslint-disable-next-line react-hooks/exhaustive-deps - }, [host, surfaceId]); + }, [host, surfaceId, scope]); React.useEffect(() => { - if (storeRef.current) storeRef.current.setTaskId(taskId); - }, [taskId]); + if (storeRef.current) storeRef.current.setScopeId(resolvedScopeId); + }, [resolvedScopeId]); return { snapshot, store: storeRef.current }; } @@ -960,10 +1051,22 @@ export function injectPluginStyles() { document.head.appendChild(style); } -function NotesEditor({ host, taskId, surfaceId, presentation }) { +// notePlaceholderFor names the thing the note is actually about. NotesEditor +// renders for both scopes now, so a single hardcoded "about this task" reads +// as wrong copy directly under a modal titled "Workspace notes —