Commit Graph
4 Commits
Author SHA1 Message Date
f0c9146c4b MUL-7437: fix(api): classify disabled capabilities as non-retryable (#8487)
* fix(plugin): return 403 when API is disabled (MUL-7437)

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

* fix(api): classify disabled capabilities as non-retryable (MUL-7437)

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

* fix(api): address disabled capability review feedback (MUL-7437)

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

---------

Co-authored-by: Sol-Boy <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-09-16 17:27:05 +08:00
0242f37154 MUL-6986 refactor(skills): merge built-in skills into one platform skill and drop source maps (#7968)
* MUL-6986 refactor(skills): merge built-in skills into one platform skill

Built-in skills are read on the user's machine, which has no checkout of
this repo. Half the shipped payload by bytes was a source-location index:
eight `references/*-source-map.md` files (103KB, 1084 lines) whose content
was `internal/handler/github.go:687` style citations, plus "verification
commands" like `go test ./internal/service` and
`grep -n 'func derivePRState' internal/handler/github.go`. Four SKILL.md
bodies also carried source paths and Go identifiers, and every one of them
pointed the reading agent at its source map ("When in doubt, read that
file"). One source map even leaked an internal branch name.

That was not an oversight: CLAUDE.md required updating a source map in the
same PR as any behavior change, and this test file asserted all seven
existed. The reasoning against it was already written at the top of this
file — anything inside a skill directory is shipped to agent machines, so
evals must stay in _test.go — it just was never applied to the maps.

The nine skills were also split along backend modules, not along how the
work arrives. An agent has one entry point ("I am doing something on the
Multica platform"), so nine descriptions competed in the always-loaded
listing with overlapping boundaries, a cross-domain task activated three
skills, and Quick start / Core model / CLI / Side effects were restated in
most of them.

- Delete all eight source maps.
- Merge the eight platform skills into `multica-platform`: an 81-line
  routing SKILL.md (positioning, the eight shared invariants that were
  duplicated across the old bodies, and a table naming one reference per
  domain) plus `references/{issues,mentions,agents,squads,autopilots,
  projects,runtimes,skill-import}.md`. This is the layout the skill spec
  documents for `references/` — a large skill split by domain, loaded on
  demand — rather than an evidence layer for source.
- Strip every source path, Go identifier, test name, migration ordinal and
  tracker citation from the payload; the behavior contracts themselves are
  unchanged, restated in user-observable terms.
- Scope `multica-onboarding` to Mika. It is one agent's procedure, and
  every agent was paying for its description and its files.
- Add TestBuiltinSkillPayloadCarriesNoSourceReferences so this cannot
  regress, and re-home the ~100 contract anchors onto the reference that
  now owns each one. Anchors match with whitespace collapsed, so a
  paragraph reflow no longer reads as a deleted contract.
- Point the brief's two skill references and CLAUDE.md at the new layout,
  and add one recall line to the brief's Skills section: eight
  descriptions became one, so an agent has one name to guess instead of
  eight. It is emitted only when a listed skill resolves to exactly
  `multica-platform`, so a workspace skill taking that slug silences the
  pointer rather than aiming it at a stranger's skill.

Payload per ordinary agent goes from ~211KB across nine skills to ~100KB
in one; the always-loaded description budget from 1,616 chars to 229.

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

* MUL-6986 fix(skills): resolve the brief's platform pointer from the built-in

Review found the brief's recall line claimed a fail-closed behavior it did not
have. The claim was that a workspace skill taking the `multica-platform` slug
would silence the pointer; in fact the workspace skill is listed first, wins the
bare slug, and the pointer named it — handing the agent a stranger's skill as
the source of Multica's platform contracts, while the built-in sat at
`multica-platform-multica` and became unreachable.

Documenting that would have left a recall hole, and recall is the one thing this
merge may not cost: eight descriptions became one, so the agent's path to a
platform contract has to stay intact.

Fixed at the source instead. Built-in skills now declare `Source` at load time
(the bundle builder already derived it for the refs path, but the older inline
claim path appended them raw), the daemon carries `Source` into the brief's view
of a skill instead of dropping it, and the pointer resolves the slug from the
built-in's own entry. A collision now points at the built-in wherever it landed.
No bundle hash changes: the bundle builder normalizes `Source` to this value
before hashing.

Also adds TestPlatformSkillDescriptionNamesEveryDomain, which fails if a
reference ships without its trigger word in the description — the merge's recall
guarantee as a test rather than an assertion.

Review nits:
- TOCs for projects.md and runtimes.md, both over the 100-line bar.
- Inlined the two reference-to-reference links in mentions.md, so the routing
  file's "they do not depend on each other" is true.
- Example issue keys use MUL-123, not MUL-2759 — a real internal issue.
- Fixtures naming the deleted skills now name live ones.
- Recorded where the description boundary sits: task nouns are trigger wording,
  body section headings are the inventory the cap rules out.

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

* MUL-6986 fix(skills): keep the brief's skill pointers resolvable across daemon skew

The runtime brief is assembled by the DAEMON, from a binary the user installs on
their own schedule. Deploying a backend does not rewrite it, and backend
upgrades do not force a daemon upgrade, so renaming the built-ins left two brief
lines — the metadata write discipline and the sub-issue playbook — naming
`multica-working-on-issues`, a skill this server no longer ships. The window is
open-ended, and a pointer to a skill that is not installed is worse than no
pointer: the agent hunts for it and may skip the contract when it misses.

Both skew directions happen, and each needs the fix on the side that is still
deployable:

- Old daemon, new backend. That daemon's brief text is frozen, so the fix has to
  come from the server: a daemon that does not advertise `platform-skill-v1`
  additionally receives a redirect stub under the old name, carrying nothing but
  the new location. A current daemon receives nothing extra, so the stub retires
  itself per install as daemons update — no cleanup release to remember, and no
  cost paid by daemons that never needed it. Gated on capability, never on a
  version string, matching how every other daemon skew is handled here.

- New daemon, old backend. The server has no idea the merge happened, so this
  side has to cope. The brief now resolves both pointers from the skills the
  task actually received: the merged skill if present, else the pre-merge one,
  else no pointer at all. The `--status todo` / `--stage <N>` / secrets rules
  stay unconditional either way — only the "read more here" clause is
  conditional.

Resolution matches on built-in source as well as slug, so a workspace skill that
sanitizes to the same name cannot be advertised as the source of Multica's
platform contracts.

The redirect stub is deliberately a signpost with no contracts of its own: a
stub that grew content would rot against references/issues.md precisely on the
installs that cannot be updated from here, and a test caps its size and forbids
supporting files. It also rides through the template, strict-frontmatter and
source-leak suites like any other shipped payload.

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

* MUL-6986 fix(skills): route by domain, not by count, and test the redirect wiring

Review must-fix: the routing text turned "load references on demand" into "load
exactly one". `SKILL.md` said to open "the ONE reference that matches your
task", the brief said "open the one reference its routing table names", and a
test pinned the singular.

That is wrong for the tasks the merge was argued on. Creating a squad, assigning
it an issue, then writing a mention needs squads.md, issues.md AND mentions.md;
wording that implies one would have the agent act on contracts it never read —
exactly the recall loss this work is not allowed to cause. Routing is now by
domain: open what the task touches, usually one and sometimes a few, and never
all eight out of uncertainty. The tests pin the domain framing and reject the
singular forms.

Also adds the wiring test the review found missing. The service test passed the
decision in as a bool and the brief test hand-built a skill set, so both ends
were covered while the middle — X-Client-Capabilities on the claim, through the
handler, into the delivered payload — was not. A break there would have left
every existing test green while an old daemon silently lost its redirect.
Covered across both claim branches, since each assembles the built-in set
separately, and verified to fail when the wire is cut. The daemon's capability
header test now pins platform-skill-v1 alongside the worktree gate.

Non-blocking review cleanups:
- The invariant claimed every domain has `list` and `get`; there is no
  `runtime get` and a mention is not a gettable entity. It now points at the
  read-only commands the opened reference actually names.
- Two comments still said the `multica-` prefix means a built-in slug can never
  collide with a workspace skill. This PR ships the fix for that exact
  collision, so they now say what is true: the prefix namespaces, nothing
  reserves it server-side, and the brief resolves pointers from built-in source
  rather than trusting the bare name.
- Recorded the allowed-tools tradeoff: merging eight skills merged their tool
  declarations, so the router declares Bash(git *) / Bash(gh *) for domains that
  do not need them. Dropping them would regress the issue workflow that runs
  `gh pr`; today the daemon's global permission mode means this grants nothing
  the agent did not already have.

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

* MUL-6986 docs: drop the CLAUDE.md payload rule that a test already enforces

CLAUDE.md is loaded into every task in this repo and asks to stay short:
rules there should be hard to infer from code or easy to get wrong. This one
was neither — TestBuiltinSkillPayloadCarriesNoSourceReferences fails CI on
every class it named, and its failure message already carries the rule and the
reason at the moment someone violates it, which is both cheaper and impossible
to drift from.

The line above it stays: nothing enforces "update the reference in the same
PR" when a CLI flag changes.

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

* MUL-6986 refactor(skills): accept the skill-name collision instead of handling it

Product decision: a workspace skill whose name sanitizes to exactly
`multica-platform` is rare enough not to design around. Half-handling it was
costing more than the risk — the brief resolved collisions dynamically while the
redirect stub could not (the stub is server-authored text, and the final slug is
only assigned when the daemon writes it to disk), so every review round found
another place that "did not handle it". Either handle it everywhere or nowhere;
this takes nowhere, and states the assumption where the pointers are.

Removed:
- builtinSlug matches the exact name again, instead of built-in source plus a
  slug prefix.
- SkillContextForEnv.Source, the line carrying it through convertSkillsForEnv,
  and the Source stamp on loaded built-ins. All three existed only for the
  collision check, and dropping the stamp also removes a wire-facing change:
  the bundle builder derives "builtin" from an empty id/source exactly as it
  did before.
- The three collision test cases.
- The "the prefix does not guarantee uniqueness, so resolve from source"
  narrative, replaced by the assumption it stands in for: `multica-` is the
  platform namespace, pointers name built-ins by bare name, and nothing
  reserves the prefix server-side yet. When that becomes real the fix is to
  reject the prefix at skill create/import, not to make every pointer
  defensive.

Kept, because it is version skew rather than collision and its window has no
upper bound: the `platform-skill-v1` capability, the redirect stub for daemons
that predate the merge, the brief's new-then-legacy-then-none resolution, and
their tests.

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

---------

Co-authored-by: J <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-09-04 13:51:12 +08:00
b6adf23f91 feat(api): emit Content-Length header on JSON responses (#5021)
The core writeJSON helpers streamed the body via json.NewEncoder(w).Encode
after WriteHeader, which forces net/http into chunked transfer encoding and
omits Content-Length. Buffer the marshaled body first, set an accurate
Content-Length, then write — so API (and health) JSON responses advertise
their exact size. writeMeasuredJSON gets the same header. Adds a test
asserting the header matches the on-wire body length.

Co-authored-by: Eve <[email protected]>
Co-authored-by: multica-agent <[email protected]>
2026-07-07 14:43:19 +08:00
Hzzzzzx 745832b536 MUL-3433: fix(daemon): restore claim slow-log payload observability without gzip
Squash merge PR #4322 after review approval.\n\nMUL-3433
2026-06-21 23:41:05 +08:00