gf-feedback

name	gf-feedback
description	Track, fix, verify, and test any bugs, improvements, or gaps reported against an OpenSpec change. MUST use this skill whenever user reports problems, defects, issues, bugs, or gaps related to existing implementations, even if they don't explicitly say "feedback" or mention OpenSpec.
compatibility	Requires openspec CLI, Go toolchain, and gf-review skill.

Feedback: Structured Fix, Verification & Test Coverage Loop

When users discover bugs or improvement points after implementation, this skill captures those issues, organizes them into a traceable task list in tasks.md, systematically fixes and verifies each one, and ensures every behavior-changing fix is covered by focused unit tests.

Core principles:

1. Spec is the source of truth — Spec-level changes require spec update before task recording
2. Write it down first, then fix it — Every issue gets recorded before any code change
3. Every fix deserves a test — Behavior-changing code changes require unit test coverage in the affected package

---

Workflow

1. Identify Target Change

CRITICAL:

1. Always append to existing active changes. Only create new change when none exist.
2. An active change is any change directory that still exists directly under openspec/changes/ and has not been moved into openspec/changes/archive/. Do not treat status: complete, all tasks checked off, or similar completion signals as "inactive" until archive actually happens.
3. Regardless of whether the feedback content is related to the main functionality of the current active iteration, it MUST be appended to the current active iteration. This ensures all changes are tracked in a single change record for unified management and archiving.

openspec list --json
# Or: ls openspec/changes/ | grep -v archive

When the two signals disagree, prefer the filesystem rule:

• If a change directory still exists under openspec/changes/ and is not inside archive/, it is active.
• openspec list --json may still report such a change as status: complete; that only means implementation tasks are done, not that the change is inactive.
• Only archived changes under openspec/changes/archive/ are inactive.

Active Changes	Action
None	Create new change (see below)
One	Auto-select it, announce and proceed
Multiple	Ask user to select from list

When multiple active changes exist:

Multiple active changes detected. Which change should this feedback be appended to?

1. config-management — System config CRUD management
2. user-auth — User authentication enhancement

Please select 1 or 2:

When no active change exists:

1. Derive kebab-case name from feedback (e.g., "fix-menu-circular-ref")
2. If name exists, append suffix ("-2")
3. Create: openspec new change "<name>"
4. Generate minimal proposal.md (one paragraph summarizing context)
5. Skip design.md for pure bug fixes unless architectural changes needed

Announce: "Applying feedback fixes to change: "

---

2. Read Current Context

File	Purpose
tasks.md	Task structure, naming conventions, numbering
design.md	Architectural context
proposal.md	Feature scope and intent
specs/	Delta spec definitions

# Locate existing unit tests in the impacted package before adding a new one
rg --files <pkg-dir> | rg '_test\\.go$'

---

3. Analyze and Organize Issues

For each reported issue:

Classify by type:

• bug — Incorrect behavior, code doesn't match spec
• missing — Feature incomplete, gaps in implementation
• ux — UX improvement, no spec change needed
• test-gap — Missing test coverage only

Classify by spec impact:

Level	Definition	Action
implementation	Spec is correct, code is wrong	Fix code only
spec-level	Requirement missing/incomplete/changed	Update spec first, then fix
internal	No user-observable change	Fix code, test optional

Group related issues — Same root cause → single task with multiple verification points.

---

4. Update Delta Specs (for Spec-Level Issues Only)

For spec-level issues, update specs before recording tasks:

1. Identify affected capability: specs/<capability>/spec.md
2. Apply delta operation:

<!-- ADDED: New requirement -->
### Requirement: Parent Selector Circular Prevention
The system SHALL disable the current menu and all its descendants in the parent selector
to prevent circular references.

#### Scenario: Edit menu with children
WHEN user edits a menu that has child menus
THEN the parent selector SHALL disable the current menu and all descendant menus

<!-- MODIFIED: Changed requirement (include full original block) -->
### Requirement: Import Error Handling
The system SHALL display error messages when import fails.
**MODIFIED:** Error messages SHALL include row number, field name, and validation failure reason.

<!-- REMOVED: Deprecated requirement -->
### Requirement: Legacy Import Format
The system SHALL support legacy CSV format.
**REMOVED:** This format is no longer supported.
**Migration:** Use the new CSV format with header row.

---

5. Write Task List to tasks.md

Append a Feedback section to tasks.md:

## Feedback

- [ ] **FB-1**: Parent selector allows circular references in menu edit
- [ ] **FB-2**: Import error messages lack row and field details
- [ ] **FB-3**: No test coverage for reset password feature

Numbering: Sequential FB-1, FB-2, etc. Continue from last number if section exists.

One line per task — No sub-fields. Analysis happens during fix phase.

Confirm with user before writing to file.

Test coverage planning (internal):

• Behavior-changing code change → Unit test required
• Internal-only optimization → Unit test optional unless logic risk increased
• Prefer extending the nearest *_z_unit*_test.go or *_test.go in the same package

---

6. Execute Fixes (Loop)

For each task:

a. Announce: ## Fixing FB-X: <issue title>

b. Investigate — Read source files, confirm root cause

c. Implement — Minimal, focused fix following existing patterns

d. Write/update unit tests — Prefer the affected package's existing *_z_unit*_test.go or *_test.go files and keep assertions focused on the changed logic

e. Assess Impact Scope (MANDATORY)

After implementing, identify regression risk:

Change Type	Map To Tests
Package-level logic	Targeted test for changed function/method + package regression tests
Shared utility	Utility package unit tests + highest-value dependent package tests already covering reuse
DB/DAO logic	DAO/model package unit tests with focused fixtures, mocks, or test helpers
Public API validation	Handler/service package unit tests that assert the changed validation path
Refactor without behavior change	Existing package tests that prove behavior parity

# Example: Find unit tests related to a changed symbol or package
rg -l "GenDao|gdao" . -g '*_test.go'

Announce:

### Impact Analysis for FB-X
- Modified: cmd/gf/internal/cmd/gendao/gendao.go
- Affected package: cmd/gf/internal/cmd/gendao
- Unit tests: cmd/gf/internal/cmd/gendao/gendao_test.go
- Regression command: go test ./cmd/gf/internal/cmd/gendao -run 'TestGenDao'

f. Verify (MANDATORY before marking complete)

1. Run new/updated unit tests for this task → must pass
2. Run ALL identified package-level regression tests → must pass
3. Only then: mark task [x] in tasks.md

If regression fails:

• Fix inline if related to current change
• Add as new FB task if separate issue

g. Run review — Invoke gf-review skill after completion

---

7. Comprehensive Verification

After all fixes:

1. Aggregate regression tests from all tasks
2. Run full set in single pass
3. Report:

### Comprehensive Verification Results
- Total tests: N
- Passed: N
- Failed: N (list with details)
- Regression tests: all passed ✓ / X failures

If failures → add new FB tasks, loop to Step 6.

---

8. Report Completion

## Feedback Complete

**Change:** <name>
**Issues reported:** X
**Issues fixed:** Y/X
**Tests added:** Z unit tests / focused assertions
**Regression tests run:** R tests across N packages
**Verification:** all passed / N issues remaining

### Fixed This Session
- [x] FB-1: <title> ✓ (unit test: TestGenDao_FiltersInvalidTables | package: ./cmd/gf/internal/cmd/gendao ✓)
- [x] FB-2: <title> ✓ (unit test: existing package coverage extended | package: ./cmd/gf/internal/cmd ✓)

### Remaining (if any)
- [ ] FB-3: <title> — blocked by <reason>

---

Edge Cases

Situation	Handling
Single issue	Still follow full workflow
Missing test cases only	Classify as test-gap, implement tests
Fix reveals more issues	Add as new FB tasks
"Bug" is actually feature request	Re-classify as spec-level, update specs first
Unit test not feasible (docs/spec only)	Note reason explicitly and skip only when no runtime code changes exist
Multiple feedback rounds	All tasks in single Feedback section, sequential numbering

---

Guardrails

• Append to active change if exists — Never create new change when active ones exist
• Specs before tasks for spec-level issues — Update delta specs first
• Write tasks before fixing — Never code without recording
• Confirm task list with user — User validates analysis
• Minimal fixes — No refactoring beyond issue scope
• Behavior-changing fix needs unit test — No exceptions unless the change is docs/spec only
• No green check without green unit tests — Mark [x] only after tests pass
• Impact analysis mandatory — Every fix requires package-level regression test identification
• Regression failures block completion — Must resolve before marking done
• Update tasks.md in real time — Mark complete immediately after verification
• Match file language — Use same language as existing content in target file