feat(workspace): a published deliverable lands in the shared tree, and the tab follows live (#552, #327) - #572
Merged
oxoxDev merged 19 commits intoAug 11, 2026
Conversation
…tinyhumansai#552) A published deliverable is about to live on two surfaces: the artifact chain, which stays the authoritative version history, and one node in the company's shared workspace tree, which holds the current body. This is the link between them. Per version rather than per record, for the same reason `run_id` is: an operator may delete a published node, and the next publish materializes a fresh one. A record-level field would rewrite history and claim the old versions had always lived in the new node. Additive on the JSON blob every backend stores, so no migration — a pre-tinyhumansai#552 record loads with None.
The test dropped an instance and re-acquired the root in the same instant. That is stricter than the lock promises. `store::lock`'s "The fork window" section says so outright: the lock belongs to the open file description, so between fork() and exec() a child transiently shares every descriptor its parent held. This binary spawns subprocesses in sibling tests, and one landing in the release window keeps the just-released root locked for the microseconds until the child execs and its O_CLOEXEC copy closes. Retry with a 10ms backoff up to 2s — several orders of magnitude above the real window — and fail with the attempt count and the last refusal if it expires. Verified not to be a blindfold: holding the first instance instead of dropping it still turns the test red, in 2s, naming the real error. No change to the lock itself; it was behaving as documented.
…ee (tinyhumansai#552) The seam between a task artifact and the workspace tree, in both directions: materialize() puts a publish at Agents/<agent>/<task>/<source>, minting the agent's folder on demand; mirror_node_edit() records an edit to a published node back onto the artifact chain. The chain stays authoritative and the node is a projection of its latest version. The two failure modes are not symmetric — a stale node is visible and self-healing, an unrecorded edit is silent and corrupts human_edit_diff — so writes go chain-first wherever there is a choice. Interior path segments become folders, so specs/a.md and docs/a.md are two deliverables rather than one. Ambiguity is refused, never guessed, matching the fail-closed rule workspace_scaffold applies one level up. In the default build, not the harness: the console's workspace and artifact routes are always compiled and could not reach an openhuman-gated module.
… artifact store (tinyhumansai#552) The publish drain filed a versioned ArtifactRecord and stopped, so an explicitly published file was reachable only from the Artifacts tab of one card — invisible to the operator browsing the tree and to every other agent, whose sole view of shared company state is that tree. The drain now also materializes the file at Agents/<agent>/<task>/<source> and stamps the node id onto the version it wrote. A re-publish revises the same node; an operator's deletion is honoured with a fresh one. A failed node write logs at error and records the artifact anyway: dropping an explicitly published deliverable over tree bookkeeping would be far worse than one the operator has to reach through the Artifacts tab. deps.workspace == None stays byte-identical to the old behaviour, which is what keeps every pre-existing publish test on the unchanged path. The drain remains the single write site — the claim sites and PendingPublishQueue are untouched, so an unclaimed publish still refuses and creates neither.
…inyhumansai#552) A note in the workspace tree may now be the projection of a task artifact, so the two console routes that write one have to keep the pair honest. The workspace PUT records an operator's save of a published note as an operator version, which is precisely the datum human_edit_diff exists to answer; without it the history would claim the agent's draft shipped unchanged. Chain first, then the node — a stale node is visible and heals on the next write, an unrecorded edit is silent and permanent. The artifact append pushes the new body into the deliverable's note, or the tree keeps serving a draft the history has superseded. A mirror failure warns rather than failing a request that already recorded the version. Both appended versions inherit the node id, without which mirroring would silently stop after one hop. An ordinary note matches no artifact and takes the unchanged path.
… chain (tinyhumansai#552) A note in the shared tree may be another agent's published deliverable. An overwrite the artifact chain never saw is the same silent divergence a console save would cause, one surface over — and the Artifacts tab and human_edit_diff read the chain, not the tree. Recorded as an agent version stamped with the writing agent's id, so a teammate's overwrite can never masquerade as the human edit the port exists to isolate. It is not a publish: no queue, no claim, tinyhumansai#445 untouched. Node first here, and forced rather than chosen — the write carries the compare-and-swap token, so until it returns there is nothing to record and a version appended earlier would claim an edit a stale-revision refusal never made. A mirror failure warns rather than reporting a landed write as failed. The artifact store is threaded in as an optional builder argument; None keeps every read tool and every path-resolution test on the old path.
…sai#327) TaskCardChanged's counterpart for the note tree, missing for the same reason: nothing on the feed said the workspace had moved, so a console with the Workspace tab open saw an agent's write only on a manual refresh. Now that agents create notes and published deliverables land in the tree, that stale window is where most of the tree's activity happens. Prunable, unlike its Permanent board sibling, and argued in-code rather than assumed: the tree is the record, nothing addresses this by sequence, nothing folds it at boot, and its whole meaning is 're-read the tree'. Structural only on every surface — id and change word, never the node name. A note's name is operator-authored free text, and none of the four consumers (SSE, insight one-liners, the Medulla wire, the cycle classifier) is a place to hand it out. The SSE projection is deny-by-default, so the explicit arm there is what makes the frame reach the console at all.
BoardAnnouncer's counterpart for the note tree, wrapped in at the one place the store is chosen so the console routes, the agent tools, the publish drain and the seeder all announce without knowing they do. An emit per write site is the shape of the bug: correct only for the paths somebody remembered. A rename that changes nothing says nothing, compared against what the tree actually held rather than what the caller asked for. Deleting a folder is one frame, not one per descendant — the console re-reads the tree and finds everything that went with it. A refusing event log never fails the write. An overwrite always announces, unlike the board's identical-re-save suppression: a write advances the revision token the agent CAS compares against, so it has changed the node in the way that matters.
…nyhumansai#327) Boot's workspace scaffold now journals a WorkspaceChanged per reserved root, so a fresh company's log is no longer empty and its first cycle event no longer lands at seq 0. Three tests encoded that as a count or a hardcoded cycle id. They now filter to the event they are about, and the hosted cycle id is derived from SYSTEM_ROOTS so adding a root cannot silently desynchronise the scripted id from the one the runtime computes.
The tab could only learn about a write by refetching on refresh or window refocus. That was tolerable when the operator was the only writer; it is not now that agents create notes and published deliverables land in the tree, because most of what appears there appears while nobody is touching it. This subscriber carries the payload rather than a counter, unlike the board's: the tree is always refetched, but what happens to the OPEN note depends on which node moved — leave it, refetch it, or close the pane because it was deleted. Two rules with teeth. The open note is refetched only in read mode, so a live write can never clobber an operator's dirty buffer and in-flight autosave. And no toast for any change word: an autosaving editor fires one frame per settled keystroke burst, and toasting those would train the operator to dismiss the toasts that matter. handleEvent is exported and its routing pinned by a test — this switch has been bitten three times by a frame the host was already sending falling through to default: and vanishing with nothing to debug.
…humansai#552) A published deliverable also lives in the shared workspace tree, which is where teammates and the operator actually browse; without a control the two surfaces were related only in the host's data. Read off the shown version rather than the record, because the node id is per version: an older revision points at the note that held it, which the operator may since have deleted. The hash router follows hashchange, so setting the hash is the whole navigation — no navigate prop threaded down for one link. WorkspaceView resolves the landed id against the host, since only it knows which node ids exist.
…yhumansai#552) The drain materialized the note first and stored the version after, so a failed artifact write could leave the tree holding a body the version history had no record of. That is not a lesser bug than the one the chain-first rule was chosen to prevent — it is the same tinyhumansai#187 rot arriving through the tree: human_edit_diff computed from the chain goes quietly wrong rather than loudly broken. Needing the store to half-fail bounds how often that happens and not at all how bad it is, and on a data path a silent wrong answer outlives the incident that caused it. A re-publish inherits the node its previous version named, so the version is stored before the tree is touched. A fresh publish has no id to inherit, so its v1 is stored unlinked and a second write stamps the link once the node exists — which also means a note is now only ever created for a deliverable already recorded, so this path can no longer leave a note in the tree with no artifact behind it. A failed link is warned, not fatal: both surfaces already hold the body and only the pointer is missing, so failing would discard the rest of the batch to report something the next publish repairs — materialize find-or-creates by path and re-adopts the same note rather than duplicating. Pinned by failure injection rather than by comment. Reverting to node-first turns two of the new tests red, one of them asserting exactly the rot: the tree reads v2 while the chain holds only v1.
Collaborator
Author
|
Merge-order note for reviewers: this branch is stacked on #571, so its diff currently includes #571's three commits ( |
|
Warning Review limit reached
Next review available in: 44 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
📝 WalkthroughWalkthroughWorkspace provisioning now creates only reserved roots and lazily creates member folders. Workspace writes emit SSE events. Published artifacts mirror into workspace nodes and preserve node links across revisions. The operator UI refreshes workspace content and supports artifact-to-workspace navigation. ChangesWorkspace provisioning and event pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ArtifactPublisher
participant ArtifactStore
participant WorkspaceStore
participant WorkspaceAnnouncer
participant OperatorSSE
participant WorkspaceView
ArtifactPublisher->>ArtifactStore: Persist artifact version
ArtifactPublisher->>WorkspaceStore: Materialize or update workspace node
WorkspaceStore->>WorkspaceAnnouncer: Complete workspace mutation
WorkspaceAnnouncer->>OperatorSSE: Emit workspace_changed
OperatorSSE->>WorkspaceView: Forward nodeId and change
WorkspaceView->>WorkspaceStore: Refresh tree or open note
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (4)
src/server/graphql/test.rs (1)
841-849: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAssert the
mayaparent path.The assertion compares only node names. It can pass if
mayais created outsideAgents, while the workspace contract requires agent content underAgents/<agent-id>/.... Include the parent or path in this query, or add a targeted workspace-store assertion, so the test verifies placement as well as authorship.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/graphql/test.rs` around lines 841 - 849, Extend the test around the tree lookup to verify maya’s parent path is Agents/<agent-id>, not merely that a maya-named node exists. Query or inspect the parent/path metadata for maya and assert it belongs under Agents, while preserving the existing name and createdBy assertions.src/company/artifact_mirror.rs (1)
258-268: 🚀 Performance & Scalability | 🔵 TrivialNote the scan's real cost dimension before it needs an index.
published_record_for_nodeloads every artifact in the company on every workspace save. AnArtifactRecordcarries all of its version bodies, so the cost grows with total stored artifact content, not with the record count. The doc bounds it by "a task's drafts and posts, not a repository", which describes the record count only.When you add the index, a
node_id → artifact_idreverse map is enough; it avoids deserializing version bodies for a lookup that only needs one field.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/company/artifact_mirror.rs` around lines 258 - 268, Update published_record_for_node to avoid scanning and deserializing all artifact version bodies; use an indexed node_id-to-artifact_id reverse lookup that returns the matching artifact record directly. Revise the associated documentation or bounds to describe the cost in terms of stored artifact content, not merely record count, while preserving the existing optional result behavior.src/runtime/workspace_events.rs (1)
360-390: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for a parent-only move.
This test covers no-op and name changes. It does not cover the
parent_idcomparison inWorkspaceAnnouncer::rename_move.Create a folder, move
n-1into it withSome(Some("folder-id")), and assert oneupdatedevent. This protects the move behavior from regressing while rename behavior remains correct.Proposed test addition
+ let folder = WorkspaceNode { + kind: NodeKind::Folder, + ..note("f-1", "Specs", None) + }; + store.create(&co, &folder, None).await.unwrap(); + log.appended.lock().unwrap().clear(); + + store + .rename_move(&co, "n-1", None, Some(Some("f-1"))) + .await + .unwrap(); + assert_eq!( + changes(&log), + vec![("n-1".to_string(), "updated".to_string())] + );As per coding guidelines,
**/*.rs: Add focused tests with every behavior change.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/workspace_events.rs` around lines 360 - 390, Add parent-only move coverage to a_rename_announces_only_when_something_actually_moved: create a folder, call rename_move for n-1 with no name change and Some(Some("folder-id")) as the parent_id, then assert exactly one updated event. Keep the existing no-op and rename assertions intact.Source: Coding guidelines
src/runtime/cycle.rs (1)
2498-2513: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse journal-read bounds that cannot hide events after boot scaffolding.
The tests filter event types correctly, but fixed ten-event reads can truncate the records before filtering.
src/runtime/cycle.rs#L2498-L2513: read the complete test journal or derive the limit from boot records before selectingOperatorMessage.src/runtime/scheduler.rs#L391-L405: use a full or derived limit before selecting the firstScheduleFiredevent.src/runtime/scheduler.rs#L424-L430: use the same safe limit before counting the secondScheduleFiredevent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/runtime/cycle.rs` around lines 2498 - 2513, Update the journal reads in the cycle test at src/runtime/cycle.rs lines 2498-2513 and both scheduler tests at src/runtime/scheduler.rs lines 391-405 and 424-430 to use a complete journal or a limit derived from boot records, so scaffolding events cannot truncate records before filtering; preserve the existing OperatorMessage and ScheduleFired selection/counting assertions.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/spec/runtime/manifest.md`:
- Around line 276-280: Update the documentation describing Agents/<agent-id>/
creation to state that both direct agent writes and artifact_mirror::materialize
via ensure_agent_folder can create the folder; remove the claim that direct
writing is the only creator while preserving the operator-only rename and delete
behavior.
In `@frontend/src/components/app-shell.tsx`:
- Around line 271-279: Scope workspaceEvent to the active client and company in
AppShell: reset it when either client or company changes, or include and
validate those scope values before WorkspaceView consumes event.nodeId. Ensure
events from a previous company or client cannot affect the currently open
workspace node.
- Around line 271-279: Update the workspace event state around workspaceEvent
and its setter to preserve pending events rather than replacing an earlier event
with a later one. Use a bounded queue or aggregate that retains every relevant
change for the currently open node, and update WorkspaceView to process those
pending changes so refresh, rename/move, and deletion actions are not lost while
preserving the existing tick-based duplicate-event behavior.
In `@frontend/src/views/WorkspaceView.tsx`:
- Around line 435-446: Update the removal handling in the WorkspaceView event
flow after loadTree refreshes nodes: determine whether the open file still
exists, including when an ancestor folder removal event does not match openId.
When it is gone, preserve any unsaved draft in an explicit recovery state before
closing the pane, and coordinate the pending autosave/debounce with that state
so flush cannot issue a failed write or discard text. Expose a save-as-new-note
action for the recovered draft, and cover both direct and ancestor-folder
removals during active editing.
In `@src/harness/orchestrator.rs`:
- Around line 784-791: Add a focused test alongside the existing summarize_event
tests for the CompanyEvent::WorkspaceChanged arm, asserting the summary contains
the change value and node_id while excluding node names and workspace/body text.
In `@src/runtime/cycle.rs`:
- Around line 1268-1272: Add focused Rust tests covering the updated
classification of CompanyEvent::WorkspaceChanged: verify it produces no cycle
task or thread ID when alone, and remains neutral when mixed with a dispatch or
addressed message so those events are not disqualified. Place the tests near the
existing cycle_task_id and cycle_thread_id coverage and preserve current
behavior for other event types.
In `@src/server/ops/workspace.rs`:
- Around line 286-299: Update mirror_node_edit to distinguish artifact lookup
failures from append failures, allowing the caller to continue the ordinary note
write when the artifact store cannot be queried while still propagating failures
after a published record is identified. Preserve fail-closed behavior for
published deliverables, and adjust the route documentation if the intentional
behavior remains to state that ordinary note saves depend on artifact-store
availability.
In `@src/server/ops/write_test.rs`:
- Around line 5651-5654: Align the documentation for the test around the
success-path assertions with its actual coverage: remove the claim that it
verifies version ordering when a node write is refused, or move that claim to
the failure-injection test that exercises the refusal path. Only add failure
injection here if this test is intended to cover that behavior.
---
Nitpick comments:
In `@src/company/artifact_mirror.rs`:
- Around line 258-268: Update published_record_for_node to avoid scanning and
deserializing all artifact version bodies; use an indexed node_id-to-artifact_id
reverse lookup that returns the matching artifact record directly. Revise the
associated documentation or bounds to describe the cost in terms of stored
artifact content, not merely record count, while preserving the existing
optional result behavior.
In `@src/runtime/cycle.rs`:
- Around line 2498-2513: Update the journal reads in the cycle test at
src/runtime/cycle.rs lines 2498-2513 and both scheduler tests at
src/runtime/scheduler.rs lines 391-405 and 424-430 to use a complete journal or
a limit derived from boot records, so scaffolding events cannot truncate records
before filtering; preserve the existing OperatorMessage and ScheduleFired
selection/counting assertions.
In `@src/runtime/workspace_events.rs`:
- Around line 360-390: Add parent-only move coverage to
a_rename_announces_only_when_something_actually_moved: create a folder, call
rename_move for n-1 with no name change and Some(Some("folder-id")) as the
parent_id, then assert exactly one updated event. Keep the existing no-op and
rename assertions intact.
In `@src/server/graphql/test.rs`:
- Around line 841-849: Extend the test around the tree lookup to verify maya’s
parent path is Agents/<agent-id>, not merely that a maya-named node exists.
Query or inspect the parent/path metadata for maya and assert it belongs under
Agents, while preserving the existing name and createdBy assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: cd646c51-c9ea-4230-aab6-9eba35e02403
📒 Files selected for processing (34)
docs/modules/runtime/README.mddocs/spec/runtime/api.mddocs/spec/runtime/manifest.mddocs/spec/runtime/ports-console.mdfrontend/src/api/artifacts.tsfrontend/src/components/app-shell.tsxfrontend/src/hooks/use-events.tsfrontend/src/views/ArtifactsTab.tsxfrontend/src/views/WorkspaceView.tsxfrontend/test/unit/workspace-events.test.tssrc/app/boot.rssrc/brain/hosted/test.rssrc/brain/medulla/effects.rssrc/company/artifact_mirror.rssrc/company/mod.rssrc/company/workspace_agents.rssrc/company/workspace_scaffold.rssrc/harness/brain.rssrc/harness/build.rssrc/harness/mod.rssrc/harness/orchestrator.rssrc/harness/workspace_tools.rssrc/ports/artifacts.rssrc/ports/types.rssrc/runtime/builder.rssrc/runtime/cycle.rssrc/runtime/mod.rssrc/runtime/scheduler.rssrc/runtime/workspace_events.rssrc/server/graphql/test.rssrc/server/operator.rssrc/server/ops/artifacts.rssrc/server/ops/workspace.rssrc/server/ops/write_test.rs
💤 Files with no reviewable changes (1)
- src/company/workspace_agents.rs
8 tasks
Collaborator
Author
|
#571 is merged ( Rebased gates on the merged tree: |
…te (tinyhumansai#552) Since tinyhumansai#552 the workspace PUT consults the artifact store on every save, to ask whether the note is a published deliverable. That lookup propagated with `?`, so an artifact-store fault rejected the save of an ordinary note — one with no chain to protect, on a write that otherwise never touches that store. `mirror_node_edit` now separates *cannot record* from *cannot tell*. A refused append stays an `Err` and still fails closed: the store answered, so the node is known to be a deliverable and must not be written behind a version that was never appended. An unreadable store answers `Undetermined`, and the caller decides — the console PUT warns and saves, the agent tool warns as before. Folding the second case into the ordinary-note result is what would have retired the fail-closed guarantee without anything appearing to change, so the two are separate variants rather than one absent value.
… write (tinyhumansai#552) The doc comment claimed the ordering was asserted — that the version is there when the node write is refused — but the body only exercised the success path. Inject the refusal instead of correcting the comment down: an artifact stamped with a node id the tree does not have makes the lookup match and the append run, then the real store refuses the write. No mock, and the ordering that this route was designed around is now actually covered.
…inyhumansai#327) The new match arms changed cycle_task_id and cycle_thread_id without tests. Assert both halves of what neutral means: alone it claims no card and no thread, and in a mixed batch it does not disqualify the dispatch or the addressed message beside it — an agent writing a note mid-cycle is the ordinary case, and treating that write as a rival would strip the stamp.
…tinyhumansai#327) summarize_event's new arm had no coverage. Assert the exact string rather than its parts: the claim is that nothing else is in there, and an arm widened to look up the node's name would keep passing every `contains` check while leaking operator-authored free text onto the insight surface.
…older The direct-write path stopped being the only creator of Agents/<agent-id>/ in this branch: artifact_mirror::materialize calls ensure_agent_folder before it writes a published deliverable.
…mansai#327) WorkspaceAnnouncer emits one `removed` frame naming the node somebody deleted — a folder — never one per descendant. So an open note inside that folder disappeared with no frame that ever said its id, the effect compared ids, decided the frame was somebody else's, and returned. The pane stayed open on a note that was gone, holding a draft and an armed autosave whose only possible outcome was a 404. A direct removal cleared the draft with the debounce still armed. Disappearance is now settled against the refreshed tree, which is the only thing that knows about descendants. When the open note is gone the debounce is cancelled first, then any text the host has not already acknowledged moves into a banner that offers it back rather than leaving with the note it was written in.
![tinysweeper[bot]](https://avatars.githubusercontent.com/in/4526585?s=60&v=4){kind=small}
There was a problem hiding this comment.
tinysweeper found nothing blocking. Approving.
$0.3738 · 377,255 in / 86,606 out · 293,111 cached (78%) · z-ai/glm-5.2
critique: $0.1925 · 125,806 in / 53,509 out · 101,185 cached (80%) · z-ai/glm-5.2
security: $0.0799 · 108,514 in / 15,940 out · 86,960 cached (80%) · z-ai/glm-5.2
tests: $0.0636 · 70,529 in / 12,920 out · 50,605 cached (72%) · z-ai/glm-5.2
description: $0.0377 · 72,406 in / 4,237 out · 54,361 cached (75%) · z-ai/glm-5.2
| // re-reading it would only 404 and leave an error where a note was. Which | ||
| // notes vanished is settled against the refreshed tree, not against the | ||
| // frame's id; [`planOpenNote`] carries the reasoning. | ||
| useEffect(() => { |
There was a problem hiding this comment.
Live-writes effect acts on a stale openId after the tree refetch
The effect captures openId, mode, draft, and openFile?.content from the render in which event.tick changed, then acts on them after await loadTree({ silent: true }). If the operator opens a different note during that refetch, the plan was computed for the old note, and the actions are applied to whatever is now open. In the reload branch, loadFile(openId) loads the stale note, overriding the operator's new selection. In the vanished branch, closeVanished calls setOpenId(null), which closes the new note the operator just opened — even though that note is untouched. The eslint-disable acknowledges that openId is read-not-watched, but the consequence goes beyond replaying a stale frame: it mutates state the operator has since changed. A ref mirroring openId (checked after the await) would let the effect bail out when the operator has moved on.
**[RULE] ** ·
What this change touches29 files, +4245 -44 across 10 components. It reaches 3 untouched components (60 graph nodes walked). flowchart LR
n0["src/harness<br/>4 files +1020 -14"]:::changed
n1["src/company<br/>3 files +945 -1"]:::changed
n2["src/server<br/>4 files +650 -1"]:::changed
n3["src/runtime<br/>5 files +573 -9"]:::changed
n4["frontend/src<br/>5 files +444 -12<br/>1 finding"]:::flagged
n5["frontend/test<br/>2 files +305 -0"]:::changed
n6["src/ports<br/>2 files +238 -1"]:::changed
n7["src/app<br/>1 file +41 -2"]:::changed
n8["src/brain<br/>2 files +25 -2"]:::changed
n9["docs/spec<br/>1 file +4 -2"]:::changed
n10["frontend/src<br/>1 file reached"]:::impacted
n11["src<br/>1 file reached"]:::impacted
n12["src/ports<br/>1 file reached"]:::impacted
n2 -->|12 refs| n12
n0 -->|10 refs| n12
n3 -->|9 refs| n12
n0 -->|5 refs| n1
n2 -->|4 refs| n11
n3 -->|4 refs| n1
n3 -->|3 refs| n10
n3 -->|3 refs| n11
n8 -->|3 refs| n12
n12 -->|3 refs| n10
n2 -->|2 refs| n1
n2 -->|2 refs| n3
n2 -->|2 refs| n10
n8 -->|2 refs| n1
n0 -->|1 ref| n10
n0 -->|1 ref| n11
n1 -->|1 ref| n11
n3 -->|1 ref| n0
n8 -->|1 ref| n3
n11 -->|1 ref| n1
n11 -->|1 ref| n3
n11 -->|1 ref| n12
n12 -->|1 ref| n1
n12 -->|1 ref| n11
classDef changed fill:#0d4429,stroke:#238636,color:#e6edf3
classDef impacted fill:#161b22,stroke:#6e7681,color:#c9d1d9
classDef flagged fill:#5a1e02,stroke:#d93f0b,color:#ffffff
classDef blocking fill:#67060c,stroke:#f85149,color:#ffffff
Green: changed. Grey: untouched, reached through an import or a call. Orange: has findings. Red: has a finding that blocks the merge.
Changed files
|
tinysweeper
Bot
added
the
priority: p2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
publish_artifactdrained into theArtifactStoreonly, so a deliverable landed on its task card and nowhere else. The workspace tree is the one surface every agent can read, and there is no agent-facing artifact read tool anywhere insrc/harness/— so an agent asked to build on another agent's output had no way to reach it. Separately, workspace writes emitted no company event, so a file written while the operator watched the Workspace tab appeared only after a refresh.Publishing now writes both places: the versioned artifact on the card, and a note under
Agents/<agent>/<task>/in the shared tree, linked so the two are one object seen twice. The tab follows writes live.Closes #552. Closes #327.
API Or Behavior Changes
A published deliverable also becomes a workspace note. The drain calls
ensure_agent_folder(member folders are minted on first use, per #570), creates the<task-id>/child and any nested source folders, and writes the file.ArtifactVersiongainsworkspace_node_id(serde(default), additive on all three backends' JSON blob storage — no migration; pre-existing records load asNone). A second agent reads the result through the existingworkspace_read; no new tool was added, which is the point of routing through the tree rather than building a second reader.The artifact chain stays authoritative; the note is its current-body projection. The alternative — node as storage, artifact referencing it — would force versioning onto
WorkspaceStoreacross three backends and turn every artifact read into a two-store join. Invariant: the note never holds a body the version history has not recorded.Writes are chain-first, uniformly. A re-publish inherits the node its previous version named, stamps the link, stores the version, then mirrors into the note. A fresh publish has no id to inherit, so v1 is stored unlinked, the node is minted, and a second write stamps the link. Ordering is pinned by failure injection, not by comment: an
ArtifactStorethat refusesupsertfrom the Nth call leaves the note on the previous body, and a refused first publish creates nothing in the tree at all. Reverting to node-first makes both tests fail with the divergence they exist to catch.Residual, stated rather than hidden: if the second artifact write fails after the node write succeeded, the version and the note both hold the same body and only the link between them is missing.
materializere-adopts by path, so the next publish of the same source reuses the same node and repairs the link — pinned byan_unlinked_first_publish_is_repaired_by_the_next_one. It requires a store failing mid-sequence, not a store being down, and it never misreports what a human changed.Edits on either surface stay in step. A console PUT to a published note appends an
Operatorversion before writing the note;append_versionmirrors the new body into the linked note; an agent'sworkspace_writeover a published note records onto the chain as anAgentversion. Without these, an edit on one surface is invisible to the other and #187'shuman_edit_diffsilently rots. Appended versions inherit the node id on all three paths — otherwise the next reverse lookup finds nothing and mirroring stops after one hop.New
CompanyEvent::WorkspaceChanged { node_id, change }(opened/updated/removed), classified Prunable — a pure refetch signal: the tree is the sole truth, nothing addresses it by sequence, nothing reads it back at boot, and it is high volume. This diverges fromTaskCardChanged's Permanent, where the board-lifecycle trail is itself evidence; flagging it since the default direction is keep-forever. AWorkspaceAnnouncerdecorator emits at the store — mirroringBoardAnnouncer, whose docs name #327 as "the obvious next one" — so publish dual-writes, console PUTs, agent writes and deletes all announce with no per-caller emits. Folder deletion announces once, not once per descendant. Projected onto the operator SSE stream asworkspace_changed; the stream is deny-by-default, so the explicit arm is what makes #327 actually fix.Console: the Workspace tab refetches the tree on a live event and refetches the open file only in read mode, so an in-progress edit is never clobbered; a removed open node closes the pane. Artifact versions carrying a node id show "Open in workspace", deep-linking via the hash router.
Also in this PR, unrelated and standalone (
5956f6c8):app::boot::test::releasing_the_instance_frees_the_rootno longer asserts that a released data root is re-acquirable in the same instant.src/store/lock.rsdocuments why that is stricter than the lock promises — the lock lives on the open file description, and afork()beforeexec()briefly shares it. The test now retries to a 2 s ceiling and reports the attempt count with the last real error. The lock itself is unchanged. Verified not to be a blindfold: holding the first instance instead of dropping it fails the test in 2.01 s after 158 refused attempts.Tests
cargo fmt --all -- --checkcargo clippy --locked --no-deps --all-targets -- -D warningscargo clippy --locked --no-deps --all-targets --features openhuman,tinycortex -- -D warningscargo build --all-targets(viacargo check --locked --all-targetsand--all-features --all-targets)cargo test— 1813 passed, 0 failedcargo test --locked --features sqlite --lib store::sqlite— 25 passedcargo test --locked --features openhuman,tinycortex --tests— 2699 passed, 0 failedfrontend:npm run typecheck,npm run build,npx vitest run— 22 files, 236 tests passedNew coverage:
materialize(folder find-vs-create through the minter, nested source segments, deleted-node recreate, same basename in two directories); the drain (dual-write,None-workspace no-op preserving today's behaviour byte-for-byte, node failure still records the artifact, re-publish updating the same node, the version carrying the id); four failure-injection tests pinning write ordering in both directions; the mirror hooks (a PUT on a published note appends an Operator version before the note write andhuman_edit_diffanswers; a PUT on an unpublished note appends nothing;append_versionmirrors); the announcer (cloned fromBoardAnnouncer's suite shape, including a refusing log not failing the write); serde round-trips; the SSE projection; and an end-to-end test proving agent A publishes and agent B reads it.Documentation
Module docs on the new
src/company/artifact_mirror.rs, the announcer, and the amended publish drain carry the ordering argument and the residual. Note on placement: the plan put this module undersrc/harness/, which is#[cfg(feature = "openhuman")], while its callers insrc/server/ops/are always compiled — a default build would not have linked. It lives insrc/company/instead, following theworkspace_links.rsprecedent.One test-only consequence worth recording: the boot scaffold's
WorkspaceChangedframes consume journal sequence 0, andwire::cycle_idembeds the first event's sequence. Five hosted-brain tests hardcodedcycle_id(…, 0)and two asserted total journal length. Production is unaffected (the id is derived at runtime), but those tests encoded an assumption that is now false. The count is derived from the system-roots list so adding a root cannot silently desynchronise it again.Related
Third of a sequence making the workspace a shared resource space:
Agents/andDesks/scaffolded, member folders minted on first use (PR feat(workspace): scaffold Agents/ and Desks/, mint member folders on first use (#570) #571; this PR is stacked on it)Summary by CodeRabbit
Agents/andDesks/roots; individual folders are created when needed.