Skip to content

chore: add /cleanup skill for periodic maintainability passes#3392

Open
Matthew Seal (MSeal) wants to merge 1 commit into
mainfrom
add-cleanup-skill
Open

chore: add /cleanup skill for periodic maintainability passes#3392
Matthew Seal (MSeal) wants to merge 1 commit into
mainfrom
add-cleanup-skill

Conversation

@MSeal
Copy link
Copy Markdown
Contributor

Summary of Changes

  • Adds a project-local Claude Code skill at .claude/skills/cleanup/SKILL.md for non-functional code cleanup, organization, and maintainability passes. The skill identifies tech debt, hygiene issues, and organization problems WITHOUT changing functionality, then reviews findings with the user and applies ONLY the subset they approve.
  • Adds .claude/skills/cleanup/conventions.md, a hand-editable style guide that consolidates team norms for this repo. CLAUDE.md wins on any conflict — conventions augment, they don't supersede.
  • Wires the skill to consult conventions.md as Core Rule 7, so its rules count alongside CLAUDE.md when bucketing findings.
  • .gitignore excludes an internal-only learn-conventions/ skill that mines team Slack and PRs to refresh conventions.md; contributors keep it locally.

Skill design (high level)

  • No behavior changes. Every proposed change must be functionally inert. Anything that could shift behavior (error timing, log output, ordering of async work) goes to a Needs decision bucket and is never auto-applied.
  • Propose before changing. Findings bucket into Test ImprovementsMust-fixShould-fixConsiderNeeds decision; the user picks the subset to apply.
  • Test-coverage analysis runs before code changes. Coverage gaps in the touched scope surface first so tests can land before refactors.
  • Scope defaults: explicit path/glob → branch-diff vs main → prompt for full-repo / directory / cancel. Never silently scans the whole repo.

Click-testing instructions

Not applicable — this PR adds Claude Code skill files only; no extension behavior changes.

Optional: discovered findings from a first full-repo run

Running /cleanup over the whole repo surfaced the items below. Not part of this PR — sharing as a follow-up roadmap for the team to triage.

Hard-signal checks

  • TypeScript check: 3 pre-existing errors in src/utils/jarInspector*.ts (missing @types/yazl, @types/yauzl, implicit any)
  • Lint: broken on missing dep eslint-plugin-playwright (pre-existing tooling issue)
  • .only left in tests: zero ✓
  • Coverage snapshot: ~90% in src/viewProviders/ and ~86% in src/commands/; critical gaps in src/webview/ (~9%) and src/sidecar/ (~44%)

Test Improvements (recommended FIRST — protect refactors)

# File LOC Tests
T1 src/sidecar/sidecarHandle.ts 641 0
T2 src/sidecar/sidecarManager.ts 632 0
T3 src/webview/message-viewer.ts 716 0
T4 src/sidecar/websocketManager.ts ~400 0

Must-fix (CLAUDE.md + conventions.md)

  1. src/webview/comms/comms.ts:11,33,72 — core message dispatch uses (type: any, body: any) => any. Replace with a MessageType discriminated union.
  2. src/sidecar/sidecarHandle.ts:82 — shared GraphQL response { data?: any }. Parameterize <T> or Record<string, unknown>.
  3. src/commands/index.ts:21,38,39,40,72registerCommandWithLogging() uses (...args: any[]). Type-narrow the context.
  4. src/models/resource.ts:46,126isResource(value: any) and isSearchable(item: any) type-guards should use unknown.
  5. Bulk logger.error()logError() across src/commands/{flinkUDFs,medusaCodeLens,schemas,flinkDatabaseView,docker,diffs,documents,flinkStatements,kafkaClusters}.ts, src/loaders/ccloudResourceLoader.ts, src/sidecar/websocketManager.ts, src/models/containers/resourceContainer.ts, src/utils/jarInspector.ts (~14 call sites).
  6. Bulk window.showErrorMessage()showErrorNotificationWithButtons() in src/commands/schemas.ts:161, src/commands/kafkaClusters.ts, src/commands/docker.ts, src/commands/medusaCodeLens.ts.
  7. src/viewProviders/resources.ts:74,105,147,264 — four (element as any).constructor.name casts. Replace with a discriminated union or instanceof checks.
  8. src/viewProviders/resources.ts:371,379,391 — three throw new Error() in DirectConnectionRow getters. Wrap with logError() and structured cause.
  9. JSDoc gaps on 16 exported symbols across src/commands/extra.ts (5), src/commands/diffs.ts:13, src/loaders/resourceLoaderInitialization.ts:9, src/sidecar/utils.ts (4), src/sidecar/websocketManager.ts (3), src/utils/bootstrapServers.ts, src/models/flinkAiModel.ts, src/viewProviders/resources.ts:498.

