Commit Graph
45 Commits
Author SHA1 Message Date
e7742b052b MUL-7776: fix(api): stop returning raw error text in 500 responses (#8905)
* fix(api): stop returning raw error text in 500 responses

39 handlers answered a 500 with the message followed by err.Error(),
sending database and store errors (constraint names, SQL states, Redis
errors) to API clients, while the other 720 500 responses in the handler
package send a fixed message. These now send their fixed message too and
log the error with the request's attributes, as the handlers that already
logged it do.

* fix(api): drop raw error text from the task callbacks and skill import 500s

completeTask and failTask sent err.Error() as the whole 500 message, and
the structured skill import put it in the result's Reason on its three
500s. They now send fixed messages and log the error. The task callbacks
keep their 500, so the daemon still retries them: isTransientError reads
the status code, never the body.

* fix(api): close remaining raw error leaks

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: Yushen <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-10-10 12:17:42 +08:00
klinsmaya 904693bed9 MUL-7336: feat(labels): add skill label CLI commands and skills list label filtering (#8368)
Brings Skill labels to the CLI and makes them usable for finding skills in the web app.

- `multica label` becomes resource-type aware (`--resource-type issue|skill`, `--description`), and `multica skill label list/add/remove` manages labels on a specific skill. Short IDs now resolve against the right catalog.
- The skills page toolbar gains a Labels filter; `GET /api/skills` bulk-attaches labels so the list does not go N+1.
- Agent labels are deliberately excluded, keeping the decision from #6245.
- CLI docs updated in all four locales.
2026-09-16 00:20:42 +08:00
54027ba763 MUL-6639: fix(skills): metadata-only skill listings and stall-based timeouts (#7574)
* MUL-6639: feat(skills): list skill files as metadata, not content

`multica skill get` and `multica skill files list` both downloaded every file
body to render output that shows none of them — `skill get`'s table view prints
four columns, and the file listing prints paths and timestamps. Past a few
hundred KB that stopped being waste and became a hard failure: a 599KB skill
could not be fetched at all, and the listing that would have shown which file
was oversized required downloading all of them first, so the diagnostic died
with the thing it was meant to diagnose (GH #7498).

Both endpoints now take `?include=content|metadata`. The metadata shape carries
path, byte size and content hash per file, plus the size and hash of the
SKILL.md body — computed in Postgres, so the bodies never leave the database.
Response size now tracks file count, not content size.

`GET /api/skills/{id}/files` defaults to metadata: it is a list endpoint, the
CLI is its only caller, and `GET /api/skills` already dropped content for this
exact reason (GH #2174). `GET /api/skills/{id}` keeps returning content by
default — installed web and desktop builds call it for the skill editor and
cannot be retrofitted with a query parameter. The CLI asks for metadata on both
and takes `--with-content` when the bodies are actually wanted.

The file listing gains a SIZE column, which is what makes an oversized skill
diagnosable without downloading it.

Co-authored-by: multica-agent <[email protected]>

* MUL-6639: fix(cli): fail skill transfers on stall, not elapsed time

`http.Client.Timeout` is a wall clock on the whole request, body included, so
it punishes exactly the transfer that is working: a 599KB skill arriving
steadily over a slow link was cut off at 30s mid-body, while a genuinely dead
connection was held open for the same 30s (GH #7498). Raising the constant
moves the cliff without removing it — the honest test is not "has this taken
too long?" but "is it still making progress?".

internal/cli/stall.go adds a client that asks the second question: bounded
connect, TLS and response-header phases, and a response body that fails after
15s with no new bytes. The wall clock stays as a loose 10-minute backstop —
demoted, not deleted, because dropping it to 0 would have left a stalled
request upload with no deadline at all.

Failures now say which thing went wrong. A new KindNetworkStalled reports "no
data received for 15s after N bytes" rather than a timeout, because only one of
those is fixed by waiting longer. Body-read errors are classified at all now:
wrapTransport only ever saw errors from http.Client.Do, which returns once the
headers arrive, so the failure #7498 actually reports reached users as a raw
"context deadline exceeded ... while reading body" out of the JSON decoder.

MULTICA_HTTP_STALL_TIMEOUT sets the no-progress budget. An explicitly set
MULTICA_HTTP_TIMEOUT still applies on this path, keeping its plain meaning —
the longest you will wait for a server — instead of silently reverting to a
total-elapsed limit.

Adoption starts with the skill commands, where the failure was reported; the
mechanism itself is not skill-specific. Graduation is one edit: point
NewAPIClient at NewStallAwareHTTPClient, fold StallAwareContext into
APIContext, drop newSkillAPIClient. A permanent second client would leave the
CLI with two timeout personalities, which is worse than either alone.

Co-authored-by: multica-agent <[email protected]>

* MUL-6639: fix(skills): hash raw UTF-8, keep the no-include default at content

Review found two problems with the previous two commits.

**The file hash was not over the file's bytes.** `sha256(content::bytea)` runs
the bytea *input* parser over the text rather than taking its bytes, so it
reads backslash escapes: a file containing `\x41` hashed as the single byte
`A`, disagreeing silently with the Go-side hash of the SKILL.md body.

Worse than the mismatch, and not caught earlier because the first check used
accented text with no backslash in it: `content::bytea` *errors* on a bare
backslash. `C:\Users\...`, a regex `\d+`, a LaTeX snippet — ordinary contents
of a skill's supporting files — fail with "invalid input syntax for type
bytea", making the listing 500 on the skills most likely to need it.
`convert_to(content, 'UTF8')` takes the bytes, and matches Go exactly.

**`/api/skills/{id}/files` must not change what a bare request returns.** The
previous commit flipped its default to metadata on the grounds that the CLI is
its only caller. That was the wrong reading of "only caller": the CLI is
installed software too, and an older `skill files list --output json` reads
`content`. It is the same argument that kept the detail endpoint's default —
applied inconsistently to a client that upgrades separately from the server.

Both endpoints now return content when `?include=` is absent. The shrink
travels with the caller instead: the CLI sends `include=metadata` itself, so
GH #7498 is fixed with no synchronized release, and the default can flip once
clients that send `include=content` have aged in. The default is now uniform,
so resolveSkillInclude no longer takes a per-endpoint parameter.

Tests: TestSkillFileHashCoversRawUTF8Bytes covers `\x41`, invalid hex, bare
backslashes, Unicode and empty bodies against the Go hash — verified to fail on
the previous SQL. TestSkillEndpointsWithoutIncludeStillReturnContent pins the
compatibility guarantee on both endpoints.

Not addressed, and noted as follow-up: the metadata path still loads the full
SKILL.md into Go, because loadSkillForUser needs the row for its tenant check.
The HTTP response no longer grows with content; the database read still does.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-26 15:45:06 +08:00
Danila Katalshov ec6a7b1285 MUL-6356: fix(skills): skip binary supporting files in runtime-local skill sync (#7175)
* fix(skills): skip binary supporting files in runtime-local skill sync

collectLocalSkillFiles read every supporting file into
SkillFileData.Content as a Go string. That string crosses the
daemon/server hop as JSON, and encoding/json rewrites each invalid UTF-8
byte to U+FFFD, so writeSkillFiles recreated a file whose bytes differ
from the original. A .docx, .xlsx or .pdf carried in a runtime-local
skill bundle therefore arrived in the prepared task environment corrupt
and would not open.

The archive/URL importer already refuses this class of file for the
mirror-image reason: those bytes cannot go into a PG TEXT column. Its
extension blacklist moves to internal/skill as
IsLikelyBinaryFilePath so both ingestion paths share one definition,
and runtime-local collection now skips the same files with a log
breadcrumb, matching the importer's behaviour.

The skip does not depend on includeContent: discovery reports the bundle
with content omitted and sync uploads it with content included, so a
skip in only one pass would leave the two disagreeing about which files
make up the bundle.

Fixes #7143

* fix(skills): catch binary supporting files by content, not just extension

collectLocalSkillFiles only skipped files whose extension matched a
fixed blacklist, so a binary file with an unlisted or missing
extension (.safetensors, .parquet, a stray no-extension blob) still
went through SkillFileData.Content and came back corrupted on the
other side. Read the file and require utf8.Valid on both the
includeContent=false (discovery) and includeContent=true (sync)
passes so they keep agreeing on the bundle; IsLikelyBinaryFilePath
stays as the cheap extension-based first pass.

Adds regression cases for an unlisted binary extension, a
no-extension binary file, and a valid-UTF-8 text file that must still
pass through untouched.

* fix(skills): require NUL-free content, check extension before reading

utf8.Valid alone is not a sufficient round-trip guarantee: a NUL byte
is legal UTF-8, but the server-side import path strips every 0x00 via
sanitizeNullBytes (server/internal/handler/skill_create.go), so a file
that is valid UTF-8 but contains an embedded NUL still comes back
different from what went in. UTF-16LE text made of ASCII characters is
a realistic example. Require content to be both valid UTF-8 and
NUL-free, with embedded_nul as its own skip reason distinct from
invalid_utf8.

Also reorders the extension check ahead of the read: IsLikelyBinaryFilePath
is genuinely the cheap first pass now, so a known-binary extension never
pays for the os.ReadFile it doesn't need.

Adds regression cases for an unlisted extension and a no-extension file,
each containing valid UTF-8 with an embedded NUL.
2026-08-20 23:57:52 +08:00
d6301091e0 MUL-6310 fix(agent tasks): stop NUL bytes wedging tasks on complete/messages/cancel (#7124)
* fix(agent tasks): stop NUL bytes wedging tasks on complete/messages/cancel

GH #7098 reported that a NUL in a failure diagnostic 500s POST
/api/daemon/tasks/{id}/fail and leaves the task stuck in 'running'.
PR #7115 fixes that one endpoint. Verifying the report turned up the
same defect on three more persistence paths, including the one users
are most likely to hit.

/complete is the important one. The whole request is marshalled into
agent_task_queue.result, a JSONB column, and encoding/json renders an
embedded NUL as a unicode escape that PostgreSQL rejects with SQLSTATE
22P05 - a different error from the TEXT case, same user-visible wedge.
Agent stdout carries a stray NUL far more readily than a curated error
string does: a tool that cats a binary, a Windows host emitting UTF-16,
a child process dumping memory as it crashes.

Nothing reaps the wedged row. Both sweepers (FailStaleTasks,
FailTasksForOfflineRuntimes) require the runtime to be offline, so a
healthy heartbeating daemon leaves the task in 'running' indefinitely,
holding one of the agent's max_concurrent_tasks slots forever.

Changes:

- util.SanitizeTextForPostgres - one shared definition of "safe to
  persist": remove NUL, replace other invalid UTF-8 with U+FFFD.
  handler.sanitizeNullBytes now delegates to it, so the task endpoints
  and the comment/skill/property endpoints cannot drift apart. This
  does change behaviour at those existing call sites: invalid UTF-8
  becomes U+FFFD instead of being dropped silently.
- util.SanitizeJSONForPostgres - depth-bounded walk over a decoded JSON
  tree, keys included. task_messages.input is JSONB, so a NUL nested in
  a tool's arguments breaks the insert even when every top-level string
  is clean.
- /complete: scrub right after decode, before the context-exhaustion
  re-route, so the failure classifier sees the text we will persist.
- /messages: scrub the strings and walk input. This endpoint has no
  daemon-side retry, so an unguarded batch is dropped silently.
- /cancel-ack and CancelTaskWithResult: same TEXT columns, same hazard.

Note for reviewers: strings.ToValidUTF8 alone does NOT solve this. NUL
is a valid UTF-8 encoding of U+0000 and passes straight through, which
is why the explicit ReplaceAll is load-bearing and has a test pinning
it.

Tests: unit coverage for both helpers, plus PostgreSQL-backed
regressions for the /complete JSONB payload and a NUL nested two levels
into task message input. Both include a control leg asserting the
database really does reject the unsanitized payload, so deleting the
sanitizer fails the test rather than silently passing.

Refs GH #7098, MUL-6310. Follow-up to #7115.

Co-authored-by: multica-agent <[email protected]>

* fix(agent tasks): sanitize messages.type; drive NUL tests through the handlers

Addresses both must-fix items from review.

1. TaskMessageRequest.Type was still written to a TEXT column unsanitized,
   so a NUL in `type` alone kept the whole batch 500ing and being dropped.
   Sanitized alongside tool/content/output, and covered by the entry test.

2. The two regression tests called the sanitizers by hand and then hit the
   query directly. That proved the helpers and the database behaviour but
   not the wiring: deleting the sanitize calls from CompleteTask /
   ReportTaskMessages left both tests green, which is the one regression
   they existed to catch.

   The positive branches now POST through testHandler.CompleteTask and
   testHandler.ReportTaskMessages via newDaemonTokenRequest, asserting HTTP
   200 plus the persisted row. The NUL travels the way the report describes
   -- escaped on the wire by encoding/json, rebuilt by the decoder -- so the
   handler sees exactly what a daemon would send.

   Verified by sabotage: commenting out the sanitize calls fails both tests
   with the reported errors (22P05 on /complete, "failed to persist task
   message" on /messages). Restored before commit.

   The direct-to-database rejection legs are kept and consolidated into
   TestPostgresRejectsUnsanitizedTaskPayloads, now also covering the TEXT
   `type` column, so the suite still proves PostgreSQL genuinely refuses
   these payloads rather than the fix being a no-op.

Seeding note: the fixture task now gets an issue, because
requireDaemonTaskAccess resolves the workspace through it and a task with no
issue / chat / autopilot link 404s before the handler reads the body.

Refs GH #7098, MUL-6310.

Co-authored-by: multica-agent <[email protected]>

* refactor(agent tasks): collapse normalizeTaskFailureDiagnostic into the shared helper

#7115 landed its own sanitizer at the FailTask service boundary while this
branch added util.SanitizeTextForPostgres for the remaining paths. The two
had identical semantics, which is exactly the drift this PR set out to
prevent: one definition of "safe to persist", not one per entry point.

FailTask now calls util.SanitizeTextForPostgres directly. The service-boundary
guard itself stays — failClaimedTaskBeforeLaunch reaches FailTask without
going through request decoding, so the handler-level scrub does not cover it.

Drops TestNormalizeTaskFailureDiagnostic, whose cases are a subset of
TestSanitizeTextForPostgres in internal/util. TestFailTaskSanitizesNULDiagnostic
is kept untouched: it drives the real FailTask against PostgreSQL and asserts
the task reaches 'failed' with the diagnostic intact, so it still guards the
wiring — and it passing against the shared helper is what demonstrates this
collapse preserved behaviour.

Refs GH #7098, MUL-6310.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-18 14:17:14 +08:00
GuillaumeandGuillaume Sayer 147cd8d84f feat(skills): update imported skills from their source (#6847)
A skill imported from GitHub, skills.sh, or ClawHub can now be re-downloaded
in place via POST /api/skills/{id}/refresh, the Update button on the skill
page / list row menu, or 'multica skill refresh <id>'. The skill id,
created_by, created_at, labels, and agent_skill bindings are preserved, so
agents keep the skill without re-adding it; name (upstream renames are
adopted, 409 on collision), description, SKILL.md, files, and config.origin
are replaced. Permission follows canManageSkill (creator or workspace
owner/admin); non-refreshable origins (manual, archive, runtime_local)
return 422. Custom config keys are preserved by merging only the origin key.

Co-authored-by: Guillaume Sayer <[email protected]>
2026-08-13 13:48:18 +08:00
71bd844955 MUL-6055 refactor(agents): remove legacy agent templates (#6820)
Co-authored-by: Lambda <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-08-12 16:09:16 +08:00
Bohan Jiang 4233e82441 fix(skills): tree-first skill import to stop 504 on large repos (MUL-5136) (#5753)
Rework skills.sh and github.com skill imports around a single recursive
git-tree fetch to stop the 504 on large mono-repos (e.g. api-gateway-skill):

- One tree call replaces the per-directory contents crawl.
- Import caps checked arithmetically from tree metadata before any download
  (fail fast with 413 instead of timing out).
- Most-specific skill-dir resolution; repo root only as a last resort, which
  fixes the root SKILL.md name collision.
- Concurrent downloads (errgroup, limit 8).
- Overall 45s fetch deadline; cancellation is fatal on every supporting-file
  path (tree downloader, crawl listing/recursion/download, ClawHub) so a
  mid-download abort never persists a half-populated bundle.
- A skills.sh tree-fetch failure returns a retryable 503 instead of an unsafe
  root-directory fallback.
- Lenient conventional-path acceptance restored for both complete and truncated
  trees.
- maxImportFileCount 128 -> 256 (aligned daemon cap); 8 MiB bundle cap remains
  the real guard.
2026-07-22 14:38:31 +08:00
6c6143e8fc fix(agents): always enable skill toggles (MUL-4520) (#5381)
Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-14 13:39:57 +08:00
411a160b99 fix(release): harden v0.3.44 migrations (#5345)
Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-13 18:13:44 +08:00
Jiayuan Zhang a14098288b feat: redesign agent Skills and MCP capabilities (#5277) 2026-07-12 02:53:17 +08:00
Bohan Jiang b90816264e feat(skills): import skills from a .skill/.zip archive (#4735)
Import a skill from a local .skill/.zip archive: POST /api/skills/import now accepts a multipart upload (file + on_conflict) alongside the JSON URL body, and the CLI gains `multica skill import --file <path>`. Reuses the existing create + on_conflict contract, per-file/bundle/count caps, reserved-SKILL.md rule, and a zip-slip guard.

Closes #4730
MUL-3865
2026-06-30 14:46:46 +08:00
39ab355585 fix(skills): authenticate raw.githubusercontent.com downloads for private repo imports (#4389)
Skill import builds raw.githubusercontent.com URLs by hand and fetches them
via fetchRawFile, which sent no Authorization header. GitHub API calls were
authenticated by #2215 (doGitHubAPIGet/addGitHubAuthHeader) but the raw
content download path was missed, so importing a skill from a private/internal
GitHub repo listed the directory fine and then 404'd on the actual file
download, surfacing as a generic 502.

Attach the existing GITHUB_TOKEN bearer header in fetchRawFile, but only when
the URL host is raw.githubusercontent.com. fetchRawFile is shared with
clawhub.ai / skills.sh downloads, so the token must not leak to those hosts.
The host gate is extracted into newRawFileRequest so it is unit-testable
without a live round-trip.

MUL-3496

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-22 13:34:06 +08:00
e4ec9dc425 MUL-2802: add skill import conflict strategies (#3997)
* feat(skills): structured conflict + overwrite path for local skill re-import

Local-skill re-import previously failed (or silently skipped) on a same-name
collision and, on delete+reimport, changed the skill UUID and dropped agent
bindings. This adds a structured conflict result and a creator-only overwrite
write path so a re-import can update the existing skill in place.

- New terminal import status `conflict` carrying { existing_skill_id,
  existing_created_by, can_overwrite }; can_overwrite = requester is the
  skill creator (canOverwriteSkillByLocalImport — intentionally narrower than
  canManageSkill: admins edit in-app, not via re-import).
- Conflict is detected at daemon-report time (the effective name is only known
  once the bundle arrives) via GetSkillByWorkspaceAndName, with the unique
  constraint as a race backstop.
- Import requests carry action=overwrite + target_skill_id, persisted through
  both the in-memory and Redis LocalSkillImportStore (the heartbeat → daemon
  payload is unchanged; overwrite is resolved server-side).
- overwriteSkillWithFiles updates by target_skill_id in one tx: re-checks
  existence (workspace-scoped) and creator permission, then replaces
  description/content/config and fully replaces files (pruning files absent
  from the new bundle). Preserves id, created_by, created_at, name, and
  agent_skill bindings. Publishes skill:updated (not skill:created).
- Boundaries: target deleted or permission lost → failed (no fallback to
  create-by-name); any mid-write error rolls back the tx, leaving the original
  skill untouched. Retrying a terminal request is a no-op.

Tests cover: creator/non-creator conflict (can_overwrite), overwrite preserves
UUID + agent binding + prunes removed files, non-creator overwrite fails,
deleted target fails without create fallback, retry idempotency, and Redis
round-trip of the new fields.

Backend half of MUL-2701. Contract change: same-name local imports now return
status `conflict` instead of `failed` — the Desktop/core client must be updated
to consume it (sibling task).

MUL-2800

Co-authored-by: multica-agent <[email protected]>

* fix(skills): gate structured conflict behind client opt-in; guard overwrite target name

Addresses review feedback on PR #3498 (MUL-2800).

Backward compatibility: a same-name local import now returns the new `conflict`
status only when the initiating client opts in via `supports_conflict` (an
overwrite request implies it). Older clients — already-installed Desktop builds
whose poll loop only understands `failed`/`timeout` — keep the legacy `failed`
+ "a skill with this name already exists" behavior, so upgrading the backend
ahead of the client no longer regresses the import UX. This is the installed-app
API-compat boundary the repo's CLAUDE.md calls out.

Also: the overwrite write path now verifies the incoming effective name matches
the target skill's current name (errSkillOverwriteNameMismatch -> failed),
preventing a stale/wrong target_skill_id from writing one skill's content onto
another. Creator-only + workspace scoping already prevent privilege escalation;
this narrows the API so it can't be misused.

Refactored LocalSkillImportStore.Create to a LocalSkillImportRequestInput params
struct (the signature had grown to 8 positional args; the opt-in flag pushed it
over). supports_conflict is persisted in both the in-memory and Redis stores.

Tests: conflict tests now opt in; added a legacy-client test (no flag ->
failed + legacy message) and an overwrite name-mismatch test.

MUL-2800

Co-authored-by: multica-agent <[email protected]>

* feat(skills): resolve local import conflicts in desktop

Co-authored-by: multica-agent <[email protected]>

* fix(skills): preserve bulk flow after conflict resolution

Co-authored-by: multica-agent <[email protected]>

* feat(cli): add skill import conflict strategies

Co-authored-by: multica-agent <[email protected]>

* fix(i18n): sync skill import locale keys

Co-authored-by: multica-agent <[email protected]>

* docs: explain skill import conflict handling

Co-authored-by: multica-agent <[email protected]>

* docs: refresh skill import source map anchors

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-11 13:00:56 +08:00
44feb3d06d fix(skill): canonicalize reserved SKILL.md path check across daemon + API (#3660)
A skill_file row whose path is the skill's own SKILL.md (persisted by
older builds or direct create/update API calls) collides with the
primary content the daemon writes itself, failing task prep with
errPathPreExists on every non-codex local runtime (#3489).

#3526 guarded this with strings.EqualFold(path, "SKILL.md") at the
daemon write site and the three API ingress points, but the stored path
is not canonicalized: "./SKILL.md" or "sub/../SKILL.md" slip past the
exact-match guard while filepath.Join still resolves them onto the same
SKILL.md, so prep can still break.

Extract one canonical helper, skill.IsReservedContentPath, that cleans
the path before the case-insensitive compare, and use it at all four
sites (execenv writeSkillFiles, skill create, update, single-file
upsert). Add a daemon-side regression test for writeSkillFiles ignoring
a bundled SKILL.md (exact + "./" spellings) — the load-bearing fix
previously had only API-layer coverage — plus a unit test for the helper.

Existing poisoned rows are intentionally left in place (skipped at prep)
per the decision on MUL-2928.

MUL-2928
Follow-up to #3526; supersedes #3560.

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-02 18:23:57 +08:00
MSandro 996eb07dc5 fix(daemon): skip duplicate SKILL.md in supporting files to prevent task prep failures (#3526)
Fixes #3489

MUL-2928
2026-06-02 17:53:20 +08:00
Naiyuan Qingandmultica-agent e36f874c86 feat: add additive agent skill assignment (#3642)
* feat: add additive agent skill assignment

Co-authored-by: multica-agent <[email protected]>

* test: cover cross-workspace agent skill add

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: multica-agent <[email protected]>
2026-06-02 15:02:24 +08:00
0f9d9d1494 fix(skills): align Go/TS frontmatter coercion for non-scalar values (#3614)
The Go SKILL.md frontmatter parser unmarshalled into a {Name,Description}
string struct, so a non-scalar value (a list/map written where a scalar
belongs) made the whole decode fail and dropped even a valid sibling
`name`. The TS parser instead kept the name and JSON-encoded the value,
so the file-viewer (TS) and the import path (Go) could disagree about
the same SKILL.md.

Decode into a generic map and coerce per key on the Go side, mirroring
the TS coercion (scalars -> literal form, sequences/mappings -> JSON), so
both sides produce identical results and a structured value never
discards a sibling key. Rename ParseFrontmatter -> ParseSkillFrontmatter
to remove the cross-language name clash with the TS parseFrontmatter
(which returns {frontmatter, body}), and drop the unused TS
parseSkillFrontmatter export.

Add parity tests for sequence/mapping values plus name-only,
description-only, leading-blank-line and triple-dash-in-body edge cases
on both sides.

Follow-up to #3543 / MUL-2842.

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-06-01 19:42:20 +08:00
YOMXXX 801c201d4c fix(skills): parse multi-line YAML frontmatter in SKILL.md (#3495) (#3543)
Three independent line-based frontmatter parsers only handled
single-line `description: value`, so a YAML block scalar
(`description: |`) collapsed to the literal "|" and the rest of the
description was dropped before it ever reached the database.

Replace all three with real YAML decoders that understand block
scalars, folded scalars and quoted values:

- server/internal/skill: shared ParseFrontmatter via gopkg.in/yaml.v3,
  used by both the handler import path and daemon local-skill discovery
- packages/core/skills: shared parseFrontmatter via the yaml package
- file-viewer renders multi-line frontmatter values (whitespace-pre-wrap)

Both parsers fall back to empty values on malformed YAML, preserving the
previous non-fatal behaviour.
2026-06-01 19:35:01 +08:00
Naiyuan Qingandmultica-agent dd4d58f20e feat: add skill search CLI (#3601)
Co-authored-by: multica-agent <[email protected]>
2026-06-01 15:19:42 +08:00
Naiyuan Qingandmultica-agent 2b2888c23a Handle duplicate skill imports as structured results (#3599)
Co-authored-by: multica-agent <[email protected]>
2026-06-01 14:45:16 +08:00
1c91c2a3b2 security(db): scope DELETE/UpdateIssueStatus by workspace_id (defense-in-depth) (#3027)
* fix(security): scope DELETE/UpdateIssueStatus by workspace_id

Add workspace_id to the WHERE clause of DeleteIssue, DeleteComment,
DeleteProject, DeleteSkill, DeleteChatSession, and UpdateIssueStatus
as SQL-layer defense-in-depth.

Handler loaders (loadIssueForUser / loadSkillForUser / etc.) already
enforce workspace membership today, so this is not patching a known
live vuln. But the tenant invariant is currently a handler-layer
guarantee — a future loader bypass or a new caller skipping the
loader would be silently catastrophic. Making workspace_id part of
the SQL identity collapses the trust surface to the schema itself:
forging a sibling-workspace UUID becomes ErrNoRows instead of a
cross-tenant write.

Reference: incident #1661 (util.ParseUUID silent zero UUID returning
204 on a DELETE that matched zero rows) — same class of failure,
prevented at a different layer.

Scope:
- 5 DELETE queries: issue, comment, project, skill, chat_session
- 1 simple UPDATE: UpdateIssueStatus (2 narg, no SET ordering risk)
- All callers updated (handlers, service, runtime sweeper fallback)

Multi-narg UPDATE queries (UpdateIssue, UpdateProject, UpdateSkill,
UpdateComment, UpdateChatSession*) are deferred to a follow-up to
keep this change reviewable: each needs its narg pinning shifted
and per-caller verification.

sqlc was regenerated by hand (no local sqlc toolchain); CI's
backend job is the authoritative compile check.

* test(security): add workspace_scope_guard regression test

Locks in the SQL-layer tenant guard added in this PR. For each of the 6
scoped queries (DeleteIssue, DeleteComment, DeleteProject, DeleteSkill,
DeleteChatSession, UpdateIssueStatus), creates the resource in workspace
A, invokes the query with a foreign workspace UUID, and asserts the row
is untouched (0 rows affected with no error for :exec; pgx.ErrNoRows for
:one). A future refactor that drops the workspace_id arg from any of
these queries will now fail loudly instead of silently regressing.

Includes a sanity sub-test that the in-workspace path still mutates, so
a buggy guard that returns no-op for every call would not pass.

Co-Authored-By: Claude Opus 4 <[email protected]>

---------

Co-authored-by: Tom Qiao <[email protected]>
Co-authored-by: Claude Opus 4 <[email protected]>
2026-05-22 14:39:47 +08:00
Tom QiaoandTom Qiao b9602adabe fix(handler): validate skill id UUID at request boundary (#3025)
loadSkillForUser was passing chi.URLParam(r, "id") directly into
parseUUID, the panic-on-invalid helper reserved for trusted UUID
round-trips. A malformed `/api/skills/{notuuid}` request panicked
in util.MustParseUUID; chi's middleware.Recoverer turned it into a
500 instead of a 400.

This violates the documented convention (CLAUDE.md → "Backend Handler
UUID Parsing Convention"): pure-UUID request inputs must use
parseUUIDOrBadRequest, which writes a 400 and short-circuits.

Switch loadSkillForUser to parseUUIDOrBadRequest. Behaviour for valid
UUIDs is unchanged; malformed input now returns 400 with a clear
"invalid skill id" message.

Test:
- TestGetSkill_MalformedUUIDReturns400 asserts GET /api/skills/not-a-uuid
  returns 400.

Co-authored-by: Tom Qiao <[email protected]>
2026-05-22 12:22:07 +08:00
623d29f276 feat(agents): one-click create from curated templates (Phase 1) (#2520)
* docs(agents): three-phase agent quick-create plan

Captures the full design for moving agent creation from manual form +
one-by-one skill attachment to a tiered experience:

- Phase 1 (this PR): one-click curated templates, AI-free.
- Phase 2 (next): AI-recommended skills via the existing quick-create
  task mechanism — no new server-side LLM dependency.
- Phase 3 (later): AI creates the whole agent end-to-end, composing
  Phase 2 with a new `multica agent create` CLI driver.

Documents the architectural decisions that keep all three phases on
existing infrastructure (no SSE, no server-side LLM SDK, no new WS
channels), the two soft blockers Phase 1 unlocks for later phases
(createSkillWithFiles TX composability + skill same-name dedupe), and
the scope decisions we explicitly opted out of (Anthropic plugin
marketplace, ClawHub UI affordances).

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

* fix(skills): harden import against invalid UTF-8 and binary files

PG rejects two byte patterns in a TEXT column. Both crashed real skill
imports we hit while assembling the template catalog:

- Embedded NUL (0x00) -> SQLSTATE 22021. Already stripped by
  sanitizeNullBytes, kept as-is.
- Other invalid UTF-8 (e.g. 0x91 — Windows-1252 smart quote in a skill
  whose author saved prose from Word). sanitizeNullBytes now also runs
  strings.ToValidUTF8 over the content so the second class no longer
  takes the whole import down.

For non-text payloads (images, fonts, archives, compiled binaries),
sanitization isn't the right fix — agents never read those as text,
and the bytes can't survive a TEXT column at all. addFile now skips
them by extension before the per-bundle cap counters tick, logging
the skip so an unexpected drop leaves a breadcrumb.

Function name kept for compatibility with the many call sites; both
behaviours are strict supersets of the original.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

* refactor(skills): split createSkillWithFiles for tx composition + add workspace find-or-create query

Two soft blockers cleared so create-from-template (next commit) can
fold N skill creates and the agent + binding writes into one outer
transaction:

1. createSkillWithFiles used to Begin/Commit its own tx. Caller
   composition was impossible — N invocations meant N separate
   transactions and no atomicity over the whole materialise step.
   Pull the body into createSkillWithFilesInTx(ctx, qtx, input); the
   original function becomes a thin wrapper that manages its own tx
   for standalone callers. Existing call sites: zero behaviour change.

2. Add GetSkillByWorkspaceAndName sqlc query — workspace skill lookup
   by name, anchored to UNIQUE(workspace_id, name) from migration
   008. Lets the template materialiser implement find-or-create:
   reuse the workspace's existing skill row when a template
   references the same name, rather than crashing on the unique
   constraint or polluting the workspace with `<name>-2` clones.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

* feat(agents): agent template catalog + create-from-template endpoint

Server-side foundation for Phase 1 of the quick-create roadmap (see
docs/agent-quick-create-plan.md). Adds:

- server/internal/agenttmpl/ — embed-loaded catalog of curated agent
  templates. Each template ships pre-written instructions plus a list
  of skill URLs that get materialised into the workspace at create
  time. Validation runs at startup (init() panics on a malformed
  template) so a bad JSON ships as a deploy-time defect, not a
  runtime 500. Slug must equal the filename basename so the URL
  router is mirror-symmetric with the file layout.

- 11 starter templates covering Engineering / Writing / Building /
  Testing (code-reviewer, frontend-builder, planner, docs-writer,
  one-pager, html-slides, full-stack-engineer, …).

- Three new endpoints, all behind RequireWorkspaceMember:
    GET  /api/agent-templates           — picker list (no instructions)
    GET  /api/agent-templates/:slug     — detail with instructions
    POST /api/agents/from-template      — materialise + create

  Create flow:
    1. Auth + runtime authorization happen BEFORE the GitHub fan-out
       so a 403 never wastes 20s of upstream fetches.
    2. Pre-flight dedupe by cached_name reuses workspace skills
       without an HTTP fetch — second create-from-the-same-template
       drops from 20s to <100ms.
    3. Parallel fetch (30s per-URL timeout) for the remaining skills.
    4. Single transaction: every skill insert, the agent insert, and
       the agent_skill bindings. On any upstream fetch failure the TX
       rolls back and the API returns 422 with `failed_urls` so the
       UI can name the bad source(s).
    5. extra_skill_ids (user-supplied additions) are verified through
       GetSkillInWorkspace per id before attach, so a malicious client
       can't graft a skill from another workspace via UUID guessing.

- multica agent create --from-template <slug> CLI flag dispatches to
  the new endpoint with a 60s ceiling, matching `multica skill import`.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

* feat(agents): one-click create-from-template UI

Frontend half of Phase 1. CreateAgentDialog becomes a state machine
spanning four steps:

  chooser          → Start blank / From template cards
  blank-form       → existing manual form (post-chooser)
  duplicate-form   → existing form pre-filled from a duplicated agent
  template-picker  → grid of templates, click navigates to detail
  template-detail  → instructions + skill list preview + one-click Use

Picking a template never lands on the form: name auto-deduped against
existingAgentNames, runtime = first usable one, visibility = private.
Refinement happens on the agent detail page if needed. Same rationale
the doc spells out — templates exist precisely to skip configuration.

New components, all collapsible-by-default so quick-create stays fast:
  - template-picker.tsx — categorised grid, lucide icons + semantic
    accent tokens resolved through static maps so Tailwind's JIT picks
    up every variant (dynamic class strings would silently miss).
  - template-detail.tsx — instructions preview, skill list with cached
    descriptions, Use CTA. Renders the failedURLs banner when a 422
    fires — the only step that can trigger that response.
  - instructions-editor.tsx — collapsed preview-card / expanded full
    ContentEditor.
  - skill-multi-select.tsx + skill-picker-list.tsx — shared multi-
    select surface, also adopted by the existing skill-add-dialog.
  - avatar-picker.tsx — agent avatar upload, mirrors the inspector's
    visual language.

Schema-defended client (CLAUDE.md → API Response Compatibility): the
three new endpoints are wired through parseWithFallback with lenient
zod schemas. Desktop builds outlive any given server — a future
field rename / wrapping must not white-screen older installs.
listAgentTemplates accepts both the current bare array and a future
{templates: [...]} envelope. Coverage: 7 new schema-test cases in
schema.test.ts (null body, missing skills/instructions, malformed
create response, envelope migration).

Catalog + detail go through TanStack Query with staleTime: Infinity —
workspace-independent static data, no per-mount refetch.

Other:
- skill-add-dialog becomes a true multi-select (Confirm button +
  checkbox list); attached skills are filtered out of the list.
- agents-page hands the freshly-created Agent back to the dialog so a
  follow-up setAgentSkills can attach the form-selected skills.
- agent-overview-pane drops the mx-auto/max-w-2xl frame on config-
  tab content; the wider dialog visual language reads better with
  tabs filling the column.
- Every new UI string lives in both en/agents.json and
  zh-Hans/agents.json under create_dialog.* / tab_body.skills.* —
  locales/parity.test.ts blocks drift in CI.

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>

* fix(ci): align skill import test + drop next-only lint suppression

- TestFetchFromSkillsSh_ResolvesRootLevelSkillMd now expects assets/logo.png
  to be skipped; matches the new addFile binary-extension guard
  (6fafd86e). The .png is intentionally dropped so PG TEXT inserts don't
  hit SQLSTATE 22021.
- packages/views shares zero next/* deps, so the @next/next/no-img-element
  eslint plugin isn't loaded there. The eslint-disable directive
  referencing it produced a hard "rule not found" error in CI lint. Raw
  <img> is the right primitive in views; remove the disable comment.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-authored-by: multica-agent <[email protected]>

* test(agents): wrap CreateAgentDialog tests in workspace/navigation providers

The dialog now calls useNavigation() and useWorkspacePaths(), both of
which throw outside their providers. The existing tests rendered the
dialog bare and tripped both new requirements:

- NavigationProvider — supply a stub adapter so push() works for the
  agent-detail redirect.
- WorkspaceSlugProvider — useWorkspacePaths() requires a slug.

The blank-vs-template chooser is now the default first step; the
existing tests target the runtime picker on the manual form, so the
helper auto-clicks "Start blank" when no template is passed
(duplicate-mode tests skip the chooser).

Manual afterEach(cleanup) + document.body wipe. Base UI's Dialog
portal renders into document.body and leaves focus-guard/inert wrapper
divs behind across tests, so the second test in the suite saw two
"All" / "My Runtime" matches and getByText failed. The wipe is local
to this file rather than the shared setup because it isn't a global
issue — only suites that open Base UI dialogs hit it.

Co-Authored-By: Claude Opus 4.7 <[email protected]>
Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: Claude Opus 4.7 (1M context) <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-05-13 18:26:04 +08:00
LinYushenandCopilot a6e8ae964e fix(skills): handle GitHub API 403 / rate limit during skill import (#2215)
Importing a skill from a github.com URL probes the commits API to
disambiguate slash-bearing refs. On self-hosted servers the IP is often
already over GitHub's 60-req/hour unauthenticated limit, so the very
first probe returns 403 and the previous code aborted the entire
import ("validating ref \"main/skills/pptx\": github API returned
status 403").

Two changes make this resilient:

* Forward GITHUB_TOKEN as a bearer token on every api.github.com request
  via a new doGitHubAPIGet / addGitHubAuthHeader helper. With a token,
  the limit becomes 5000 req/hour and the issue disappears entirely.
* When the API still returns 401/403/429 (no token, or limit exhausted
  on the higher tier) treat the probe as indeterminate via
  errGitHubAPIBlocked, keep trying remaining candidates, and finally
  fall back to parseGitHubURL's optimistic single-segment split. This
  covers the common case (single-word refs like "main") even when the
  API is fully blocked. A warn log points operators at GITHUB_TOKEN.

Co-authored-by: Copilot <[email protected]>
2026-05-07 16:28:34 +08:00
Multica Eve d82a2d8a04 feat(skills): support importing skills from github.com URLs (#2209) 2026-05-07 15:22:34 +08:00
Bohan Jiangandmultica-agent d0ac67dea2 fix(skills): drop SKILL.md content from list endpoints (#2180)
* fix(skills): drop SKILL.md content from list endpoints (#2174)

`GET /api/skills` and `GET /api/agents/{id}/skills` were SELECT *'ing the
skill row and shipping the full SKILL.md `content` blob to every caller.
SKILL.md bodies routinely run 50–200KB each, so a workspace with 30–40
skills returned multi-megabyte JSON arrays — past the CLI's 15s timeout
on high-latency links and locking out non-US users entirely.

Add `ListSkillSummariesByWorkspace` / `ListAgentSkillSummaries` sqlc
queries that omit `content`, plus a dedicated `SkillSummaryResponse`
wire shape so the contract is explicit (versus stuffing
`Content: ""` back into the existing struct). Detail endpoints
(`GET /api/skills/{id}`, agent CRUD return values) keep returning the
full body.

`AgentResponse.skills` and the matching TS `Agent.skills` now use
`SkillSummary[]` — frontend list/columns code already only read
id/name/description/config.origin, so the type narrowing matches actual
usage and prevents new code from accidentally depending on a content
field that won't be there.

Co-authored-by: multica-agent <[email protected]>

* fix(agents): narrow embedded skills to AgentSkillSummary; gofmt agent.go

GPT-Boy review of #2180: the previous commit typed AgentResponse.Skills as
[]SkillSummaryResponse, but the agent list batch query
(ListAgentSkillsByWorkspace) only joins agent_id/id/name/description, so
the wider type left workspace_id/config/created_at/updated_at as zero
values. Define a dedicated AgentSkillSummary {id,name,description} that
matches what the batch query actually returns and what the frontend
actually reads (`agent.skills.map(s => s.name|s.id)`); the standalone
GET /api/agents/{id}/skills endpoint keeps SkillSummaryResponse for
callers that need the source/origin info.

Switch GetAgent's per-agent skills load from ListAgentSkills (full Skill
rows including content) back to ListAgentSkillSummaries to avoid reading
SKILL.md bodies just to discard them.

Re-run gofmt on agent.go to fix the field-tag alignment that drifted when
Skills changed type.

Co-authored-by: multica-agent <[email protected]>

* docs(types): correct SkillSummary JSDoc — Agent.skills is AgentSkillSummary[]

GPT-Boy spotted on review: comment said SkillSummary was "embedded in
Agent.skills", but that field is now AgentSkillSummary[]. Re-point the
reader at the right type to avoid future confusion.

Co-authored-by: multica-agent <[email protected]>

---------

Co-authored-by: multica-agent <[email protected]>
2026-05-07 01:36:29 +08:00
wucm667 2305f7d180 fix(skill): sanitize null bytes in all skill update/upsert paths to prevent PostgreSQL UTF8 error (#1959) 2026-04-30 22:34:24 +02:00
Bohan JiangandEve f628e48775 refactor(server): error-returning ParseUUID to prevent silent data loss
* refactor(server): make ParseUUID error-returning to prevent silent data loss (MUL-1410)

util.ParseUUID previously swallowed errors and returned a zero pgtype.UUID
on invalid input. When this zero UUID reached a write query (DELETE/UPDATE),
the SQL matched zero rows and the handler returned 2xx success — producing
silent data corruption. #1661 (DeleteIssue with identifier-style ID) was the
visible symptom; PR #1680 patched that one site, this commit closes the
class of bug.

Changes:

- util.ParseUUID now returns (pgtype.UUID, error). Add util.MustParseUUID
  for trusted round-trips that should panic on invalid input.
- handler/handler.go: parseUUID wrapper now calls MustParseUUID — any
  unguarded user-input string reaching it surfaces as a recovered panic
  (chi middleware.Recoverer → 500) instead of silently corrupting data.
  Add parseUUIDOrBadRequest(w, s, fieldName) for handler entry points.
- Convert every Queries.Delete*/Update* call site reachable from raw user
  input (autopilot, comment, project, skill, skill_file, label, pin,
  attachment, feedback, issue assignee, daemon runtime, workspace) to
  validate UUIDs explicitly with parseUUIDOrBadRequest, returning 400 on
  invalid input. Where a resolved entity.ID is already in scope, write
  queries now use it directly instead of re-parsing the URL string.
- Update getWorkspaceMember + loadIssueForUser to handle invalid UUIDs
  gracefully (404/400 instead of panic).
- Update util/middleware/cmd-level callers (subscriber_listeners,
  notification_listeners, activity_listeners, scope_authorizer,
  middleware/workspace) to use the error-returning API.
- Add server/internal/util/pgx_test.go covering valid/invalid input and
  the MustParseUUID panic contract.
- Add TestDeleteIssueByIdentifier + TestDeleteIssueRejectsInvalidUUID
  regression tests in handler_test.go (the original #1661 bug + the
  invalid-input case).
- Document the handler UUID parsing convention in CLAUDE.md so the rule
  is enforceable in future PR review.

* fix(server): address GPT-Boy review of #1748

P1 fixes from PR #1748 review:

1. Migrate remaining request-boundary UUIDs to parseUUIDOrBadRequest so
   malformed input returns 400 instead of panic/500. Was missing on:
   - issue.go: workspace_id in CreateIssue/ChildIssueProgress/ListIssues/
     SearchIssues/BatchUpdateIssues/BatchDeleteIssues; project_id /
     parent_issue_id / lead_id / assignee_id / assignee_ids / creator_id
     filters; batch issue_ids and assignee/parent/project fields in
     BatchUpdateIssues (skip on bad input via util.ParseUUID, matching
     the existing per-row continue semantics).
   - project.go: project id + workspace_id in GetProject/UpdateProject/
     DeleteProject; lead_id in CreateProject/UpdateProject;
     workspace_id in ListProjects + SearchProjects.
   - handler.go: resolveActor now uses util.ParseUUID for X-Agent-ID /
     X-Task-ID headers; invalid UUID falls back to "member" (matches
     pre-existing semantics) instead of panicking.
   - issue.go: validateAssigneePair returns 400 on invalid workspace_id
     instead of panicking.

2. Fix issue:deleted WS event payloads to emit uuidToString(issue.ID)
   instead of the raw URL string. After an identifier-path delete
   ("MUL-7"), the previous payload would have leaked the identifier to
   subscribers, leaving stale entries in frontend caches that key by
   UUID. Updated DeleteIssue (issue.go:1341) and BatchDeleteIssues
   (issue.go:1641). The slog "issue deleted" log line also now records
   the resolved UUID so logs match the WS payload.

3. Extend TestDeleteIssueByIdentifier to subscribe to the bus and
   assert issue:deleted.payload.issue_id is the resolved UUID, not
   the identifier.

* fix(server): validate remaining reviewed UUID inputs

* fix(server): validate remaining handler UUID inputs

* fix(server): finish request boundary UUID audit

* fix(server): validate remaining request body UUIDs

* fix(server): validate runtime path UUIDs

* fix(server): validate remaining audit UUID inputs

---------

Co-authored-by: Eve <[email protected]>
2026-04-28 14:50:28 +08:00
Truffle 93fe324bb9 fix(skills): fast-path root-level SKILL.md with frontmatter guard (#1625)
Closes the functional gap the reporter hit on alchaincyf/huashu-design
(skills.sh/alchaincyf/huashu-design/huashu-design) without expanding
candidatePaths unconditionally, which would let an unrelated root
SKILL.md hijack a different skill URL in a multi-skill repo.

Try SKILL.md at the repo root before falling into the recursive tree
fallback added in #1432. Verify the frontmatter name matches the
requested skill so only genuine single-skill repos take the fast path.
For those repos this also shaves the recursive tree API call.

Also clarifies the candidate-path comment so the root case is
explicit.
2026-04-25 01:40:23 +08:00
affe (Yufei Zhang) 5ef957ca1b fix(skills): resolve aliased skills.sh imports (#1432)
* fix(skills): resolve aliased skills.sh imports

* fix(skills): harden alias fallback scan
2026-04-23 17:33:30 +08:00
gezilinllandzhangliang f247a4f544 feat(skills): import runtime local skills into workspace (#1431)
* feat(skills): import runtime local skills into workspace

* fix(skills): address runtime local skill review feedback

* docs(skills): annotate local provider skill paths

---------

Co-authored-by: zhangliang <[email protected]>
2026-04-22 13:16:51 +08:00
devv-eveandEve 03e21aee80 Fix skills.sh nested directory imports (#1423)
Co-authored-by: Eve <[email protected]>
2026-04-20 23:11:33 -07:00
Naiyuan QingandClaude Opus 4.6 f0f3cb5c3a fix(server): resolve X-Workspace-Slug in middleware-less handlers (#1165)
Problem
-------
The v2 workspace URL refactor (#1141) switched the frontend from sending
X-Workspace-ID (UUID) to X-Workspace-Slug. The workspace middleware was
updated to accept the slug and translate it via GetWorkspaceBySlug.

But the handler package maintained a PARALLEL resolver
(`resolveWorkspaceID` in handler.go) used by endpoints that sit outside
the workspace middleware — and that resolver was never updated. It only
checked context / ?workspace_id / X-Workspace-ID, never the slug.

/api/upload-file is the one production route that hit the broken path:
it's user-scoped (not behind workspace middleware) because it also
serves avatar uploads (no workspace). Post-refactor requests from the
frontend arrived with only X-Workspace-Slug; the handler resolver
returned "", the code fell into the "no workspace context" branch, and
every file upload since v2 landed in S3 with no corresponding DB
attachment row — files orphaned, invisible to the UI.

Root cause is structural: two resolvers doing the same job, written
independently, diverged silently when one was updated.

Fix
---
Collapse to a single shared helper. middleware.ResolveWorkspaceIDFromRequest
is the new canonical resolver; both the middleware's internal
`resolveWorkspaceUUID` (for middleware gating) and the handler-side
`(h *Handler).resolveWorkspaceID` (promoted from a package function)
now delegate to it. Priority order matches what the middleware has had
since v2: context > X-Workspace-Slug header > ?workspace_slug query >
X-Workspace-ID header > ?workspace_id query.

Impact analysis
---------------
47 call sites of the old `resolveWorkspaceID(r)` are renamed to
`h.resolveWorkspaceID(r)`. 46 of them sit behind workspace middleware,
so they hit the context fast path and see zero behavior change. The
one caller that actually gains capability is UploadFile — which now
correctly recognizes slug requests and creates DB attachment rows.

Tests
-----
- New table-driven unit test for ResolveWorkspaceIDFromRequest covers
  all priority levels and the unknown-slug fallback.
- Regression tests for UploadFile: once with X-Workspace-Slug only
  (the broken path), once with X-Workspace-ID only (legacy CLI/daemon
  compat path). Both assert that a DB attachment row is created.
- Full Go test suite passes; typecheck + pnpm test unaffected.

Plan
----
See docs/plans/2026-04-16-unify-workspace-identity-resolver.md for the
full first-principles writeup.

Co-authored-by: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-16 18:01:56 +08:00
Bohan Jiang b4b69f89f6 fix(server): allow members to create and manage their own skills (#1017)
Remove admin/owner-only restriction from skill creation and import routes.
Add canManageSkill helper that lets skill creators manage their own skills,
matching the existing canManageAgent pattern for agents.
2026-04-14 23:48:57 +08:00
Bohan Jiang 1ee4e0501a fix(handler): add .claude/skills/ candidate path for skills.sh import (#792)
Skills stored under .claude/skills/{name}/SKILL.md (the Claude Code
native discovery convention) were not found during skills.sh import,
causing a 502 error. Add this path to the candidate list.

Fixes #777
2026-04-12 23:39:46 +08:00
Bohan Jiang ee3c849c52 fix(skills): detect GitHub default branch instead of hardcoding "main" for skills.sh imports (#632)
Repos hosted on GitHub can use any branch name as default (main, master, etc.).
The skills.sh import was hardcoding "main" in raw.githubusercontent.com URLs,
causing 404s when fetching SKILL.md from repos with a different default branch.

Now queries the GitHub API (/repos/{owner}/{repo}) to get the actual default
branch before fetching files.

Fixes #517
2026-04-10 15:44:18 +08:00
yushenandClaude Opus 4.6 f05f3face3 fix(agent): allow members to manage skills on their own agents
SetAgentSkills previously only allowed workspace owner/admin roles,
blocking members from adding skills to their own agents. Now uses
canManageAgent which allows agent owners too.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-04-02 12:25:19 +08:00
Naiyuan QingandClaude Opus 4.6 9ede795c5b feat(api): strict workspace isolation + agent parity fixes
Enforce workspace isolation at every layer:

- Router: move RequireWorkspaceMember middleware to group level so ALL
  workspace-scoped routes (issues, agents, skills, runtimes, inbox,
  comments) require workspace context
- SQL: add GetXxxInWorkspace queries that filter by workspace_id,
  eliminating cross-workspace data access at the query level
- Handlers: loadXForUser functions use workspace-scoped queries,
  no fallback to unscoped queries
- Migration 025: add workspace_id column to comment table with backfill
- ListComments: add workspace_id filter for defense-in-depth

Fix daemon workspace mapping:
- Server returns workspace_id in task claim response (from issue)
- Daemon uses task.WorkspaceID directly instead of unreliable
  workspaceIDForRuntime() local map lookup
- Remove workspaceIDForRuntime function

Fix agent/human parity:
- Comment update/delete: use resolveActor for isAuthor check so agents
  can edit/delete their own comments
- Event attribution: replace hardcoded "member" with resolveActor in
  agent, skill, and subscriber publish calls

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-30 16:49:13 +08:00
Jiayuan f4a6e7c475 refactor(server): consolidate workspace permission checks into middleware
Move workspace membership and role validation from individual handlers
into dedicated Chi middleware. The new middleware resolves workspace ID
(from query param, X-Workspace-ID header, or URL param), validates
membership via DB, and injects the member into request context.

Handlers now read workspace ID and member from context instead of
calling requireWorkspaceMember/requireWorkspaceRole directly. This
eliminates ~17 duplicated permission checks across handlers and makes
it harder to accidentally omit access control on new routes.
2026-03-30 03:40:20 +08:00
Naiyuan QingandClaude Opus 4.6 9236674667 feat(realtime): WS invalidation + refetch pattern, inbox bugfixes, UI polish
Refactor real-time sync from per-event precise mutations to WS-as-invalidation-signal + debounced refetch.

Backend:
- Add SubscribeAll to Event Bus — auto-broadcasts ALL events, eliminates manual 25-item allEvents list
- Add skill event constants to protocol, fix skill handler string literals
- Add title_changed activity tracking

Frontend:
- WSClient: add onAny() method for wildcard event subscription
- useRealtimeSync: rewrite to refreshMap + prefix routing + 100ms debounce
- Precise handlers only for side effects: workspace:deleted, member:removed, member:added (self-check)
- Reconnect now refetches all stores (fixes missing members/skills/workspace refresh)
- Stale-while-revalidate: fetch() only shows loading spinner on initial load, not on refetch
- Remove redundant useWSEvent in agents/page.tsx and skills-page.tsx
- WSClient.disconnect() now clears all handler registrations

Inbox bugfixes:
- Unify sidebar badge count with page count via dedupedItems + unreadCount in store
- Sort by time DESC (removed severity-first ordering)
- Ellipsis on truncated detail labels

UI:
- Status/Priority pickers: replace RadioGroup with MenuItem for auto-close on selection

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-29 13:49:40 +08:00
JiayuanandClaude Opus 4.6 9bc0366662 feat(skills): add skill import from ClawHub and Skills.sh
Support importing skills from external sources (clawhub.ai and skills.sh)
via a new POST /api/skills/import endpoint. The backend auto-detects the
source from the URL, fetches skill metadata and files, and creates the
skill in the workspace. The frontend CreateSkillDialog now has two tabs:
Create (manual) and Import (paste URL with source auto-detection badge).

Co-Authored-By: Claude Opus 4.6 <[email protected]>
2026-03-26 18:21:28 +08:00
Naiyuan QingandClaude Opus 4.6 06122dfe9e merge: resolve conflicts with main (skills feature)
Merge origin/main which added the skills system (structured skills
with meta skill runtime injection). Resolve 4 conflicts:

- workspace/store.ts: keep both skills state + issue/inbox fetch
- types/index.ts: keep Skill types + our event exports
- handler/agent.go: merge visibility filtering + skills batch loading
- pnpm-lock.yaml: accept main's lockfile with skills deps

Also fix skill.go: migrate h.broadcast → h.publish (event bus)
to match our architecture where all WS events go through the bus.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
2026-03-25 16:43:21 +08:00
Jiayuan ZhangandClaude Opus 4.6 42aef6e13e fix: address review issues in skills system
- Fix authorization bypass in DeleteSkillFile (missing workspace/role check)
- Extract skills page into features/skills/ module (thin route shell)
- Fix skill files disappearing after save (use API return values + merge in refreshSkills)
- Fix silently swallowed DB errors in ListAgents/GetAgent skill queries

Co-Authored-By: Claude Opus 4.6 <[email protected]>
2026-03-25 15:37:08 +08:00
Jiayuan ZhangandClaude Opus 4.6 02df33803a feat: structured skills system with meta skill runtime injection
Replace agent.skills TEXT field with structured skill/skill_file/agent_skill
tables. Skills are workspace-level entities with supporting files, reusable
across agents via many-to-many bindings.

Backend: migration 008, sqlc queries, CRUD handler, agent-skill junction,
structured skill loading in task context snapshot.

Daemon: meta skill injection via runtime-native config (.claude/CLAUDE.md
for Claude, AGENTS.md for Codex) so agents discover .agent_context/ skills
through their native mechanism. Lean prompt without inlined skill content.

Frontend: Skills management page, agent Skills tab picker, SDK methods,
TypeScript types, workspace store integration.

Also removes auto-creation of init issues when creating agents.

Co-Authored-By: Claude Opus 4.6 <[email protected]>
2026-03-25 15:17:59 +08:00