feat: implement shared workspace for agent teams (#62)#74
feat: implement shared workspace for agent teams (#62)#74Deepak-negi11 wants to merge 3 commits into
Conversation
- Add workspace module with artifact store, context store, lock manager, change history, and filesystem helpers (atomic writes, exclusive locks) - Artifact CRUD with version history, rollback, and conflict detection - Exclusive per-artifact locking with agent ownership - Shared context API (decisions, constraints, glossary) - Task tracking with status lifecycle and dashboard - Integrate SharedWorkspace into AgentLoop with subagent task tracking - 23 integration tests
There was a problem hiding this comment.
Pull request overview
Introduces a new core::workspace module that provides a filesystem-backed shared workspace (artifacts, shared context, locking, history, and active task tracking) and integrates it into the agent runtime to support multi-agent collaboration.
Changes:
- Added
core/src/workspace/*implementing artifact CRUD/versioning, context store, lock manager, history log, atomic write + exclusive lock helpers, and aSharedWorkspacefaçade. - Integrated
SharedWorkspaceintoAgentLoopand added automatic task tracking for spawned subagents. - Added integration tests covering workspace lifecycle, concurrency controls, history, and dashboard aggregation.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| core/src/workspace/types.rs | Defines workspace data types (artifacts, locks, history, tasks, dashboard) and base64 serde for bytes. |
| core/src/workspace/mod.rs | Implements SharedWorkspace façade tying together artifact/context/locks/history/tasks and recording history. |
| core/src/workspace/artifact.rs | Artifact persistence with CRUD, snapshots, listing filters, rollback, and per-artifact mutation locking. |
| core/src/workspace/context.rs | JSON-backed shared context stores with exclusive mutation locks and atomic writes. |
| core/src/workspace/lock.rs | Per-artifact lock files with acquire/release/force-release/status/list operations. |
| core/src/workspace/history.rs | Append-only NDJSON change log with read-all and read-recent helpers. |
| core/src/workspace/fs.rs | Exclusive lock primitive and “atomic write” helpers used by workspace subsystems. |
| core/tests/workspace_test.rs | Integration test suite validating workspace behavior end-to-end. |
| core/src/agent/loop_.rs | Adds workspace: Arc<SharedWorkspace> and subagent task lifecycle updates. |
| core/src/error.rs | Adds WorkspaceError and plumbs it into MofaclawError. |
| core/src/lib.rs | Exposes the workspace module and re-exports SharedWorkspace. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| file.flush().await?; | ||
| drop(file); | ||
|
|
||
| fs::rename(&temp_path, path).await?; | ||
| Ok(()) |
There was a problem hiding this comment.
atomic_write uses tokio::fs::rename to move the temp file onto an existing target. On Windows, rename fails if the destination already exists, so updates to JSON files (tasks/context/artifacts metadata) can error in CI. Consider removing/replacing the destination first while holding a lock, or using a Windows-safe atomic replace (e.g., platform-specific replace, or a crate that wraps ReplaceFileW).
| Err(err) if err.kind() == std::io::ErrorKind::AlreadyExists => { | ||
| if Instant::now() >= deadline { | ||
| return Err(WorkspaceError::Busy(path.display().to_string()).into()); | ||
| } | ||
| sleep(Duration::from_millis(10)).await; |
There was a problem hiding this comment.
ExclusiveLock can leave stale .lock files behind on process crash (Drop won't run), which will cause all future callers to spin until the 5s deadline and then permanently fail with Busy. Consider writing metadata (pid/timestamp) into the lock file and supporting stale-lock eviction/TTL, or using an OS-level lock that is released automatically when the process exits.
| // Write metadata (without bulky content duplicated – content lives in the .content file) | ||
| atomic_write_json(&meta, artifact).await?; | ||
| atomic_write(&content, &artifact.content).await?; |
There was a problem hiding this comment.
The comment says metadata is written "without bulky content duplicated", but atomic_write_json(&meta, artifact) serializes the full Artifact including content (base64), and .content is also written. Additionally, get() only reads the JSON, so the .content file isn't used for reads. Consider either removing .content entirely or changing the JSON format to exclude content and load it from the .content file.
| let subdirs: Vec<&str> = if let Some(ref at) = filter.artifact_type { | ||
| vec![match at { | ||
| ArtifactType::Design => "designs", | ||
| ArtifactType::Code => "code", | ||
| ArtifactType::Review => "reviews", |
There was a problem hiding this comment.
list() treats filter.artifact_type only as a directory selector. For ArtifactType::Other("..."), this will return all artifacts in artifacts/other/ (regardless of the inner string), and it also never verifies that artifact.artifact_type == filter.artifact_type after deserialization. Consider applying an explicit artifact.artifact_type equality check when filter.artifact_type is set (especially for Other).
| // Check lock – if locked by someone else, reject | ||
| if let Some(lock) = self.locks.is_locked(id).await? { | ||
| if lock.agent != agent { | ||
| return Err(crate::error::WorkspaceError::ArtifactLocked { | ||
| artifact_id: id, | ||
| held_by: lock.agent, | ||
| } | ||
| .into()); | ||
| } | ||
| } | ||
|
|
||
| let artifact = match self | ||
| .artifacts | ||
| .update(id, content, agent.clone(), expected_version, strategy) |
There was a problem hiding this comment.
Lock enforcement is subject to a TOCTOU race: the code checks locks.is_locked() and then performs the mutation in artifacts.update(...). Another process can acquire the lock between those steps, allowing an update/delete/rollback to proceed while locked. Consider performing the lock check under the same per-artifact mutation lock used by ArtifactStore (or making LockManager::acquire/release also participate in that lock) so the check+mutate sequence is atomic.
| let mut file = OpenOptions::new() | ||
| .create(true) | ||
| .append(true) | ||
| .open(&self.log_path) | ||
| .await?; |
There was a problem hiding this comment.
ChangeHistory::record appends without any cross-process serialization. Multiple writers can interleave writes to changes.log and corrupt NDJSON lines (especially since write_all may perform multiple underlying writes). Consider guarding appends with ExclusiveLock (similar to tasks/context) or another file-locking mechanism so each record is written atomically.
Summary
Implements a shared, persistent workspace where multiple agents can collaborate on common artifacts, share project knowledge, and maintain consistency across a team. This is the foundation for multi-agent coordination in mofaclaw.
Closes #62
Parent: #58 (Multi-Agent Collaboration System)
Problem
When multiple agents work on related tasks, they operate in isolation. There is no shared location for intermediate artifacts, no way to track who changed what, no conflict detection when two agents modify the same resource, and no shared understanding of project decisions or constraints.
Solution
A new
core::workspacemodule that provides a complete shared workspace backed by the filesystem at~/.mofaclaw/workspace/. The workspace is structured into four subsystems: Artifact Management, Context Sharing, Locking, and Change History, all unified behind aSharedWorkspacefaçade and integrated into the agent runtime loop.Architecture
Directory Structure
Module Overview
typesworkspace/types.rsartifactworkspace/artifact.rscontextworkspace/context.rslockworkspace/lock.rshistoryworkspace/history.rsfsworkspace/fs.rsmodworkspace/mod.rsSharedWorkspacefaçadeDetailed Feature Breakdown
1. Artifact Management (
artifact.rs)Each artifact is a typed, versioned, named binary blob owned by an agent.
Data model:
Operations:
artifact_type,owner, orname_contains(case-insensitive substring match); returns results sorted newest-firstStorage format:
<id>.json— Full artifact metadata (including base64-encoded content)<id>.content— Raw content bytes<id>.v<N>.json— Immutable version snapshot for rollbackVersion History & Rollback:
<id>.v<N>.jsonsnapshotget_versions(id)returns all historical snapshots sorted oldest → newestrollback(id, target_version, agent)restores a previous version's content as a new head version (creating a new version number, not rewriting history)2. Conflict Detection & Resolution (
artifact.rs+types.rs)Supports optimistic concurrency control via expected version checks:
update_artifact_if_version(id, content, agent, expected_version)— Rejects the update if the artifact's current version doesn't matchexpected_versionupdate_artifact_with_strategy(id, content, agent, expected_version, strategy)— Full control: passOverwriteto force through a stale updateChangeAction::ConflictError types:
3. Exclusive Locking (
lock.rs)Per-artifact locks for exclusive access, using atomic
create_newfile creation for race-free acquisition:<artifact-id>.lock.jsonatomically; if the file already exists, checks if the same agent owns it (idempotent re-lock) or rejects withArtifactLockederrorLockNotOwnederror if a different agent triesLock enforcement is integrated into the
SharedWorkspacefaçade —update_artifact,delete_artifact, androllback_artifactall check locks before proceeding. If a different agent holds the lock, the operation is rejected.4. Context Sharing (
context.rs)Three JSON files maintain shared project knowledge:
Decisions (
context/decisions.json):Operations:
list_decisions(),add_decision(title, rationale, agent),remove_decision(id)Constraints (
context/constraints.json):Operations:
list_constraints(),add_constraint(name, description, agent),remove_constraint(id)Glossary (
context/glossary.json):Operations:
list_glossary(),add_glossary_entry(term, definition, agent),remove_glossary_entry(term)All context mutations go through
ExclusiveLockto prevent concurrent corruption.5. Change History (
history.rs)Append-only NDJSON (newline-delimited JSON) log at
history/changes.log:SharedWorkspacefaçaderead_all()returns full history oldest-firstread_recent(n)returns the last N entries newest-first6. Task Tracking (
mod.rs)Active task management stored in
state/active_tasks.json:Operations:
add_task(),add_task_with_status(),update_task_status(),remove_task(),list_tasks()All task mutations are serialized via
ExclusiveLockand recorded in change history.7. Dashboard (
mod.rs)dashboard(recent_changes)returns an aggregated view of the workspace state.8. Filesystem Safety (
fs.rs)Two low-level primitives used throughout the module:
ExclusiveLock — Cross-process file lock using
create_new(atomic file creation):Drop(RAII pattern)Atomic Writes —
atomic_write()andatomic_write_json():rename()to the target path (atomic on most filesystems)9. Agent Loop Integration (
agent/loop_.rs)SharedWorkspaceis integrated into the core agent runtime:workspace: Arc<SharedWorkspace>onAgentLoopwith_agent()andwith_agent_and_tools()constructorsspawn_subagent()is called, a task is automatically created withInProgressstatus. On completion, it's updated toCompleted; on failure, toFailed.workspace()returns&Arc<SharedWorkspace>for use by tools and handlersError Handling
New error types in
core/src/error.rs:Integrated into
MofaclawErrorvia#[from]:MofaclawError::Workspace(WorkspaceError).Files Changed
New (8 files, +2258 lines)
core/src/workspace/types.rscore/src/workspace/artifact.rscore/src/workspace/context.rscore/src/workspace/lock.rscore/src/workspace/history.rscore/src/workspace/fs.rscore/src/workspace/mod.rscore/tests/workspace_test.rsModified (3 files)
core/src/error.rsWorkspaceErrorenum (6 variants) +Workspacevariant inMofaclawErrorcore/src/lib.rspub mod workspace;+pub use workspace::SharedWorkspace;core/src/agent/loop_.rsworkspacefield, initialization in constructors, subagent task tracking, accessor methodTesting
23 integration tests in
core/tests/workspace_test.rs:test_workspace_creates_directory_treetest_artifact_create_and_gettest_artifact_update_bumps_versiontest_artifact_deletetest_artifact_list_with_filterstest_version_history_and_rollbacktest_lock_and_unlocktest_lock_idempotent_same_agenttest_lock_nonexistent_artifact_failstest_unlock_wrong_agent_failstest_decisions_crudtest_constraints_crudtest_glossary_crudtest_history_records_operationstest_task_lifecycletest_reopen_preserves_datatest_artifact_serde_roundtriptest_version_conflict_rejectedtest_overwrite_strategy_accepts_staletest_delete_blocked_by_locktest_rollback_through_facadetest_context_via_facade_records_historytest_dashboardAcceptance Criteria Checklist