mirror of
https://github.com/multica-ai/multica.git
synced 2026-07-05 21:39:54 +02:00
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720) (#4608)
* feat(composio): server-side connect flow + connections REST (Notion MVP) (MUL-3720)
Compose the merged server/pkg/composio SDK into a user-facing connection
manager: signed-state connect handshake, local user_composio_connection
mirror, idempotent disconnect, and a per-user MCP session helper (not yet
wired into task dispatch).
- migration 127_user_composio_connection (no FK/cascade, per DB rules)
- sqlc queries: upsert (idempotent on user_id+connected_account_id), list
active, owner-scoped get, mark revoked
- internal/integrations/composio: signed HMAC-SHA256 state, BeginConnect,
CompleteCallback (idempotent upsert), ListConnections, Disconnect
(upstream 404 = idempotent success), CreateMCPSession (no-op when empty,
pins connected_accounts per toolkit), CallbackRedirect
- REST handlers under /api/integrations/composio (user-scoped, 503 when
COMPOSIO_API_KEY unset): connect/init, callback (302), connections list,
delete
- router wiring gated by COMPOSIO_API_KEY; COMPOSIO_AUTH_CONFIGS_JSON maps
toolkit->auth_config (MVP: notion); state secret from COMPOSIO_STATE_SECRET
or derived from JWT_SECRET; callback base from COMPOSIO_CALLBACK_BASE_URL
or MULTICA_PUBLIC_URL
- tests: state (expire/tamper/wrong-secret), service (mapping, callback
idempotency, non-success, disconnect owner/404 idempotency, MCP pin),
handlers (httptest), redact regression for Bearer mcp_ tokens
MVP scope: Notion only; no task-dispatch overlay, sharing, or webhook
event handling (later stages).
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): bind callback account to user + idempotent revoked disconnect (MUL-3720)
Address PR 4608 review (CHANGES_REQUESTED):
- callback: verify connected_account_id with Composio before mirroring it.
The signed state only proved user/toolkit/exp, so a valid state paired with
a tampered connected_account_id would be written verbatim. CompleteCallback
now calls ListConnectedAccounts and fails closed (ErrAccountVerification)
unless the account belongs to the state's user (composio_user_id == multica
user id) and was created under the toolkit's auth config. No row is written
on mismatch / unknown account / upstream error.
- disconnect: short-circuit to a no-op when the local row is already revoked,
before touching upstream. Previously a second DELETE re-hit Composio and a
non-404 upstream error surfaced as a 502, breaking the 204-idempotent
contract.
- CreateMCPSession: document the v1 single-active-connection-per-(user,toolkit)
constraint and make duplicate selection deterministic (newest-wins, rows are
connected_at DESC) instead of order-dependent map overwrite. Stage 3 owns the
real single-account-enforcement vs multi-account-shape decision.
Tests: tampered/wrong-auth-config/unknown-account callback rejection, revoked-row
disconnect no-op (asserts upstream not re-hit). composio pkg 85% coverage; all
green.
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): list all toolkits + dynamic auth-config resolution (MUL-3720)
Yushen's follow-up to the Notion MVP: surface the full Composio toolkit
catalog, render it in Settings, and drop the static env mapping in favor of
dynamic auth-config discovery.
Config correctness (per Composio docs):
- Remove COMPOSIO_AUTH_CONFIGS_JSON entirely. The toolkit→auth_config mapping
is now resolved at request time from the project's /auth_configs (cached,
5-min TTL), so enabling a toolkit is a dashboard action, not a redeploy.
- Do NOT add COMPOSIO_PROJECT_ID. The project API key (x-api-key) authenticates
to exactly one project; the project is resolved from the key. Only org-level
endpoints use x-org-api-key, which this integration never calls.
Backend:
- SDK: server/pkg/composio/auth_configs.go — ListAuthConfigs (toolkit_slug,
is_composio_managed, show_disabled, limit, cursor).
- service: dynamic resolver (authConfigMap cache; betterAuthConfig prefers a
custom/white-label config over Composio-managed, newest wins); BeginConnect
and CompleteCallback resolve via it; ListToolkits fetches the full catalog
(paginated, capped) annotated with connectable = has an enabled auth config,
connectable-first ordering.
- handler + route: GET /api/integrations/composio/toolkits (user-scoped, 503
when COMPOSIO_API_KEY unset) returning slug/name/logo/category/connectable.
Frontend:
- core: ComposioToolkit/ComposioConnection types, api client methods, and
composio query options (@multica/core/composio).
- views: Settings → Integrations now has a Composio section rendering every
toolkit as a card with search. Connect is gated on `connectable`;
non-connectable toolkits show a muted "not configured" hint instead of a
dead button. Connected toolkits show a badge + Disconnect (with confirm).
- i18n: composio block added to en/zh-Hans/ja/ko settings.
Tests: SDK + service (dynamic resolution, custom-over-managed preference,
connectable flag, resolver-error soft-degrade) and handler toolkits endpoint;
composio pkg 85.7% coverage. go build/vet/gofmt clean; core+views typecheck,
core+views lint, and core tests (691) all green.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): close cross-toolkit callback fail-open by signing auth_config_id into state (MUL-3720)
Re-review blocker: CompleteCallback resolved the toolkit's auth config at
callback time and ignored a resolve error/empty result, while
verifyAccountOwnership skipped the auth-config comparison when the expected
value was empty. A user could then pass another toolkit's connected_account_id
into this toolkit's callback — the owner check passed and it was written under
the wrong toolkit_slug/account binding.
Fix: the auth_config_id is already resolved in BeginConnect (before the state
is signed), so sign it into the state and compare it exactly at callback. No
re-resolve, no fail-open. verifyAccountOwnership now fails closed when the
expected auth config is empty (rejects instead of skipping) and requires an
exact match — closing the cross-toolkit binding gap.
Tests: state round-trips auth_config_id; BeginConnect signs it; callback
rejects wrong/cross-toolkit auth config and an empty (no-mapping) auth config
fails closed. composio pkg 85.2% coverage, all green.
Frontend (non-blocking): the Composio settings tab now surfaces an error when
the connections query fails instead of silently rendering everything as
unconnected.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): hide Settings section entirely when integration unconfigured (MUL-3720)
Decision (option 2, hide-then-merge): don't show a card that leaks the internal
COMPOSIO_API_KEY env-var name to every end user. IntegrationsTab now gates the
whole Composio section (heading + body) on the toolkits query — a 503 means the
key is unset, so the section is withheld instead of rendering the not-configured
card. Admin-only setup guidance is a later, role-gated affordance.
Removed the notConfigured card (and now-unused ApiError import) from
ComposioTab; it only mounts when configured. views typecheck + lint clean.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): Stage 2 frontend polish — callback toast, last_used & expired UI, e2e (MUL-3718) (#4688)
* feat(composio): callback toast + refresh, last_used & expired UI, e2e (MUL-3718)
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): real callback redirect route + StrictMode-safe toast dedup (MUL-3718 review)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): callback endpoint should not require Multica auth (MUL-3843) (#4709)
* fix(composio): move OAuth callback out of the Auth group (MUL-3843)
Composio 302-redirects the browser to /api/integrations/composio/callback
at the end of the OAuth flow, but PR #4608 mounted it inside the cookie-auth
middleware group. When the session cookie is absent (expired session,
SameSite=Strict / Safari ITP, private window, self-hosted callback subdomain)
the Auth middleware returned a hard 401 and a JSON blob instead of the
settings redirect, breaking the flow.
Identity never came from the cookie anyway: it is carried by the HMAC-signed
state param that CompleteCallback verifies (signature, expiry, replay) and
cross-checked by verifyAccountOwnership; h.Composio == nil still 503s. So the
callback is registered alongside the other public OAuth/webhook routes; the
other four composio endpoints stay session-gated.
Refs MUL-3843, MUL-3715.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): correct stale callback routing comments (MUL-3843)
The package header and ComposioCallback doc comments still described the
callback as sitting under the Auth middleware group. After the route was
moved out (this PR), update both to state it is a public route whose identity
comes from the signed state — addressing review nit from 张大彪.
Refs MUL-3843.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): inject MCP overlay into agent runtime at task dispatch (MUL-3721) (#4704)
Stage 3 of the Composio epic. Wires the per-user Composio MCP session into
every agent task so the agent process sees the initiator's connected tools
without any prompt-time plumbing.
Server side
- Migration 128 adds agent_task_queue.runtime_mcp_overlay JSONB plus a
BEFORE-UPDATE trigger that wipes the column on any transition into a
terminal status (completed / failed / cancelled). A trigger is the single
source of truth — future queries that flip status cannot bypass it.
- composio.Service.BuildTaskOverlay(userID) reuses CreateMCPSession and
emits the Claude-style { mcpServers: { composio: { type: http, url,
headers } } } shape the daemon's existing sidecar generators consume.
Returns (nil, nil) on zero active connections so we never burn a
Composio session for a user with nothing to call.
- TaskService grows a Composio ComposioOverlayBuilder seam, wired in
router.go after composiointeg.NewService succeeds. Five enqueue paths
(issue / mention / quick-create / chat / auto-retry) attach the overlay
after CreateAgentTask returns and before the daemon is notified — so
every claim reads a settled row, with no second daemon hop. Best-effort:
a builder failure logs and proceeds with no overlay.
- resolveInitiatorFromTriggerComment derives the initiator user from the
trigger comment when it was authored by a member. Agent-authored
triggers are not treated as initiators (their connected-apps view is
empty by construction).
Daemon side
- handler/daemon.go claim path merges task.runtime_mcp_overlay onto
agent.mcp_config via mergeMCPOverlay before populating
TaskAgentData.McpConfig. Overlay wins on server-name collisions
because it carries the live user-scoped session URL. Errors fall back
to the agent config unchanged — a bad overlay must not surprise-disable
saved MCP tools. The existing execenv sidecar generators (cursor /
codex / openclaw / opencode / hermes-kiro) need no changes: they keep
consuming the merged result through TaskAgentData.McpConfig.
Tests
- 9 merge cases (mcp_overlay_test): both-nil short-circuit, agent-only
pass-through, overlay-only canonicalization, two-side merge, name
collision (overlay wins), top-level key preservation, malformed agent
fallback, malformed overlay fallback, non-object server rejection.
- 4 dispatch cases (composio): zero-connections returns nil without
CreateSession, happy-path emits the right shape with the right user
id, empty-URL defensive branch, SDK error surfacing.
- 4 TaskService helper cases: nil Composio is a no-op (Queries-safe),
invalid initiator does not call the builder, nil overlay skips the
UPDATE, builder error swallowed without panic.
- Migration 128 verified to roll up + down + up cleanly against the test
database.
Out of scope (deferred): assignment-triggered enqueue paths with no
trigger comment get no overlay attached today (no initiator UUID flows
through enqueueIssueTask in that case). Retry paths recompute the overlay
fresh from the parent's initiator_user_id instead of inheriting the bearer
from the parent row, so a stale token can never resurface on a retry.
Co-authored-by: Eve <eve@multica.ai>
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869) (#4736)
* feat(composio): per-agent allowlist + originator-scoped MCP overlay (MUL-3869)
Stage 3.1 of the Composio epic (MUL-3721 parent). PR #4704 wired in the
runtime_mcp_overlay column and a per-task dispatch hook; this change
inverts the default from "all-on" to opt-in and locks the overlay to the
agent owner's own connected apps:
- Agents carry composio_toolkit_allowlist TEXT[]. NULL or [] => no MCP.
Owner-only read/write; non-owner GET/PUT silently redacts/drops the
field (same shape as mcp_config).
- agent_task_queue carries originator_user_id UUID. Set from the
top-of-chain HUMAN at every enqueue path:
* issue/mention comment by member -> author_id
* issue/mention comment by agent -> inherit via comment.source_task_id
-> parent task originator_user_id
* quick-create -> requester_id
* chat -> initiator_user_id
* retry -> SQL-inherited from parent row
* autopilot -> NULL (system-driven)
- BuildTaskOverlay (composio dispatch) now takes (ctx, originatorUserID,
agent) and short-circuits on five gates: invalid originator,
originator != agent.owner_id, empty allowlist, empty intersection of
allowlist ∩ active connections, defensive empty session URL. Composio
CreateSession is called with BOTH `toolkits.slugs` (the intersection)
AND `connected_accounts` (the pinned account ids), narrowing the
tool-router twice.
- The originator-vs-owner gate closes the agent-fanout privacy hole: any
workspace member who can @-mention a public agent used to project the
owner's connected apps into their run. Now the overlay only mounts
when the human at the top of the chain IS the agent owner.
Tests:
- dispatch_test.go covers all 5 gates plus uppercase/whitespace slug
normalisation.
- task_runtime_mcp_overlay_test.go covers the no-op gates of the new
applyRuntimeMCPOverlay signature.
- agent_composio_allowlist_test.go (handler): owner roundtrip
(list/empty/null), workspace-admin silent-drop, owner-only GET
visibility, pure normaliseComposioToolkitAllowlist.
- resolve_originator_test.go (service, DB-backed): member-authored,
agent-authored inherits via comment.source_task_id, invalid id.
Migration 129 up/down/up verified against docker postgres.
Co-authored-by: multica-agent <github@multica.ai>
* chore(composio): gofmt + regenerate sqlc with v1.31.1 (MUL-3869 review nits)
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: Eve <eve@multica-ai.local>
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): accept nested connected account auth config
* feat(views): creator-only MCP tab for per-agent Composio allowlist (MUL-3870) (#4743)
Stage 3.2 frontend on top of the Stage 3.1 backend (MUL-3869, 4708dba97).
Adds an agent-detail tab that lets the agent owner pick which of their own
active Composio connections this agent may mount as MCP servers, writing the
selection to agent.composio_toolkit_allowlist via the existing PUT /api/agents.
- core/types: composio_toolkit_allowlist (+ _redacted) on Agent; tri-state
composio_toolkit_allowlist on UpdateAgentRequest (omit/no-change, null/clear,
array/replace), matching the backend contract.
- core/agents: useUpdateAgentAllowlist - optimistic mutation hook (patches the
cached workspace agent list, rolls back on error, invalidates on settle).
- views: AgentMcpTab renders the owner's active connections as checkboxes;
empty state links to Settings -> Integrations; defensive redacted state.
- views: wired into AgentOverviewPane as tab "composio_mcp", labeled "MCP Apps"
to disambiguate from the existing raw-JSON "MCP" (mcp_config) tab. The entry
is gated to the creator (currentUserId === agent.owner_id), matching the
backend's owner-only read/write of the allowlist.
- i18n: tabs.composio_mcp + tab_body.composio_mcp.* in en/ja/ko/zh-Hans.
- tests: agent-mcp-tab.test.tsx (gating, toggle->allowlist body, active-only,
empty, redacted); e2e/agent-mcp.spec.ts (creator sees tab + PUT body,
non-creator hidden) with Composio + agent endpoints mocked at the boundary.
Note: the product spec says "creator"; the schema has no creator_id - the
backend gate and redaction are keyed on owner_id, so the tab uses owner_id.
Co-authored-by: multica-agent <github@multica.ai>
* fix(composio): mount remote MCP for codex
* feat(agents): agent invocation permission system (MUL-3963) (#4844)
* feat(agents): agent invocation permission system (permission_mode + invocation targets)
MUL-3963: split who may INVOKE an agent out of the overloaded visibility
column into an explicit, extensible model on feature/composio-integration.
- DB: agent.permission_mode (private|public_to) + agent_invocation_target
table (workspace/member/team targets) + lossless backfill from visibility
(migration 130).
- canInvokeAgent: owner-only for private (NO admin bypass, NO A2A bypass);
public_to honours the allow-list; A2A judged by the top-of-chain originator.
- All trigger paths rewired: issue assign, comment @agent/@squad, chat,
quick-create, autopilot, squad leader, child-done.
- Agent API: permission_mode + invocation_targets on responses and
create/update (owner-only writes); legacy visibility kept as a derived field
so old clients never see a permission widening.
- Composio: BuildTaskOverlay now FOLLOWS invocation permission and uses the
agent OWNER connection (removed the originator==owner gate); front-end warns
when a shared agent enables Composio apps.
- CLI: --permission-mode / --public-to-workspace / --public-to-member (legacy
--visibility still mapped).
- Frontend: AccessPicker (Private / workspace / specific people / team soon),
permission rules mirror canInvokeAgent, Composio warning banner.
- Tests: migration backfill, admin cannot invoke others private, public_to
workspace/member whitelist, A2A by originator, Composio overlay uses owner
connection.
Co-authored-by: multica-agent <github@multica.ai>
* feat(agents): stackable, mixed public_to invocation targets (MUL-3963)
Follow-up on PR #4844: public_to now supports selecting MULTIPLE, MIXED
targets on one agent (e.g. Public to workspace + specific people + team),
with canInvokeAgent admitting on ANY matching target (OR).
- Frontend AccessPicker: reworked from a single exclusive kind into a
stackable multi-select — an "Everyone in workspace" toggle, a member
multi-select checklist, and a (disabled, v1) team placeholder can be
combined freely. Emits the full union of selected targets; empty union
collapses to Private. Existing team targets are preserved across saves.
Added the access.public_group locale string (en/zh-Hans/ja/ko).
- Backend already supported this (agent_invocation_target is multi-row per
agent; create/update take a target ARRAY and batch-replace the whole
allow-list; canInvokeAgent OR-matches). Added tests to lock it in:
mixed member+team targets, overlapping-member batch replace, and
workspace+member stacking then narrowing.
Refs MUL-3963.
Co-authored-by: multica-agent <github@multica.ai>
* fix(agents): address review on invocation permission (MUL-3963)
张大彪 review on PR #4844 — three blockers + product ruling + nits:
1. Migration 130: drop the FK/cascade on agent_invocation_target
(agent_id, created_by) per the Multica no-FK rule; relationships are now
maintained in the app layer (matching MUL-3515 §4). Added
DeleteAgentInvocationTargetsByArchivedRuntimeAgents and call it before
DeleteArchivedAgentsByRuntime in all three runtime-delete paths
(runtime.go x2, runtime_profile.go) so hard-deleting agents can't orphan
target rows.
2. revokeAndRemoveMember: prune the leaving member's member-target grants
(DeleteAgentInvocationTargetsByMember) in the same tx as the member-row
delete, so a re-invited user can't reclaim a stale invocation grant.
3. Empty public_to is a phantom — parsePermissionInput now normalises a
public_to with no resolvable targets to a single workspace target, so
`--permission-mode public_to` alone (and any empty target array) means
"public to workspace" instead of "shared but nobody can run it".
Product ruling: the system/no-human-originator → workspace-target path in
canInvokeAgent is a deliberate, documented exception (webhook/system/
workspace-wide automation); member/team targets still fail closed without a
resolved originator. Documented in code + locked with a test.
Nits: refreshed the stale "originator must be owner" comments — models.go
(via migration 130 COMMENT ON COLUMN + sqlc regen for composio_toolkit_allowlist
and originator_user_id) and agent-mcp-tab.tsx — to the owner-connection +
invocation-permission rules.
Tests: member remove/re-add regression, system workspace exception + member
fail-closed, empty public_to → workspace (plus the earlier mixed/overlap/
batch-replace suite). Migration 130 applied to the test DB; Go handler/service/
composio suites green; views typecheck clean.
Refs MUL-3963.
Co-authored-by: multica-agent <github@multica.ai>
* fix(agents): scope member invocation-target cleanup to one workspace (MUL-3963)
张大彪 3rd review — cross-workspace permission bug + comment nits:
- DeleteAgentInvocationTargetsByMember was a GLOBAL delete by user id, so
removing a user from workspace A also wiped their member-target grants on
agents in workspace B. Scoped it to a single workspace by joining through
agent.workspace_id; revokeAndRemoveMember now passes (workspaceID, userID).
- Regression test TestRevokeMember_InvocationTargetCleanupIsWorkspaceScoped:
same user allow-listed by agents in two workspaces; removal from one leaves
the other workspace's target intact.
- Nits: refreshed the remaining stale "originator == agent.owner_id" /
"owner-vs-originator" comments — CreateRetryTask (agent.sql, regenerated),
and the AgentResponse allowlist doc + ListAgents/UpdateAgent redaction
rationale in agent.go — to the owner-connection + invocation-permission rule.
Migration 130 applied to the test DB; Go handler/service/composio suites green;
go vet clean.
Refs MUL-3963.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* fix(agents): agent access owner-only editable, read-only for others (MUL-3963) (#4853)
* fix(agents): make agent access owner-only editable, read-only for others (MUL-3963)
Interaction bug: a non-owner (incl. workspace admin) could open the AccessPicker
and set an agent public — the backend silently ignored it and the UI bounced
back to private. Access is owner-only, so non-owners must see a read-only state
and the backend must reject real changes explicitly.
Frontend:
- AccessPicker renders a static, non-interactive read-only state when the
viewer is not the owner: the current access value + a lock affordance + a
tooltip "Only the agent owner can change who can run this agent." No clickable
trigger is rendered, so a non-owner can never open a control the backend would
reject (the GitHub/Notion pattern for permission settings you can see but not
edit). The editable multi-select picker is unchanged for the owner.
- agent-detail-inspector gates the picker on ownership specifically
(currentUserId === agent.owner_id), NOT the general canEdit (which also admits
admins, who may edit other fields but not access).
- New locale key access.owner_only_readonly (en/zh-Hans/ja/ko).
Backend:
- UpdateAgent now returns an explicit 403 when a non-owner submits a REAL
permission change (permissionInputChangesAgent compares requested mode +
target set against the persisted state); a no-op resubmit (admin PATCH-as-PUT
echoing unchanged permission) is still tolerated so admin edits of other
fields keep working. Replaces the previous silent-drop that caused the bounce.
Tests:
- access-picker.test.tsx: non-owner gets a non-interactive read-only display
with the owner-only tooltip; owner gets an interactive picker; owner can pick
a member and stack workspace + member.
- TestUpdateAgent_AccessChangeIsOwnerOnly: admin real change → 403; admin no-op
resubmit → 200; admin editing other fields → 200; owner change → 200.
Incidental: fixed a pre-existing base typecheck break in
slash-command-suggestion.test.tsx (stray `signal` arg not in the suggestion
items type) that otherwise fails the whole @multica/views typecheck.
Refs MUL-3963.
Co-authored-by: multica-agent <github@multica.ai>
* fix(agents): compare legacy visibility, not expanded permission, for no-op detection (MUL-3963)
PR #4853 review: permissionInputChangesAgent expanded a legacy-only
visibility:"private" into a real private permission and compared it against the
agent's actual permission. A member-only public_to agent derives legacy
visibility "private", so an admin PATCH-as-PUT echoing visibility:"private"
while editing another field was misread as a public_to→private downgrade and
rejected with 403 — contradicting the "unchanged permission no-op is allowed"
contract.
Fix (per review): when a request carries ONLY legacy `visibility` (no
permission_mode / invocation_targets), derive the agent's CURRENT legacy
visibility from its real targets and compare the legacy string values. Equal =
no-op (allowed); a real legacy change (e.g. "workspace") still returns 403.
Requests that carry permission_mode / invocation_targets keep the precise
mode+target comparison.
Regression test TestUpdateAgent_LegacyVisibilityNoOpForMemberOnlyPublicTo:
member-only public_to agent — admin submitting visibility:"private" + a
non-permission field → 200 with targets unchanged; admin submitting
visibility:"workspace" → 403.
Go handler/composio suites green; migration 130 applied; go vet clean.
Refs MUL-3963.
Co-authored-by: multica-agent <github@multica.ai>
---------
Co-authored-by: multica-agent <github@multica.ai>
* feat(composio): brief agents on connected apps
* feat(composio): gate MCP apps behind feature flag
* fix(mobile): parse agent invocation permissions
* fix(tests): update agent fixtures for access fields
---------
Co-authored-by: multica-agent <github@multica.ai>
Co-authored-by: Multica Eve <eve@devv.ai>
Co-authored-by: Eve <eve@multica.ai>
Co-authored-by: Eve <eve@multica-ai.local>
1419 lines
49 KiB
Go
1419 lines
49 KiB
Go
package execenv
|
|
|
|
import (
|
|
"fmt"
|
|
"os"
|
|
"path/filepath"
|
|
"strings"
|
|
"testing"
|
|
|
|
"github.com/multica-ai/multica/server/internal/runtimeapps"
|
|
)
|
|
|
|
// Sub-issue Creation section — after MUL-2538 the platform posts the
|
|
// child-done parent notification itself, so the brief no longer carries
|
|
// any parent-notification rule (per Bohan's call on PR #3055: delete the
|
|
// guidance entirely, do not replace it with a "do not post one" sentence
|
|
// — the agent should not be thinking about parent comments at all). All
|
|
// that remains is the `--status todo` vs `--status backlog` rule for
|
|
// creating sub-issues, which is unrelated to the notification path.
|
|
|
|
func TestSubIssueCreationSectionPresentForIssueRuns(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
name string
|
|
ctx TaskContextForEnv
|
|
}{
|
|
{
|
|
name: "assignment-triggered",
|
|
ctx: TaskContextForEnv{IssueID: "11111111-2222-3333-4444-555555555555"},
|
|
},
|
|
{
|
|
name: "comment-triggered",
|
|
ctx: TaskContextForEnv{
|
|
IssueID: "22222222-3333-4444-5555-666666666666",
|
|
TriggerCommentID: "33333333-4444-5555-6666-777777777777",
|
|
},
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
out := buildMetaSkillContent("claude", tc.ctx)
|
|
|
|
if !strings.Contains(out, "## Sub-issue Creation") {
|
|
t.Fatalf("expected Sub-issue Creation section in %s brief", tc.name)
|
|
}
|
|
for _, want := range []string{
|
|
"**Choosing `--status` when creating sub-issues.**",
|
|
"`--status todo` = **start now**",
|
|
"`--status backlog` = **wait**",
|
|
"`multica issue status <child-id> todo`",
|
|
"all `--status todo`",
|
|
"`--status backlog` from the start",
|
|
// Stage guidance must reach the always-on brief so agents
|
|
// reach for stages instead of only the manual backlog chain
|
|
// (MUL-3508 follow-up).
|
|
"**Ordering with stages.**",
|
|
"`--stage <N>`",
|
|
"`multica issue children <id>`",
|
|
} {
|
|
if !strings.Contains(out, want) {
|
|
t.Errorf("[%s] section missing %q", tc.name, want)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// The brief must no longer carry any parent-notification guidance. PR
|
|
// #2918 added a "Tell the parent when you finish a child" rule that
|
|
// turned into noise (self-mention loops, planner ack ping-pong,
|
|
// hardcoded `MUL-` prefix). PR #3055 first downgraded it to a "do NOT
|
|
// post one" guardrail, but Bohan's product call was to remove the
|
|
// guidance entirely rather than substitute a new prohibition. These
|
|
// canaries lock that in: any wording that re-introduces the
|
|
// parent-comment concept — positive, negative, or descriptive — must
|
|
// not come back through future edits.
|
|
func TestBriefHasNoParentNotificationGuidance(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []TaskContextForEnv{
|
|
{IssueID: "11111111-2222-3333-4444-555555555555"},
|
|
{
|
|
IssueID: "22222222-3333-4444-5555-666666666666",
|
|
TriggerCommentID: "33333333-4444-5555-6666-777777777777",
|
|
},
|
|
}
|
|
for _, ctx := range cases {
|
|
ctx := ctx
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
|
|
// The pre-MUL-2538 phrasing instructed the agent to compose a
|
|
// parent comment by hand — including a hardcoded `MUL-` prefix
|
|
// and an assignee mention. The intermediate revision (PR #3055
|
|
// before Bohan's call) instead told the agent NOT to post one.
|
|
// Both framings must stay out.
|
|
for _, banned := range []string{
|
|
// Old "do it yourself" framing (PR #2918).
|
|
"## Parent / Sub-issue Protocol",
|
|
"**Tell the parent when you finish a child.**",
|
|
"multica issue comment add <parent-id>",
|
|
"with NO `--parent`",
|
|
"link the child as `[MUL-",
|
|
"`@mention` the parent's assignee",
|
|
"`mention://agent/<id>`",
|
|
"`mention://member/<id>`",
|
|
"`mention://squad/<id>`",
|
|
// Intermediate "do NOT do it yourself" framing (PR #3055
|
|
// before Bohan's call) — also out per product direction.
|
|
"**Do NOT post your own parent-notification comment.**",
|
|
"Do NOT post your own parent-notification comment",
|
|
"parent-notification comment",
|
|
"system comment on the parent fires from the status transition",
|
|
"re-trigger the parent's assignee for nothing",
|
|
"platform posts a top-level system comment on the parent",
|
|
// Earlier revisions split rules by trigger type or used
|
|
// table/subsection layouts. None of those structures should
|
|
// come back either.
|
|
"| Parent assignee | Parent status |",
|
|
"The same agent as yourself",
|
|
"| Member or squad |",
|
|
"### A. Notify the parent",
|
|
"### B. Choose",
|
|
"When this issue has `parent_issue_id`:",
|
|
"**Closing out child work** (only if this issue has `parent_issue_id`)",
|
|
"**Notify the parent** (only if this issue has `parent_issue_id`",
|
|
"**Creating sub-issues** (applies to any issue-bound run)",
|
|
"For parent/child work, use these best-effort rules",
|
|
// The protocol must no longer emit a placeholder
|
|
// `<this-issue-id>` status flip — the workflow above owns
|
|
// that command with the real issue id substituted.
|
|
"`multica issue status <this-issue-id> in_review`",
|
|
// Non-existent CLI form Elon's earlier review flagged.
|
|
"issue list --parent",
|
|
} {
|
|
if strings.Contains(out, banned) {
|
|
t.Errorf("expected %q to be removed from the brief", banned)
|
|
}
|
|
}
|
|
}
|
|
}
|
|
|
|
// Comment-triggered briefs must NOT carry any unconditional status-flip
|
|
// command targeting the current issue. Previous revisions had a
|
|
// dedicated protocol step that wrote `multica issue status <this-issue-id> in_review`;
|
|
// the comment-triggered workflow rule "Do NOT change the issue status
|
|
// unless the comment explicitly asks for it" must remain the source of
|
|
// truth (Elon's blocking review on PR #2918).
|
|
func TestCommentTriggeredProtocolDoesNotForceInReview(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := TaskContextForEnv{
|
|
IssueID: "55555555-6666-7777-8888-999999999999",
|
|
TriggerCommentID: "66666666-7777-8888-9999-aaaaaaaaaaaa",
|
|
}
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
|
|
if strings.Contains(out, "`multica issue status <this-issue-id> in_review`") {
|
|
t.Errorf("comment-triggered brief must not contain a placeholder `<this-issue-id> in_review` flip — that conflicts with the comment-triggered \"do not change status unless asked\" rule")
|
|
}
|
|
|
|
const guardrail = "Do NOT change the issue status unless the comment explicitly asks for it"
|
|
if !strings.Contains(out, guardrail) {
|
|
t.Errorf("expected the comment-triggered workflow guardrail %q to be present", guardrail)
|
|
}
|
|
}
|
|
|
|
// The CLAUDE.md workflow surface must carry the same issue-wide since-delta
|
|
// new-comment hint as the per-turn prompt. PR #2816 requires the two surfaces
|
|
// stay in sync.
|
|
func TestCommentTriggeredBriefCarriesNewCommentsHint(t *testing.T) {
|
|
t.Parallel()
|
|
const (
|
|
issueID = "55555555-6666-7777-8888-999999999999"
|
|
since = "2026-05-28T11:00:00Z"
|
|
)
|
|
ctx := TaskContextForEnv{
|
|
IssueID: issueID,
|
|
TriggerCommentID: "reply-abc",
|
|
NewCommentCount: 4,
|
|
NewCommentsSince: since,
|
|
}
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
|
|
// Issue-wide count.
|
|
if !strings.Contains(out, "4 new comment(s) on this issue since your last run") {
|
|
t.Errorf("comment brief must report the issue-wide new-comment count, got:\n%s", out)
|
|
}
|
|
if !strings.Contains(out, "blindly") {
|
|
t.Errorf("comment brief must discourage blindly reading every new comment, got:\n%s", out)
|
|
}
|
|
// Parent thread first.
|
|
if !strings.Contains(out, "--thread reply-abc --since "+since+" --output json") {
|
|
t.Errorf("comment brief must point at the triggering (parent) thread --since read first, got:\n%s", out)
|
|
}
|
|
if !strings.Contains(out, "--tail 30") {
|
|
t.Errorf("comment brief must offer the full-thread (--tail 30) option, got:\n%s", out)
|
|
}
|
|
// Issue-wide catch-up demoted to an only-if-needed fallback.
|
|
if !strings.Contains(out, "multica issue comment list "+issueID+" --since "+since+" --output json") {
|
|
t.Errorf("comment brief must keep the issue-wide --since catch-up fallback, got:\n%s", out)
|
|
}
|
|
// The removed resolve step must not reappear.
|
|
if strings.Contains(out, "multica comment resolve") {
|
|
t.Errorf("comment brief must not carry the dropped resolve step, got:\n%s", out)
|
|
}
|
|
}
|
|
|
|
// Cold start (no prior run → no since anchor) must point the agent at the
|
|
// triggering CONVERSATION (--thread <trigger> --tail 30) instead of the flat
|
|
// timeline dump or the since-delta hint.
|
|
func TestCommentTriggeredBriefColdStartThreadRead(t *testing.T) {
|
|
t.Parallel()
|
|
const issueID = "55555555-6666-7777-8888-999999999999"
|
|
ctx := TaskContextForEnv{
|
|
IssueID: issueID,
|
|
TriggerCommentID: "trigger-1",
|
|
TriggerThreadID: "thread-root-1",
|
|
NewCommentCount: 0,
|
|
NewCommentsSince: "",
|
|
}
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
if strings.Contains(out, "new comment(s) since your last run") {
|
|
t.Errorf("no since-delta hint should render on cold start, got:\n%s", out)
|
|
}
|
|
if !strings.Contains(out, "multica issue comment list "+issueID+" --thread thread-root-1 --tail 30 --output json") {
|
|
t.Errorf("cold start must point at the triggering thread read, got:\n%s", out)
|
|
}
|
|
}
|
|
|
|
// A resumed comment session with no since-delta should not fall back to the
|
|
// cold-start "read the triggering conversation first" instruction. The trigger
|
|
// body is already embedded in the per-turn prompt and the resumed session should
|
|
// carry prior thread context, so the thread read is only a fallback.
|
|
func TestCommentTriggeredBriefResumedNoDeltaSkipsDefaultThreadRead(t *testing.T) {
|
|
t.Parallel()
|
|
const issueID = "55555555-6666-7777-8888-999999999999"
|
|
ctx := TaskContextForEnv{
|
|
IssueID: issueID,
|
|
TriggerCommentID: "trigger-1",
|
|
TriggerThreadID: "thread-root-1",
|
|
PriorSessionResumed: true,
|
|
NewCommentCount: 0,
|
|
NewCommentsSince: "",
|
|
}
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
|
|
for _, want := range []string{
|
|
"triggering comment is already included above",
|
|
"No other new comments on this issue since your last run",
|
|
"active thread anchor `thread-root-1` and triggering comment ID `trigger-1`",
|
|
"If your reply depends on thread context",
|
|
"do not rely only on resumed session memory",
|
|
"multica issue comment list " + issueID + " --thread thread-root-1 --tail 30 --output json",
|
|
} {
|
|
if !strings.Contains(out, want) {
|
|
t.Errorf("resumed/no-delta brief missing %q\n--- output ---\n%s", want, out)
|
|
}
|
|
}
|
|
if strings.Contains(out, "scoped to the triggering thread") {
|
|
t.Errorf("resumed/no-delta brief must not claim the delta is thread-scoped, got:\n%s", out)
|
|
}
|
|
if strings.Contains(out, "Read the triggering conversation first") {
|
|
t.Errorf("resumed/no-delta brief must not use the cold-start forced-read wording, got:\n%s", out)
|
|
}
|
|
}
|
|
|
|
// Assignment-triggered briefs are the high-risk path for role conflicts:
|
|
// non-executor agents still need issue context, but the runtime workflow must
|
|
// not turn status changes, investigation, implementation, or delegation into
|
|
// permissions that override Agent Identity.
|
|
func TestAssignmentTriggeredProtocolHonorsAgentIdentity(t *testing.T) {
|
|
t.Parallel()
|
|
const issueID = "77777777-8888-9999-aaaa-bbbbbbbbbbbb"
|
|
ctx := TaskContextForEnv{IssueID: issueID}
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
|
|
for _, want := range []string{
|
|
"## Instruction Precedence",
|
|
"Agent Identity instructions have priority over the assignment workflow below.",
|
|
"If a workflow step conflicts with Agent Identity, skip the conflicting action",
|
|
"Never treat this runtime workflow as permission to change issue status, investigate, implement",
|
|
"Run `multica issue status " + issueID + " in_progress` unless your Agent Identity forbids issue status changes; if it does, skip this step.",
|
|
"Complete the task within your Agent Identity boundaries.",
|
|
"Do not investigate, implement, create issues, update issues, or delegate if your Agent Identity forbids that action",
|
|
"When done, run `multica issue status " + issueID + " in_review` unless your Agent Identity forbids issue status changes; if it does, skip this step.",
|
|
"If blocked, run `multica issue status " + issueID + " blocked` unless your Agent Identity forbids issue status changes.",
|
|
} {
|
|
if !strings.Contains(out, want) {
|
|
t.Errorf("assignment-triggered brief missing identity-bound workflow text %q\n---\n%s", want, out)
|
|
}
|
|
}
|
|
|
|
for _, banned := range []string{
|
|
"4. Run `multica issue status " + issueID + " in_progress`\n",
|
|
"5. Follow your Skills and Agent Identity to complete the task (write code, investigate, etc.)",
|
|
"8. When done, run `multica issue status " + issueID + " in_review`\n",
|
|
} {
|
|
if strings.Contains(out, banned) {
|
|
t.Errorf("assignment-triggered brief still contains unconditional legacy workflow text %q\n---\n%s", banned, out)
|
|
}
|
|
}
|
|
}
|
|
|
|
func TestInstructionPrecedenceOnlyAppliesToAssignmentWorkflow(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
name string
|
|
ctx TaskContextForEnv
|
|
}{
|
|
{
|
|
name: "comment-triggered",
|
|
ctx: TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
TriggerCommentID: "22222222-3333-4444-5555-666666666666",
|
|
},
|
|
},
|
|
{
|
|
name: "chat",
|
|
ctx: TaskContextForEnv{ChatSessionID: "chat-1"},
|
|
},
|
|
{
|
|
name: "quick-create",
|
|
ctx: TaskContextForEnv{QuickCreatePrompt: "create me an issue"},
|
|
},
|
|
{
|
|
name: "autopilot run-only",
|
|
ctx: TaskContextForEnv{AutopilotRunID: "run-1"},
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
out := buildMetaSkillContent("claude", tc.ctx)
|
|
for _, banned := range []string{
|
|
"## Instruction Precedence",
|
|
"assignment workflow below",
|
|
"Never treat this runtime workflow as permission to change issue status",
|
|
} {
|
|
if strings.Contains(out, banned) {
|
|
t.Errorf("%s brief must not inherit assignment-only precedence text %q\n---\n%s", tc.name, banned, out)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestChatOutputDoesNotRequireIssueComment(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
out := buildMetaSkillContent("claude", TaskContextForEnv{ChatSessionID: "chat-1"})
|
|
|
|
for _, want := range []string{
|
|
"This is a chat session",
|
|
"Your reply is delivered directly to the chat window the user is reading",
|
|
} {
|
|
if !strings.Contains(out, want) {
|
|
t.Errorf("chat brief missing chat output guidance %q\n---\n%s", want, out)
|
|
}
|
|
}
|
|
|
|
for _, banned := range []string{
|
|
"Final results MUST be delivered via `multica issue comment add`",
|
|
"The user does NOT see your terminal output",
|
|
"do not call `multica issue comment add`",
|
|
"unless the user explicitly asks",
|
|
} {
|
|
if strings.Contains(out, banned) {
|
|
t.Errorf("chat brief must not inherit issue-comment output warning %q\n---\n%s", banned, out)
|
|
}
|
|
}
|
|
}
|
|
|
|
// The Output section for issue tasks must forbid mid-run progress
|
|
// comments and require the single final result comment. Guards the
|
|
// MUL-3605 regression where a review agent surfaced its progress
|
|
// narration as the result instead of posting a conclusion. (The
|
|
// pre-existing "Final results MUST be delivered … invisible without it"
|
|
// and "state the outcome, not the process" lines already carry the
|
|
// mandatory-comment and no-process-dump halves.) Chat / quick-create /
|
|
// autopilot kinds keep their own delivery channels and must NOT inherit
|
|
// this rule. Runs both the legacy and slim paths.
|
|
func TestOutputForbidsMidRunProgressComments(t *testing.T) {
|
|
wantPhrases := []string{
|
|
"Post exactly ONE comment per run",
|
|
"Do NOT post progress updates",
|
|
}
|
|
issueCtxs := map[string]TaskContextForEnv{
|
|
"assignment": {IssueID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee"},
|
|
"comment": {IssueID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee", TriggerCommentID: "tc-1"},
|
|
}
|
|
|
|
run := func(t *testing.T, label string) {
|
|
for name, ctx := range issueCtxs {
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
for _, want := range wantPhrases {
|
|
if !strings.Contains(out, want) {
|
|
t.Errorf("%s/%s brief missing output rule %q\n---\n%s", label, name, want, out)
|
|
}
|
|
}
|
|
}
|
|
// Chat keeps its own delivery channel; it must not inherit the
|
|
// issue-task "post a final comment" rules.
|
|
chat := buildMetaSkillContent("claude", TaskContextForEnv{ChatSessionID: "chat-1"})
|
|
for _, banned := range wantPhrases {
|
|
if strings.Contains(chat, banned) {
|
|
t.Errorf("%s chat brief must not inherit issue output rule %q", label, banned)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Not parallel: the slim subtest toggles a process-wide feature flag.
|
|
t.Run("legacy", func(t *testing.T) { run(t, "legacy") })
|
|
t.Run("slim", func(t *testing.T) {
|
|
withSlimBrief(t)
|
|
run(t, "slim")
|
|
})
|
|
}
|
|
|
|
// The sub-issue creation rule must reach top-level parents that have no
|
|
// `parent_issue_id` of their own — that is where the `todo` vs `backlog`
|
|
// decision matters most. The section must not gate on this issue being
|
|
// a child, and must not even mention `parent_issue_id`.
|
|
func TestSubIssueCreationSectionIsUnconditional(t *testing.T) {
|
|
t.Parallel()
|
|
ctx := TaskContextForEnv{
|
|
IssueID: "aaaaaaaa-bbbb-cccc-dddd-eeeeeeeeeeee",
|
|
}
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
|
|
const header = "## Sub-issue Creation"
|
|
start := strings.Index(out, header)
|
|
if start == -1 {
|
|
t.Fatalf("sub-issue creation section missing")
|
|
}
|
|
rest := out[start:]
|
|
end := strings.Index(rest[len(header):], "\n## ")
|
|
var section string
|
|
if end == -1 {
|
|
section = rest
|
|
} else {
|
|
section = rest[:len(header)+end]
|
|
}
|
|
|
|
if strings.Contains(section, "parent_issue_id") {
|
|
t.Errorf("Sub-issue Creation section must not reference `parent_issue_id` — it applies to any issue-bound run, including top-level parents:\n%s", section)
|
|
}
|
|
}
|
|
|
|
// Workspace Context block: workspace.context (the per-workspace system prompt
|
|
// owners set in Settings → General) must reach the brief as `## Workspace
|
|
// Context` for every task kind so agents see a consistent shared system prompt
|
|
// regardless of how they were triggered. Empty content must skip the heading
|
|
// entirely — bare headings would just add noise.
|
|
func TestWorkspaceContextRenderedAcrossTaskKinds(t *testing.T) {
|
|
t.Parallel()
|
|
const wsContext = "All comments must be in English. Prefer concise PR descriptions."
|
|
cases := []struct {
|
|
name string
|
|
ctx TaskContextForEnv
|
|
}{
|
|
{
|
|
name: "assignment-triggered",
|
|
ctx: TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
WorkspaceContext: wsContext,
|
|
},
|
|
},
|
|
{
|
|
name: "comment-triggered",
|
|
ctx: TaskContextForEnv{
|
|
IssueID: "22222222-3333-4444-5555-666666666666",
|
|
TriggerCommentID: "33333333-4444-5555-6666-777777777777",
|
|
WorkspaceContext: wsContext,
|
|
},
|
|
},
|
|
{
|
|
name: "chat",
|
|
ctx: TaskContextForEnv{
|
|
ChatSessionID: "chat-1",
|
|
WorkspaceContext: wsContext,
|
|
},
|
|
},
|
|
{
|
|
name: "quick-create",
|
|
ctx: TaskContextForEnv{
|
|
QuickCreatePrompt: "create me an issue",
|
|
WorkspaceContext: wsContext,
|
|
},
|
|
},
|
|
{
|
|
name: "autopilot run-only",
|
|
ctx: TaskContextForEnv{
|
|
AutopilotRunID: "run-1",
|
|
WorkspaceContext: wsContext,
|
|
},
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
out := buildMetaSkillContent("claude", tc.ctx)
|
|
|
|
if !strings.Contains(out, "## Workspace Context") {
|
|
t.Fatalf("[%s] expected `## Workspace Context` heading", tc.name)
|
|
}
|
|
if !strings.Contains(out, wsContext) {
|
|
t.Errorf("[%s] brief missing workspace context body %q", tc.name, wsContext)
|
|
}
|
|
// The block must precede Available Commands so it acts as
|
|
// background framing, not a footer hidden below CLI usage.
|
|
ctxIdx := strings.Index(out, "## Workspace Context")
|
|
cmdsIdx := strings.Index(out, "## Available Commands")
|
|
if ctxIdx == -1 || cmdsIdx == -1 || ctxIdx > cmdsIdx {
|
|
t.Errorf("[%s] `## Workspace Context` must appear above `## Available Commands` (ctx=%d, cmds=%d)", tc.name, ctxIdx, cmdsIdx)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestWorkspaceContextHeadingSkippedWhenEmpty(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
name string
|
|
ctx TaskContextForEnv
|
|
}{
|
|
{
|
|
name: "empty string",
|
|
ctx: TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
WorkspaceContext: "",
|
|
},
|
|
},
|
|
{
|
|
name: "whitespace only",
|
|
ctx: TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
WorkspaceContext: " \n\t \r\n",
|
|
},
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
out := buildMetaSkillContent("claude", tc.ctx)
|
|
if strings.Contains(out, "## Workspace Context") {
|
|
t.Errorf("[%s] empty workspace context must NOT emit the heading", tc.name)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestConnectedAppsRenderedAcrossBriefModes(t *testing.T) {
|
|
ctx := TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
WorkspaceContext: "Prefer source-of-truth systems.",
|
|
ConnectedApps: []runtimeapps.ConnectedApp{{
|
|
Provider: "composio",
|
|
ServerName: "composio",
|
|
ToolkitSlug: "notion",
|
|
ToolkitName: "Notion",
|
|
}},
|
|
}
|
|
|
|
run := func(t *testing.T, label string) {
|
|
out := buildMetaSkillContent("claude", ctx)
|
|
for _, want := range []string{
|
|
"## Connected Apps",
|
|
"- Notion (`notion`) via MCP server `composio`",
|
|
"Use the listed MCP server when the task asks to read or act in one of these apps.",
|
|
} {
|
|
if !strings.Contains(out, want) {
|
|
t.Fatalf("%s brief missing connected app text %q\n---\n%s", label, want, out)
|
|
}
|
|
}
|
|
wsIdx := strings.Index(out, "## Workspace Context")
|
|
appIdx := strings.Index(out, "## Connected Apps")
|
|
cmdIdx := strings.Index(out, "## Available Commands")
|
|
if wsIdx == -1 || appIdx == -1 || cmdIdx == -1 || !(wsIdx < appIdx && appIdx < cmdIdx) {
|
|
t.Fatalf("%s connected apps should sit between workspace context and available commands (ws=%d app=%d cmd=%d)", label, wsIdx, appIdx, cmdIdx)
|
|
}
|
|
}
|
|
|
|
t.Run("legacy", func(t *testing.T) { run(t, "legacy") })
|
|
t.Run("slim", func(t *testing.T) {
|
|
withSlimBrief(t)
|
|
run(t, "slim")
|
|
})
|
|
}
|
|
|
|
func TestConnectedAppsHeadingSkippedWhenEmpty(t *testing.T) {
|
|
t.Parallel()
|
|
out := buildMetaSkillContent("claude", TaskContextForEnv{IssueID: "11111111-2222-3333-4444-555555555555"})
|
|
if strings.Contains(out, "## Connected Apps") {
|
|
t.Fatalf("empty connected apps must not emit the heading")
|
|
}
|
|
}
|
|
|
|
func TestSubIssueCreationSectionSkippedForNonIssueModes(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
name string
|
|
ctx TaskContextForEnv
|
|
}{
|
|
{
|
|
name: "chat",
|
|
ctx: TaskContextForEnv{ChatSessionID: "chat-1"},
|
|
},
|
|
{
|
|
name: "quick-create",
|
|
ctx: TaskContextForEnv{QuickCreatePrompt: "create me an issue"},
|
|
},
|
|
{
|
|
name: "autopilot run-only",
|
|
ctx: TaskContextForEnv{AutopilotRunID: "run-1"},
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
out := buildMetaSkillContent("claude", tc.ctx)
|
|
if strings.Contains(out, "## Sub-issue Creation") {
|
|
t.Errorf("%s mode must NOT emit the Sub-issue Creation section", tc.name)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// writeRuntimeConfigFile is the safe replacement for the previous
|
|
// unconditional os.WriteFile of CLAUDE.md / AGENTS.md. The two
|
|
// states it must handle correctly are: file missing, file present without
|
|
// markers (user-authored content already there — the regression case from
|
|
// MUL-2753), and file present with markers (idempotent second-run replace).
|
|
|
|
func TestWriteRuntimeConfigFileCreatesMissingFile(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
const brief = "# Multica Agent Runtime\n\nbrief body line"
|
|
|
|
if err := writeRuntimeConfigFile(path, brief); err != nil {
|
|
t.Fatalf("writeRuntimeConfigFile returned error: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back file: %v", err)
|
|
}
|
|
s := string(got)
|
|
if !strings.HasPrefix(s, runtimeMarkerBegin+"\n") {
|
|
t.Errorf("output should start with begin marker, got:\n%s", s)
|
|
}
|
|
if !strings.Contains(s, brief) {
|
|
t.Errorf("output should contain brief body, got:\n%s", s)
|
|
}
|
|
if !strings.Contains(s, "\n"+runtimeMarkerEnd+"\n") {
|
|
t.Errorf("output should contain end marker followed by newline, got:\n%s", s)
|
|
}
|
|
}
|
|
|
|
func TestWriteRuntimeConfigFilePreservesUserContent(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
const userContent = "# User repo CLAUDE.md\n\n- rule one\n- rule two\n"
|
|
if err := os.WriteFile(path, []byte(userContent), 0o644); err != nil {
|
|
t.Fatalf("seed user file: %v", err)
|
|
}
|
|
|
|
const brief = "## Multica brief\n\ninjected body"
|
|
if err := writeRuntimeConfigFile(path, brief); err != nil {
|
|
t.Fatalf("writeRuntimeConfigFile returned error: %v", err)
|
|
}
|
|
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back file: %v", err)
|
|
}
|
|
s := string(got)
|
|
// The user's original content must be untouched and appear before the
|
|
// injected marker block; this is the core regression case from MUL-2753.
|
|
if !strings.HasPrefix(s, userContent) {
|
|
t.Errorf("user content must be preserved verbatim at the top of the file, got:\n%s", s)
|
|
}
|
|
beginIdx := strings.Index(s, runtimeMarkerBegin)
|
|
endIdx := strings.Index(s, runtimeMarkerEnd)
|
|
if beginIdx < 0 || endIdx <= beginIdx {
|
|
t.Fatalf("expected a well-formed marker block in:\n%s", s)
|
|
}
|
|
if beginIdx < len(userContent) {
|
|
t.Errorf("begin marker must appear after user content, beginIdx=%d userLen=%d", beginIdx, len(userContent))
|
|
}
|
|
if !strings.Contains(s, brief) {
|
|
t.Errorf("brief body missing from output:\n%s", s)
|
|
}
|
|
}
|
|
|
|
func TestWriteRuntimeConfigFileReplacesExistingBlock(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "AGENTS.md")
|
|
const userBefore = "# User AGENTS.md\n\nuser line above\n"
|
|
const userAfter = "\nuser line below the block\n"
|
|
original := userBefore +
|
|
runtimeMarkerBegin + "\n" +
|
|
"OLD BRIEF CONTENT THAT MUST GO AWAY\n" +
|
|
runtimeMarkerEnd + "\n" +
|
|
userAfter
|
|
if err := os.WriteFile(path, []byte(original), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
const newBrief = "## New Multica brief\n\nfresh body"
|
|
if err := writeRuntimeConfigFile(path, newBrief); err != nil {
|
|
t.Fatalf("writeRuntimeConfigFile returned error: %v", err)
|
|
}
|
|
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back file: %v", err)
|
|
}
|
|
s := string(got)
|
|
if !strings.HasPrefix(s, userBefore) {
|
|
t.Errorf("content above the marker block must be preserved, got:\n%s", s)
|
|
}
|
|
if !strings.HasSuffix(s, userAfter) {
|
|
t.Errorf("content below the marker block must be preserved, got:\n%s", s)
|
|
}
|
|
if strings.Contains(s, "OLD BRIEF CONTENT THAT MUST GO AWAY") {
|
|
t.Errorf("previous block body must be replaced, got:\n%s", s)
|
|
}
|
|
if !strings.Contains(s, newBrief) {
|
|
t.Errorf("new brief body missing from output:\n%s", s)
|
|
}
|
|
if strings.Count(s, runtimeMarkerBegin) != 1 || strings.Count(s, runtimeMarkerEnd) != 1 {
|
|
t.Errorf("there must be exactly one begin/end marker pair, got:\n%s", s)
|
|
}
|
|
}
|
|
|
|
func TestWriteRuntimeConfigFileIsIdempotent(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
const userContent = "# User CLAUDE.md\n\nimportant rules\n"
|
|
if err := os.WriteFile(path, []byte(userContent), 0o644); err != nil {
|
|
t.Fatalf("seed user file: %v", err)
|
|
}
|
|
|
|
const brief = "## Multica brief\n\nbody"
|
|
for i := 0; i < 5; i++ {
|
|
if err := writeRuntimeConfigFile(path, brief); err != nil {
|
|
t.Fatalf("iteration %d: %v", i, err)
|
|
}
|
|
}
|
|
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back file: %v", err)
|
|
}
|
|
s := string(got)
|
|
if strings.Count(s, runtimeMarkerBegin) != 1 {
|
|
t.Errorf("repeated runs must not duplicate the begin marker, count=%d, file:\n%s", strings.Count(s, runtimeMarkerBegin), s)
|
|
}
|
|
if strings.Count(s, runtimeMarkerEnd) != 1 {
|
|
t.Errorf("repeated runs must not duplicate the end marker, count=%d, file:\n%s", strings.Count(s, runtimeMarkerEnd), s)
|
|
}
|
|
if strings.Count(s, brief) != 1 {
|
|
t.Errorf("repeated runs must not duplicate the brief body, count=%d, file:\n%s", strings.Count(s, brief), s)
|
|
}
|
|
if !strings.HasPrefix(s, userContent) {
|
|
t.Errorf("user content must remain intact at the top of the file, got:\n%s", s)
|
|
}
|
|
}
|
|
|
|
// InjectRuntimeConfig is the production entry point — verify the marker
|
|
// semantics propagate through it for each provider's target filename.
|
|
func TestInjectRuntimeConfigPreservesUserContent(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
provider string
|
|
filename string
|
|
}{
|
|
{"claude", "CLAUDE.md"},
|
|
{"codex", "AGENTS.md"},
|
|
{"copilot", "AGENTS.md"},
|
|
{"opencode", "AGENTS.md"},
|
|
{"openclaw", "AGENTS.md"},
|
|
{"hermes", "AGENTS.md"},
|
|
{"pi", "AGENTS.md"},
|
|
{"cursor", "AGENTS.md"},
|
|
{"kimi", "AGENTS.md"},
|
|
{"kiro", "AGENTS.md"},
|
|
{"antigravity", "AGENTS.md"},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.provider, func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, tc.filename)
|
|
const userContent = "# User-authored file\n\ndon't touch this\n"
|
|
if err := os.WriteFile(path, []byte(userContent), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
content, err := InjectRuntimeConfig(dir, tc.provider, TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
})
|
|
if err != nil {
|
|
t.Fatalf("InjectRuntimeConfig: %v", err)
|
|
}
|
|
if content == "" {
|
|
t.Fatalf("returned brief content must be non-empty")
|
|
}
|
|
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
s := string(got)
|
|
if !strings.HasPrefix(s, userContent) {
|
|
t.Errorf("[%s] user content must be preserved verbatim at the top of %s, got:\n%s", tc.provider, tc.filename, s)
|
|
}
|
|
if !strings.Contains(s, runtimeMarkerBegin) || !strings.Contains(s, runtimeMarkerEnd) {
|
|
t.Errorf("[%s] %s must contain the runtime marker block, got:\n%s", tc.provider, tc.filename, s)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
func TestInjectRuntimeConfigUnknownProviderSkipsWrite(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
// Seed all three candidate filenames so we can verify none of them get
|
|
// written when the provider is unknown.
|
|
for _, name := range []string{"CLAUDE.md", "AGENTS.md"} {
|
|
if err := os.WriteFile(filepath.Join(dir, name), []byte("untouched\n"), 0o644); err != nil {
|
|
t.Fatalf("seed %s: %v", name, err)
|
|
}
|
|
}
|
|
|
|
if _, err := InjectRuntimeConfig(dir, "totally-unknown-provider", TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
}); err != nil {
|
|
t.Fatalf("InjectRuntimeConfig: %v", err)
|
|
}
|
|
for _, name := range []string{"CLAUDE.md", "AGENTS.md"} {
|
|
got, err := os.ReadFile(filepath.Join(dir, name))
|
|
if err != nil {
|
|
t.Fatalf("read %s: %v", name, err)
|
|
}
|
|
if string(got) != "untouched\n" {
|
|
t.Errorf("unknown provider must not write %s; got:\n%s", name, string(got))
|
|
}
|
|
}
|
|
}
|
|
|
|
// Parser hardening: the end marker must be found strictly after the begin
|
|
// marker so a stray end marker that appears earlier in user content (e.g.
|
|
// a documentation snippet showing what the wire format looks like) doesn't
|
|
// trick writeRuntimeConfigFile into thinking the file is malformed and
|
|
// appending another block on every run.
|
|
func TestWriteRuntimeConfigFileIgnoresStrayEndMarkerBeforeBegin(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
|
|
// Seed a file whose user-authored portion documents the marker format
|
|
// (so the *end* marker appears before any *begin* marker), then has a
|
|
// real block authored by an earlier Multica run below.
|
|
const userDoc = "# Repo CLAUDE.md\n\nExample of what Multica writes:\n" +
|
|
runtimeMarkerEnd + "\n\n# Real config below\n"
|
|
original := userDoc +
|
|
runtimeMarkerBegin + "\nFIRST BRIEF\n" + runtimeMarkerEnd + "\n"
|
|
if err := os.WriteFile(path, []byte(original), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
const newBrief = "SECOND BRIEF"
|
|
if err := writeRuntimeConfigFile(path, newBrief); err != nil {
|
|
t.Fatalf("writeRuntimeConfigFile: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
s := string(got)
|
|
|
|
// The user's stray end marker line plus surrounding doc text must still
|
|
// be present, and the file must contain exactly one begin marker and
|
|
// one *additional* end marker (so two end markers total — the stray
|
|
// one and the one closing our block).
|
|
if !strings.Contains(s, userDoc) {
|
|
t.Errorf("user doc with stray end marker must be preserved verbatim, got:\n%s", s)
|
|
}
|
|
if got, want := strings.Count(s, runtimeMarkerBegin), 1; got != want {
|
|
t.Errorf("expected exactly %d begin markers, got %d:\n%s", want, got, s)
|
|
}
|
|
if got, want := strings.Count(s, runtimeMarkerEnd), 2; got != want {
|
|
t.Errorf("expected exactly %d end markers (1 user stray + 1 closing our block), got %d:\n%s", want, got, s)
|
|
}
|
|
if strings.Contains(s, "FIRST BRIEF") {
|
|
t.Errorf("previous brief body must be replaced, got:\n%s", s)
|
|
}
|
|
if !strings.Contains(s, newBrief) {
|
|
t.Errorf("new brief body missing from output:\n%s", s)
|
|
}
|
|
|
|
// Idempotency under the stray-end pattern: a second write must not
|
|
// stack another block.
|
|
if err := writeRuntimeConfigFile(path, newBrief); err != nil {
|
|
t.Fatalf("second writeRuntimeConfigFile: %v", err)
|
|
}
|
|
got2, _ := os.ReadFile(path)
|
|
s2 := string(got2)
|
|
if got, want := strings.Count(s2, runtimeMarkerBegin), 1; got != want {
|
|
t.Errorf("repeat write must not grow begin markers, got %d, want %d:\n%s", got, want, s2)
|
|
}
|
|
}
|
|
|
|
// Parser hardening: a file containing only a begin marker (e.g. a previous
|
|
// run that crashed mid-write) must not cause every subsequent run to stack
|
|
// another block beneath the half-block.
|
|
func TestWriteRuntimeConfigFileReplacesMalformedHalfBlock(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "AGENTS.md")
|
|
|
|
const userTop = "# Repo AGENTS.md\n\nrules above\n"
|
|
const halfBlock = "leftover from crashed write\nsecond line\n"
|
|
original := userTop + runtimeMarkerBegin + "\n" + halfBlock
|
|
if err := os.WriteFile(path, []byte(original), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
const newBrief = "recovered brief"
|
|
if err := writeRuntimeConfigFile(path, newBrief); err != nil {
|
|
t.Fatalf("writeRuntimeConfigFile: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
s := string(got)
|
|
if !strings.HasPrefix(s, userTop) {
|
|
t.Errorf("user content above the half-block must be preserved, got:\n%s", s)
|
|
}
|
|
if strings.Contains(s, "leftover from crashed write") {
|
|
t.Errorf("half-block contents must be replaced, got:\n%s", s)
|
|
}
|
|
if got, want := strings.Count(s, runtimeMarkerBegin), 1; got != want {
|
|
t.Errorf("expected exactly %d begin marker, got %d:\n%s", want, got, s)
|
|
}
|
|
if got, want := strings.Count(s, runtimeMarkerEnd), 1; got != want {
|
|
t.Errorf("expected exactly %d end marker after recovery, got %d:\n%s", want, got, s)
|
|
}
|
|
if !strings.Contains(s, newBrief) {
|
|
t.Errorf("new brief body missing from output:\n%s", s)
|
|
}
|
|
}
|
|
|
|
// Cleanup excises the marker block, preserving every byte of surrounding
|
|
// user content. This is the local_directory invariant: a `claude` /
|
|
// `codex` run started by the user after a Multica task must see the same
|
|
// file the user wrote.
|
|
func TestCleanupRuntimeConfigPreservesUserContent(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
|
|
const userBefore = "# Repo CLAUDE.md\n\nuser line above\n"
|
|
const userAfter = "\nuser line below the block\n"
|
|
const userExpected = "# Repo CLAUDE.md\n\nuser line above\n\nuser line below the block\n"
|
|
// Inject via the production write path so we exercise the actual
|
|
// marker block format, not a hand-rolled approximation.
|
|
if err := os.WriteFile(path, []byte(userBefore+userAfter), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
if err := writeRuntimeConfigFile(path, "brief body"); err != nil {
|
|
t.Fatalf("seed brief: %v", err)
|
|
}
|
|
|
|
if err := CleanupRuntimeConfig(dir, "claude"); err != nil {
|
|
t.Fatalf("CleanupRuntimeConfig: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
s := string(got)
|
|
if strings.Contains(s, runtimeMarkerBegin) || strings.Contains(s, runtimeMarkerEnd) {
|
|
t.Errorf("marker block must be removed, got:\n%s", s)
|
|
}
|
|
if strings.Contains(s, "brief body") {
|
|
t.Errorf("brief body must be removed, got:\n%s", s)
|
|
}
|
|
if s != userExpected {
|
|
t.Errorf("user content must be preserved byte-for-byte\n got:\n%q\nwant:\n%q", s, userExpected)
|
|
}
|
|
}
|
|
|
|
// Cleanup removes the file entirely when the marker block was the only
|
|
// content — i.e. we created the file from scratch in a directory that had
|
|
// no pre-existing CLAUDE.md / AGENTS.md.
|
|
func TestCleanupRuntimeConfigRemovesFileWhenOnlyBlockRemained(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
|
|
// No seed — writeRuntimeConfigFile creates the file with only the
|
|
// marker block inside.
|
|
if err := writeRuntimeConfigFile(path, "brief body"); err != nil {
|
|
t.Fatalf("seed brief: %v", err)
|
|
}
|
|
|
|
if err := CleanupRuntimeConfig(dir, "claude"); err != nil {
|
|
t.Fatalf("CleanupRuntimeConfig: %v", err)
|
|
}
|
|
if _, err := os.Stat(path); !os.IsNotExist(err) {
|
|
t.Errorf("expected file to be removed, stat err=%v", err)
|
|
}
|
|
}
|
|
|
|
// Cleanup is a no-op when no marker block exists or when the file is
|
|
// missing — Cleanup is safe to call defensively from the daemon's defer.
|
|
func TestCleanupRuntimeConfigNoOpCases(t *testing.T) {
|
|
t.Parallel()
|
|
|
|
t.Run("missing file", func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
if err := CleanupRuntimeConfig(dir, "claude"); err != nil {
|
|
t.Errorf("missing file must be no-op, got: %v", err)
|
|
}
|
|
// And the directory must remain untouched.
|
|
entries, err := os.ReadDir(dir)
|
|
if err != nil {
|
|
t.Fatalf("readdir: %v", err)
|
|
}
|
|
if len(entries) != 0 {
|
|
t.Errorf("expected dir to remain empty, got: %v", entries)
|
|
}
|
|
})
|
|
|
|
t.Run("file without marker block", func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
const userContent = "# Repo CLAUDE.md\n\nrules\n"
|
|
if err := os.WriteFile(path, []byte(userContent), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
if err := CleanupRuntimeConfig(dir, "claude"); err != nil {
|
|
t.Errorf("no-marker-block file must be no-op, got: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
if string(got) != userContent {
|
|
t.Errorf("file must be untouched\n got:\n%q\nwant:\n%q", string(got), userContent)
|
|
}
|
|
})
|
|
|
|
t.Run("unknown provider", func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
// Seed every candidate filename to verify none of them get touched.
|
|
for _, name := range []string{"CLAUDE.md", "AGENTS.md"} {
|
|
if err := os.WriteFile(filepath.Join(dir, name), []byte("untouched\n"), 0o644); err != nil {
|
|
t.Fatalf("seed %s: %v", name, err)
|
|
}
|
|
}
|
|
if err := CleanupRuntimeConfig(dir, "totally-unknown-provider"); err != nil {
|
|
t.Errorf("unknown provider must be no-op, got: %v", err)
|
|
}
|
|
for _, name := range []string{"CLAUDE.md", "AGENTS.md"} {
|
|
got, err := os.ReadFile(filepath.Join(dir, name))
|
|
if err != nil {
|
|
t.Fatalf("read %s: %v", name, err)
|
|
}
|
|
if string(got) != "untouched\n" {
|
|
t.Errorf("unknown provider must not touch %s; got:\n%s", name, string(got))
|
|
}
|
|
}
|
|
})
|
|
}
|
|
|
|
// Cleanup must handle a half-block left by a previous crashed run: begin
|
|
// marker present but no end. Otherwise the half-block would survive
|
|
// cleanup and pollute the next manual CLI invocation in the same dir.
|
|
func TestCleanupRuntimeConfigRemovesMalformedHalfBlock(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "AGENTS.md")
|
|
|
|
const userTop = "# Repo AGENTS.md\n\nrules\n"
|
|
original := userTop + runtimeMarkerBegin + "\nhalf-written brief no end\n"
|
|
if err := os.WriteFile(path, []byte(original), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
if err := CleanupRuntimeConfig(dir, "codex"); err != nil {
|
|
t.Fatalf("CleanupRuntimeConfig: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
s := string(got)
|
|
if strings.Contains(s, runtimeMarkerBegin) {
|
|
t.Errorf("half-block begin marker must be excised, got:\n%s", s)
|
|
}
|
|
if strings.Contains(s, "half-written brief no end") {
|
|
t.Errorf("half-block body must be excised, got:\n%s", s)
|
|
}
|
|
if !strings.HasPrefix(s, userTop) {
|
|
t.Errorf("user content above the half-block must remain, got:\n%s", s)
|
|
}
|
|
}
|
|
|
|
// Cleanup must remove the marker block for every provider's target file,
|
|
// using the same provider→filename mapping as InjectRuntimeConfig — so a
|
|
// new provider added to one side cannot drift past the other.
|
|
func TestCleanupRuntimeConfigByProvider(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
provider string
|
|
filename string
|
|
}{
|
|
{"claude", "CLAUDE.md"},
|
|
{"codex", "AGENTS.md"},
|
|
{"copilot", "AGENTS.md"},
|
|
{"opencode", "AGENTS.md"},
|
|
{"openclaw", "AGENTS.md"},
|
|
{"hermes", "AGENTS.md"},
|
|
{"pi", "AGENTS.md"},
|
|
{"cursor", "AGENTS.md"},
|
|
{"kimi", "AGENTS.md"},
|
|
{"kiro", "AGENTS.md"},
|
|
{"antigravity", "AGENTS.md"},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.provider, func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, tc.filename)
|
|
const userContent = "# User file\n\ndon't touch this\n"
|
|
if err := os.WriteFile(path, []byte(userContent), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
// Inject through the production path so cleanup runs against
|
|
// the same wire format the agent saw.
|
|
if _, err := InjectRuntimeConfig(dir, tc.provider, TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
}); err != nil {
|
|
t.Fatalf("InjectRuntimeConfig: %v", err)
|
|
}
|
|
if err := CleanupRuntimeConfig(dir, tc.provider); err != nil {
|
|
t.Fatalf("CleanupRuntimeConfig: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
s := string(got)
|
|
if strings.Contains(s, runtimeMarkerBegin) || strings.Contains(s, runtimeMarkerEnd) {
|
|
t.Errorf("[%s] marker block must be removed from %s, got:\n%s", tc.provider, tc.filename, s)
|
|
}
|
|
if s != userContent {
|
|
t.Errorf("[%s] user content in %s must be preserved byte-for-byte\n got:\n%q\nwant:\n%q", tc.provider, tc.filename, s, userContent)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// Inject → Cleanup → manual edit → Inject must converge back to the
|
|
// pre-injection state on the next Cleanup. This is the end-to-end
|
|
// regression that locks in: the user's repo is byte-identical to what
|
|
// they had before the task, every task cycle.
|
|
func TestInjectThenCleanupRoundTrip(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
const userContent = "# User-authored CLAUDE.md\n\n- rule A\n- rule B\n"
|
|
if err := os.WriteFile(path, []byte(userContent), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
// Two full inject→cleanup cycles — covers both the "first task on a
|
|
// fresh user file" path and the "subsequent task hits a clean file
|
|
// again" path.
|
|
for i := 0; i < 2; i++ {
|
|
if _, err := InjectRuntimeConfig(dir, "claude", TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
}); err != nil {
|
|
t.Fatalf("iter %d inject: %v", i, err)
|
|
}
|
|
if err := CleanupRuntimeConfig(dir, "claude"); err != nil {
|
|
t.Fatalf("iter %d cleanup: %v", i, err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("iter %d read back: %v", i, err)
|
|
}
|
|
if string(got) != userContent {
|
|
t.Errorf("iter %d: user file must be byte-identical to pre-injection state\n got:\n%q\nwant:\n%q", i, string(got), userContent)
|
|
}
|
|
}
|
|
}
|
|
|
|
// Byte-exact boundary coverage flagged in PR #3438 review (Elon): the
|
|
// previous cleanup used TrimRight + "\n" and TrimSpace-based file removal,
|
|
// which created a real diff in three boundary cases. The table walks each
|
|
// one through a full inject→cleanup cycle and asserts the file ends up
|
|
// byte-identical (or, for missing-file, that it stays missing).
|
|
func TestInjectThenCleanupRoundTripByteExactBoundaries(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
name string
|
|
// seed describes the pre-inject filesystem state. When seedExists
|
|
// is false the file is absent; when true the file is created with
|
|
// seedContent (which may be empty / whitespace-only / arbitrary
|
|
// bytes).
|
|
seedExists bool
|
|
seedContent string
|
|
}{
|
|
{
|
|
name: "file missing — Inject creates, Cleanup removes",
|
|
seedExists: false,
|
|
seedContent: "",
|
|
},
|
|
{
|
|
name: "pre-existing empty file (zero bytes)",
|
|
seedExists: true,
|
|
seedContent: "",
|
|
},
|
|
{
|
|
name: "pre-existing whitespace-only file",
|
|
seedExists: true,
|
|
seedContent: " \n",
|
|
},
|
|
{
|
|
name: "no trailing newline",
|
|
seedExists: true,
|
|
seedContent: "rules",
|
|
},
|
|
{
|
|
name: "one trailing newline (the common markdown shape)",
|
|
seedExists: true,
|
|
seedContent: "# Rules\n\nbody\n",
|
|
},
|
|
{
|
|
name: "two trailing newlines",
|
|
seedExists: true,
|
|
seedContent: "rules\n\n",
|
|
},
|
|
{
|
|
name: "many trailing newlines",
|
|
seedExists: true,
|
|
seedContent: "rules\n\n\n\n",
|
|
},
|
|
{
|
|
name: "CRLF line endings",
|
|
seedExists: true,
|
|
seedContent: "rule A\r\nrule B\r\n",
|
|
},
|
|
{
|
|
name: "no final newline AND embedded blank lines",
|
|
seedExists: true,
|
|
seedContent: "para 1\n\npara 2\n\npara 3",
|
|
},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
|
|
if tc.seedExists {
|
|
if err := os.WriteFile(path, []byte(tc.seedContent), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
}
|
|
|
|
// Two cycles to cover both "first inject hits user file" and
|
|
// "subsequent inject hits a cleaned file" paths.
|
|
for i := 0; i < 2; i++ {
|
|
if _, err := InjectRuntimeConfig(dir, "claude", TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
}); err != nil {
|
|
t.Fatalf("iter %d inject: %v", i, err)
|
|
}
|
|
if err := CleanupRuntimeConfig(dir, "claude"); err != nil {
|
|
t.Fatalf("iter %d cleanup: %v", i, err)
|
|
}
|
|
|
|
if !tc.seedExists {
|
|
// Missing file must remain missing after the cycle so
|
|
// the user's directory listing is also byte-identical
|
|
// (no zero-byte stub left behind).
|
|
if _, err := os.Stat(path); !os.IsNotExist(err) {
|
|
t.Errorf("iter %d: file must remain missing, stat err=%v", i, err)
|
|
}
|
|
continue
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("iter %d read back: %v", i, err)
|
|
}
|
|
if string(got) != tc.seedContent {
|
|
t.Errorf("iter %d: file must be byte-identical to seed\n got: %q\n want: %q", i, string(got), tc.seedContent)
|
|
}
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// Idempotency across the byte-exact boundaries: when a second Inject runs
|
|
// against a file that already carries a marker block (the "replace in
|
|
// place" branch), the surrounding bytes must stay untouched and the
|
|
// subsequent Cleanup must still restore the user's original file
|
|
// byte-exactly. This guards against a regression where the replace path
|
|
// would re-normalise pre/post bytes the way the old cleanup did.
|
|
func TestInjectReplaceThenCleanupRestoresByteExact(t *testing.T) {
|
|
t.Parallel()
|
|
cases := []struct {
|
|
name string
|
|
seedContent string
|
|
}{
|
|
{name: "no trailing newline", seedContent: "rules"},
|
|
{name: "two trailing newlines", seedContent: "rules\n\n"},
|
|
{name: "empty file", seedContent: ""},
|
|
}
|
|
for _, tc := range cases {
|
|
tc := tc
|
|
t.Run(tc.name, func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
if err := os.WriteFile(path, []byte(tc.seedContent), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
|
|
// First inject — append path.
|
|
if _, err := InjectRuntimeConfig(dir, "claude", TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
}); err != nil {
|
|
t.Fatalf("first inject: %v", err)
|
|
}
|
|
// Second inject — replace-in-place path.
|
|
if _, err := InjectRuntimeConfig(dir, "claude", TaskContextForEnv{
|
|
IssueID: "11111111-2222-3333-4444-555555555555",
|
|
}); err != nil {
|
|
t.Fatalf("second inject: %v", err)
|
|
}
|
|
if err := CleanupRuntimeConfig(dir, "claude"); err != nil {
|
|
t.Fatalf("cleanup: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
if string(got) != tc.seedContent {
|
|
t.Errorf("file must be byte-identical to seed after replace+cleanup\n got: %q\n want: %q", string(got), tc.seedContent)
|
|
}
|
|
})
|
|
}
|
|
}
|
|
|
|
// The fixed managed separator is the invariant that makes byte-exact
|
|
// cleanup possible. This test pins it: writeRuntimeConfigFile must
|
|
// produce exactly `<user-bytes><\n\n><marker-block>` for ANY non-empty
|
|
// or empty pre-existing file, with no trailing-newline normalisation.
|
|
func TestWriteRuntimeConfigFileAlwaysInsertsFixedManagedSeparator(t *testing.T) {
|
|
t.Parallel()
|
|
for _, seed := range []string{"", "rules", "rules\n", "rules\n\n", "rules\n\n\n\n"} {
|
|
seed := seed
|
|
t.Run(fmt.Sprintf("seed=%q", seed), func(t *testing.T) {
|
|
t.Parallel()
|
|
dir := t.TempDir()
|
|
path := filepath.Join(dir, "CLAUDE.md")
|
|
if err := os.WriteFile(path, []byte(seed), 0o644); err != nil {
|
|
t.Fatalf("seed: %v", err)
|
|
}
|
|
if err := writeRuntimeConfigFile(path, "brief body"); err != nil {
|
|
t.Fatalf("write: %v", err)
|
|
}
|
|
got, err := os.ReadFile(path)
|
|
if err != nil {
|
|
t.Fatalf("read back: %v", err)
|
|
}
|
|
s := string(got)
|
|
// The seed must appear verbatim at the start of the file —
|
|
// no extra newline appended, no trailing newline trimmed.
|
|
if !strings.HasPrefix(s, seed) {
|
|
t.Errorf("seed bytes must survive verbatim at the start of the file\n got: %q\n seed: %q", s, seed)
|
|
}
|
|
// Immediately after the seed we must see the fixed managed
|
|
// separator, then the begin marker.
|
|
markerStart := len(seed) + len(runtimeManagedSeparator)
|
|
if len(s) < markerStart+len(runtimeMarkerBegin) {
|
|
t.Fatalf("file shorter than expected layout\n got: %q", s)
|
|
}
|
|
if got, want := s[len(seed):markerStart], runtimeManagedSeparator; got != want {
|
|
t.Errorf("expected managed separator %q immediately after seed, got %q", want, got)
|
|
}
|
|
if got, want := s[markerStart:markerStart+len(runtimeMarkerBegin)], runtimeMarkerBegin; got != want {
|
|
t.Errorf("expected begin marker after managed separator, got %q", got)
|
|
}
|
|
})
|
|
}
|
|
}
|