M5c-1 plan: task 08, the turn decision in memoryd
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Claude Opus 5.5
@@ -0,0 +1,255 @@
|
||||
# M5c-1 task 08: `memoryd` decides where a finished turn goes in Honcho
|
||||
|
||||
**Branch:** `m5c1` (run `git switch m5c1`; `git status --short` must be empty, otherwise stop)
|
||||
**Commit subject:** `memoryd: where a finished turn goes in Honcho, decided from brokerd and the log`
|
||||
**Log row:** the Task cell is `M5c1/08-memoryd-honcho-decide`.
|
||||
|
||||
## Goal
|
||||
|
||||
When a turn ends, `memoryd` decides once where it goes in Honcho, and records the decision in
|
||||
the ledger (task 07) at once, so that a Honcho outage later cannot change it (M5c spec, section
|
||||
5). This task is the decision itself: a pure function, with no file, socket or clock. Task 09
|
||||
calls it with `brokerd`'s answer and the session's log, and appends what it returns.
|
||||
|
||||
A turn goes to this agent's **trusted** workspace, its **untrusted** workspace, or **nowhere**
|
||||
(`Action::Skip`, with a fixed reason). The spec's table (section 4):
|
||||
|
||||
| The turn's session at its end | Goes to |
|
||||
|---|---|
|
||||
| clean, class `public` or `private` | the trusted workspace |
|
||||
| untrusted (read untrusted content), class `public` or `private` | the untrusted workspace |
|
||||
| class `secret` | nowhere |
|
||||
| no speaker, or incomplete speaker information | nowhere |
|
||||
| the text holds a known-prefix secret | nowhere |
|
||||
|
||||
**The guarantee.** A turn goes to the trusted workspace only when `brokerd` says the session is
|
||||
not untrusted, no tool result in the log up to the turn's end is labelled untrusted, and the
|
||||
class is `public` or `private`.
|
||||
|
||||
## The checks, in order
|
||||
|
||||
`decide` runs these checks in this order; the first that holds decides. Every `Skip` carries one
|
||||
of six fixed texts, the constants in the skeleton.
|
||||
|
||||
| # | When | The action |
|
||||
|---|---|---|
|
||||
| 0 | the log has no turn `turn` (`turn` is 0, or more than the log's `User` records) | `Err(NoSuchTurn { turn, turns })`: nothing is decided |
|
||||
| 1 | `brokerd`'s class is `secret` | `Skip`, `SECRET_CLASS`: `the session is secret` |
|
||||
| 2 | the turn's `User` record has no speaker | `Skip`, `NO_SPEAKER`: `the turn has no single speaker` |
|
||||
| 3 | the speaker's peer name (each `.` made `_`) is not a Honcho name | `Skip`, `BAD_SPEAKER`: `the speaker is not a Honcho peer name` |
|
||||
| 4 | the speaker's peer name is the agent's own (`[honcho] agent`) | `Skip`, `AGENT_SPEAKER`: `the speaker has the agent's peer name` |
|
||||
| 5 | the turn has no final reply, or an empty or white-space one | `Skip`, `NO_REPLY`: `the turn has no reply` |
|
||||
| 6 | the user's text or the reply holds a known-prefix secret | `Skip`, `SECRET_TEXT`: `the turn's text holds a secret` |
|
||||
| 7 | `brokerd` says untrusted, or a `ToolResult` from the log's start to the turn's end is labelled `untrusted` | `Turn { workspace: Untrusted, speaker: <peer name> }` |
|
||||
| 8 | otherwise | `Turn { workspace: Trusted, speaker: <peer name> }` |
|
||||
|
||||
`decide` is given whole in the skeleton: it is these rows as code. The helpers it calls are
|
||||
yours.
|
||||
|
||||
## Interfaces
|
||||
|
||||
In `crates/memoryd/src/honcho/decide.rs` (the module `memoryd::honcho::decide`):
|
||||
|
||||
```rust
|
||||
pub const SECRET_CLASS: &str = "the session is secret";
|
||||
pub const NO_SPEAKER: &str = "the turn has no single speaker";
|
||||
pub const BAD_SPEAKER: &str = "the speaker is not a Honcho peer name";
|
||||
pub const AGENT_SPEAKER: &str = "the speaker has the agent's peer name";
|
||||
pub const NO_REPLY: &str = "the turn has no reply";
|
||||
pub const SECRET_TEXT: &str = "the turn's text holds a secret";
|
||||
|
||||
pub struct NoSuchTurn { pub turn: u64, pub turns: u64 } // Debug, Clone, Copy, PartialEq, Eq
|
||||
|
||||
pub fn turns(records: &[LogRecord]) -> u64;
|
||||
pub fn turn_records(records: &[LogRecord], turn: u64) -> Option<&[LogRecord]>;
|
||||
pub fn peer_name(speaker: &str) -> String;
|
||||
pub fn has_secret(text: &str) -> bool;
|
||||
pub fn decide(
|
||||
state: &SessionStateReply,
|
||||
records: &[LogRecord],
|
||||
turn: u64,
|
||||
agent: &str,
|
||||
) -> Result<Action, NoSuchTurn>;
|
||||
```
|
||||
|
||||
`NoSuchTurn` displays as `turn 3 is not in the session's log, which has 2 turns`. `Action` and
|
||||
`Workspace` are the ledger's (task 07); `LogRecord` and `SessionStateReply` are `bxcore`'s.
|
||||
Task 09 uses `turns` to walk every undecided turn from 1 up to the smaller of the turn
|
||||
`loopd` named and `turns(log)`, so a turn number that came over a socket never drives the loop.
|
||||
|
||||
## Every place that changes
|
||||
|
||||
This is the complete list.
|
||||
|
||||
| Place | What happens there |
|
||||
|---|---|
|
||||
| `crates/memoryd/src/honcho/mod.rs` | `pub mod decide;` (edit H1) |
|
||||
| `crates/memoryd/src/honcho/decide.rs` | new: a skeleton you copy, then fill 8 bodies (step 4) |
|
||||
| everything else (`ledger.rs`, `config.rs`, `serve.rs`, other crates, the runbook) | **no change** |
|
||||
|
||||
## Files
|
||||
|
||||
- Copy, from `docs/plans/M5c1/files/` (the same path under it):
|
||||
- the new test `crates/memoryd/tests/honcho_decide.rs`
|
||||
- the skeleton `crates/memoryd/src/honcho/decide.rs` (you fill 8 bodies in it)
|
||||
- Modify: `crates/memoryd/src/honcho/mod.rs`, `docs/implementer-log.md`.
|
||||
- Do not touch: any other source file, any file of another crate, any `Cargo.toml`, any other
|
||||
test, and in the skeleton anything but the 8 `todo!()` bodies.
|
||||
|
||||
## Facts you need
|
||||
|
||||
- **A session's log** (`bxcore::LogRecord`, one per line of `sessions/<id>/0.jsonl`). `loopd`
|
||||
writes each turn as: one `User` record (the text, and the speaker when there is one), then for
|
||||
each completion an `Assistant` record and a `Usage` record (sometimes a `CacheLoss`), and after
|
||||
an `Assistant` with tool calls one `ToolResult` per call, labelled with its `class` and
|
||||
`untrusted`. A turn ends with an `Assistant` record that has no tool calls: its `content` is the
|
||||
reply. A turn that failed (an inference error, the tool-round limit) has its `User` record and
|
||||
no such final record. The log starts with a `SessionStart` record.
|
||||
- **Turn numbers** (task 02): turn `n` is the `n`-th `User` record of the log, counted from 1. A
|
||||
failed turn counts. Its records run from its `User` record up to, not including, the next
|
||||
`User` record, or to the log's end.
|
||||
- **Labels up to the turn's end.** Row 7 looks at every `ToolResult` from the log's start to the
|
||||
turn's end, earlier turns included (spec 5: "any `ToolResult` so far labelled untrusted"), and
|
||||
not at later turns: turn 1 of a session whose turn 2 fetched a page is decided on its own
|
||||
records and those before it (`brokerd`'s answer is the session's state when it is asked, and
|
||||
may already say untrusted: a late decision only moves a turn towards untrusted).
|
||||
- **The speaker.** `gatewayd` sets the Mattermost username as it is (task 03), `a-z`, `0-9`,
|
||||
`.`, `-`, `_`; `bxctl chat --speaker` any text (task 04). Honcho allows `a-zA-Z0-9_-`, so the
|
||||
peer name is the username with each `.` made `_` (spec 3), and that name is checked with
|
||||
`honcho::config::is_name`. The `Turn` action carries the peer name.
|
||||
- **The secret rules.** `bxcore::sanitize::refuse(text)` returns the **first** rule a text breaks,
|
||||
in this order: `control`, `invisible`, `private-use`, `noncharacter`, then the secret rules
|
||||
(`secret-github`, `secret-gitlab`, `secret-sk`, `secret-slack`, `secret-aws`,
|
||||
`secret-private-key`), then `tag`. Only the secret rules count here (a chat-template tag in a
|
||||
reply is not a reason to skip), but a text with a carriage return (a `control` character) would
|
||||
hide a key after it. So `has_secret` first replaces every character that `refuse` refuses on
|
||||
its own with a space, then asks `refuse` of the whole text and keeps only a `secret-` rule. A
|
||||
space ends a key and starts a word exactly as the replaced character did (none of them is an
|
||||
ASCII letter, digit, `_`, `-` or `.`), so no key is lost by the replacement. (One more may be
|
||||
found: a private key's `-----BEGIN` line with a control character before it on its line.)
|
||||
- **Tests build keys from pieces**, so that no file holds a key whole: do not write one in a
|
||||
comment or a command either.
|
||||
- **The given test** `honcho_decide.rs` (18 tests): the six texts; `turns` and `turn_records`; a
|
||||
turn not in the log; each row of the table; the speaker's peer name, its edges (512 and 513
|
||||
characters, a space, a non-ASCII letter or digit) and the agent's name; the replies a failed
|
||||
turn leaves; secrets in the text or the reply, after a carriage return, a bell or an invisible
|
||||
character; secrets where nothing is sent (a tool result, reasoning, another turn) not counted;
|
||||
tags and short keys not counted; and one case per row that breaks every later row too, to pin
|
||||
the order.
|
||||
- The calls the bodies use:
|
||||
|
||||
| Call | Signature, and what it gives |
|
||||
|---|---|
|
||||
| `matches!` | `matches!(r, LogRecord::User { .. }) -> bool` |
|
||||
| `Iterator::filter`, `count`, `any` | `records.iter().filter(f).count() -> usize`; `.any(f) -> bool` |
|
||||
| `u64::try_from` | `u64::try_from(count).unwrap_or(u64::MAX)`: a `usize` as `u64`, no `as` cast |
|
||||
| `Iterator::enumerate` | `records.iter().enumerate()`: `(index, &record)` |
|
||||
| `slice::get` | `records.get(start..end) -> Option<&[LogRecord]>` |
|
||||
| `slice::first` | `turn.first() -> Option<&LogRecord>` |
|
||||
| `Iterator::rev`, `find` | `turn.iter().rev().find(f) -> Option<&LogRecord>`: the last match |
|
||||
| `Option::as_deref` | `speaker.as_deref()`: `&Option<String>` to `Option<&str>` |
|
||||
| `str::replace` | `speaker.replace('.', "_") -> String` |
|
||||
| `char::encode_utf8` | `c.encode_utf8(&mut buf) -> &mut str`, `buf: [u8; 4]`: the char as a `&str` |
|
||||
| `refuse` | `bxcore::sanitize::refuse(text: &str) -> Option<Refusal>`; `Refusal.rule: &'static str` |
|
||||
| `Option::is_some_and` | `refuse(&masked).is_some_and(\|r\| r.rule.starts_with("secret-"))` |
|
||||
|
||||
## The edit, exactly
|
||||
|
||||
### `crates/memoryd/src/honcho/mod.rs`
|
||||
|
||||
**H1.** The line
|
||||
|
||||
```rust
|
||||
pub mod config;
|
||||
```
|
||||
|
||||
becomes
|
||||
|
||||
```rust
|
||||
pub mod config;
|
||||
pub mod decide;
|
||||
```
|
||||
|
||||
## Steps
|
||||
|
||||
- [ ] **1. Copy the given test.** From the repository root:
|
||||
|
||||
```sh
|
||||
cp docs/plans/M5c1/files/crates/memoryd/tests/honcho_decide.rs crates/memoryd/tests/
|
||||
```
|
||||
|
||||
- [ ] **2. See it fail.** `cargo build --tests --keep-going -p memoryd`, with a shell timeout of
|
||||
600000 ms: one test target does not compile, `honcho_decide`, with one error, `E0432`
|
||||
(``unresolved import `memoryd::honcho::decide` ``). Every other target compiles.
|
||||
- [ ] **3. The module.** Make edit H1, then copy the skeleton:
|
||||
|
||||
```sh
|
||||
cp docs/plans/M5c1/files/crates/memoryd/src/honcho/decide.rs crates/memoryd/src/honcho/
|
||||
```
|
||||
|
||||
`cargo check -p memoryd`: no error, and 11 warnings, all from the unfilled bodies:
|
||||
``unused import: `bxcore::sanitize::refuse` ``, and ten `unused variable`: `records` four
|
||||
times, `turn` four times, `speaker` and `text` once each. Then
|
||||
`cargo test -p memoryd --test honcho_decide`: 1 test passes (`the_skip_reasons_are_these_texts`)
|
||||
and 17 fail with `not yet implemented`.
|
||||
- [ ] **4. Fill the eight bodies of `honcho/decide.rs`, one at a time, in the order of the
|
||||
file**, each from its comment, and run `cargo check -p memoryd` after each: `turns`, `bounds`,
|
||||
`turn_records`, `peer_name`, `has_secret`, `user_text`, `final_reply`, `labelled_untrusted`.
|
||||
When a body is filled, delete its instruction comment (the `//` lines inside the body) and keep
|
||||
the function's doc comment (`///`) as it is. Change nothing else in the file: not `decide`, not
|
||||
the constants, not `NoSuchTurn`. After the eighth, `cargo check -p memoryd` prints no warning.
|
||||
- [ ] **5. The test.** `cargo test -p memoryd --test honcho_decide`: all 18 tests pass.
|
||||
- [ ] **6. Check.** Three commands:
|
||||
- `grep -c 'fs::\|std::io\|Timestamp::now' crates/memoryd/src/honcho/decide.rs` prints `0`:
|
||||
the decision reads and writes nothing, and takes no time.
|
||||
- `grep -n 'refuse(' crates/memoryd/src/honcho/decide.rs` prints exactly two lines, both in
|
||||
`has_secret`.
|
||||
- `git diff --stat -- crates/bxcore crates/loopd crates/brokerd crates/gatewayd crates/bxctl
|
||||
Cargo.toml crates/memoryd/Cargo.toml docs/runbook.md` prints nothing.
|
||||
- [ ] **7. Run the gate.** `cargo fmt --all`, then `make gate`, with a shell timeout of 600000 ms.
|
||||
Expected last line: `gate: ok`, with 1230 tests: 1212 before this task, plus 18 new ones in
|
||||
`honcho_decide.rs`.
|
||||
- [ ] **8. Log and commit.** Add your row to `docs/implementer-log.md`; its Task cell is
|
||||
`M5c1/08-memoryd-honcho-decide`. `git status --short` lists `crates/memoryd/src/honcho/mod.rs`
|
||||
and your log as modified, and `crates/memoryd/src/honcho/decide.rs` and
|
||||
`crates/memoryd/tests/honcho_decide.rs` as new. Stage with
|
||||
`git add crates/memoryd docs/implementer-log.md Cargo.lock`, then commit as `AGENTS.md` says.
|
||||
`Cargo.lock` does not change in this task; the `git add` is harmless.
|
||||
|
||||
## Done when
|
||||
|
||||
- `make gate` prints `gate: ok` with 1230 tests.
|
||||
- `git diff HEAD~1 --stat` lists only `honcho/mod.rs`, `honcho/decide.rs`,
|
||||
`tests/honcho_decide.rs` and `docs/implementer-log.md`.
|
||||
|
||||
## Stop and report if
|
||||
|
||||
- The line of edit H1 is not in `crates/memoryd/src/honcho/mod.rs` exactly as shown, or is there
|
||||
more than once.
|
||||
- Step 2 or 3 shows a different result: other test targets, errors of another kind, other
|
||||
warnings, or other tests passing or failing.
|
||||
- `cargo check` reports an error after a body you filled, and the comment does not tell you how
|
||||
to fix it. Quote the error, and stop.
|
||||
- A comment in the skeleton does not fit the code under it, or a row of "The checks, in order"
|
||||
does not fit `decide`. Quote both and stop.
|
||||
- After step 4 a test fails although each body follows its comment. Do not change the test or
|
||||
the skeleton's given code: stop, and give the failing tests' names and their messages.
|
||||
- The fix seems to need a change anywhere else: another `memoryd` file, `bxcore`'s sanitizer,
|
||||
another crate, or a dependency. It does not; stop and say why you think so.
|
||||
- A test outside this file fails in the gate: check once whether it fails without your change,
|
||||
then stop and report both results.
|
||||
- The gate passes with a count other than 1230.
|
||||
|
||||
## What the given test pins
|
||||
|
||||
Each of these changes to a finished `decide.rs` makes at least one given test fail: counting
|
||||
other records as turns; a turn's bounds off by one at either end, or ended at the next record;
|
||||
turn 0 or a turn past the end accepted; the peer name keeping `.` or making it `-`; the name
|
||||
checked before the `.` is replaced, or compared with the agent's before it; `has_secret` without
|
||||
the replacement, replacing with a letter, deleting instead of replacing, counting any rule, or
|
||||
asking `refuse` of the text as written; the speaker ignored; the first `Assistant` record taken
|
||||
for the reply, or its tool calls ignored; an empty or white-space reply accepted; the labels
|
||||
ignored, taken from the turn alone or from the whole log; `brokerd`'s `untrusted` ignored; any
|
||||
tool result taken as untrusted; the class `secret` ignored, or `public` taken for it; any two
|
||||
checks in another order; a secret in the text or in the reply ignored; the raw username sent as
|
||||
the speaker.
|
||||
Reference in New Issue
Block a user