fix(store): pointer-based stores, concurrency and robustness fixes

[Copilot]

The library had several correctness and concurrency bugs: mutations to stores were silently lost due to value-copy semantics, systemModCache was unprotected against concurrent access, and Config could panic on type mismatch via unchecked reflection.

Storage: value → pointer semantics

August.storage was map[string]AugustStore. GetStore returned &localCopy, so pointer-receiver mutations (e.g. Set, Delete) operated on a throwaway copy.

// before: mutations on the returned pointer were lost
storage map[string]AugustStore
return &store, nil  // pointer to local copy

// after: returns the canonical stored pointer
storage map[string]*AugustStore
return store, nil

Concurrency: systemModCache data race

event() appended to parent.systemModCache and handleModCacheSkip iterated/deleted from it — both without holding any lock.

• event() now wraps the append with as.parent.mu.Lock()/Unlock()
• handleModCacheSkip() now acquires a.mu.Lock() for the full scan+delete
• eventFunc is called outside the lock to avoid re-entrant deadlock if the callback calls back into August

populateRegistry: deadlock + panic fixes

The function held a.mu.Lock() across store.loadFromFile() → event() → a.mu.Lock() — a self-deadlock. Fixed by reading config/registry data under a brief RLock, releasing before file I/O.

File extension check file.Name()[len-len(format):] could also panic on short filenames; replaced with strings.HasSuffix.

GetStore: wrong lock + copy return

Was using a write lock for a pure read; switched to RLock and eliminated the redundant else branch.

Config: reflection-based set → explicit switch

reflect...Set(reflect.ValueOf(v)) panics if the type doesn't match exactly. Replaced with a switch on AugustConfigOption with type assertions and a proper error return.

GetAll: deferred unlock inside loop

Each iteration deferred an RUnlock, piling up N defers. Lock is now acquired once before the loop.

Original prompt

This PR fixes multiple correctness, concurrency, and robustness bugs in the solafide-dev/august repository so the library behaves as intended.

Summary of issues found and changes requested:

1. storage map stores AugustStore values (map[string]AugustStore) which causes GetStore to return pointer to a copy. Methods with pointer receivers then operate on the copy and mutations are lost. Fix: change August.storage to map[string]*AugustStore and update all usages (Register, GetStore, populateRegistry, initStorage, Run watcher loop) so they store and return pointers.

Files to change: august.go, augustStore.go

2. Concurrency on systemModCache is not protected. AugustStore.event appends to parent.systemModCache without synchronization and August.handleModCacheSkip iterates/removes entries without locking. Fix: protect systemModCache accesses with the August.mu mutex (use parent.mu.Lock/Unlock around appends) and acquire the same lock in handleModCacheSkip.

Files to change: august.go, augustStore.go

3. GetStore currently uses a.mu.Lock() for reads and returns pointer to copy; change to RLock and return stored pointer (after changing storage type). Also improve error handling.

File: august.go

4. populateRegistry file extension handling can panic for short filenames and the file extension check is brittle. Add length checks and only attempt to trim extension when it matches. Also when store directory doesn't exist (shouldn't happen after initStorage) handle errors gracefully.

File: august.go

5. GetAll defers Unlock inside a loop causing multiple defers and inefficient locking. Acquire read lock once for the duration of the loop instead.

File: augustStore.go

6. August.Config uses reflect to set options which can panic when types don't match. Replace with an explicit switch on AugustConfigOption to set fields with type assertions and proper error handling.

File: august.go

7. Misc: minor code cleanup and comments.

Tests: There are no unit tests in the repo. This PR focuses on correctness fixes. I will:

• Modify august.go and augustStore.go as described.
• Run go vet and go build locally to ensure code compiles.
• Add concise commit message and PR title: "fix(store): use pointer-based stores, concurrency and robustness fixes"

Please open a pull request in solafide-dev/august with these changes targeting the default branch. If you want me to target a specific branch or include additional changes (e.g., add unit tests, CI), tell me and I'll update the PR.

Changes to implement (detailed):

