Skip to content

Commit da56cef

Browse files
authored
Merge branch 'main' into main
2 parents fd2e756 + 8c70343 commit da56cef

32 files changed

Lines changed: 336 additions & 42 deletions
Lines changed: 218 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,218 @@
1+
---
2+
name: implement-story
3+
description: Implements a GitHub user story from planning through PR creation, with research, codebase analysis, and structured commits.
4+
---
5+
6+
# Implement User Story
7+
8+
Takes a GitHub user story issue and produces well-organized PR(s) that reliably meet the acceptance criteria.
9+
10+
## Arguments
11+
12+
The user provides a GitHub issue number or URL. Example:
13+
14+
```
15+
/implement-story #4550
16+
/implement-story https://github.com/stacklok/toolhive/issues/4550
17+
```
18+
19+
---
20+
21+
## Phase 1: Gather Context
22+
23+
### 1.1 Read the Issue
24+
25+
Fetch the issue body using GitHub tools. Extract:
26+
27+
- **User story**: The "As a / I want / so that" statement
28+
- **Acceptance criteria**: The checkbox list — this is the contract
29+
- **Context links**: RFC links, related issues, dependencies
30+
- **Out of scope**: What NOT to do
31+
32+
### 1.2 Fetch RFC Context
33+
34+
If the issue links to an RFC (look for `THV-XXXX` references or links to `toolhive-rfcs`):
35+
36+
1. Clone or locate the RFC repo locally (check `../toolhive-rfcs/` first)
37+
2. Read the full RFC document
38+
3. Extract design decisions relevant to this story — config shapes, algorithm details, error formats, key schemas, etc.
39+
40+
If no RFC is linked, skip this step.
41+
42+
### 1.3 Find Related Stories
43+
44+
Search for sibling stories that share context with this one. These inform how to factor the code for extensibility:
45+
46+
```bash
47+
# Search by keywords from the issue title
48+
gh search issues "<keywords>" --repo stacklok/toolhive --state open --limit 10
49+
50+
# Search for issues linking to the same RFC
51+
gh search issues "THV-XXXX" --repo stacklok/toolhive --limit 10
52+
```
53+
54+
For each related story, read its acceptance criteria. Ask:
55+
56+
- Will a future story need to extend a type, interface, or package I'm creating?
57+
- Should I define an interface now that a sibling story will implement later?
58+
- Are there naming conventions or patterns I should establish that siblings will follow?
59+
60+
**Do not implement sibling stories.** But design the code so they can be implemented without refactoring what you build here.
61+
62+
### 1.4 Research the Codebase
63+
64+
Use the Explore agent or direct search to understand:
65+
66+
1. **Where does this change fit?** Identify the packages, files, and functions that need modification.
67+
2. **What patterns exist?** Find analogous features already implemented. For example, if adding a new middleware, study how existing middleware (auth, mcp-parser, authz) is registered and wired.
68+
3. **What gets generated?** Identify files that are auto-generated (CRD manifests, mocks, docs) so you know what to regenerate.
69+
4. **What tests exist?** Find the test patterns used for similar features (table-driven tests, testcontainers, Chainsaw E2E).
70+
71+
Document your findings before writing any code.
72+
73+
---
74+
75+
## Phase 2: Plan the Work
76+
77+
### 2.1 Map AC to Changes
78+
79+
For each acceptance criterion, identify:
80+
81+
- Which files need to change
82+
- Whether it's new code or a modification
83+
- What tests verify it (unit, integration, or E2E)
84+
85+
### 2.2 Decide PR Strategy
86+
87+
Evaluate the total scope against the project's PR guidelines:
88+
89+
- **< 10 files changed** (excluding tests, generated code, docs)
90+
- **< 400 lines of code changed** (excluding tests, generated code, docs)
91+
92+
If the story fits in one PR, use a single PR. If not, split into multiple PRs following these patterns:
93+
94+
1. **Foundation first**: New types, interfaces, packages
95+
2. **Wiring second**: Integration into existing code (middleware chain, reconciler, CRD)
96+
3. **Tests alongside**: Each PR includes its own tests
97+
4. **Generated code with its trigger**: CRD type changes + `task operator-manifests operator-generate` output in the same PR
98+
99+
### 2.3 Present the Plan
100+
101+
Show the user a plan that covers PR boundaries AND commit boundaries within each PR:
102+
103+
```markdown
104+
## Implementation Plan
105+
106+
**Story**: #XXXX — [title]
107+
**PRs**: [1 or N]
108+
109+
### PR 1: [title]
110+
**Commits**:
111+
1. [commit message][what changes and why]
112+
2. [commit message][what changes and why]
113+
3. [commit message][what changes and why]
114+
**Tests**:
115+
- [Unit/E2E]: [what is tested]
116+
**AC covered**: [which acceptance criteria this satisfies]
117+
**Regeneration**: [which `task` commands need to run and in which commit]
118+
119+
### PR 2: [title] (if needed)
120+
...
121+
```
122+
123+
Wait for user approval before proceeding. Adjust if the user has feedback.
124+
125+
---
126+
127+
## Phase 3: Implement
128+
129+
### 3.1 Create a Branch
130+
131+
```bash
132+
git checkout -b <user>/<short-description> main
133+
```
134+
135+
### 3.2 Write Code
136+
137+
Implement the changes from the plan. Follow these principles:
138+
139+
- **Match existing patterns**: Don't invent new conventions. Study the codebase and follow what's there.
140+
- **Design for siblings**: If related stories will extend this code, use interfaces and clear extension points. But don't build speculative abstractions — just leave the door open.
141+
- **Tests are not optional**: Every AC that says "Unit:" or "E2E:" must have a corresponding test. Write tests as you go, not at the end.
142+
143+
### 3.3 Commit Per the Plan
144+
145+
Follow the commit boundaries from the plan. Each commit should:
146+
147+
- Be independently compilable (`go build ./...` passes)
148+
- Have a clear, descriptive message
149+
- Group related changes (e.g., don't mix CRD type changes with middleware logic)
150+
151+
### 3.4 Run Regeneration Tasks
152+
153+
After changes that affect generated artifacts, run the appropriate tasks:
154+
155+
| Change Type | Regeneration Command |
156+
|-------------|---------------------|
157+
| CRD type definitions (`api/v1alpha1/*_types.go`) | `task operator-manifests operator-generate` |
158+
| Mock interfaces | `task gen` |
159+
| CLI commands or API endpoints | `task docs` |
160+
| Helm chart values | `task helm-docs` |
161+
| Any Go file | `task license-fix` |
162+
163+
Run these **before committing** the related changes. Include the generated output in the same commit as the trigger.
164+
165+
---
166+
167+
## Phase 4: Create PR
168+
169+
### 4.1 Push and Create PR
170+
171+
Follow the PR template at `.github/pull_request_template.md` and the rules in `.claude/rules/pr-creation.md`:
172+
173+
- Title: under 70 chars, imperative mood, no conventional commit prefix
174+
- Summary: why first, then what. Reference the issue with `Closes #XXXX`
175+
- Type of change: check exactly one
176+
- Test plan: check every verification step actually run
177+
178+
### 4.2 Verify AC Coverage
179+
180+
Before submitting, review each acceptance criterion from the issue:
181+
182+
- [ ] Is there code that implements it?
183+
- [ ] Is there a test that verifies it?
184+
- [ ] Has the test passed?
185+
186+
If any AC is not covered, either implement it or flag it to the user with a reason.
187+
188+
### 4.3 Babysit CI
189+
190+
After pushing, monitor CI status:
191+
192+
```bash
193+
gh pr checks <pr-number> --repo stacklok/toolhive --watch
194+
```
195+
196+
If CI fails:
197+
1. Read the failure logs
198+
2. Fix the issue
199+
3. Push the fix as a new commit (don't amend — keep the history clean for review)
200+
4. Re-check CI
201+
202+
### 4.4 Multi-PR Workflow
203+
204+
If the story spans multiple PRs:
205+
206+
1. Create the first PR targeting `main`
207+
2. After merge, create subsequent PRs targeting `main`
208+
3. Each PR references the story issue (`Part of #XXXX`)
209+
4. The final PR uses `Closes #XXXX`
210+
211+
---
212+
213+
## Edge Cases
214+
215+
- **AC references another story**: If an acceptance criterion depends on work from another story (e.g., "STORY-001 core middleware exists"), check if that story is merged. If not, flag it to the user.
216+
- **Generated code is large**: CRD manifest regeneration can produce hundreds of lines of diff. This is expected — note it in the PR description under "Special notes for reviewers."
217+
- **Tests require infrastructure**: E2E tests may need a Kind cluster, Redis, or Keycloak. Document the setup in the test plan. Don't skip the test — write it even if the user will run it separately.
218+
- **RFC is ambiguous**: If the RFC doesn't specify a detail needed for implementation, make a pragmatic choice, document it in a code comment, and flag it in the PR description.

.github/workflows/claude.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ jobs:
5959

6060
- name: Run Claude Code
6161
id: claude
62-
uses: anthropics/claude-code-action@88c168b39e7e64da0286d812b6e9fbebb6708185 # v1
62+
uses: anthropics/claude-code-action@1eddb334cfa79fdb21ecbe2180ca1a016e8e7d47 # v1
6363
with:
6464
anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }}
6565
# Security: Restrict tools to prevent arbitrary code execution.

cmd/thv-operator/api/v1alpha1/embeddingserver_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,8 @@ type EmbeddingStatefulSetOverrides struct {
156156
// EmbeddingServerStatus defines the observed state of EmbeddingServer
157157
type EmbeddingServerStatus struct {
158158
// Conditions represent the latest available observations of the EmbeddingServer's state
159+
// +listType=map
160+
// +listMapKey=type
159161
// +optional
160162
Conditions []metav1.Condition `json:"conditions,omitempty"`
161163

cmd/thv-operator/api/v1alpha1/mcpexternalauthconfig_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -704,6 +704,8 @@ type UpstreamInjectSpec struct {
704704
// MCPExternalAuthConfigStatus defines the observed state of MCPExternalAuthConfig
705705
type MCPExternalAuthConfigStatus struct {
706706
// Conditions represent the latest available observations of the MCPExternalAuthConfig's state
707+
// +listType=map
708+
// +listMapKey=type
707709
// +optional
708710
Conditions []metav1.Condition `json:"conditions,omitempty"`
709711

cmd/thv-operator/api/v1alpha1/mcpgroup_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,8 @@ type MCPGroupStatus struct {
4242
RemoteProxyCount int `json:"remoteProxyCount,omitempty"`
4343

4444
// Conditions represent observations
45+
// +listType=map
46+
// +listMapKey=type
4547
// +optional
4648
Conditions []metav1.Condition `json:"conditions,omitempty"`
4749
}

cmd/thv-operator/api/v1alpha1/mcpoidcconfig_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,8 @@ type WorkloadReference struct {
168168
// MCPOIDCConfigStatus defines the observed state of MCPOIDCConfig
169169
type MCPOIDCConfigStatus struct {
170170
// Conditions represent the latest available observations of the MCPOIDCConfig's state
171+
// +listType=map
172+
// +listMapKey=type
171173
// +optional
172174
Conditions []metav1.Condition `json:"conditions,omitempty"`
173175

cmd/thv-operator/api/v1alpha1/mcpremoteproxy_types.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,8 @@ type MCPRemoteProxyStatus struct {
157157
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
158158

159159
// Conditions represent the latest available observations of the MCPRemoteProxy's state
160+
// +listType=map
161+
// +listMapKey=type
160162
// +optional
161163
Conditions []metav1.Condition `json:"conditions,omitempty"`
162164

@@ -304,6 +306,7 @@ const (
304306
//+kubebuilder:printcolumn:name="Phase",type="string",JSONPath=".status.phase"
305307
//+kubebuilder:printcolumn:name="Remote URL",type="string",JSONPath=".spec.remoteURL"
306308
//+kubebuilder:printcolumn:name="URL",type="string",JSONPath=".status.url"
309+
//+kubebuilder:printcolumn:name="Ready",type="string",JSONPath=".status.conditions[?(@.type=='Ready')].status"
307310
//+kubebuilder:printcolumn:name="Age",type="date",JSONPath=".metadata.creationTimestamp"
308311

309312
// MCPRemoteProxy is the Schema for the mcpremoteproxies API

cmd/thv-operator/api/v1alpha1/mcpserver_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,8 @@ type OpenTelemetryMetricsConfig struct {
882882
// MCPServerStatus defines the observed state of MCPServer
883883
type MCPServerStatus struct {
884884
// Conditions represent the latest available observations of the MCPServer's state
885+
// +listType=map
886+
// +listMapKey=type
885887
// +optional
886888
Conditions []metav1.Condition `json:"conditions,omitempty"`
887889

cmd/thv-operator/api/v1alpha1/mcptelemetryconfig_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,8 @@ type MCPTelemetryConfigSpec struct {
9898
// MCPTelemetryConfigStatus defines the observed state of MCPTelemetryConfig
9999
type MCPTelemetryConfigStatus struct {
100100
// Conditions represent the latest available observations of the MCPTelemetryConfig's state
101+
// +listType=map
102+
// +listMapKey=type
101103
// +optional
102104
Conditions []metav1.Condition `json:"conditions,omitempty"`
103105

cmd/thv-operator/api/v1alpha1/virtualmcpcompositetooldefinition_types.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ type VirtualMCPCompositeToolDefinitionStatus struct {
3939
ObservedGeneration int64 `json:"observedGeneration,omitempty"`
4040

4141
// Conditions represent the latest available observations of the workflow's state
42+
// +listType=map
43+
// +listMapKey=type
4244
// +optional
4345
Conditions []metav1.Condition `json:"conditions,omitempty"`
4446
}

0 commit comments

Comments
 (0)