refactor(paths): absorb trimmed-path guards into pathspec.PathSpec (links-va-001-pathspec-drq.1)#203
Conversation
…inks-va-001-pathspec-drq.1) PathSpec carries a possibly-absent filesystem path as a value: trimmed once at construction ([LAW:single-enforcer]), absence as the zero value, and absence propagating through Or/Join as data instead of per-site guards ([LAW:dataflow-not-control-flow]). Deviations from the epic sketch, per the recorded va-001 precedents: - Dropped the sketch's 'valid bool' field — derivable from value; two fields that can disagree violate [LAW:one-source-of-truth]. - Added Or/Join — both demanded by consumers (env-override chains, dir+name joins with absence propagation). - workspace.normalizeRemoteName excluded: a git remote name is not a path ([FRAMING:representation]), and it is already its own single enforcer — no duplicated variance there. Deleted the dead firstNonEmptyTrimmed helper instead. - Absorbed a 9th pellet the audit missed: cli planEject re-trimming GlobalPath's result. Verified: go test ./... clean; old-vs-new binary output byte-identical on quickstart/backlog/queue/show against the live store.
There was a problem hiding this comment.
Z.ai Coding Agent Review
Clean refactoring that introduces pathspec.PathSpec to absorb scattered strings.TrimSpace(path) == "" guards into a single authority. The type is well-designed: unexported value field enforces construction through New, zero value represents absence, and Or/Join propagate absence as data rather than re-guarding.
No LAW violations introduced. The new type correctly realizes [LAW:single-enforcer] (trimming once, in New), [LAW:types-are-the-program] (absence is the zero value, not a runtime check), and [LAW:dataflow-not-control-flow] (absence flows through Or/Join as data). Behavioral tests satisfy [LAW:behavior-not-structure].
Pre-existing note: ejectResult.Path and quickstartRefreshItem.Path remain string fields, so the PathSpec guarantee is unwound at the DTO boundary via .String(). This is a reasonable seam choice for JSON-serializable output structs, but if these structs ever gain path-computation logic, they'd benefit from carrying PathSpec directly.
✅ Approved
Implements links-va-001-pathspec-drq.1 (epic links-va-001-pathspec-drq, audit finding F-20). Second ticket in the va-001 absorption family, mirroring the shape piloted by #202.
What
New leaf package
internal/pathspec.PathSpeccarries a possibly-absent filesystem path as a value: whitespace-trimmed exactly once in the constructor ([LAW:single-enforcer]), absence represented as the zero value, and absence propagating throughOr/Joinas data rather than per-site guards ([LAW:dataflow-not-control-flow]).Absorbed sites (old per-site
strings.TrimSpace(x) == ""logic deleted, not left beside):internal/config/config.go—globalConfigPath,projectConfigPath(env-override chains becomeNew(env).Or(default)),mergeConfigFile,Load's workspace-root checkinternal/templates/templates.go—GlobalPath,ProjectPath,ActiveOverride(now return/traffic inPathSpec),readOptionalFile,projectTemplatePath,globalTemplatesDirinternal/cli/quickstart_eject.go— a 9th pellet the audit missed:planEjectre-trimmedGlobalPath's resultDeviations from the epic sketch (per the precedents recorded on this ticket)
valid boolfrom the sketch's struct — it is derivable fromvalue; two fields that must agree is[LAW:one-source-of-truth]violated inside the type itself.OrandJoin— both demanded by actual consumers;Joinpreserves absence (filepath.Join("", x)would invent a relative path from nothing).workspace.normalizeRemoteNameexcluded. The audit counted it as a site, but a git remote name is not a path — naming itPathSpecwould make the type lie ([FRAMING:representation]). It is also already its own single enforcer (one function, two callers): no duplicated variance to absorb. Deleted the deadfirstNonEmptyTrimmedhelper in that file instead — zero callers, pure residue of this pattern.Verification
go test ./...clean;go vetclean.pathspec(trim, absence,Or,Joinpropagation) andTestLoadAbsentWorkspaceRootFallsThroughpinning the behavior the deleted guards carried ([LAW:behavior-not-structure]).quickstart/backlog/queue/showagainst the live store.gofmt -ldelta vs master: empty (the two flagged touched files were already unformatted on master).