From 69aec677c812329842189385af2d6c888b3f02c7 Mon Sep 17 00:00:00 2001
From: luvs01 <luvs01@hanmail.net>
Date: Sun, 30 Aug 2026 03:15:36 +0900
Subject: [PATCH] fix(cli): send the cleanup percent in the policy target the
 server reads

`ocx storage policy set --percent N` serialized a top-level `percent` field. The
PUT contract has no such field: `normalizeStorageCleanupPolicy` reads only
`target`, so the value was dropped and the previously stored target survived.

The request still answered 200 with a policy body, so the operator saw success
while the stored target was unchanged. On a policy holding the 25% default,
`--percent 10` left cleanup authorized to remove considerably more data than was
asked for.

Send `target: { removeOldestPercent: N }` instead. Out-of-range values are still
forwarded so the server answers with its named 400 rather than the CLI
duplicating the 1-100 vocabulary; a rejected write is the correct outcome, and
the silent accepted write is what this removes.
---
 src/cli/storage.ts                | 11 ++++++++++-
 tests/cli-storage-inspect.test.ts | 26 +++++++++++++++++++++++++-
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/src/cli/storage.ts b/src/cli/storage.ts
index d23c181050..ed13aa6710 100644
--- a/src/cli/storage.ts
+++ b/src/cli/storage.ts
@@ -173,7 +173,16 @@ async function policy(argv: string[], deps: RuntimeApiDeps): Promise<void> {
     }
     const body: Record<string, unknown> = {};
     if (enabled !== undefined) body.enabled = enabled === "true";
-    if (percent !== undefined) body.percent = percent;
+    // The policy target is nested. A top-level `percent` is not part of the PUT contract:
+    // `normalizeStorageCleanupPolicy` reads only `target`, so the field was dropped and the
+    // previously stored target survived. `--percent 10` on a policy still holding the
+    // default 25% therefore reported success while leaving cleanup authorized to delete
+    // more than the operator asked for.
+    //
+    // An out-of-range value is deliberately still sent: the server owns the 1-100
+    // vocabulary and answers with a named 400, which is a rejected write rather than the
+    // silent wrong write this replaces.
+    if (percent !== undefined) body.target = { removeOldestPercent: percent };
     if (mode !== undefined) body.mode = mode;
     if (schedule !== undefined) body.schedule = schedule;
     if (Object.keys(body).length === 0) {
diff --git a/tests/cli-storage-inspect.test.ts b/tests/cli-storage-inspect.test.ts
index 3a39627f01..cdf992e852 100644
--- a/tests/cli-storage-inspect.test.ts
+++ b/tests/cli-storage-inspect.test.ts
@@ -142,7 +142,31 @@ describe("ocx storage trash and policy", () => {
     expect(calls[0]?.method).toBe("PUT");
     // `enabled` is absent, which the server reads as "keep the stored value". Sending
     // `enabled: false` here would silently disable a policy the operator never mentioned.
-    expect(calls[0]?.body).toEqual({ percent: 40 });
+    // The percent travels inside `target`: the PUT contract has no top-level `percent`, so
+    // that shape was accepted, dropped, and left the stored target in place.
+    expect(calls[0]?.body).toEqual({ target: { removeOldestPercent: 40 } });
+  });
+
+  test("--percent reaches the server in the shape the policy target actually reads", async () => {
+    // A top-level `percent` round-trips as HTTP 200 while changing nothing:
+    // `normalizeStorageCleanupPolicy` reads only `target`, so a policy still holding the
+    // default 25% stayed at 25% after `--percent 10` reported success — cleanup remained
+    // authorized to delete more than the operator asked for.
+    const { calls, deps } = harness(() => ({ json: { ok: true, policy: {} } }));
+    const cap = capture();
+    try { await handleStorageCommand(["policy", "set", "--percent", "10"], deps); } finally { cap.restore(); }
+    const body = calls[0]?.body as Record<string, unknown>;
+    expect(body).toEqual({ target: { removeOldestPercent: 10 } });
+    expect(body).not.toHaveProperty("percent");
+  });
+
+  test("an out-of-range percent is still sent so the server can name the rejection", async () => {
+    // Rejecting locally would duplicate the server's 1-100 vocabulary. A named 400 is a
+    // refused write; the defect being fixed here was a silent accepted one.
+    const { calls, deps } = harness(() => ({ json: { ok: true, policy: {} } }));
+    const cap = capture();
+    try { await handleStorageCommand(["policy", "set", "--percent", "0"], deps); } finally { cap.restore(); }
+    expect(calls[0]?.body).toEqual({ target: { removeOldestPercent: 0 } });
   });
 
   test("policy set with no fields is refused rather than sent as an empty write", async () => {