Should-fix

  1. src/loaders/ccloudResourceLoader.ts (1130 LOC) mixes GraphQL queries, REST wrappers, and model transformers. Extract restFlinkArtifactToModel and other transformers into co-located helpers without restructuring the loader itself.
  2. src/models/containers/resourceContainer.ts:124,136 — hardcoded 10000 timeout. Introduce DEFAULT_LOAD_TIMEOUT_MS at module level.

Needs decision

  1. src/viewProviders/resources.ts (940 LOC) — split tree-provider logic from DirectConnectionRow. Touches import boundaries, possible serialization-timing impact.
  2. Verify DisposableCollection chain for flinkStatements, flinkDatabase, topics, schemas view providers (inherit via ParentedBaseViewProvider).

Remaining lower-priority items (not shown above)

  • Test Improvements: 5 more (src/commands/{debugtools,schemaRegistry,providers,resources,organizations}.ts uncovered; thin coverage in src/authn/ ~33%, src/chat/ ~29%, src/storage/ ~40%)
  • Should-fix: ~10 more (JSDoc gaps in src/quickpicks/, src/utils/, src/models/, src/webview/bindings/; Map<any,any> cleanup in storage; webview message-handler body: any overloads blocked on Add templates for some types of issues and discussions #1)
  • Consider: ~20 more (large-file split candidates in src/sidecar/, src/loaders/; comment-refresh in src/utils/fsWrappers.ts; TODO/FIXME density: 13 in src/docker/, 10 in src/commands/, 5 each in src/sidecar/ and src/quickpicks/)
  • Needs decision: 3 more (adding @types/yazl/@types/yauzl devDeps; WriteableTmpDir singleton mutability; JSDoc on DisposableCollection itself)

Pull request checklist

Tests

  • Added new
  • Updated existing
  • Deleted existing

(No tests — skill files are instructions for Claude Code, not extension code.)

Release notes

  • Does anything in this PR need to be mentioned in the user-facing CHANGELOG?

(Not user-facing — internal tooling for contributors.)

🤖 Generated with Claude Code

Adds a project-local Claude Code skill at .claude/skills/cleanup/SKILL.md
that runs non-functional cleanup passes. Identifies tech debt, hygiene,
and organization issues, analyzes test coverage over the touched scope,
and reviews proposed changes with the user before applying any subset.

Key properties:
- No behavior changes: every proposed edit must be functionally inert;
  anything that could shift behavior goes to a "needs-decision" bucket and
  is never auto-applied.
- Propose before changing: findings are bucketed (Test Improvements,
  Must-fix, Should-fix, Consider, Needs decision) and the user picks the
  subset to apply.
- Test coverage analysis runs before code changes; coverage gaps in the
  touched scope are surfaced first so tests can land before refactors.
- Scope defaults: explicit path/glob > branch-diff vs main > prompt for
  full repo scan vs directory vs cancel. Never silently scans the whole
  repo.
- Reads .claude/skills/cleanup/conventions.md (if present) and treats its
  rules as team norms alongside CLAUDE.md. The conventions doc is a
  hand-editable style guide; CLAUDE.md wins on conflicts.

Also adds .claude/skills/cleanup/conventions.md with team-derived
conventions covering style, testing, error handling, architecture, sidecar
usage, and anti-patterns. Excludes the internal "learn-conventions"
skill that mines team Slack/PRs to refresh this doc — that skill stays
on contributors' machines via .gitignore.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings May 15, 2026 00:24
@MSeal Matthew Seal (MSeal) requested a review from a team as a code owner May 15, 2026 00:24
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new project-local Claude Code /cleanup skill to support periodic, non-functional maintainability passes, plus a companion conventions document to codify team norms that augment (but never override) CLAUDE.md.

Changes:

  • Added .claude/skills/cleanup/SKILL.md defining a scoped, “propose-before-changing” cleanup workflow with verification steps and conventions-loading.
  • Added .claude/skills/cleanup/conventions.md as a hand-editable repository conventions reference for the cleanup skill to consult.
  • Updated .gitignore to exclude a local-only .claude/skills/learn-conventions/ directory from being committed.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
.gitignore Ignores an internal-only local Claude skill directory (learn-conventions/).
.claude/skills/cleanup/SKILL.md Introduces the /cleanup skill process, scoping rules, bucketing, and verification workflow.
.claude/skills/cleanup/conventions.md Adds repo-specific conventions that the cleanup skill treats as authoritative alongside CLAUDE.md.


- `git log --follow -- <file>` for files where age/ownership matters
- `grep -rn "TODO\|FIXME\|XXX\|HACK" <scope>` for self-flagged debt
- `grep -rn ": any\b\|as any\b" <scope>` for `any` usage (a CLAUDE.md violation)
Comment on lines +29 to +30
- Mocha tests must be isolated. Rely on `globalBeforeAll` to activate the extension rather than
assuming shared state from prior tests.
@sonarqube-confluent
Copy link
Copy Markdown

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants