diff --git a/devlog/_plan/261009_n5_small_sweep/000_plan.md b/devlog/_plan/261009_n5_small_sweep/000_plan.md new file mode 100644 index 00000000000..374540b08d8 --- /dev/null +++ b/devlog/_plan/261009_n5_small_sweep/000_plan.md @@ -0,0 +1,75 @@ +# 261009 N5 — small PR and issue sweep + +**Reader summary.** Sweep lane N5 drives three items to a terminal state: contributor PR #6734 (paste a +`codex://threads/` link into the Logs conversation filter), contributor PR #6813 (an explicit Codex +account selection can silently lose to a newer disk snapshot, so routing and auth use another account), and +issue #6757 (Aside profiles show `off (stale)` and `ocx integration client sync --client aside` gives no +actionable recovery). Each item ends READY, NEEDS_HUMAN, BLOCKED or SUPERSEDED with evidence. + +## Loop spec + +- **Loop archetype:** satisfy-spec, multi-cycle HOTL (cxc-loop), one work-phase per PABCD cycle. +- **Trigger:** coordinator chat 01a11fe0 dispatched lane N5 ("ㄱㄱ 하자 너 상속으로 레인 파견하고 sol 서브에이전트 + 무제한으로 쓰라고") with the N5 scope #6734, #6813, #6757. +- **Goal:** #6734 READY as-is or carried; #6813 carried onto current dev in a maintainer branch so repository + CI runs, security-reviewed; #6757 recovery guidance shipped with OFF preferences preserved and the cause + reported or its unknown documented. +- **Non-goals:** other lanes (N1–N4, L1–L7 PRs #6802–#6818 and #6806); `.github/workflows`; the user's live + Desktop sidecar (port 10100), `~/.claude`, `~/.aside`, PATH shims; merging, commenting on or closing other + authors' PRs and issues; approving fork workflows; release. +- **Verifier:** per work-phase focused Bun test files named in each decade doc, `bun run typecheck`, and + `bun run structure:check` / `bun run privacy:scan` where structure or docs change; exact-head hosted CI is + the broad gate. Full `bun run test` is left to CI (lane instruction: minimal local tests). +- **Stop condition:** every item terminal; one final lane report. Merge is outside authority. +- **Memory artifact:** this unit; goalplan `.codexclaw/goalplans/opencodex-sweep-lane-n5-small-prs-issues-forked` + in the coordinator cwd `/Users/jun/.codex/worktrees/7301/opencodex`. +- **Expected terminal outcomes:** DONE = all three items READY or terminal with evidence; NEEDS_HUMAN = a product + or security decision the evidence cannot settle; BLOCKED = CI infrastructure or permission; UNSAFE = a fix + that needs the user's live environment. +- **Escalation condition:** reviewer FAIL twice on the same packet; any need to write to another author's PR. +- **Resource bounds:** tools = local shell in lane worktrees, gh for this repository's own branches/PRs/CI, + gpt-6.1-sol subagents without a count limit; write scope = `opencodex-lanes/261009-N5-small*`; no token or + time budget was stated (host limits apply). + +## Work-phase map (dependency order) + +The three items are independent; order follows risk of drift (a contributor head that is already green +first, then the auth carry, then the new CLI behavior). + +| Goalplan id | Doc | Slice | PR | Depends on | +|---|---|---|---|---| +| wp0 | this unit | docs-first roadmap | none | — | +| wp1 | `010_pr6734_thread_links.md` | review and union check of #6734; READY or carry | original #6734 (or carry) | wp0 | +| wp2 | `020_pr6813_account_selection_carry.md` | carry #6813 onto dev, security review | new PR → dev | wp0 | +| wp3 | `030_issue6757_aside_recovery.md` | Aside off/stale recovery guidance + cause report | new PR → dev (carries this unit) | wp0 | +| wp4 | `040_closeout.md` | merge #6824 and the #6757 PR under the gate, close originals, final report | none | wp2, wp3 | + +## Evidence baseline (origin/dev 730d898457) + +- #6734 head `ca2391373f`: `git merge-tree --write-tree origin/dev pr/6734` exit 0, tree `9e44b77e61`; union + focused tests 96 + 28 pass, typecheck exit 0 (explorer receipt `/tmp/n5-6734-evidence/receipt.txt`). +- #6813 head `8441b1bc68` (base `f7d6c049`): merge-tree clean, tree `a86859108a`; #6811 merged as `c3bbaaa342` + and shares only the two layout registries and `structure/config.md`; union 460 focused tests pass. +- #6757: no automatic path flips an explicit true preference to false; candidates are schema fallback of a + malformed policy (`src/config/schema/config-schema.ts:239`) and loss of legacy-derived activation + (`src/integrations/aside-profile-context.ts:167-181`). + +## Source-of-truth sync + +`structure/` docs owned by touched areas: `structure/dashboard-and-usage.md` (#6734, already in the PR), +`structure/config.md` + `structure/providers/openai-accounts.md` (#6813, already in the PR), and the +integrations doc that owns `src/cli/integration-aside-sync.ts` for wp3 (resolved in 030 via +`structure/manifest.json`). + +## Authority amendment (coordinator steering, 2026-10-09) + +The coordinator relayed the user's instruction "머지까지해": this lane now merges its own PRs and closes the +originals. Per-PR gate immediately before merge: base dev; no failed/cancelled/pending exact-head job (including +triggered Windows jobs; a queued-only enforce-target over one hour with all code CI green is non-blocking and +reported); `mergeable == MERGEABLE`; independent gpt-6.1-sol review PASS (plus sol security review PASS for #6813); +fresh origin/dev with no conflict, and a union focused-test run when another PR touched the same files. Then +`gh pr merge --squash --admin` (carry bodies keep `Co-authored-by`), close carried/replaced originals with a +credit comment (merged PR, merge commit, author), and close fixed issues with a dev note. Partial items stay open +with a comment. Still out of scope: release, main/preview, other lanes' PRs, fork workflow approval, the user's +live environment. +Gate addition (audit wp2): no outstanding maintainer change request on the PR at merge time (MAINTAINERS.md). diff --git a/devlog/_plan/261009_n5_small_sweep/010_pr6734_thread_links.md b/devlog/_plan/261009_n5_small_sweep/010_pr6734_thread_links.md new file mode 100644 index 00000000000..8b3373737e8 --- /dev/null +++ b/devlog/_plan/261009_n5_small_sweep/010_pr6734_thread_links.md @@ -0,0 +1,54 @@ +# 010 — wp1: PR #6734, Codex thread links in the conversation filter + +**Reader summary.** #6734 (luvs01) lets the Logs conversation filter, `GET /api/logs?conversationId=` and +`ocx logs filter --conversation` accept a pasted `codex://threads/` link. Its CI ran green on an older dev +base; this phase proves it still holds on current dev and gets an independent review. No code is written +unless the review finds a defect. + +## Facts (verified against origin/dev 730d898457) + +- Head `ca2391373f10795bf9270ba388344bec2ec2301f`, MERGEABLE, all triggered checks green on that head + (Cross-platform CI run 37725885408; enforce-target/hygiene reruns 37846215811/37846185830). +- Union: `git merge-tree --write-tree origin/dev pr/6734` exit 0, no conflicts, tree `9e44b77e61`. +- Ten touched files; only `structure/dashboard-and-usage.md` changed on dev since the PR base (lines 336, 500); + the union reaches the 600-line structure budget exactly (`structure/manifest.json:3`, enforced by + `scripts/structure-ssot.ts:350-357`). No file-size baseline caps apply; no test-layout registration changes. + +## Plan + +1. Independent gpt-6.1-sol review (no builder context; the lane did not author the change) of the PR diff + against current dev: bounded unwrap (512 chars, `src/server/request-log-conversation.ts:40-66`), digest + bound (4096, lines 20-37), case handling (57, 61, 92-94), dual candidates (75-78), GUI mirror + `gui/src/log-conversation-id.ts`, privacy (no raw ID logging, no persistence change), docs/structure + contract wording. +2. Union verification (already run by the explorer; re-run by the reviewer only if it doubts it): + `bun test tests/usage/request-log-conversation.test.ts tests/cli/cli-log-view-filter.test.ts` (exit 0, 96 pass; + reads the target: both files are named directly), `(cd gui && bun test tests/logs-filter.test.ts + tests/logs-filter-bar.test.ts)` (exit 0, 28 pass), `bun run typecheck` (exit 0; covers `src/` only, GUI + compile is covered by hosted `gates`), `bun run structure:check` on the union tree for the 600-line budget. +3. Decision rule: review PASS + union checks pass → original PR READY (no carry; lane cannot rerun another + author's CI, and the exact head is unchanged). Review FAIL with a fix → carry to + `codex/n5-6734-thread-links` (cherry-pick the six commits, keep authors, add the fix, `Co-authored-by: luvs01`), + open a dev PR. Structure budget failure on the union → carry with a one-line structure trim. + +## Acceptance + +- Reviewer verdict PASS (or FAIL folded via carry and re-reviewed PASS). +- Union structure:check exit 0 (activation: the 600-line budget is exactly hit; a 601st line would fail). +- Final report lists #6734 as READY (original) or the carry PR, and nothing to close until merge. + +## Cycle revision (wp1 P, origin/dev bb36029ef9) + +The facts above were observed at dev `730d898457` and are historical. dev moved one commit (#6820, Codex home +restore; it touches none of the ten files). This cycle replaces "already run" with fresh checks: + +- **W1 union.** In the lane worktree, create the unpublished branch `n5-union-6734` from origin/dev `bb36029ef9` and + `git merge --no-ff refs/remotes/pr/6734` (head `ca2391373f`). Record parents, union commit and tree in 011. A conflict + switches to the carry path. The branch is never pushed. +- **W2 review.** The independent gpt-6.1-sol review (reviewer 01a1205e, no builder context) is bound to the PR head and + current dev; its verdict, findings and limitations go to `011_pr6734_review.md` on `codex/n5-small`. +- **W3 checks.** Session source is the lane worktree; C runs `cxc receipt test` with the union branch checked out: + the two root test files, the two GUI test files, `bun run typecheck`, `bun run structure:check`. 011 is committed on + `codex/n5-small` after switching back, so the doc commit is never confused with the tested tree. +- **W4 READY.** Original READY additionally requires a fresh read of the PR's exact-head checks, review threads and + mergeability at report time; an unchanged head alone does not refresh that evidence. diff --git a/devlog/_plan/261009_n5_small_sweep/011_pr6734_review.md b/devlog/_plan/261009_n5_small_sweep/011_pr6734_review.md new file mode 100644 index 00000000000..459ac54c64c --- /dev/null +++ b/devlog/_plan/261009_n5_small_sweep/011_pr6734_review.md @@ -0,0 +1,23 @@ +# 011 — wp1 record: PR #6734 merged + +**Reader summary.** #6734 (luvs01) merged into dev unchanged as `37e9294125` on 2026-10-09 (squash, admin). Its head +`ca2391373f` merged cleanly onto dev `bb36029ef9`, fresh checks on that union passed, and an independent review +passed with one minor test-coverage note. + +## Evidence + +- Union: local unpublished branch `n5-union-6734`, commit `a3714f0ebe` (parents `bb36029ef9`, `ca2391373f`), tree + `436eff12c5`, identical to the auditor's `git merge-tree` result. +- Receipt checks on the union (temporary HOME): `bun test tests/usage/request-log-conversation.test.ts + tests/cli/cli-log-view-filter.test.ts` 96 pass / 0 fail; `(cd gui && bun test tests/logs-filter.test.ts + tests/logs-filter-bar.test.ts)` 28 pass / 0 fail; `bun run typecheck` clean; `bun run structure:check` passed + (`structure/dashboard-and-usage.md` exactly at its 600-line budget). +- Gate at merge time: base dev; exact-head checks 23 pass / 8 skipped / 0 failed or pending; MERGEABLE; no review + threads; dev unchanged since the union. +- Independent review (gpt-6.1-sol 01a1205e, no builder context): PASS. Minor: the oversized-input assertions + (`tests/usage/request-log-conversation.test.ts:102`, `gui/tests/logs-filter.test.ts:225`) would not catch a raised + 512-character unwrap limit, and mixed-case IDs are untested. Not merge-blocking; left as a follow-up. + +## Outcome + +READY → merged. Nothing to close: the PR links no issue. diff --git a/devlog/_plan/261009_n5_small_sweep/020_pr6813_account_selection_carry.md b/devlog/_plan/261009_n5_small_sweep/020_pr6813_account_selection_carry.md new file mode 100644 index 00000000000..0926d4a2484 --- /dev/null +++ b/devlog/_plan/261009_n5_small_sweep/020_pr6813_account_selection_carry.md @@ -0,0 +1,86 @@ +# 020 — wp2: carry PR #6813, atomic explicit account selection + +**Reader summary.** Selecting a Codex account (or pin) through the management API can report success while a +later whole-config reconcile adopts a newer disk value, so the next request routes and authenticates as a +different account. #6813 (hulkbig, fork PR) commits the selection through the scoped persisted-config +transaction first and only then adopts it and resets routing. Fork CI needs approval the lane does not +have, so the lane carries it onto current dev in a maintainer branch where repository CI runs. + +## Facts (origin/dev 730d898457) + +- Before: `src/codex/auth-api/routes.ts:269-280` mutates live selection, resets routing, then + `saveRuntimeConfig` → whole reconcile (`src/codex/auth-api/runtime-config.ts:24-25`); + `src/config/live-reconcile.ts:325-341` adopts disk when live equals baseline. +- PR head `8441b1bc68`: new `src/codex/auth-api/account-selection.ts` (107 lines) using + `mutatePersistedConfig` (`src/config/persisted-mutation.ts:37-90`, SQLite `BEGIN IMMEDIATE` lock + `src/config/mutation-lock.ts:104-152`); `routes.ts` -49/+10; `live-reconcile.ts` +17 (selection-only + advance); `rebase-provenance.ts` +11; docs in five locales; structure `config.md`, + `providers/openai-accounts.md`; new test `tests/codex-integration/codex-account-selection-atomicity.test.ts` + registered in both layout files. +- Merge onto dev is clean (tree `a86859108a`); #6811 (merged `c3bbaaa342`) overlaps only on the layout + registries and `structure/config.md` and does not change the transaction API; #6792 has no runtime overlap. + +## Change map + +| Path | Action | Note | +|---|---|---| +| branch `codex/n5-6813-account-selection` from origin/dev | NEW | worktree `opencodex-lanes/261009-N5-small-6813` | +| commits `91e24b0c30`, `8441b1bc68` | CHERRY-PICK | authorship (hulkbig) preserved; no content change expected | +| any conflict in layout registries / `structure/config.md` | MODIFY | keep both sides' entries | +| security-review folds (if any) | MODIFY | separate maintainer commit with `Co-authored-by: hulkbig` | + +## Verification + +- `bun test tests/codex-integration/codex-account-selection-atomicity.test.ts tests/codex-integration/codex-auth-api.test.ts tests/config/config-user-edits.test.ts tests/codex-integration/codex-pool-rotation.test.ts` + (targets named directly). +- `bun test tests/test-layout.test.ts tests/test-layout-tooling.test.ts` (registration). +- `bun run typecheck`; `bun run structure:check` (structure docs changed); `bun run privacy:scan`. +- Activation scenarios to confirm in tests: same-value re-select of main against a newer disk snapshot + (the original red case); persistence failure leaves routing untouched; post-publication exception + reconciles to durable state. +- Independent gpt-6.1-sol security review (auth selection, races, credential exposure, error projection), + then a general correctness review; exact-head hosted CI on the carry PR. + +## Terminal + +READY = carry PR exact-head CI green, both reviews PASS, MERGEABLE. After merge the coordinator closes #6813 +with credit. Residual risks reported: recovery accepts any valid matching snapshot (observed-state, not +writer attribution); non-cooperating external writers after the final freshness check. + +## Cycle revision (wp2 P, origin/dev 37e9294125) + +dev moved to `37e9294125` (#6820 and the #6734 merge). #6820 also edits both layout registries that #6813 touches; the +union resolves that cleanly and the layout guards stay in C3. `git merge-tree --write-tree +origin/dev refs/remotes/pr/6813` is clean (tree `3d755f0d31`). #6811 is merged (`c3bbaaa342`) and orthogonal. + +- **C1 branch.** In the lane worktree: `git switch -c codex/n5-6813-account-selection origin/dev`, then + `git cherry-pick 91e24b0c30 8441b1bc68`. Both commits keep author Hulk . No content edits unless a + review finding requires one, in a separate maintainer commit carrying `Co-authored-by: hulkbig `. +- **C2 reviews.** Independent gpt-6.1-sol security reviewer (01a12063-915d) and correctness reviewer (01a12063-926c), + neither given builder context, review #6813's head against current dev in their own /tmp worktrees. +- **C3 checks (receipt, temporary HOME, carry branch checked out).** `bun test + tests/codex-integration/codex-account-selection-atomicity.test.ts tests/codex-integration/codex-auth-api.test.ts + tests/config/config-user-edits.test.ts tests/codex-integration/codex-pool-rotation.test.ts`, `bun test + tests/test-layout.test.ts tests/test-layout-tooling.test.ts`, `bun run typecheck`, `bun run structure:check`, + `bun run privacy:scan`. +- **C4 publish and merge.** Push the branch, open a dev PR with the full template (Summary, Verification, Checklist), + `Carries #6813`, and `Co-authored-by: hulkbig ` in the body. After the exact-head CI and both + reviews pass and the merge gate holds, `gh pr merge --squash --admin --body` with the trailer, then close #6813 with a + credit comment (merged PR, merge commit, author). D closes on the carry branch before any record commit. + +### Architect K1–K4 folds + +- **K1/K2 tree binding.** Reviews start on the fork head; they count for the carry only if the carry branch tree equals + `git merge-tree --write-tree origin/dev refs/remotes/pr/6813` for the dev SHA the branch was cut from (recorded in the + PR). Any fold commit invalidates that equivalence and requires both reviewers to re-review the fold diff. +- **K3 receipt binding.** Receipts run with the carry branch's final SHA checked out (cxc binds commit + clean state); any + later edit requires a fresh receipt. Minimal local scope is stated in the PR Verification section; the full suite and + Windows/macOS shards are left to hosted CI. +- **K4 merge.** `gh pr merge --squash --admin --match-head-commit --body-file ` where the file ends with + `Co-authored-by: hulkbig `. The PR description carries the maintainer-integration record + (MAINTAINERS.md `dev` exception) with exact-head verification. The dev ruleset (20763889) defines no required status + checks; the gate requires every triggered check to pass, and the queued-enforce-target exemption applies only because + no check is ruleset-required. Close #6813 only after the merge is confirmed. +- **Audit fold.** The merge gate also requires a fresh review-state read: no outstanding maintainer change request + (resolved or withdrawn), recorded with the exact-head CI evidence. The queued enforce-target exemption keeps all three + conditions (over one hour, all code CI green, reported). Each reviewer's examined union tree is recorded. diff --git a/devlog/_plan/261009_n5_small_sweep/030_issue6757_aside_recovery.md b/devlog/_plan/261009_n5_small_sweep/030_issue6757_aside_recovery.md new file mode 100644 index 00000000000..0e7b8cee9f3 --- /dev/null +++ b/devlog/_plan/261009_n5_small_sweep/030_issue6757_aside_recovery.md @@ -0,0 +1,106 @@ +# 030 — wp3: issue #6757, Aside profiles that read "off (stale)" + +**Reader summary.** `ocx integration client status --client aside` can show every profile as `off (stale)`, +and `ocx integration client sync --client aside` then prints "No eligible Aside profiles to synchronize. Check +integration status and profile sync preferences." with no next step. Sync only refreshes profiles whose sync +preference is on, which is correct and stays. This phase makes sync print the recovery procedure (the status +command, then preview and enable with a `` placeholder), makes status print the exact preview and enable +commands with the real ID for each profile that is off with a stale managed block, and warns at load time +when a malformed `asideProfileSync` block silently turns every profile off. Nothing is re-enabled +automatically. + +## Cause investigation (origin/dev 730d898457) + +- `off (stale)` is two independent values: preference (`enabled`) and block state. `src/cli/integrations.ts:255`. +- Preference: explicit override → `allProfiles` → legacy root-ownership match (`src/integrations/aside-profile-context.ts:181,210-214`). +- Stale: safe sibling edit, missing desired fragment, or changed desired fingerprint (`src/integrations/state.ts:430-470`). Stale + never writes the preference. +- Paths that produce `enabled=false`: explicit disable (`aside-profile-context.ts:276-280`), Undo/restore of a + snapshot that was not owned (`aside-profile-journal.ts:223`, `136-144`), **schema fallback of an invalid policy + to `{ allProfiles: false }`** (`src/config/schema/config-schema.ts:239`; one non-boolean value is enough, + `leaf-validators.ts:813-819`), loss of the legacy-derived default when no policy exists and the root ownership + record no longer matches (`aside-profile-context.ts:167-181`), or a different/replaced `OPENCODEX_HOME`. +- No automatic stale/fingerprint/attestation/restart path writes `false` over an explicit `true` + (`ocx sync` → `src/cli/dispatch.ts:455-483`; attestation failures throw, `src/cli/aside-profiles.ts:29-45`). +- Conclusion: the reporter's runtime evidence (persisted `asideProfileSync`, root ownership record) is needed to + pick between schema fallback and lost legacy default. The fallback is silent today; this phase makes it loud. + +## Decisions (architect proposal D1–D8, handle 01a12051; revised after A round 1) + +A round 1 (auditor 01a12056) FAILED the first revision: the server projection called `listAsideProfileStates`, +whose `readIntegrationState` runs `retryPendingPrunesOnce` (`src/integrations/state.ts:581-583`) and, through +the guarded Aside store, can prune snapshots and clear maintenance markers +(`src/integrations/aside-profile-context.ts:117-120`). The empty sync path performs no writes today; adding that +lookup would add maintenance writes and widen a management response. Revision 2 drops the server change. + +| ID | Disposition (revision 2) | +|---|---| +| D1 keep preference/state/cause separate; OFF authoritative | accepted | +| D2 projection on the empty sync response | **rejected** (A round 1 blocker 1: not effect-free; the status GET already reports the same rows) | +| D3 helper callback | **rejected** with D2; `src/cli/aside-profiles.ts` and the attested exchange are untouched | +| D4 one CLI owner for recovery text | accepted as `src/cli/aside-profile-recovery.ts` (render only; no wire parsing needed) | +| D5 human text with conditional enable wording; JSON unchanged | accepted: sync prints the generic recovery procedure with the exact status/preview/enable command forms; status (list and `--profile N`) prints per-profile commands with real IDs | +| D6 no follow-up GET or capability expansion | accepted (trivially: no new request at all) | +| D7 load-time warning for an invalid `asideProfileSync` | accepted (factual wording, value-free) | +| D8 verification matrix | accepted, reduced to the CLI and config surfaces below | + +## Change map + +| Path | Action | Change | +|---|---|---| +| `src/cli/aside-profile-recovery.ts` | NEW | `ASIDE_SYNC_EMPTY_LINES: readonly string[]` = "No eligible Aside profiles to synchronize. Sync refreshes only profiles whose sync preference is on." / "List profiles with: ocx integration client status --client aside" / "For a profile shown as off (stale), review: ocx integration client preview --client aside --operation apply --profile " / "Then, if the preview permits the change and you accept it: ocx integration client enable --client aside --profile ". `asideProfileRecoveryLines(rows: unknown[]): string[]` selects rows with `enabled === false && state === "stale"` and a safe nonnegative integer `profileId` (not `-0`, deduplicated), and renders per profile the off (stale) line, the preview command, the conditional "Then, if the preview permits the change and you accept it:" line and the enable command. conflict/unsafe/absent/current rows and enabled rows get nothing. | +| `src/cli/integration-aside-sync.ts:59` | MODIFY | empty `results` → `[...ASIDE_SYNC_EMPTY_LINES]`. JSON stays `{ results }`; exit code stays 0. | +| `src/cli/integrations.ts:253-256` | MODIFY | Aside list output appends `asideProfileRecoveryLines(profiles)` after the rows. | +| `src/cli/integrations.ts:195` `singleClientStatusLines` | MODIFY | when the result is an Aside profile object (`clientId === "aside"` with numeric `profileId`), append `asideProfileRecoveryLines([result])`. JSON unchanged. | +| `src/config/load-degrade.ts` `warnDegradedTopLevelOptIns` | MODIFY | if raw `asideProfileSync` is present and `asideProfileSyncSchema.safeParse` fails, `console.warn` once: "⚠️ config.json asideProfileSync is invalid — Aside profile sync falls back to all profiles off; fix the block or inspect `ocx integration client status --client aside`". No value or key echoed. `config.ts` is not touched. | +| `tests/cli/cli-aside-sync.test.ts` | MODIFY | empty-set test asserts the four lines, `` placeholders, no enable request, `--json` still `{ results: [] }`. | +| `tests/cli/cli-headless-parity.test.ts` (describe "Aside profile integration CLI", ~L1553-1601) | MODIFY | list status with rows [0 off/stale, 1 on/stale, 2 off/conflict, 3 off/current, 4 off/stale] → commands for 0 and 4 only, conditional wording present, no extra request; `--profile 4` off/stale → hint; `--json` output equals the response. | +| `tests/config/config-load-degrade.test.ts` | MODIFY | `asideProfileSync: { profiles: { "0": "secret-shaped-off" } }` → one warning containing "asideProfileSync is invalid", no "secret-shaped-off", loaded policy `{ allProfiles: false }`, bytes unchanged; valid and absent policy → no such warning. | +| `structure/cli-management.md:87` | MODIFY | one sentence: an empty sync prints the status/preview/enable recovery procedure; status names per-profile commands for off (stale) rows; neither enables anything. | +| `structure/clients/integrations.md:147` | REVIEWED, unchanged | its sentence "uses the established attested sync helper unchanged" stays true. | +| `structure/config.md` | MODIFY | one sentence where `load-degrade.ts` warnings are described (L123 area): an invalid `asideProfileSync` keeps the schema fallback `{ allProfiles: false }` and now warns once without echoing values. | +| `docs-site/src/content/docs/guides/integrations.md:524-532` | MODIFY | after "`{results:[]}` exits 0 and means no eligible profiles": one sentence on reading status and using preview then enable for an off (stale) profile; translated locales are left unchanged (they do not contradict). | + +No new test files, so no layout registration. `src/cli/aside-profile-recovery.ts` lies in the already owned `src/cli` +area (`structure/manifest.json`). + +## Verification + +Commands (each names its target file directly): + +- `bun test tests/cli/cli-aside-sync.test.ts` +- `bun test tests/cli/cli-headless-parity.test.ts -t "Aside"` +- `bun test tests/config/config-load-degrade.test.ts` +- `bun run typecheck` (covers `src/`), `bun run structure:check` (structure docs changed), `bun run privacy:scan` +- `bun test tests/ci-workflows/file-size-ratchet.test.ts` if present (file growth in `src/cli/integrations.ts`, `load-degrade.ts`) + +Activation scenarios: (a) empty sync → four guidance lines, no enable/preview request issued, JSON unchanged; (b) status +list mixed rows → hints only for off+stale, conflict/unsafe/current/enabled none; (c) `--profile N` off+stale → hint; +(d) malformed profileId values (-1, -0, 1.5, duplicate) → skipped; (e) invalid `asideProfileSync` → one value-free +warning, fallback policy unchanged, file bytes unchanged; valid/absent → none. + +Reviews: independent gpt-6.1-sol correctness review, plus a security pass on the boundary question (no management +response, auth or transport change; the warning echoes no config value), then exact-head hosted CI. + +## Bypass record (PLAN-BYPASS-NAMED-01) + +No enforcement is added; the change is guidance and a warning. Residual: sync cannot name real profile IDs without a +read that may perform maintenance, so it names the status command instead. + +## Architect reflection (handle 01a12051) + +Revision 1: D4/D5/D7/D8 gaps folded, re-check ALIGNED. Revision 2 (after A round 1) removes D2/D3; reflection +re-requested below. + +Revision 2 reflection: D1/D4/D5/D6/D7/D8 ALIGNED; reader-summary gap (placeholders in sync vs real IDs in status) +folded. D2/D3 superseded; the architect confirmed its earlier effect-free claim was wrong. + +## Cycle revision (wp3 P, origin/dev 37e9294125) + +Revalidated: none of the wp3 files changed on dev since `730d898457`. `structure/config.md` is at exactly 600 lines, +so its sentence joins the existing `load-degrade.ts` paragraph (line 123) instead of adding a line. +`structure/cli-management.md` (115 lines) takes its sentence in the line-87 paragraph. Branch: `codex/n5-6757-aside-recovery` +from origin/dev with the plan-unit commits cherry-picked first (the unit lands with this PR); D closes on that branch +before any record commit. Merge and issue closure move to wp4 (`040_closeout.md`). +wp3 ends after pushing the branch, opening a template-complete dev PR with `Closes #6757` (closed manually after the +merge, because GitHub does not auto-close from dev), and recording receipt + review verdicts against that head. diff --git a/devlog/_plan/261009_n5_small_sweep/040_closeout.md b/devlog/_plan/261009_n5_small_sweep/040_closeout.md new file mode 100644 index 00000000000..84bb63900a5 --- /dev/null +++ b/devlog/_plan/261009_n5_small_sweep/040_closeout.md @@ -0,0 +1,40 @@ +# 040 — wp4: merge and closeout + +**Reader summary.** Added at wp3 P (LOOP-UNIT-CHAIN-01) after the coordinator granted merge authority. It merges the +lane's two published PRs under the gate in `000_plan.md`, closes the originals they replace or fix with credit, and +produces the final lane report. #6734 already merged in wp1 (`37e9294125`). + +## Steps + +1. **#6824 (carry of #6813).** Refresh: `gh pr view 6824 --json baseRefName,headRefOid,mergeable,reviewDecision`, + `gh pr checks 6824` (every triggered job pass or skipped; none failed, cancelled or pending; queued-only + enforce-target over one hour with all code CI green is reported as non-blocking), review threads and change + requests (none outstanding), fresh `git fetch origin dev` + `git merge-tree --write-tree origin/dev ` clean. + If dev changed any file the PR touches since `37e9294125`, rerun the wp2 focused tests on a union in a lane-owned + worktree first. Post the maintainer-integration record and both review verdicts as a PR comment. Merge with + `gh pr merge 6824 --squash --admin --match-head-commit --body-file ` whose body ends with + `Co-authored-by: hulkbig `. Confirm `state == MERGED` and the trailer in the merge commit, then + close #6813 with a thank-you comment naming #6824, the merge commit and the credit. +2. **#6757 PR.** Same gate; standard review PASS plus the boundary security pass named in 030. Merge, confirm, then + comment on #6757 with the PR and merge commit and close it. The comment states the residual: the root cause of the + preference loss is unproven; the new load warning covers the schema-fallback path. +3. **Flaky handling.** A failure that looks like a timeout gets one rerun of the failed jobs. Conflicts or real failures + go back to a fix commit on the lane branch, then fresh CI and, if code changed, re-review. +4. **Report.** Record merged PRs, merge commits, closed items, NEEDS_HUMAN items and worktree paths in + `041_closeout_record.md` on `codex/n5-small`, and deliver one final lane report. + +## Verification + +Evidence is GitHub state read at the time of each action: `gh pr view --json state,mergeCommit`, `gh issue view +--json state`, and `git log -1 --format=%B ` for the trailer. No code changes in this phase; if a fix +becomes necessary, it follows the owning phase's tests. + +## Architect Z1–Z4 folds + +- Check classes: the dev ruleset (20763889) defines no required status checks, so "skipped" means a job whose path or + trigger condition did not apply on that head; any other non-success fails the gate. The queued enforce-target + exemption applies only under its three stated conditions. +- Union reruns use the **owning unit's** commands: 020 C3 for #6824, 030 Verification for the #6757 PR, each bound to + that PR's current head and fresh dev. +- wp3 ends with its PR published and its final-head evidence recorded (receipt, reviews, PR link) in the D attest; + wp4 starts from those published heads. diff --git a/docs-site/src/content/docs/guides/integrations.md b/docs-site/src/content/docs/guides/integrations.md index c92d3b5d316..d0c46601aad 100644 --- a/docs-site/src/content/docs/guides/integrations.md +++ b/docs-site/src/content/docs/guides/integrations.md @@ -532,6 +532,14 @@ per-profile refusal, residual and separately labeled redacted backup information inspect the affected profile before retrying. This refresh is separate from each profile's preview/bound mutation workflow. +Sync refreshes only profiles whose sync preference is on, so an empty result is not a failure. +If `ocx integration client status --client aside` shows a profile as `off (stale)`, status +prints the commands to reconnect it: review `ocx integration client preview --client aside +--operation apply --profile N`, and run `ocx integration client enable --client aside --profile N` +only if the preview permits the change and you accept it. Nothing is re-enabled automatically. +A malformed `asideProfileSync` block in `config.json` turns every profile off and logs a +warning when the config loads. + Each profile has separate ownership and history. Existing user edits, unsafe paths and linked catalogs are refused; the existing explicit overwrite and drift-confirmation controls remain available. Fully quit and reopen Aside to load changed model files. diff --git a/src/cli/aside-profile-recovery.ts b/src/cli/aside-profile-recovery.ts new file mode 100644 index 00000000000..d3b81ded0a5 --- /dev/null +++ b/src/cli/aside-profile-recovery.ts @@ -0,0 +1,33 @@ +/** + * Recovery guidance for Aside profiles whose sync preference is off while their managed block is stale (#6757). + * + * Sync refreshes only profiles whose preference is on, and an OFF preference is the operator's decision, so an + * "off (stale)" profile needs an explicit, reviewed enable. This module renders text only: it issues no request, + * reads no state and never changes a preference. Sync cannot name real profile IDs without a status read that may + * run integration maintenance, so its empty result points at status, which already holds the rows. + */ +export const ASIDE_SYNC_EMPTY_LINES: readonly string[] = [ + "No eligible Aside profiles to synchronize. Sync refreshes only profiles whose sync preference is on.", + "List profiles with: ocx integration client status --client aside", + "For a profile shown as off (stale), review: ocx integration client preview --client aside --operation apply --profile ", + "Then, if the preview permits the change and you accept it: ocx integration client enable --client aside --profile ", +]; + +function recoveryProfileId(row: unknown): number | null { + if (!row || typeof row !== "object") return null; + const { profileId, enabled, state } = row as Record; + // Only an explicit OFF beside a stale block qualifies. Conflict and unsafe blocks need inspection, not an enable. + if (enabled !== false || state !== "stale") return null; + return typeof profileId === "number" && Number.isSafeInteger(profileId) && profileId >= 0 && !Object.is(profileId, -0) + ? profileId : null; +} + +/** Per-profile preview-then-enable commands for status rows that are off with a stale managed block. */ +export function asideProfileRecoveryLines(rows: readonly unknown[]): string[] { + const ids = [...new Set(rows.map(recoveryProfileId).filter((id): id is number => id !== null))]; + return ids.flatMap(id => [ + `Aside profile ${id} is off (stale). To reconnect it, review: ocx integration client preview --client aside --operation apply --profile ${id}`, + `Then, if the preview permits the change and you accept it: ocx integration client enable --client aside --profile ${id}`, + ]); +} + diff --git a/src/cli/integration-aside-sync.ts b/src/cli/integration-aside-sync.ts index e5eb270657e..fe2b6a303be 100644 --- a/src/cli/integration-aside-sync.ts +++ b/src/cli/integration-aside-sync.ts @@ -1,6 +1,7 @@ import { z } from "zod"; import { redactUserPath } from "../lib/redact"; import type { RefusalReason } from "../integrations/mutation-plan"; +import { ASIDE_SYNC_EMPTY_LINES } from "./aside-profile-recovery"; import { refreshAsideProfilesThroughServer } from "./aside-profiles"; import { runCatalogAction } from "./catalog-command-result"; import { CliUsageError, RuntimeApiError, printData, takeFlag, takeOption, type RuntimeApiDeps } from "./runtime-api"; @@ -56,7 +57,7 @@ export function handleIntegrationAsideSync(argv: string[], deps: AsideSyncCliDep const parsed = resultsSchema.safeParse(outcomes); if (!parsed.success) throw new Error("Invalid Aside synchronization outcome"); const results = parsed.data; - const lines = results.length === 0 ? ["No eligible Aside profiles to synchronize. Check integration status and profile sync preferences."] + const lines = results.length === 0 ? [...ASIDE_SYNC_EMPTY_LINES] : results.flatMap(row => [ `Aside profile ${row.profileId}: ${row.ok ? row.changed ? "updated" : "unchanged" : "failed"}.`, ...(row.reason ? [row.reason] : []), diff --git a/src/cli/integrations.ts b/src/cli/integrations.ts index 016e50003ac..c553068e0f0 100644 --- a/src/cli/integrations.ts +++ b/src/cli/integrations.ts @@ -13,6 +13,7 @@ import { type RuntimeApiDeps, } from "./runtime-api"; import { clientIntegrationPath, validateAsideProfile } from "./integration-input"; +import { asideProfileRecoveryLines } from "./aside-profile-recovery"; import type { IntegrationClientId } from "../integrations/registry"; const CLAUDE_USAGE = `Usage: @@ -194,7 +195,10 @@ function raycastBlock(result: unknown): RaycastStatusBlock | null { */ function singleClientStatusLines(result: unknown): string[] { const raycast = raycastBlock(result); - if (!raycast) return summaryLines(result); + if (!raycast) { + const aside = (result as { clientId?: unknown } | null)?.clientId === "aside"; + return [...summaryLines(result), ...(aside ? asideProfileRecoveryLines([result]) : [])]; + } const rest = Object.fromEntries(Object.entries(result as Record).filter(([key]) => key !== "raycast")); const lines = [...summaryLines(rest), `plan: ${raycast.plan}`]; if (!raycast.aiDirPresent) { @@ -252,7 +256,10 @@ export async function handleClientIntegrationCommand( const profiles = (result as { profiles?: Array> }).profiles; printData(result, wantsJson, profiles ? profiles.length > 0 - ? profiles.map(row => `${String(row.profileId)} ${String(row.name ?? "Aside")}: ${row.enabled ? "on" : "off"} (${String(row.state)})${row.current ? " [current]" : ""}`) + ? [ + ...profiles.map(row => `${String(row.profileId)} ${String(row.name ?? "Aside")}: ${row.enabled ? "on" : "off"} (${String(row.state)})${row.current ? " [current]" : ""}`), + ...asideProfileRecoveryLines(profiles), + ] : [String((result as { error?: string }).error ?? "No Aside profiles found.")] : rows /* diff --git a/src/config/load-degrade.ts b/src/config/load-degrade.ts index 3cd5dcdb577..1b1c16ebe05 100644 --- a/src/config/load-degrade.ts +++ b/src/config/load-degrade.ts @@ -26,6 +26,7 @@ import { refreshConfigDerivedRegistries } from "./derived-registries"; import { type OcxClaudeCodeConfig, type OcxConfig } from "../types"; import { agentTaskRecoverySchema, + asideProfileSyncSchema, catalogAutoRefreshSchema, clientConnectionSchema, isUsableApiKeySecret, @@ -131,6 +132,11 @@ export function warnDegradedTopLevelOptIns(rawParsed: unknown, validated: OcxCon warnDegradedStreamMode(rawParsed, validated); warnDegradedCompactionRouting(rawParsed, validated); warnDegradedMemoryModels(rawParsed, validated); + // The schema replaces a malformed policy with { allProfiles: false }, which silently turns every Aside profile off (#6757). + const asidePolicy = rawParsed && typeof rawParsed === "object" ? (rawParsed as Record).asideProfileSync : undefined; + if (asidePolicy !== undefined && !asideProfileSyncSchema.safeParse(asidePolicy).success) { + console.warn("⚠️ config.json asideProfileSync is invalid — Aside profile sync falls back to all profiles off; fix the block or inspect `ocx integration client status --client aside`"); + } } /** diff --git a/structure/cli-management.md b/structure/cli-management.md index f00ba15f814..bac73785776 100644 --- a/structure/cli-management.md +++ b/structure/cli-management.md @@ -84,7 +84,7 @@ Management route declarations describe actual HTTP calls only; local Lab automat `src/cli/integration-input.ts` shares pure profile paths/validation and owns exact optional Droid-map/fingerprint grammar. `src/cli/integration-preview.ts` handles explicit preview and new-option writes; `src/cli/integration-plan-dto.ts` validates value-free plans without runtime imports from GUI/planner/writer code. The original direct mutation bodies remain when new options are absent. Refused and no-op previews are completed observations; stale commits return a re-preview instruction without adopting a replacement token. The server owns coordinated binding and writes. -`src/cli/integration-journal.ts` addresses exact global/Aside/profile history with an explicit confirmation. It reports retired records separately from incomplete snapshot cleanup. `src/cli/integration-aside-sync.ts` retains the existing attested helper and its original dependency object rather than selecting a different transport through synthesized baseUrl. Empty, malformed and partial outcomes remain distinct. New fixed runtime requests reject redirects; this is not a global legacy transport rewrite. +`src/cli/integration-journal.ts` addresses exact global/Aside/profile history with an explicit confirmation. It reports retired records separately from incomplete snapshot cleanup. `src/cli/integration-aside-sync.ts` retains the existing attested helper and its original dependency object rather than selecting a different transport through synthesized baseUrl. Empty, malformed and partial outcomes remain distinct. New fixed runtime requests reject redirects; this is not a global legacy transport rewrite. An empty sync prints guidance instead of a bare no-op: `src/cli/aside-profile-recovery.ts` names the status command and the preview-then-enable commands, and `ocx integration client status --client aside` (list or `--profile`) adds those commands with the real ID for each profile that is off with a stale block. The text issues no request and enables nothing; `--json` output is unchanged. `src/cli/claude-desktop-profile.ts` saves runtime profiles through the canonical parser and management owner, without applying or falling back to local state. `src/cli/integration-cursor.ts` exposes installation/capability and installer-link observations only. `src/cli/remote-workspace-hub.ts` selects Hub reads before executor storage: available empty results succeed, outer available:false observations remain unavailable/nonzero, and runtime availability retains its named object shape. These observations can trigger the existing inventory/probe work and are not advertised as offline. diff --git a/structure/clients/integrations.md b/structure/clients/integrations.md index b5c15c5facf..d62657871b5 100644 --- a/structure/clients/integrations.md +++ b/structure/clients/integrations.md @@ -477,7 +477,7 @@ connection defaults all profiles on; explicit per-profile changes materialize th pin one legacy root owner before changing it. Sibling stores remain independent. Policy saves precede coordinated writes under one scoped flight, and actual file state/refusals remain separate. Restore reconciles target intent from validated snapshot ownership without changing -sibling policy. Profile journal views retain source-store provenance for older legacy entries. +sibling policy. Profile journal views retain source-store provenance for older legacy entries. A profile that is off beside a stale block is never re-enabled automatically; the CLI names its preview-then-enable recovery (`src/cli/aside-profile-recovery.ts`), and a malformed policy that falls back to all-off produces a load-time warning. The shared atomic replacement publisher also identifies explicit Remote Workspace file writes as `remote-workspace`; its isolated owner and support limits are documented in [Remote Workspace](../remote-workspace.md). diff --git a/structure/config.md b/structure/config.md index 13505a50bd0..e2e1e04b32c 100644 --- a/structure/config.md +++ b/structure/config.md @@ -597,4 +597,4 @@ so wrong types and unknown nested fields are rejected rather than silently saved `apiSurfaces` and `protocols` on `src/types/config.ts` are parsed by `src/protocols/settings.ts` only; [Protocol Paths](data-planes/protocol-paths.md#settings) owns their schema handling, meaning and the one writer (`PATCH /api/protocols/settings`), including why closing Messages also writes `claudeCode.enabled` through `commitClaudeCodeBlock` (`src/claude/claude-code-block.ts`, the sentinel-stamping block writer every management route uses). Stored Direct substitution follows the [credential identity contract](providers/openai-accounts.md#sidecars-management-and-ui): both synchronous and asynchronous materializers discard the caller account header before applying the stored credential; ordinary native Direct passthrough is unchanged. -Proxy activation and credential-safe CLI output follow [Proxy Configuration](config-proxy.md). The experimental `chatgptDesktop` leaf accepts only optional boolean `appServerShim`, default off. Malformed reads disable this leaf; live writes reject malformed values and unknown keys. `src/config/diagnostics.ts` reports the reason a whole `chatgptDesktop` block is dropped on both normal and salvaged reads; salvage retains its fallback source and schema error. Its activation and local executable boundary follow [ChatGPT Desktop](clients/chatgpt-desktop.md). `claudeCode.subagentModelForce` is an optional safe roster-style id. Invalid hand edits degrade with a warning without losing other config; stale but syntactically valid targets remain stored for repair and are skipped at launch. Management force updates are field-scoped against current disk state, preserving concurrent Claude sibling edits. A config absent at request start uses canonical create-only initialization; a racing file or invalid initial state refuses the save, and an existing-file mutation never recreates deleted config. Clearing removes only that leaf. +Proxy activation and credential-safe CLI output follow [Proxy Configuration](config-proxy.md). The experimental `chatgptDesktop` leaf accepts only optional boolean `appServerShim`, default off. Malformed reads disable this leaf; live writes reject malformed values and unknown keys. `src/config/diagnostics.ts` reports the reason a whole `chatgptDesktop` block is dropped on both normal and salvaged reads; salvage retains its fallback source and schema error. Its activation and local executable boundary follow [ChatGPT Desktop](clients/chatgpt-desktop.md). `claudeCode.subagentModelForce` is an optional safe roster-style id. Invalid hand edits degrade with a warning without losing other config; stale but syntactically valid targets remain stored for repair and are skipped at launch. Management force updates are field-scoped against current disk state, preserving concurrent Claude sibling edits. A config absent at request start uses canonical create-only initialization; a racing file or invalid initial state refuses the save, and an existing-file mutation never recreates deleted config. Clearing removes only that leaf. A malformed `asideProfileSync` still loads as `{ allProfiles: false }`, which turns every Aside profile off, so `warnDegradedTopLevelOptIns` in `src/config/load-degrade.ts` now reports it once per load without echoing a key or value. diff --git a/tests/cli/cli-aside-sync.test.ts b/tests/cli/cli-aside-sync.test.ts index e335961b2ac..429ec1f3052 100644 --- a/tests/cli/cli-aside-sync.test.ts +++ b/tests/cli/cli-aside-sync.test.ts @@ -2,6 +2,7 @@ import { afterEach, beforeEach, describe, expect, spyOn, test } from "bun:test"; import { writeFileSync } from "node:fs"; import { handleClientIntegrationCommand } from "../../src/cli/integrations"; import { handleIntegrationAsideSync, type AsideSyncCliDeps } from "../../src/cli/integration-aside-sync"; +import { asideProfileRecoveryLines } from "../../src/cli/aside-profile-recovery"; import { getRuntimePortPath } from "../../src/config/process-state"; import { LOCAL_ATTESTATION_CHALLENGE_HEADER, LOCAL_ATTESTATION_PROOF_HEADER, createLocalAttestationProof } from "../../src/lib/local-management-attestation"; import { LOCAL_ASIDE_SYNC_CAPABILITY_HEADER, LOCAL_ASIDE_SYNC_CAPABILITY_VERSION, LOCAL_ASIDE_SYNC_EXPECTED_PID_HEADER, LOCAL_ASIDE_SYNC_EXPIRES_AT_HEADER, LOCAL_ASIDE_SYNC_NONCE_HEADER, LOCAL_ASIDE_SYNC_PATH, verifyLocalAsideSyncCapability } from "../../src/lib/local-aside-sync-contract"; @@ -52,9 +53,35 @@ describe("Aside-only synchronization command", () => { expect(f.calls[0]!.init).toMatchObject({ method: "POST", body: "{}" }); expect(json()).toEqual({ results: [{ client: "aside", profileId: 2, ok: true, changed: true }] }); }); - test("empty eligible set is described as no work", async () => { + test("empty eligible set is described as no work and names the reviewed recovery commands", async () => { const f = fixture(); expect(await handleIntegrationAsideSync(["--client", "aside"], f.deps)).toBe(0); - expect(channels()).toContain("No eligible Aside profiles"); expect(channels()).not.toContain("updated"); + const text = channels(); + expect(text).toContain("No eligible Aside profiles"); expect(text).not.toContain("updated"); + expect(text).toContain("Sync refreshes only profiles whose sync preference is on."); + expect(text).toContain("List profiles with: ocx integration client status --client aside"); + expect(text).toContain("review: ocx integration client preview --client aside --operation apply --profile "); + expect(text).toContain("Then, if the preview permits the change and you accept it: ocx integration client enable --client aside --profile "); + // Guidance only: the one sync POST is the whole exchange; nothing previews or enables on the user's behalf. + expect(f.calls).toHaveLength(1); expect(f.calls[0]!.url).toEndWith("/api/client-integrations/aside/sync"); + }); + test("recovery lines cover only valid, distinct off (stale) profile ids", () => { + const lines = asideProfileRecoveryLines([ + { profileId: 0, enabled: false, state: "stale" }, { profileId: 1, enabled: true, state: "stale" }, + { profileId: 2, enabled: false, state: "conflict" }, { profileId: 3, enabled: false, state: "unsafe" }, + { profileId: 5, enabled: false, state: "current" }, { profileId: 6, state: "stale" }, + { profileId: -1, enabled: false, state: "stale" }, { profileId: -0, enabled: false, state: "stale" }, + { profileId: 1.5, enabled: false, state: "stale" }, { profileId: "7", enabled: false, state: "stale" }, + { profileId: Number.MAX_SAFE_INTEGER + 1, enabled: false, state: "stale" }, + { profileId: 4, enabled: false, state: "stale" }, { profileId: 4, enabled: false, state: "stale" }, null, + { profileId: Number.MAX_SAFE_INTEGER, enabled: false, state: "stale" }, + ]); + expect(lines.filter(line => line.startsWith("Aside profile")).map(line => line.split(" ")[2])).toEqual(["0", "4", String(Number.MAX_SAFE_INTEGER)]); + expect(lines.slice(0, 4)).toEqual([ + "Aside profile 0 is off (stale). To reconnect it, review: ocx integration client preview --client aside --operation apply --profile 0", + "Then, if the preview permits the change and you accept it: ocx integration client enable --client aside --profile 0", + "Aside profile 4 is off (stale). To reconnect it, review: ocx integration client preview --client aside --operation apply --profile 4", + "Then, if the preview permits the change and you accept it: ocx integration client enable --client aside --profile 4", + ]); }); test.each([true, false])("partial results preserve safe outcomes and return nonzero (JSON %s)", async wantsJson => { const f = fixture({ results: [ diff --git a/tests/cli/cli-headless-parity.test.ts b/tests/cli/cli-headless-parity.test.ts index 2c59a884dfc..dd178a6b779 100644 --- a/tests/cli/cli-headless-parity.test.ts +++ b/tests/cli/cli-headless-parity.test.ts @@ -1598,6 +1598,44 @@ test("Aside status prints the empty-profile diagnostic for humans", async () => } finally { log.mockRestore(); } }); +test("Aside status names preview-then-enable commands only for off (stale) profiles", async () => { + const profiles = [ + { clientId: "aside", profileId: 0, name: "A", enabled: false, state: "stale" }, + { clientId: "aside", profileId: 1, name: "B", enabled: true, state: "stale" }, + { clientId: "aside", profileId: 2, name: "C", enabled: false, state: "conflict" }, + { clientId: "aside", profileId: 3, name: "D", enabled: false, state: "current" }, + { clientId: "aside", profileId: 4, name: "E", enabled: false, state: "stale" }, + ]; + const runtime = fakeRuntime(() => ({ clientId: "aside", profiles })); + const log = spyOn(console, "log").mockImplementation(() => {}); + try { + expect(await handleClientIntegrationCommand(["status", "--client", "aside"], runtime.deps)).toBe(0); + const text = log.mock.calls.flat().join("\n"); + expect(text).toContain("0 A: off (stale)"); + expect(text.match(/--operation apply --profile \d+/g)).toEqual(["--operation apply --profile 0", "--operation apply --profile 4"]); + expect(text.match(/enable --client aside --profile \d+/g)).toEqual(["enable --client aside --profile 0", "enable --client aside --profile 4"]); + expect(text).toContain("Then, if the preview permits the change and you accept it:"); + log.mockClear(); + expect(await handleClientIntegrationCommand(["status", "--client", "aside", "--json"], runtime.deps)).toBe(0); + expect(JSON.parse(log.mock.calls.flat().join("\n"))).toEqual({ clientId: "aside", profiles }); + expect(runtime.requests.map(row => row.method)).toEqual(["GET", "GET"]); + } finally { log.mockRestore(); } +}); + +test("single Aside profile status adds recovery commands only when off (stale)", async () => { + let profile: Record = { clientId: "aside", profileId: 4, enabled: false, state: "stale", installed: true }; + const runtime = fakeRuntime(() => profile); + const log = spyOn(console, "log").mockImplementation(() => {}); + try { + expect(await handleClientIntegrationCommand(["status", "--client", "aside", "--profile", "4"], runtime.deps)).toBe(0); + expect(log.mock.calls.flat().join("\n")).toContain("ocx integration client enable --client aside --profile 4"); + profile = { ...profile, state: "conflict" }; log.mockClear(); + expect(await handleClientIntegrationCommand(["status", "--client", "aside", "--profile", "4"], runtime.deps)).toBe(0); + expect(log.mock.calls.flat().join("\n")).not.toContain("enable --client aside"); + expect(runtime.requests.map(row => row.path)).toEqual(["/api/client-integrations/aside/profiles/4", "/api/client-integrations/aside/profiles/4"]); + } finally { log.mockRestore(); } +}); + describe("Aside CLI recovery metadata", () => { test.each([ { name: "a long POSIX backup path", snapshotPath: `/tmp/aside-recovery/${"profile-2-snapshot/".repeat(80)}models.json.bak` }, diff --git a/tests/config/config-load-degrade.test.ts b/tests/config/config-load-degrade.test.ts index 807175eb520..1efec0e6a67 100644 --- a/tests/config/config-load-degrade.test.ts +++ b/tests/config/config-load-degrade.test.ts @@ -503,3 +503,32 @@ test("malformed protocol containers and siblings retain disabled native policy a expect(validateConfigCandidate(raw).ok).toBe(false); } }); + +test("a malformed Aside profile sync policy warns once without echoing it and keeps the all-off fallback", () => { + const warn = spyOn(console, "warn").mockImplementation(() => {}); + try { + const raw = { ...candidate(undefined), asideProfileSync: { profiles: { "0": "secret-shaped-aside-off" } } }; + const bytes = JSON.stringify(raw); + writeFileSync(getConfigPath(), bytes, "utf8"); + const loaded = loadConfig(); + expect(loaded.asideProfileSync).toEqual({ allProfiles: false }); + expect(loaded.providers.xai).toMatchObject({ note: "keep me" }); + const messages = warn.mock.calls.flat().join("\n"); + expect(messages.match(/asideProfileSync is invalid/g)).toHaveLength(1); + expect(messages).toContain("ocx integration client status --client aside"); + expect(messages).not.toContain("secret-shaped-aside-off"); + expect(readFileSync(getConfigPath(), "utf8")).toBe(bytes); + for (const invalid of [null, [], "on", { allProfiles: "yes" }, { profiles: { "01": true } }, { legacyProfileId: -1 }]) { + warn.mockClear(); + writeFileSync(getConfigPath(), JSON.stringify({ ...candidate(undefined), asideProfileSync: invalid }), "utf8"); + expect(loadConfig().asideProfileSync).toEqual({ allProfiles: false }); + expect(warn.mock.calls.flat().join("\n").match(/asideProfileSync is invalid/g)).toHaveLength(1); + } + for (const valid of [undefined, { allProfiles: true }, { profiles: { "0": false } }, { allProfiles: false, futureKey: "kept" }]) { + warn.mockClear(); + writeFileSync(getConfigPath(), JSON.stringify({ ...candidate(undefined), asideProfileSync: valid }), "utf8"); + loadConfig(); + expect(warn.mock.calls.flat().join("\n")).not.toContain("asideProfileSync"); + } + } finally { warn.mockRestore(); } +});