From f3fb4d8e911210d53cb43e8e3163b24bda70cca1 Mon Sep 17 00:00:00 2001 From: Dennison Date: Thu, 30 Jul 2026 17:06:30 +0200 Subject: [PATCH] Fix repeated default registry tests --- docs/logs/engineering-log.md | 16 +++++ docs/logs/long-term-thinking-log.md | 15 +++++ ...1042-default-registry-repeat-impact-map.md | 66 +++++++++++++++++++ ...issue-1042-default-registry-repeat-plan.md | 61 +++++++++++++++++ docs/plans/INDEX.md | 2 + internal/workspace/workspace_test.go | 8 ++- 6 files changed, 166 insertions(+), 2 deletions(-) create mode 100644 docs/plans/2026-07-30-issue-1042-default-registry-repeat-impact-map.md create mode 100644 docs/plans/2026-07-30-issue-1042-default-registry-repeat-plan.md diff --git a/docs/logs/engineering-log.md b/docs/logs/engineering-log.md index b0f9ef16..9b064f0c 100644 --- a/docs/logs/engineering-log.md +++ b/docs/logs/engineering-log.md @@ -88,6 +88,22 @@ the full harness package passes in normal and race modes; and `./scripts/test-regression.sh` passes normal, full race, and `coveragegate` at 85.6% with zero uncovered functions. +## 2026-07-30 (Default Workspace Registry Test Isolation — Issue #1042) + +- Symptom: `TestDefaultRegistry_Functions` fails on its second in-process + invocation with `workspace: implementation already registered`. +- Cause: every invocation registers the fixed + `test-default-impl-unique-12345` name in the intentionally persistent package + registry. +- Planned fix: assign each invocation a process-local atomic suffix while + retaining the same-name duplicate assertion inside that invocation. +- Verification contract: focused normal/race `-count=100`, workspace + normal/race, and full repository normal/race/coverage gate. +- Result: the focused normal/race tests passed at `-count=100`, the complete + workspace package passed normal/race at `-count=5`, and + `./scripts/test-regression.sh` passed with 85.6% total coverage and zero + uncovered functions. + ## 2026-07-30 (Workflow Subscription Cancellation Test — Issue #1035) - Symptom: the full race gate failed in diff --git a/docs/logs/long-term-thinking-log.md b/docs/logs/long-term-thinking-log.md index c81c1801..39fe2fc5 100644 --- a/docs/logs/long-term-thinking-log.md +++ b/docs/logs/long-term-thinking-log.md @@ -74,6 +74,21 @@ stress plus the full repository gate. - Guardrails: issue-first isolated worktree, deterministic red evidence, test-only synchronization, no production lifetime change, and no sleeps. +## 2026-07-30 (Default Workspace Registry Test Isolation — Issue #1042) + +- Command intent: clear the repeatable workspace-package blocker discovered by + the mandatory post-merge coverage gate. +- User intent: do not merge the cron/callback GUI work over any red accepted + baseline, even when the failure is in an unrelated test fixture. +- Success definition: every in-process invocation owns a unique registry name, + retains the duplicate-registration contract, and passes repeated/race/full + gates. +- Guardrails: issue-first isolated worktree, test-only atomic identity, no + production reset/unregister or weakened assertion. +- Outcome: the atomic invocation identity retained every public registry + assertion and cleared focused stress, package stress, normal, race, and + coverage verification. + ## 2026-07-30 (Workflow Subscription Cancellation Test — Issue #1035) - Command intent: clear the race-gate blocker exposed while verifying the diff --git a/docs/plans/2026-07-30-issue-1042-default-registry-repeat-impact-map.md b/docs/plans/2026-07-30-issue-1042-default-registry-repeat-impact-map.md new file mode 100644 index 00000000..c9cf7e5b --- /dev/null +++ b/docs/plans/2026-07-30-issue-1042-default-registry-repeat-impact-map.md @@ -0,0 +1,66 @@ +# Cross-Surface Impact Map: Issue #1042 Default Registry Repeatability + +## Task + +- Task / issue: isolate repeated default-registry test invocations, #1042. +- Plan: `2026-07-30-issue-1042-default-registry-repeat-plan.md`. +- Owner: Codex. +- Status: implemented and fully verified; merge pending. + +## Current Ownership, Callers, and Data Flow + +- Entry: `TestDefaultRegistry_Functions`. +- Source of truth: package-level `defaultRegistry`; top-level + `Register/List/New` delegate to it. +- Search: all `workspace.Register`, default-registry tests, init registration, + and reset/unregister APIs. +- Conclusion: production correctly retains registrations; only the fixed test + identity incorrectly assumes one invocation per process. + +## Config, API, CLI, and Tools + +- Config/env/defaults: none. +- API/CLI/wire/tools: production top-level registry API remains exercised. +- Errors: retain explicit `ErrAlreadyExists` and provisioning checks. + +## Persistence and Compatibility + +- State: process-local test registry only. +- Schemas/migrations/caches: none. +- Compatibility: no runtime change. + +## Lifecycle, Security, and Reliability + +- Concurrency: process-local atomic counter assigns invocation ownership. +- Auth/privacy/secrets: none. +- Failure/recovery: assertion output includes the generated name. + +## Product and Integration Surfaces + +- Server/TUI/web/macOS/providers: none. +- Automation: normal, race, repeated, and coverage gates become deterministic. +- UX/accessibility: none. + +## Deployment and Operations + +- Deployment/migration/flags: none. +- Rollback: revert if first/duplicate registration is no longer distinguished. +- Operator docs: none. + +## Regression Tests + +- Red: focused `-count=2` fixed-name failure. +- Green: focused normal/race `-count=100`. +- Controls: List contains the invocation's name; New provisions exactly its + factory; duplicate still fails. +- Full: workspace normal/race and `./scripts/test-regression.sh`. + +## Documentation and Handoff + +- Update engineering log, long-term log, plan, impact map, and plans index. +- No public docs. + +## Warning Check + +- All runtime/product surfaces are explicitly unaffected because the change is + confined to fixture identity. diff --git a/docs/plans/2026-07-30-issue-1042-default-registry-repeat-plan.md b/docs/plans/2026-07-30-issue-1042-default-registry-repeat-plan.md new file mode 100644 index 00000000..91aa2054 --- /dev/null +++ b/docs/plans/2026-07-30-issue-1042-default-registry-repeat-plan.md @@ -0,0 +1,61 @@ +# Plan: Make the Default Registry Contract Test Repeatable + +## Context + +- Governing GitHub issue: #1042. +- Problem: `TestDefaultRegistry_Functions` registers a fixed name in a + process-global registry, so the second `-count` invocation fails before it + can test its own duplicate-registration contract. +- User impact: an unrelated workspace package failure blocks the accepted + cron/callback GUI merge chain. +- Constraint: keep production registry semantics and every top-level API + assertion unchanged. + +## Scope + +- In scope: allocate a collision-free name for each test invocation. +- Out of scope: production reset/unregister APIs, registry ownership, package + serialization, and runtime behavior. + +## Documentation Contract + +- Feature status: test-only bug repair in implementation. +- Public docs affected: none. +- Evidence: engineering and long-term logs, plan, impact map, plans index. + +## Test Plan (TDD) + +- Red: existing focused test at `-count=2` fails deterministically on the second + invocation with `ErrAlreadyExists`. +- Green: focused normal/race at `-count=100`; each invocation still asserts + first registration, duplicate rejection, listing, and provisioning. +- Full: workspace normal/race and repository normal/race/coverage gate. + +## Cross-Surface Impact Map + +- See `2026-07-30-issue-1042-default-registry-repeat-impact-map.md`. + +## Implementation Checklist + +- [x] Create contract-complete bug #1042. +- [x] Capture deterministic red evidence. +- [x] Record registry ownership/search evidence. +- [x] Write plan and impact map before code. +- [x] Implement invocation-unique identity. +- [x] Run focused stress and full gates. +- [ ] Merge through a closing PR. + +## Verification + +- Focused normal and race tests passed at `-count=100`. +- The complete workspace package passed normal and race tests at `-count=5`. +- `./scripts/test-regression.sh` passed normal, race, and the 85.6% coverage + gate with zero uncovered functions. + +## Risks and Mitigations + +- Risk: unique names could stop testing duplicate rejection. +- Mitigation: register the same invocation-local name twice and retain the + explicit `ErrAlreadyExists` assertion. +- Risk: a timing-derived name could collide. +- Mitigation: use a process-local atomic sequence, not wall-clock time. diff --git a/docs/plans/INDEX.md b/docs/plans/INDEX.md index ed55ff18..1ad32473 100644 --- a/docs/plans/INDEX.md +++ b/docs/plans/INDEX.md @@ -10,6 +10,8 @@ - `2026-07-30-issue-1044-ask-status-race-impact-map.md` — Cross-surface impact map for Issue #1044. - `2026-07-30-issue-1039-worktree-containment-ci-plan.md` — Issue #1039 planned synchronization of live worktree containment assertions with terminal cleanup. - `2026-07-30-issue-1039-worktree-containment-ci-impact-map.md` — Cross-surface impact map for Issue #1039. +- `2026-07-30-issue-1042-default-registry-repeat-plan.md` — Issue #1042 planned repeatable top-level workspace registry contract test. +- `2026-07-30-issue-1042-default-registry-repeat-impact-map.md` — Cross-surface impact map for Issue #1042. - `2026-07-30-issue-1035-workflow-subscription-cancel-plan.md` — Issue #1035 planned deterministic workflow subscription-cancellation regression repair. - `2026-07-30-issue-1035-workflow-subscription-cancel-impact-map.md` — Cross-surface impact map for Issue #1035. - `2026-07-30-issue-1026-feedback-direct-publish-plan.md` — Issue #1026 attached-image, publish-by-default GitHub feedback intake (implemented and verified; promotion pending). diff --git a/internal/workspace/workspace_test.go b/internal/workspace/workspace_test.go index 3b7ed934..8ed89c97 100644 --- a/internal/workspace/workspace_test.go +++ b/internal/workspace/workspace_test.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "sync" + "sync/atomic" "testing" "go-agent-harness/internal/workspace" @@ -13,6 +14,8 @@ import ( // Compile-time interface compliance check. var _ workspace.Workspace = (*mockWorkspace)(nil) +var defaultRegistryTestSequence atomic.Uint64 + // mockWorkspace is a test implementation of the Workspace interface. type mockWorkspace struct { mu sync.Mutex @@ -279,8 +282,9 @@ func TestRegistry_Concurrent(t *testing.T) { // -------------------------------------------------------------------------- func TestDefaultRegistry_Functions(t *testing.T) { - // Use unique names to avoid conflicts with other tests sharing the default registry. - const implName = "test-default-impl-unique-12345" + // The default registry intentionally persists for the process lifetime, so + // each `-count` invocation must own a distinct name. + implName := fmt.Sprintf("test-default-impl-%d", defaultRegistryTestSequence.Add(1)) f, created := newMockFactory("http://harness.local", "/ws/default") if err := workspace.Register(implName, f); err != nil {