Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 17 additions & 2 deletions packages/loopover-mcp/bin/loopover-mcp.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,24 @@ const CLI_COMMAND_SPEC = {
};
const COMPLETION_SHELLS = ["bash", "zsh", "fish", "powershell"];
const AGENT_PROFILE_IDS = ["miner-planner", "miner-auto-dev", "maintainer-triage", "repo-owner-intake"];
// #784 maintain set-level — the autonomy dial's action classes + levels (must mirror src/settings/autonomy.ts).
// #784 maintain set-level — the autonomy dial's action classes + levels.
//
// Both are hand-synced literals, not imports: this file resolves @loopover/engine through the PUBLISHED package
// (`^3.0.0`), whose export map exposes only `.` + a few `./scoring/*`/`./signals/*` subpaths — neither surfaces
// AUTONOMY_LEVELS, so importing the canonical list would mean widening the engine's public API (#6153). The
// drift this invites is real and has bitten once already, so test/unit/mcp-cli-maintain.test.ts pins LEVELS
// against the live enum and fails the moment the two disagree.
//
// LEVELS mirrors AUTONOMY_LEVELS (src/settings/autonomy.ts -> packages/loopover-engine/src/settings/autonomy.ts)
// exactly. #6153: it carried "suggest"/"propose" for the whole life of #4620, which dropped them server-side --
// PUT /settings validates against the live enum (src/api/routes.ts), so every value this list accepted but the
// server didn't turned an immediate, clear client-side error into a confusing 400 from the API.
//
// ACTION_CLASSES is deliberately NOT the engine's full AGENT_ACTION_CLASSES: it is the operator-settable subset
// the maintain surface exposes, and src/mcp/server.ts's MAINTAIN_AUTONOMY_ACTION_CLASSES mirrors these six on
// purpose. Do not "sync" it to the engine list.
const MAINTAIN_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label"];
const MAINTAIN_AUTONOMY_LEVELS = ["observe", "suggest", "propose", "auto_with_approval", "auto"];
const MAINTAIN_AUTONOMY_LEVELS = ["observe", "auto_with_approval", "auto"];
const AGENT_PROFILES = {
"miner-planner": {
id: "miner-planner",
Expand Down
10 changes: 5 additions & 5 deletions src/mcp/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -551,11 +551,11 @@ const setAgentPausedOutputSchema = {
agentPaused: z.boolean().optional(),
};

// `action` mirrors the CLI's MAINTAIN_ACTION_CLASSES exactly (loopover-mcp.js:65). `level` intentionally
// validates against the LIVE AUTONOMY_LEVELS (src/settings/autonomy.ts), not the CLI's own stale
// MAINTAIN_AUTONOMY_LEVELS -- that list still carries "suggest"/"propose", both removed server-side by #4620
// and silently dropped by normalizeAutonomyPolicy on persist, so accepting them here would report success on a
// write that never actually took effect.
// `action` mirrors the CLI's MAINTAIN_ACTION_CLASSES exactly (loopover-mcp.js). `level` validates against the
// LIVE AUTONOMY_LEVELS (src/settings/autonomy.ts) rather than restating one: "suggest"/"propose" were removed
// server-side by #4620 and are silently dropped by normalizeAutonomyPolicy on persist, so accepting either here
// would report success on a write that never actually took effect. (#6153: the CLI's own MAINTAIN_AUTONOMY_LEVELS
// carried both until then -- binding to the live enum is what kept this surface correct while that one drifted.)
const MAINTAIN_AUTONOMY_ACTION_CLASSES = ["review", "request_changes", "approve", "merge", "close", "label"] as const;

const setActionAutonomyShape = {
Expand Down
40 changes: 39 additions & 1 deletion test/unit/mcp-cli-maintain.test.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,22 @@
import { mkdtempSync, rmSync } from "node:fs";
import { mkdtempSync, readFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import { join } from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { AUTONOMY_LEVELS } from "../../src/settings/autonomy";
import { closeFixtureServer, runAsync, startFixtureServer } from "./support/mcp-cli-harness";

// #6153: MAINTAIN_AUTONOMY_LEVELS is a hand-synced copy of the live enum (the CLI reaches @loopover/engine only
// through its published export map, which doesn't surface AUTONOMY_LEVELS), so nothing but a test can catch the
// two drifting apart. The source is parsed rather than imported because bin/loopover-mcp.js is an executable
// entrypoint that starts a server on import.
const CLI_SOURCE = readFileSync(join(process.cwd(), "packages/loopover-mcp/bin/loopover-mcp.js"), "utf8");

/** The `maintain set-level` levels the committed CLI source really accepts. */
function declaredLevels(): string[] {
const raw = /const MAINTAIN_AUTONOMY_LEVELS = \[([^\]]*)\];/.exec(CLI_SOURCE)?.[1] ?? "";
return [...raw.matchAll(/"([^"]+)"/g)].map((m) => m[1]!);
}

describe("loopover-mcp CLI — maintain (#784)", () => {
let tempDir: string | null = null;

Expand Down Expand Up @@ -92,6 +105,31 @@ describe("loopover-mcp CLI — maintain (#784)", () => {
await expect(runAsync(["maintain", "set-level", "merge", "bogus", "--repo", "owner/repo"], e)).rejects.toThrow(/Unknown level/);
}, 45_000);

// Pins the INVARIANT (the two lists agree), not today's three values -- restating the literal here would just
// create a third hand-synced copy that rots alongside the one this guards.
it("set-level's levels stay in sync with the live autonomy enum (#6153)", () => {
expect(declaredLevels()).toEqual([...AUTONOMY_LEVELS]);
});

// #6153 regression: the CLI accepted "suggest"/"propose" for the whole life of #4620, which dropped them
// server-side. The fixture's PUT /settings echoes any autonomy body back as a success, exactly like a server
// with no enum -- so a rejection here can only have come from the CLI's own check, before any round-trip.
it("rejects levels #4620 removed server-side, client-side rather than via a 400 (#6153)", async () => {
const e = await env();
for (const removed of ["suggest", "propose"]) {
// Derived from the live enum for the same reason as above: the point is that the error names exactly the
// levels the server accepts, not that it names three particular strings.
await expect(runAsync(["maintain", "set-level", "review", removed, "--repo", "owner/repo"], e)).rejects.toThrow(
new RegExp(`Unknown level: ${removed}\\. Use ${AUTONOMY_LEVELS.join(", ")}\\.`),
);
}
// The dial still accepts every level the server does -- the fix narrowed the list, it didn't break it.
const json = JSON.parse(await runAsync(["maintain", "set-level", "review", "observe", "--repo", "owner/repo", "--json"], e)) as {
autonomy: Record<string, string>;
};
expect(json.autonomy).toMatchObject({ review: "observe" });
}, 45_000);

it("prints help when invoked with no subcommand", async () => {
const e = await env();
const out = await runAsync(["maintain"], e);
Expand Down