HIP reviewer skill

.agents/skills/hip-reviewer/SKILL.md

@@ -0,0 +1,111 @@
+---
+name: hip-reviewer
+description: "Review and critique Helm Improvement Proposals (HIPs) from a maintainer perspective. Use this skill when asked to review a HIP, evaluate a proposal, provide feedback on a HIP, critique a HIP, or assess whether a proposed change to Helm is appropriate. Also trigger when the user says things like 'what do you think of this HIP', 'review this proposal', 'is this HIP ready', 'give feedback on hip-NNNN', or pastes HIP content for evaluation."
+---
+
+# HIP Review Guide
+
+This skill helps Helm maintainers and community members evaluate proposed HIPs against the project's standards and values. The review produces a structured assessment covering five dimensions, with a clear verdict and actionable feedback.
+
+Read `references/review-criteria.md` for the detailed domain knowledge (backwards compatibility rules from HIP-0004, Helm's mission scope, user base breadth expectations, implementation complexity signals, and security checklist).
+
+## Review Process
+
+### Step 1: Read the HIP
+
+If the user provides a HIP number (e.g., "review hip-0029"), read it from `hips/hip-NNNN.md`. If they paste content directly, work from that.
+
+### Step 2: Evaluate against five dimensions
+
+Assess the HIP on each dimension below. For each, provide:
+- A **rating**: Strong / Adequate / Weak / Insufficient
+- A brief explanation (2-4 sentences) justifying the rating
+- Specific concerns or suggestions if the rating is below "Strong"
+
+#### 1. Motivation and Rationale
+
+Does this proposal solve a real problem for Helm users? Is the motivation grounded in concrete pain points, or is it speculative? Does it align with Helm's core mission (package, distribute, and manage Kubernetes applications)?
+
+**Strong**: Clear user pain, concrete examples, obvious alignment with Helm's purpose.
+**Weak**: Vague motivation, hypothetical benefits, or solves a problem that isn't really Helm's to solve.
+
+Look for: named user roles affected, existing workarounds described, real-world scenarios (not just "it would be nice if...").
+
+#### 2. Change Applicability (Breadth)
+
+Will this benefit a broad cross-section of Helm users, or is it niche? Does it introduce complexity that all users must contend with even if they don't use the feature?
+
+**Strong**: Benefits most Helm users or a large segment; complexity is opt-in and invisible to non-users.
+**Weak**: Benefits only a narrow use case; adds concepts or flags everyone encounters; could be a plugin instead.
+
+Ask yourself: would a typical chart author and a typical cluster operator both understand why this exists? If neither would notice it in their daily workflow, it may be too niche for core.
+
+#### 3. Backwards Compatibility
+
+Does the proposal respect HIP-0004's compatibility rules? If it introduces breaking changes, does it properly scope them to a major version or use experimental feature flags?
+
+**Strong**: Purely additive, no breakage, existing users/charts/tools unaffected.
+**Adequate**: Minor changes with clear migration path, properly acknowledged.
+**Weak/Insufficient**: Breaks CLI contracts, file formats, template behavior, or Go API without acknowledging it or providing mitigation.
+
+Check the specific rules in `references/review-criteria.md` — CLI flags, file format fields, template functions, return types, structured output. Authors often miss subtle breaks like changing the type of a field or adding a required parameter.
+
+#### 4. Implementation Complexity and Feasibility
+
+Is this implementable by the Helm maintainer team (small volunteer group)? Is the scope bounded? Does a proof-of-concept exist or is it purely theoretical?
+
+**Strong**: Bounded scope, single-project change, proof-of-concept exists, follows existing patterns.
+**Weak**: Requires multi-project coordination, unbounded ongoing maintenance, depends on unmerged upstream features, or no one has demonstrated feasibility.
+
+Consider: who will actually implement this? If the author isn't planning to, is there a realistic path to someone else doing it? Orphaned HIPs with no implementor become dead weight.
+
+#### 5. Security Implications
+
+Has the proposal thoroughly considered security? Are the defaults safe? Does it add attack surface, expose data, or change trust boundaries?
+
+**Strong**: Proactively addresses security, defaults to safe behavior, minimal new attack surface.
+**Weak**: Dismisses security ("No security implications") for a feature that clearly touches trust or data boundaries. Missing analysis of how a malicious chart or user could abuse the feature.
+
+Use the checklist in `references/review-criteria.md`: data exposure, trust boundaries, supply chain, privilege escalation, denial of service, default safety.
+
+### Step 3: Produce the review
+
+Structure the output as follows:
+
+```
+## HIP Review: [HIP title]
+
+### Summary Verdict
+
+[One sentence: accept / request changes / major concerns]
+
+### Dimension Ratings
+
+| Dimension | Rating | Key Concern |
+|-----------|--------|-------------|
+| Motivation & Rationale | [rating] | [one-line summary] |
+| Change Applicability | [rating] | [one-line summary] |
+| Backwards Compatibility | [rating] | [one-line summary] |
+| Implementation Feasibility | [rating] | [one-line summary] |
+| Security Implications | [rating] | [one-line summary] |
+
+### Detailed Assessment
+
+[2-4 sentences per dimension, with specific line references or quotes from the HIP where relevant]
+
+### Recommendations
+
+[Bulleted list of specific, actionable changes the author should make]
+```
+
+### Verdict guidance
+
+- **All Strong/Adequate**: "This HIP is well-prepared for acceptance. Minor suggestions below."
+- **One or more Weak**: "This HIP needs revisions before it's ready for acceptance." Focus recommendations on the weak areas.
+- **Any Insufficient**: "This HIP has fundamental issues that need to be addressed before further review." Explain what's missing.
+
+## Tone
+
+Be constructive but honest. The goal is to help the author improve their proposal, not to gatekeep. When something is weak, explain what would make it strong. When something is good, say so briefly and move on — don't pad the review with praise.
+
+Helm is a community project. Assume good intent. The author invested time in writing this; respect that while being direct about problems.

.agents/skills/hip-reviewer/references/review-criteria.md

@@ -0,0 +1,103 @@
+# HIP Review Criteria Reference
+
+This file provides the detailed evaluation criteria for reviewing Helm Improvement Proposals. The skill's SKILL.md defines the review process; this file provides the domain knowledge needed to apply each criterion.
+
+## Helm's mission and scope
+
+Helm is the package manager for Kubernetes. Its core functions are:
+
+- **Create**: package Kubernetes resources into charts
+- **Distribute**: share charts via repositories (HTTP or OCI registries)
+- **Manage**: install, upgrade, rollback, and uninstall releases on clusters
+
+Proposals that align with these functions have a natural home in Helm. Proposals that push Helm toward being an application platform, a CI/CD tool, a service mesh manager, or a cluster provisioner are likely out of scope — even if they touch Kubernetes.
+
+## Backwards compatibility rules (HIP-0004)
+
+Changes to Helm minor and patch releases must be 100% backwards compatible per Semantic Versioning. The following rules apply:
+
+### CLI
+- Commands and flags must not be removed, renamed, or moved
+- Commands and flags cannot be repurposed to provide different behavior
+- Flag types must not change (unless the new type is a superset: int8 → int32, int → float, float → string)
+- Short name and long name rules are independent — changing one while keeping the other is still a break
+
+### CLI output
+- Help text may change
+- Structured output format (tables, JSON, YAML) must not change
+- Error messages can change if doing so increases usability
+- Bug fixes (spelling, misinformation) are allowed
+- Return codes should not change unless incorrect
+
+### File formats (Chart.yaml, index.yaml, repositories.yaml, values.yaml)
+- No fields may be removed from any of these files
+- No fields may be added or modified on Chart.yaml
+- Added fields in other files must be optional and accompanied by backward compatibility checks
+- No existing optional field can be made required
+- No 3rd-party-specific fields (use annotations instead)
+
+### Charts
+- Template handling must not change
+- Reserved directory names (e.g., crds) must not be removed
+- Allowed file types cannot become disallowed
+- New objects or file types added to charts must be optional
+
+### Templates (functions, syntax, variables)
+- Template functions cannot be removed
+- Return types cannot change
+- Function signatures can only grow additively (new optional parameters)
+- Built-in directives, constants, and variables cannot be removed or changed in breaking ways
+- Template syntax cannot change (except upstream Go template changes)
+
+### Exceptions
+- Backwards compatibility may be broken for security reasons
+- Experimental features behind feature flags are exempt from these rules (but cannot break non-experimental features)
+
+## User base breadth
+
+Helm serves a wide range of users:
+
+- **Chart authors** writing and maintaining charts
+- **Chart consumers** installing and managing releases
+- **Platform/ops teams** managing Helm across fleets of clusters
+- **Tool builders** integrating Helm's SDK or CLI into CI/CD pipelines, GitOps tools, etc.
+- **Registry operators** hosting chart repositories
+
+A change that serves only one narrow segment (e.g., only benefits users of a specific cloud provider, or only helps teams using a particular deployment pattern) needs to justify why it belongs in Helm core rather than in a plugin, chart convention, or external tool.
+
+Questions to ask:
+- What fraction of Helm users will benefit from this change?
+- Does this add complexity for users who don't need the feature?
+- Could this be implemented as a plugin instead of a core feature?
+- Does the feature require all users to learn new concepts even if they don't use it?
+
+## Implementation complexity signals
+
+Helm is maintained by a small group of volunteers. Implementation burden is a real factor in whether a HIP can succeed:
+
+**Red flags:**
+- Requires changes across multiple Helm projects (helm, helm-www, chartmuseum, etc.)
+- Introduces new long-lived infrastructure the maintainers must operate
+- Requires ongoing maintenance of external integrations
+- Adds significant new surface area to the Go SDK that must be kept stable
+- Depends on features that don't exist yet in Kubernetes or Go
+- Requires coordination with external projects for correctness
+
+**Green flags:**
+- Self-contained change within a single package
+- Clear, bounded scope with a definitive "done" state
+- Author has a working proof-of-concept
+- Follows established patterns in the codebase
+
+## Security evaluation checklist
+
+When reviewing security implications, consider:
+
+1. **Data exposure**: Does this feature expose data that wasn't previously accessible? Could chart values, release secrets, or cluster state leak?
+2. **Trust boundaries**: Does this change who can see or modify what? Does it add new trust relationships?
+3. **Supply chain**: Does this affect chart provenance, signing, or verification? Could a malicious chart exploit this feature?
+4. **Privilege escalation**: Could this be used to gain Kubernetes permissions beyond what the user already has?
+5. **Denial of service**: Could this be used to exhaust cluster resources, fill storage, or create runaway processes?
+6. **Default safety**: Are the defaults secure? Does the user have to opt-in to risky behavior, or opt-out?
+
+A well-written HIP addresses these proactively. A poorly-written one says "No security implications" for a feature that clearly touches trust boundaries.

.gitignore

@@ -1 +1,2 @@
-.idea
+.idea
+.claude/