Skip to content

docs(conductor): rfc v3 -- shared board visibility and conductor-gated worker communication - #9394

Merged
iamwhatever merged 1 commit into
mainfrom
docs/work-ledger-rfc-v3
Sep 8, 2026
Merged

docs(conductor): rfc v3 -- shared board visibility and conductor-gated worker communication#9394
iamwhatever merged 1 commit into
mainfrom
docs/work-ledger-rfc-v3

Conversation

@iamwhatever

Copy link
Copy Markdown
Collaborator

Problem / Motivation

docs/request-for-change/rfc-conductor-work-ledger.md is at v2.1 and says two things that are no longer true.

It says status: draft and "Nothing in this document exists on main", but three phases have merged: the store (#8855), the four tools and their routes (#9152), and the isolation into a separate kirocrew-ledger-conductor spec (#9277). A reader acts on that status field, and the index row still says "Nothing".

It also describes a two-party record — one conductor, one worker, one item each, and a worker that can see nothing but its own row. The store that shipped is not that: every item of one conductor lives in one directory, which is a shared board. Because the document never said so, it also never wrote down who may read which field, and the one real need that follows from a board — item B building on item A's output — had no answer except the conductor reading A's summary and retyping it at B.

Why it matters

Two costs, one for readers and one for the next phase.

A draft RFC whose first three phases are on main sends every future reader to the wrong place: they read a proposal and go looking for code that is already there, or they trust "Nothing" and duplicate it. docs/request-for-change/README.md states that statuses are verified against the code, so a stale row is a broken promise of that file specifically.

And Phase 3 onward now has to be designed against a visibility model nobody wrote. work_brief returns seven keys today; whether an eighth may carry a sibling's summary, or a session key, or the conductor's goal is currently a judgement call per PR rather than a rule. The same gap makes worker-to-worker help unimplementable: the only sanctioned answer is "route it through the conductor's prose", which is the interpretation cost this RFC exists to remove.

What changed (motivation → approach → change)

Goal: make the document describe the shipped board, and give the remaining phases a rule to design against.

Approach: re-characterise rather than re-scope. The ledger is one board per conductor, masked by identity — the conductor sees every square, a worker sees its own square whole and its siblings only as pointers. Everything else in v3 falls out of that one sentence, so no new mechanism is introduced that the mask does not already justify.

What the diff does:

  • New §Visibility model tabulating every field against four readers (conductor / this item's worker / a sibling worker / Crew page user), plus the three rules that generate the table: a sibling sees pointers and enumerations but never prose, no reader but the conductor sees a session key, and a worker never learns the goal.
  • Phase 3b — visibility. work_brief gains siblings: item_id, title, status, artifacts, last_report_at, open items only. No summary, no worker_session_key, no acceptance, no verdict, no decision. The concrete use is item B rebasing on item A's artifacts.pr. Read-only, pointers and enums only, so it adds no surface artifacts did not already carry inside one item.
  • Phase 5 — communication. Conductor-gated, on two paths. work_request (kind: relay | channel, target a sibling item_id, text ≤ 500) appends a request event to the requester's own square and wakes the conductor; the target is a field, not a destination. The conductor answers either by relaying with session_send — zero new code, one approval click per message — or by writing a channel with work_ledger_record action=channel (ttl_secs ≤ 86400, max_messages ≤ 50), after which work_message lets the pair exchange bodies directly. A message lands as an event on the target's item and is read as data through work_brief's new inbox. Every request and every message is an event on the board the conductor already reads once a cycle, which is the point: the manager does not have to be in the conversation to know what the conversation was.
  • Data model and storage grow four event kinds (request, channel_open, channel_close, message), a derived unread_inbox, and a channels/ directory whose pair id is two server-minted item_ids sorted and joined — so no model-supplied string reaches a path component, which is what lets these two tools take an item_id argument at all.
  • Fact correction on session_send. The earlier text said it is "withheld from workers", which reads as an allowlist decision that could be reversed by adding a line. It is structural: kirocrew-worker is build_agent_config() plus @kirocrew-work, and session_send lives on the opt_in kirocrew-dashboard server a worker spec never mounts. A worker's only tools that reach a human are send_message / send_notification, which address a person, not a session's turn queue. The ledger is a worker's only channel to its conductor.
  • Threat model gains the rows the new surface needs, and states the work_message-vs-session_send distinction as categorical rather than a matter of degree: session_send reaches enqueue_or_run_prompt, so its body runs as the target's user-role turn under the target's grants — lateral prompt injection. work_message writes a JSON field, and Phase 5's exit criteria patch enqueue_or_run_prompt to fail the test if it is called at all, because "we did not wire delivery" is exactly the property a later convenience patch would quietly add.
  • Shipped phases rewritten as implemented, including the two places the code corrected the design (_coalesce_progress; MAX_RECORD_BYTES reading an over-large file as absent) and the follow-up that the cross-platform locking argument turned out to need (fix(work-ledger): stop the lock-file open from truncating on Windows #9237). A new Phase 2.5 section records the isolation walk-back and its met exit criteria, and §Agent spec changes is corrected to the grants actually on main — three refs on kirocrew-ledger-conductor including work_brief, and no work-ledger grant on kirocrew-conductor or kirocrew-pipeline-conductor.
  • Open questions. Q2 closed: the ledger gate wins, because a worker reports the pull request it produced anyway, so the ledger observes a superset one report later. Q6 closed: consecutive progress reports coalesce, as shipped. Q7, Q8, Q9 opened for sibling summary visibility (currently no), a conductor as one end of a channel (currently no), and cross-level requests in a two-level conductor (currently no).
  • Frontmatter and index. revision: v3, status: partial, implementation-prs: [8855, 9152, 9277], re-audited at 7fa305f35. The index row moves from draft/"Nothing" to partial with what is and is not on main.

One thing is deliberately not claimed: Phase 3 is not a precondition for use. kirocrew-ledger-conductor as shipped already runs a goal on a plain timer, paying one turn per quiet tick. Phase 3 is a cost optimisation on that loop, and half of the merge criterion for folding the two conductors back together — not the thing that makes the ledger usable.

Tests

N/A — documentation only. No code changes, so nothing to pin with a test. The design's own testable claims live in each phase's exit criteria, and Phase 5's are written as assertions rather than intentions (the enqueue_or_run_prompt absence, the byte-identical target after a request, the distinct channel_expired / channel_exhausted codes).

Manual verification

All four diff-scoped gates run with their BASE_REF exported, since running them bare reverts to full-tree report mode and always exits 0:

  • sh scripts/docs-lint.shAll documentation checks passed (303 files). The 14 dead-identifier and 1 stale-baseline findings are report-only and pre-existing in unrelated files.
  • BRAND_BASE_REF=$(git merge-base HEAD origin/main) python3 scripts/check_brand_name.py → clean.
  • CHANGELOG_BASE_REF=... python3 scripts/check_changelog_history.py → 8 shipped sections intact.
  • FOCUS_BASE_REF=... python3 scripts/check_focus_cue.py → clean for this diff (4 pre-existing frontend elements reported, not gated).

Every code claim added in this revision was read at 7fa305f35, not carried over from the previous revision: work_ledger.py's vocabularies and caps, read_work_brief's seven keys, accept_batch's composition from acceptance alone, mcp_work.py's four tool definitions, the _reaches_a_channel re-check after the thread hop in handlers/work_ledger.py, _MANAGED_MCP_SERVERS' two opt_in entries, _LEDGER_CONDUCTOR_WORK_GRANTS / _WORKER_WORK_GRANTS, _narrow_conductor_mcp_servers' work parameter, and _WORKER_SYSTEM_PROMPT.

Related Issues

Follows #8842 (v1), #8920 (v2). Documents the design behind #8855, #9152 and #9277.

@iamwhatever
iamwhatever requested a review from a team as a code owner September 8, 2026 05:45
@iamwhatever
iamwhatever requested a review from dwu96 September 8, 2026 05:45
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 8, 2026
@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 669410cdeed4efb894d77bfab2796a5c16dff070 — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 669410c

Verdict parsed from the review's SHA-scoped output markers for commit 669410cdeed4efb894d77bfab2796a5c16dff070.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 669410cdeed4efb894d77bfab2796a5c16dff070: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — 🟡 CONCERNS

Design-level review of 669410cdeed4efb894d77bfab2796a5c16dff070 — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Solid RFC revision with verified code claims, but the visibility rule "a sibling never sees prose" is false for artifacts, which is worker-authored free text.

Watch

The Phase 3b sibling digest breaks the mask rule it claims to enforce. The Visibility model says "a sibling sees pointers and enumerations, never prose" and the threat row asserts steering is "Prevented by the mask: … no free-text field" — but artifacts is characterized as "a string→string map of pointers" while the shipped store (work_ledger.py:123-125) enforces only type and size: 16 keys × 512 chars of worker-chosen text, ~8KB. Chain: worker A (steerable by untrusted external content) writes persuasion into artifacts values → the digest delivers it to sibling B with no conductor grant → exactly the lateral-influence path the summary exclusion and the channel gate exist to close. The RFC's own defense — "adds no attack surface that artifacts did not already carry within one item" — is wrong on the reader set: within one item only the conductor and the item's own worker read it; the digest is what puts it in front of an untrusted peer. Phase 3b's exit criteria pin key presence, not value shape, so nothing catches this at implementation time.
Clears when: the RFC either drops artifacts from the sibling digest, constrains digest-visible values to pointer-shaped content (and adds an exit criterion pinning it), or amends the visibility rule and threat row to honestly state that bounded worker free text crosses the sibling boundary.

Suggestions

  • Fold the artifacts question into Q7 rather than a new mechanism: the channel path already exists as the sanctioned way for one worker's text to reach another, so "B rebases on A's branch" may only need pr (integer-validated) in the digest, not the whole map.

[DESIGN-REVIEWED] 669410c

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 669410cdeed4efb894d77bfab2796a5c16dff070 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 669410c

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 669410cdeed4efb894d77bfab2796a5c16dff070: <one-sentence reason>

@github-actions github-actions Bot added readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge and removed readiness: checking Automated validation is still running labels Sep 8, 2026
…d worker communication

Re-characterises the work ledger as one shared board per conductor, masked by
identity, and promotes the three shipped phases from "proposed" to "as
implemented".

Direction change. v1 and v2 described a two-party record: one conductor, one
worker, one item each, and a worker that could see nothing but its own row. The
shipped store is already a board -- every item lives in one conductor directory --
so v3 names it that and writes the mask down. A new Visibility model section
tabulates every field against four readers (conductor, this item's worker, a
sibling worker, Crew page user) and gives the three rules that generate the table:
a sibling sees pointers and enumerations but never prose, no reader but the
conductor sees a session key, and a worker never learns the goal.

Two new phases follow from that. Phase 3b adds a `siblings` digest to
`work_brief` -- item_id, title, status, artifacts, last_report_at, open items only
-- so item B can rebase on item A's branch without a round trip through the
conductor. Phase 5 adds conductor-gated worker communication: `work_request` lets
a worker ask for relay or a channel, recorded as an event on its OWN square;
`work_ledger_record action=channel` writes a bounded, expiring pairwise channel;
`work_message` writes a body into the target's inbox only while that channel
covers the pair. Nothing on the path enqueues a prompt, and Phase 5's exit
criteria assert the absence of that call directly, because that absence is what
distinguishes a channel from `session_send` granted downward.

Fact correction. The RFC's earlier framing said `session_send` is "withheld from
workers", which reads as an allowlist decision. It is structural:
`kirocrew-worker` is `build_agent_config()` plus `@kirocrew-work`, and
`session_send` lives on the opt-in `kirocrew-dashboard` server that a worker spec
never mounts. A worker has `send_message` / `send_notification`, which address a
person rather than a session's turn queue, so the ledger is its only channel to
its conductor.

Also: Phase 1/2 sections rewritten as implemented with the two places the code
corrected the design (`_coalesce_progress`, `MAX_RECORD_BYTES` reading as absent);
a Phase 2.5 section recording the isolation walk-back and its met exit criteria;
the agent-spec section corrected to the shipped grants; Q2 closed (the ledger gate
wins, because a worker reports its PR anyway) and Q6 closed (consecutive progress
reports coalesce); Q7, Q8 and Q9 opened for sibling summary visibility, a
conductor as a channel end, and cross-level requests. The index row moves from
`draft`/"Nothing" to `partial` with what is and is not on main.

Tests: none -- documentation only. docs-lint, brand-name, changelog-history and
focus-cue gates all run with their BASE_REF exported and pass.
@iamwhatever
iamwhatever force-pushed the docs/work-ledger-rfc-v3 branch from 9689d31 to 669410c Compare September 8, 2026 07:00
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: passed Eligible automated validation passed for the current revision merge conflict Branch has merge conflicts with its base — author must resolve before merge readiness: checking Automated validation is still running labels Sep 8, 2026
@chenmingwei23

Copy link
Copy Markdown
Contributor

Triage review (comment only, no verdict). RFC v3 re-characterises the conductor work ledger as one shared board per conductor masked by identity, adds a Visibility model table plus Phase 3b (sibling digest) and Phase 5 (conductor-gated worker channels), and promotes the three shipped phases from proposed to as-implemented; the sibling README moves the index row from draft to partial. Pure documentation: zero source files touched, and the diff matches its docs: claim.

design-doc-gate: Rule A (docs/request-for-change/), also Rule D on README.md and Rule C on rfc-*.md, docs-dominant with no code in the diff. The document specifies trust-boundary and inter-session-communication design (worker channels, identity masks, session_send withholding), which is exactly the class of decision an agent approval cannot stand in for. Not approved and not rejected by the pipeline: it needs a human to agree to the design. Evaluated at head SHA 669410c.

@iamwhatever
iamwhatever enabled auto-merge (squash) September 8, 2026 16:07

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Read the whole diff and checked every code claim in it against origin/main. The as-implemented rewrite of Phases 1/2/2b is accurate — read_work_brief's seven keys, accept_batch composing from acceptance alone, the exact grant sets in _LEDGER_CONDUCTOR_WORK_GRANTS / _WORKER_WORK_GRANTS, _narrow_conductor_mcp_servers(work=…), kirocrew-dashboard being opt_in, probes/__init__.py mapping only gh-pr, _coalesce_progress and MAX_RECORD_BYTES — all verified, and no internal references leaked.

One blocking factual problem, and it is in the new document's central claim rather than at its edges.

§Visibility model — "a sibling sees pointers and enumerations, never prose" is false for artifacts. The same section calls artifacts "a string→string map of pointers", but the shipped store validates only type and size: work_ledger.py:962-985 accepts MAX_ARTIFACT_KEYS=16 keys of MAX_ARTIFACT_VALUE_CHARS=512 each — roughly 8 KB of worker-chosen free text, with no shape constraint making a value a pointer. Phase 3b puts artifacts into the sibling digest, so worker-authored prose crosses exactly the boundary that excluding summary and gating the Phase 5 channels exist to close.

That makes the threat row "A worker reads a sibling's prose and is steered by it … Prevented by the mask … no free-text field" wrong for the same reason, and the stated defence ("adds no attack surface that artifacts did not already carry within one item") misstates the reader set: within one item the readers are the conductor and that item's own worker, not siblings. Phase 3b's exit criteria pin key presence, not value shape, so nothing would catch this at implementation time either.

This is the same finding Design Review raised as 🟡 CONCERNS on head 669410cd, still without a disposition. Two ways out, and the choice is a design decision, not an editorial one: constrain artifacts values to a pointer shape (URL / path / id) and say so in Phase 3b's exit criteria, or drop the "never prose" guarantee and treat artifacts as prose that the mask deliberately lets through, with the threat row rewritten to match.

Separately, a nit: the body says "re-audited at 7fa305f35" and "Every code claim added in this revision was read at 7fa305f35", but the diff writes audited-at: c02cdd67c and the doc text says c02cdd67c. 7fa305f35 is a real commit but not the audit SHA the artifact claims.

Not approving on the design itself — per the design-doc-gate triage this specifies a trust boundary and an inter-session communication channel, so it needs a human owner to agree to the design rather than a review bot or me.

@iamwhatever
iamwhatever merged commit fd89051 into main Sep 8, 2026
65 checks passed
@iamwhatever
iamwhatever deleted the docs/work-ledger-rfc-v3 branch September 8, 2026 21:59

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting changes, formalising the finding I left in my earlier comment on this PR.

The document's central guarantee is false as written. §Visibility model states "a sibling sees pointers and enumerations, never prose" and calls artifacts "a string→string map of pointers", but the shipped store validates only type and size: work_ledger.py:962-985 accepts MAX_ARTIFACT_KEYS=16 keys of MAX_ARTIFACT_VALUE_CHARS=512 each — about 8 KB of worker-chosen text, with nothing constraining a value to a pointer shape. Phase 3b puts artifacts in the sibling digest, so worker-authored prose crosses exactly the boundary that excluding summary and gating the Phase 5 channels exist to close.

Two consequences that also need correcting:

  • The threat row "A worker reads a sibling's prose and is steered by it … Prevented by the mask … no free-text field" is wrong for the same reason.
  • The stated defence — "adds no attack surface that artifacts did not already carry within one item" — misstates the reader set. Within one item the readers are the conductor and that item's own worker; Phase 3b widens that to siblings, which is the whole change.

Phase 3b's exit criteria pin key presence, not value shape, so nothing catches this at implementation time either.

Either fix closes it, and the choice is a design decision rather than an editorial one: constrain artifacts values to a pointer shape (URL / path / id) and say so in Phase 3b's exit criteria, or drop the "never prose" guarantee, treat artifacts as prose the mask deliberately lets through, and rewrite the threat row to match. Design Review raised the same thing as 🟡 CONCERNS on head 669410cd and it still has no disposition.

Separately, a nit worth fixing in the same pass: the body says "re-audited at 7fa305f35" and "Every code claim added in this revision was read at 7fa305f35", but the diff writes audited-at: c02cdd67c and the doc text says c02cdd67c. 7fa305f35 is a real commit but not the audit SHA the artifact claims.

For the record, everything else in the as-implemented rewrite checks out against origin/main: read_work_brief's seven keys, accept_batch composing from acceptance alone, the exact grant sets in _LEDGER_CONDUCTOR_WORK_GRANTS and _WORKER_WORK_GRANTS, _narrow_conductor_mcp_servers(work=…), kirocrew-dashboard being opt_in, probes/__init__.py mapping only gh-pr, _coalesce_progress and MAX_RECORD_BYTES, and goal-conductor/SKILL.md plus ledger_entry.py being untouched. Phases 3/3b/4/5 describe behaviour that does not exist yet but are labelled "not started", which is correct RFC practice. No leaked internal references.

@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants