Skip to content

Commit 079b913

Browse files
authored
feat(#96): linker cleanup — LinkerResult, slug-matching, dead code removal (#97)
## What ships - **LinkerResult with Source** — sync now reports whether an issue was resolved from branch name or cache - **Slug-aware branch matching** — `feat/42-my-change` only resolves for `my-change`, not all changes - **Dead code removed** — MarkerResolver (expensive API call) and ExternalResolver (placeholder) gone - **Pull simplified** — uses BranchResolver directly instead of full Linker chain - **Site** — added linker feature to features.json - **npm** — bumped to 0.10.0 ## Tests 395 tests pass, 2 dead tests removed (ExternalResolver, resolveOpenSpecDir).
1 parent 47cedb8 commit 079b913

10 files changed

Lines changed: 404 additions & 614 deletions

File tree

cmd/specsync/main.go

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -256,6 +256,7 @@ func runSync(args []string) {
256256
Reconcile: *reconcile,
257257
CloseCompleted: *closeCompleted,
258258
Project: target,
259+
Linker: buildSyncLinker(*repo, providers),
259260
})
260261
if err != nil {
261262
fail(err)
@@ -351,9 +352,6 @@ func runPull(args []string) {
351352
StatusMapping: statusMapping,
352353
}
353354

354-
// Auto-resolve issue from branch name when -issue is not provided.
355-
linker := buildPullLinker(*repo)
356-
357355
if *worktree {
358356
runPullWithWorktree(*issue, *change, *repo, *dryRun, target, *worktreeDir)
359357
return
@@ -366,7 +364,6 @@ func runPull(args []string) {
366364
Slug: *change,
367365
DryRun: *dryRun,
368366
Project: target,
369-
Linker: linker,
370367
})
371368
if err != nil {
372369
fail(err)
@@ -703,6 +700,23 @@ func detectProvider(provider string) (string, string) {
703700
return "github", ""
704701
}
705702

703+
// buildSyncLinker builds a ChainLinker for sync with discovery resolvers
704+
// in priority order: branch only. The cache is NOT a resolver here — the
705+
// provider loop already reads the cache for each provider. Including the
706+
// cache in the Linker would return a ref from one provider and use it as
707+
// a fallback for another, causing cross-provider ref pollution.
708+
func buildSyncLinker(repo string, providers []specsync.WorkProvider) specsync.Linker {
709+
var resolvers []specsync.Resolver
710+
711+
// Branch resolver — slug-aware to prevent resolving all changes to the
712+
// same issue when syncing multiple changes at once.
713+
if repo != "" {
714+
resolvers = append(resolvers, specsync.NewBranchResolver(repo))
715+
}
716+
717+
return specsync.NewChainLinker(resolvers...)
718+
}
719+
706720
// makeProvider builds the selected work provider, substituting a dry-runner that
707721
// prints commands instead of executing them when dryRun is set. github
708722
// (default) targets repo (auto-detect when empty); beads drives the local `bd`
@@ -725,18 +739,6 @@ func makeProvider(repo string, dryRun bool, provider string) specsync.WorkProvid
725739
}
726740
}
727741

728-
// buildPullLinker creates a Linker for the pull command. It resolves the issue
729-
// from the current branch name (e.g., feat/42-change → #42) so -issue is
730-
// optional when on an issue-linked branch.
731-
func buildPullLinker(repo string) specsync.Linker {
732-
r, err := getRepoName(context.Background(), repo)
733-
if err != nil {
734-
return nil
735-
}
736-
br := specsync.NewBranchResolver(r)
737-
return specsync.NewChainLinker(br)
738-
}
739-
740742
// parseStatusMapping parses "-status-map" (falling back to $SPECSYNC_STATUS_MAP)
741743
// into per-stage Status-name overrides. The format is comma-separated
742744
// stage=Name pairs where stage is active, complete, or archived; Status names

linker.go

Lines changed: 77 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,24 @@ import (
88
"strings"
99
)
1010

11+
// LinkerResult holds a resolved Ref and the source that produced it
12+
// (e.g., "branch", "cache"). Used by sync for dry-run visibility.
13+
type LinkerResult struct {
14+
Ref *Ref
15+
Source string // human-readable source label
16+
}
17+
1118
// Linker resolves a change to its issue ref by consulting multiple sources
1219
// in priority order. The first hit wins; the result is cached for the
1320
// duration of the sync run.
1421
type Linker interface {
15-
// Resolve returns the Ref for the given change directory. Returns nil if
16-
// no resolver found a match.
17-
Resolve(ctx context.Context, changeDir string) (*Ref, error)
22+
// Resolve returns the LinkerResult for the given change directory, or
23+
// (nil, nil) if no resolver found a match.
24+
Resolve(ctx context.Context, changeDir string) (*LinkerResult, error)
1825
// ResolveFromContext resolves without a change directory, using only
1926
// the context (e.g., current branch name). Returns (nil, nil) if not
2027
// supported by this linker.
21-
ResolveFromContext(ctx context.Context) (*Ref, error)
28+
ResolveFromContext(ctx context.Context) (*LinkerResult, error)
2229
}
2330

2431
// ChainLinker tries each resolver in order until one returns a non-nil Ref.
@@ -30,9 +37,9 @@ type ChainLinker struct {
3037
// Resolver is a single link resolution strategy. Returns (nil, nil) if it
3138
// cannot resolve (not a match), or (nil, err) if it encounters an error.
3239
type Resolver interface {
33-
// Resolve returns the Ref if this resolver can match, or (nil, nil) to
34-
// pass to the next resolver.
35-
Resolve(ctx context.Context, changeDir string) (*Ref, error)
40+
// Resolve returns the LinkerResult if this resolver can match, or
41+
// (nil, nil) to pass to the next resolver.
42+
Resolve(ctx context.Context, changeDir string) (*LinkerResult, error)
3643
}
3744

3845
// NewChainLinker creates a ChainLinker with the given resolvers in priority
@@ -41,28 +48,28 @@ func NewChainLinker(resolvers ...Resolver) *ChainLinker {
4148
return &ChainLinker{resolvers: resolvers}
4249
}
4350

44-
func (c *ChainLinker) Resolve(ctx context.Context, changeDir string) (*Ref, error) {
51+
func (c *ChainLinker) Resolve(ctx context.Context, changeDir string) (*LinkerResult, error) {
4552
for _, r := range c.resolvers {
46-
ref, err := r.Resolve(ctx, changeDir)
53+
result, err := r.Resolve(ctx, changeDir)
4754
if err != nil {
4855
return nil, fmt.Errorf("resolver: %w", err)
4956
}
50-
if ref != nil {
51-
return ref, nil
57+
if result != nil && result.Ref != nil {
58+
return result, nil
5259
}
5360
}
5461
return nil, nil
5562
}
5663

57-
func (c *ChainLinker) ResolveFromContext(ctx context.Context) (*Ref, error) {
64+
func (c *ChainLinker) ResolveFromContext(ctx context.Context) (*LinkerResult, error) {
5865
for _, r := range c.resolvers {
5966
if cr, ok := r.(contextResolver); ok {
60-
ref, err := cr.ResolveFromContext(ctx)
67+
result, err := cr.ResolveFromContext(ctx)
6168
if err != nil {
6269
return nil, fmt.Errorf("resolver: %w", err)
6370
}
64-
if ref != nil {
65-
return ref, nil
71+
if result != nil && result.Ref != nil {
72+
return result, nil
6673
}
6774
}
6875
}
@@ -71,11 +78,13 @@ func (c *ChainLinker) ResolveFromContext(ctx context.Context) (*Ref, error) {
7178

7279
// contextResolver is a resolver that can resolve without a change directory.
7380
type contextResolver interface {
74-
ResolveFromContext(ctx context.Context) (*Ref, error)
81+
ResolveFromContext(ctx context.Context) (*LinkerResult, error)
7582
}
7683

7784
// BranchResolver extracts an issue number from the current git branch name.
7885
// The pattern is configurable; default: `feat/(\d+)-.*` or `fix/(\d+)-.*`.
86+
// When given a changeDir, it verifies the slug matches the branch suffix
87+
// to avoid resolving all changes to the same issue.
7988
type BranchResolver struct {
8089
repo string // "owner/name" — required to build the URL
8190
pats []*regexp.Regexp
@@ -87,21 +96,13 @@ type BranchResolver struct {
8796
func NewBranchResolver(repo string, pats ...*regexp.Regexp) *BranchResolver {
8897
if len(pats) == 0 {
8998
pats = []*regexp.Regexp{
90-
regexp.MustCompile(`^(?:feat|fix)/(\d+)-.*`),
99+
regexp.MustCompile(`^(?:feat|fix)/(\d+)-(.+)$`),
91100
}
92101
}
93102
return &BranchResolver{repo: repo, pats: pats}
94103
}
95104

96-
func (b *BranchResolver) Resolve(_ context.Context, changeDir string) (*Ref, error) {
97-
return b.resolveFromBranch()
98-
}
99-
100-
func (b *BranchResolver) ResolveFromContext(_ context.Context) (*Ref, error) {
101-
return b.resolveFromBranch()
102-
}
103-
104-
func (b *BranchResolver) resolveFromBranch() (*Ref, error) {
105+
func (b *BranchResolver) Resolve(ctx context.Context, changeDir string) (*LinkerResult, error) {
105106
if b.repo == "" {
106107
return nil, nil
107108
}
@@ -113,50 +114,63 @@ func (b *BranchResolver) resolveFromBranch() (*Ref, error) {
113114

114115
for _, pat := range b.pats {
115116
matches := pat.FindStringSubmatch(branch)
116-
if len(matches) < 2 {
117+
if len(matches) < 3 {
117118
continue
118119
}
119120
num := matches[1]
121+
branchSuffix := matches[2]
122+
123+
// When resolving for a specific change, verify the slug matches
124+
// the branch suffix. This prevents all changes from resolving to
125+
// the same issue when syncing multiple changes at once.
126+
if changeDir != "" {
127+
slug := filepath.Base(changeDir)
128+
if slug != branchSuffix {
129+
continue
130+
}
131+
}
132+
120133
url := fmt.Sprintf("https://github.com/%s/issues/%s", b.repo, num)
121-
return &Ref{
122-
Provider: "github:" + b.repo,
123-
ID: num,
124-
URL: url,
134+
return &LinkerResult{
135+
Ref: &Ref{
136+
Provider: "github:" + b.repo,
137+
ID: num,
138+
URL: url,
139+
},
140+
Source: "branch",
125141
}, nil
126142
}
127143

128144
return nil, nil
129145
}
130146

131-
// MarkerResolver resolves via the <!-- specsync:change=<slug> --> marker in
132-
// an existing issue body. It uses the provider's Find method.
133-
type MarkerResolver struct {
134-
provider WorkProvider
135-
}
136-
137-
// NewMarkerResolver creates a MarkerResolver that uses the given provider's
138-
// Find method to search for the specsync marker.
139-
func NewMarkerResolver(provider WorkProvider) *MarkerResolver {
140-
return &MarkerResolver{provider: provider}
141-
}
142-
143-
func (m *MarkerResolver) Resolve(ctx context.Context, changeDir string) (*Ref, error) {
144-
openspecDir := resolveOpenSpecDir(changeDir)
145-
if openspecDir == "" {
147+
func (b *BranchResolver) ResolveFromContext(ctx context.Context) (*LinkerResult, error) {
148+
if b.repo == "" {
146149
return nil, nil
147150
}
148-
c, err := LoadChangeBySlug(openspecDir, filepath.Base(changeDir))
149-
if err != nil {
151+
152+
branch, err := currentBranch()
153+
if err != nil || branch == "" {
150154
return nil, nil
151155
}
152-
ref, err := m.provider.Find(ctx, c.Slug)
153-
if err != nil {
154-
return nil, fmt.Errorf("marker find: %w", err)
156+
157+
for _, pat := range b.pats {
158+
matches := pat.FindStringSubmatch(branch)
159+
if len(matches) < 2 {
160+
continue
161+
}
162+
num := matches[1]
163+
url := fmt.Sprintf("https://github.com/%s/issues/%s", b.repo, num)
164+
return &LinkerResult{
165+
Ref: &Ref{
166+
Provider: "github:" + b.repo,
167+
ID: num,
168+
URL: url,
169+
},
170+
Source: "branch",
171+
}, nil
155172
}
156-
return ref, nil
157-
}
158173

159-
func (m *MarkerResolver) ResolveFromContext(_ context.Context) (*Ref, error) {
160174
return nil, nil
161175
}
162176

@@ -170,74 +184,38 @@ func NewCacheResolver(providerName string) *CacheResolver {
170184
return &CacheResolver{providerName: providerName}
171185
}
172186

173-
func (c *CacheResolver) Resolve(_ context.Context, changeDir string) (*Ref, error) {
187+
func (c *CacheResolver) Resolve(_ context.Context, changeDir string) (*LinkerResult, error) {
174188
refs, err := loadRefs(changeDir)
175189
if err != nil {
176190
return nil, fmt.Errorf("cache read: %w", err)
177191
}
178192

179193
// Try the exact provider key first.
180194
if ref, ok := refs[c.providerName]; ok {
181-
return &ref, nil
195+
return &LinkerResult{Ref: &ref, Source: "cache"}, nil
182196
}
183197

184198
// Try the legacy bare "github" key for github: prefixed providers.
185199
if strings.HasPrefix(c.providerName, "github:") {
186200
if ref, ok := refs["github"]; ok {
187-
return &ref, nil
201+
return &LinkerResult{Ref: &ref, Source: "cache"}, nil
188202
}
189203
}
190204

191205
return nil, nil
192206
}
193207

194-
func (c *CacheResolver) ResolveFromContext(_ context.Context) (*Ref, error) {
195-
return nil, nil
196-
}
197-
198-
// ExternalResolver is a configurable hook for external relation sources
199-
// (e.g. MCP, database, or custom logic).
200-
type ExternalResolver struct {
201-
fn func(ctx context.Context, changeDir string) (*Ref, error)
202-
}
203-
204-
// NewExternalResolver creates an ExternalResolver with the given function.
205-
func NewExternalResolver(fn func(ctx context.Context, changeDir string) (*Ref, error)) *ExternalResolver {
206-
return &ExternalResolver{fn: fn}
207-
}
208-
209-
func (e *ExternalResolver) Resolve(ctx context.Context, changeDir string) (*Ref, error) {
210-
return e.fn(ctx, changeDir)
211-
}
212-
213-
func (e *ExternalResolver) ResolveFromContext(_ context.Context) (*Ref, error) {
214-
return nil, nil
215-
}
216-
217-
// currentBranch returns the current git branch name, or "" if not on a branch.
218-
func currentBranch() (string, error) {
208+
// currentBranchFn is the function that returns the current git branch name.
209+
// It is a variable so it can be overridden in tests.
210+
var currentBranchFn = func() (string, error) {
219211
out, err := runGit(context.Background(), "rev-parse", "--abbrev-ref", "HEAD")
220212
if err != nil {
221213
return "", err
222214
}
223215
return strings.TrimSpace(out), nil
224216
}
225217

226-
// resolveOpenSpecDir returns the openspec/ directory that contains the given
227-
// change directory. It walks up from changeDir looking for a parent named
228-
// "changes" or "archive", then returns that parent.
229-
func resolveOpenSpecDir(changeDir string) string {
230-
dir := changeDir
231-
for {
232-
parent := filepath.Dir(dir)
233-
base := filepath.Base(dir)
234-
if base == "changes" || base == "archive" {
235-
return parent
236-
}
237-
dir = parent
238-
if dir == "/" || dir == "" || dir == "." {
239-
break
240-
}
241-
}
242-
return ""
218+
// currentBranch returns the current git branch name, or "" if not on a branch.
219+
func currentBranch() (string, error) {
220+
return currentBranchFn()
243221
}

0 commit comments

Comments
 (0)