[Spec 8] feat: ci-channel remove command (v0.5.0)#10
Merged
waleedkadous merged 9 commits intoApr 11, 2026
Conversation
Address Codex REQUEST_CHANGES from iteration 1: - Canonical .mcp.json rule now uses strict full-array equality - Idempotency clarified: 2nd run fails fast with exit 1 - state.json validity checks enumerated (missing/malformed/no smeeUrl) - 404 handling disambiguated (list-404 hard, delete-404 soft) Address Claude minor concerns: - R1 test response shape clarified - Defensive .mcp.json shape handling added - findProjectRoot null-return handling added - Order-of-operations paragraph cleaned up - R9 (404-during-DELETE race) promoted to mandatory - R10 (invalid state.json) added as mandatory
Address Codex and Claude REQUEST_CHANGES from iteration 1: - Inline Codev revert into remove() (no separate helper) - save 7 lines - Add nested cliDelete helper for GitHub+GitLab DELETE - save 16 lines - Gitea DELETE uses direct fetch (LIST still uses giteaFetch) - Honest line-cap budget with 10 tightening levers; add escalation gate Test corrections (both reviewers flagged factual errors): - runRemove is in-process, mirrors runSetup at lines 64-76 (not dist/server.js) - mkFakeCli is counter-based; no helper extension needed - npm test baseline is 191 (full suite), not 18 (setup.test.ts alone) - R3 assertion string syncs with actual remove() log line New mandatory tests (was 9, now 10): - R4 now folds missing-state + missing-smeeUrl sub-assertions - R10 is new: LIST-404 hard-fail (repo not found) - R1 now folds remove-twice assertion
Phase 1 of Spec 8: implement remove() and dispatch. lib/setup.ts (300 → 396 lines, under 400 cap): - Rename parseArgs → parseCommandArgs with command: 'setup' | 'remove' param - Add remove() export with three forge branches and inline Codev revert - Nested cliDelete helper shares list→match→DELETE-with-404-soft logic for gh+glab - Gitea LIST uses existing giteaFetch; Gitea DELETE uses direct fetch for 404-soft - Strict state.json precondition check (missing/malformed/no-smeeUrl → fail fast) - Canonical .mcp.json check: strict full-array equality against forge-specific expected args - Compressed existing classifyForgeError and codevIntegrate helpers for space server.ts dispatch extended to recognize 'remove' (5 lines, under 8-line cap). Version bump to 0.5.0 in package.json and package-lock.json (both fields). Docs updated in README, INSTALL, CLAUDE, AGENTS with uninstall section. Baseline test count: 191. After Phase 1: 191 (no new tests yet, Phase 2 adds them).
Phase 2 of Spec 8: add automated tests for ci-channel remove. tests/setup.test.ts (399 → 580 lines, under 600 cap): - Import remove alongside setup - Refactor runSetup into shared runCommand(fn, argv) helper; runRemove uses the same - R1: GitHub happy path + second-run-fails-fast assertion (remove-twice contract) - R2: GitHub no matching webhook → skip DELETE, still clean up locally - R3: GitHub customized .mcp.json → leave entry alone, still delete webhook+state - R4: Missing state.json OR missing smeeUrl → fail fast, no mutations (folds R10 invalid-state case) - R5: GitLab happy path with path-encoded LIST + DELETE - R6: Gitea happy path with GET + DELETE via withGiteaServer - R7: Gitea missing token → exit 1, state.json intact, no HTTP - R8: Codev revert strips the loader flag with leading space - R9: DELETE-404 race → already-gone, continue, exit 0 - R10: LIST-404 hard fail → Could not find repo, state + .mcp.json untouched No modifications to mkFakeCli or withGiteaServer (both already support DELETE via counter-based and method-agnostic patterns respectively). Test count: 18 setup + 10 remove = 28 in setup.test.ts; full suite 201/201 pass.
Review document covers spec compliance, lessons learned, and 3-way consultation feedback from all phases (specify/plan/impl/tests). Architecture document updated: - lib/setup.ts now mentioned as installer/uninstaller - remove() section added with 404 handling, canonical check, second-run rules - tests/setup.test.ts count updated to 28 (18 setup + 10 remove) Lessons learned updated with 3 new entries from Spec 8: - Inline small helpers under a hard line cap - Plan budget math must count from actual baseline - Plan-phase 3-way review catches factual errors about existing code
Addresses Codex PR review feedback: R4 previously covered missing state and parseable-but-missing-smeeUrl, but did not exercise the JSON.parse failure branch in remove(). Added a third sub-case writing invalid JSON to state.json and asserting 'unreadable or malformed' exit 1 + no gh call + state.json byte-equal + .mcp.json untouched. All three spec-enumerated state.json precondition failure cases are now tested: - Case (a): no state.json - Case (b): state.json present but malformed - Case (c): state.json present but missing smeeUrl File size: 580 → 591 lines (still under 600 cap). Test count unchanged at 28 (R4 still counts as one test). Full suite: 201/201 pass.
Apply three targeted simplifications per architect intervention:
1. Drop nested cliDelete helper; inline three forge branches directly in
remove() body. Gitea already duplicated the shape separately via fetch,
so the 'sharing' was only across gh+glab and added indirection without
much savings at this scale.
2. Simplify .mcp.json canonical check. Old: strict full-array equality
against forge-specific expected args (meant users who customized --repos
or --port would see 'does not match canonical shape' and have to clean
up manually). New: loose match — entry.command === 'npx' AND args
includes 'ci-channel'. If the user invoked 'ci-channel remove --repo X',
they want it gone regardless of customization. Only entries that don't
look like ci-channel at all (e.g., command: 'node' with a custom path)
are left alone with 'not recognized — leaving alone'.
3. Narrow types instead of biome-ignore any. Added 5 local interfaces
(RemoveState, McpEntry, McpFile, CodevConfig, Hook) so state.json,
.mcp.json, .codev/config.json, and hook list responses are all
properly typed. Reduced biome-ignore any count from ~9 to ~3 (only
for subprocess error shapes and unlink errno, which are genuinely
opaque until caught).
R3 test updated: the old test seeded { command: 'npx', args: ['-y',
'ci-channel'], env: { FOO: 'bar' } } and asserted the entry was left
alone. Under the new loose check, that entry matches (command=npx + args
includes ci-channel) and gets removed — which is the new intended
behavior. R3 now seeds a genuinely unrecognized entry { command: 'node',
args: ['/custom/path/...'] } and asserts the 'not recognized — leaving
alone' path.
lib/setup.ts: 407 → 399 lines (under 400 cap)
remove() body: 120 → 111 lines
tests/setup.test.ts: 591 lines (unchanged)
Full suite: 201/201 pass
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a `ci-channel remove` subcommand that reverses what `ci-channel setup` did: deletes the forge webhook (matched by smee URL from `state.json`), deletes `state.json`, strips the canonical `ci` entry from `.mcp.json` (leaves non-canonical entries alone with a warning), and reverts the Codev integration if `.codev/config.json` contains the loader flag. Supports all three forges (GitHub, GitLab, Gitea) with the same flag shape as `setup`.
Ships as v0.5.0.
Changes
Testing
Spec
Review
All three 3-way consultations (Gemini, Codex, Claude) across all four phases (specify, plan, impl, tests) converged on APPROVE. Two phases (specify and plan) had REQUEST_CHANGES iterations that were fully resolved before proceeding.
Closes #8