[kavix]

Problem

Two shell processes can stage, move aside the same version, restore the shared .replaced directory, and write policy and active state independently. The final state can mix decisions from different installs (#240).

Solution

Serialize module activation per product namespace using the shell's advisory file lock (internal/lockfile) in the state root:

• Added Store.LockPath(namespace) in internal/modules/store.go to return the namespace lock path (<store>/<namespace>.lock). Per ADR 0007, the lock sits beside the namespace directory so rollback cleanup of a failed first install never unlinks the lock file.
• Wrapped Installer.activate() in internal/install/install.go inside lockfile.With(), bounding wait time under 15 * time.Second (above declarationTimeout of 10s plus margin to accommodate running the installed executable).
• Contention and filesystem lock errors are mapped to storeFailure.
• Added <namespace>.lock to the storage layout in docs/architecture.md.

Acceptance

• Concurrent install test: TestConcurrentInstallsActivateCleanlyMatchingExactlyOneInstall in internal/install/install_test.go and TestConcurrentInstallsMatchExactlyOneInstall in test/acceptance/moduleinstall_test.go.
• Failure-injection test: TestInterruptedInstallLeavesPreviousVersionActive in internal/install/install_test.go exercising same-version reinstall rollback from .replaced.

Closes #240

[coderabbitai]

Important

• 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 2b6def9a-78a0-483f-a79c-98ed26aed009

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

_{Comment @coderabbitai help to get the list of available commands.}

[CLAassistant]

All committers have signed the CLA.

[kanushka]

The concurrent-install test passes and the namespace lock addresses the main race. Before merging, could you please:

• run gofmt on the three new test files;
• add the persistent <namespace>.lock file to the storage layout in docs/architecture.md;
• strengthen the failure-injection test to exercise a same-version reinstall after the existing version has moved to .replaced. The current different-version validation failure does not test that rollback path.

[kavix]

Thanks for the review @kanushka! I have updated the branch with the requested changes

[kanushka]

The lock deadline can expire while a healthy activation still holds the lock.

[kavix]

Thanks @kanushka! Updated lockDeadline to 15s (above declarationTimeout), updated the comment to mention the executable run, and rebased onto main.

[kanushka]

Thanks, the lock is in the right place and all earlier comments are addressed. Two things before merging:

• The concurrency tests don't catch the race. With the install.go from main and the tests from this PR, TestConcurrentInstallsActivateCleanlyMatchingExactlyOneInstall passes 300/300 and TestConcurrentInstallsMatchExactlyOneInstall passes 20/20. Both install different versions (1.0.0 and 1.1.0), so they never share .replaced. Could you make them run concurrent installs of the same version, or add a test hook that pauses activation between staging and the policy write, and confirm they fail without the lock?
• The busy-lock error is wrong (internal/install/install.go:353-358). When the lock is held, lockfile.ErrBusy goes through storeFailure. The user is told "locking the module namespace failed: lockfile: …" and is asked to check the state-directory permissions. Please map busy the same way internal/contexts/save.go:169 does: another invocation is updating the product, retry the command. Please keep the lockfile: package name out of the message as well, and add a test for the busy path.

Non-blocking, fine as follow-ups:

• Store.Remove (wso2 product remove) doesn't take the namespace lock, so a remove that runs during an install can leave a half-removed namespace.
• updateOne reads the channel and pin before taking the lock, so a pin that lands in between can be overwritten.
• Per ADR 0007, declarationTimeout should point back to lockDeadline, since the two values are coupled.