• august.go:

  • change storage type to map[string]*AugustStore
  • update Init() to initialize storage as make(map[string]*AugustStore)
  • update GetStore to RLock and return stored pointer (or error)
  • update Register to store &AugustStore{...}
  • update populateRegistry to handle short filenames and only process files whose extension matches "."+a.config.Format; call store.loadFromFile on pointer
  • update handleModCacheSkip to acquire lock (a.mu.Lock / Unlock) while iterating and removing
  • replace Config reflection-based field set with explicit type-checked assignment for supported keys (StorageDir string, Verbose bool, Format string, FSNotify bool)

• augustStore.go:

  • in event(), wrap append to parent.systemModCache with as.parent.mu.Lock()/Unlock()
  • in GetAll(), acquire read lock once before loop (as.mu.RLock(); defer as.mu.RUnlock())
  • ensure other usages of as.event, loadFromFile, saveToFile work with pointer-based parent store map

If this plan looks good I will implement the changes and open the PR.

The following is the prior conversation context from the user's chat exploration (may be truncated):

User: Update my August Library to be functional

Assistant: GitHub Copilot Chat Assistant here. I can help—first I need a few clarifying details so I do the right work.

Questions:

1. By “functional” do you mean:

   • Make the library build and tests pass locally/CI?
   • Fix runtime bugs so it works for a particular use-case?
   • Update it to support a newer language/runtime version?
   • Something else (describe)?

2. Do you want me to:

   • Inspect the repository and report what’s broken and how to fix it, or
   • Make code changes and open a pull request with fixes?

3. Which branch should I target (default branch if you don’t specify)? If you want a PR, do you want it against main/master or a specific branch?

4. Any constraints or priorities (keep API compatibility, support only Node X / Python X / Go X, add tests, add CI)?

If you want me to proceed, I’ll:

• Scan the repo for build/test failures, obvious runtime issues, outdated deps, and missing CI.
• Provide a prioritized list of fixes with proposed code changes.
• (If approved) implement the fixes and open a PR with tests and notes.

Tell me which option you prefer and confirm whether I should scan the repo now.

User: Scan it, find bugs, fix bugs, open a PR. Make it not suck. Lol

Assistant: GitHub Copilot Chat Assistant here ...

This pull request was created from Copilot chat.

[Copilot]

Pull request overview

This PR addresses multiple correctness and concurrency issues in the August storage library, primarily ensuring store mutations persist (pointer semantics) and removing data races / deadlocks around file watching and registry population.

Changes:

• Switches August.storage from value to pointer semantics (map[string]*AugustStore) and updates GetStore/Register accordingly.
• Adds synchronization around systemModCache to eliminate concurrent access races.
• Hardens startup loading/configuration: avoids deadlocks in populateRegistry, uses strings.HasSuffix for extension checks, and replaces reflection-based config setting with explicit type-checked options.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File	Description
go.mod	Promotes fsnotify to a direct dependency and cleans up requires.
august.go	Pointer-based store map, safer GetStore, concurrency fixes for mod cache, safer populateRegistry, config setter refactor.
augustStore.go	Fixes inefficient locking in GetAll and synchronizes systemModCache writes in event().

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

populateRegistry calls store.loadFromFile(id) but ignores the returned error, which can silently skip corrupted/unreadable entries and leave the in-memory store partially populated. Please handle/propagate the error (or at least log it) so startup failures are visible to callers.

Suggested change

	store.loadFromFile(id)
	if err := store.loadFromFile(id); err != nil {
	log.Printf("Error loading file: %s for registry %s as ID %s: %v", fname, name, id, err)
	return err
	}

[Copilot]

Error message grammar: "does not exists" should be "does not exist".

Suggested change

	return fmt.Errorf("store %s does not exists", name)
	return fmt.Errorf("store %s does not exist", name)

[Copilot]

Changing Config to return an error is a breaking API change for downstream users (previously it was void). If you need to keep compatibility, consider adding a new method (e.g., ConfigE/SetConfig) that returns an error while keeping the old signature as a wrapper (possibly logging or panicking on invalid types), or bumping the module major version if breaking changes are intended.