From c5ec068b7dcb9344c783740cf7590b5903803009 Mon Sep 17 00:00:00 2001 From: meh Date: Sun, 2 Aug 2026 04:12:06 +0700 Subject: [PATCH] fix: unify agent actions under prompt syntax --- AGENTS.md | 15 +- AgencyZero.md | 17 +- Cargo.lock | 14 +- Cargo.toml | 2 +- .../src/features/settings/SettingsTab.tsx | 2 +- apps/gui/frontend/src/types/index.ts | 4 +- apps/gui/src/db/schema/pull_request.rs | 2 +- apps/gui/src/directives.rs | 98 +- apps/gui/src/projects.rs | 1451 +++++------------ apps/gui/src/prs.rs | 157 +- apps/gui/src/tasks.rs | 395 +---- docs/gui-wiring-plan.md | 12 +- docs/ps-capability.yaml | 22 +- docs/task-manager.md | 105 +- 14 files changed, 631 insertions(+), 1665 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 2ca1470..ee3602a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -46,18 +46,21 @@ Run what you build before calling it done. **If you can't run it, say so.** ## PR discipline - Paste the full PR URL, not the number. +- Record it through the declared surface too: prose URLs are inert, so use + `@agency:pr.link(url: "", item: "")` or the `pr:` field on + `items.state` when shipping an item. - When the change lands on master, close the PR and delete the branch in the same breath (`gh pr merge --delete-branch`). An open PR whose content already shipped reads as unfinished work. ## Closing an item you were given -Reply checkboxes edit the project's item list: `- [ ]` proposes, `- [x] ` -closes, `- [-] ` strikes an obsolete row. Titles match exactly and -case-insensitively, so a paraphrase silently appends a near-duplicate. Full contract: -[`docs/task-manager.md`](docs/task-manager.md#the-project-session-contract-three-checkboxes). -Read that, not `projects.rs`: the verbs arrived late, and an older tree reads as -append-only. [`wt-tools`](crates/wt-tools) reads the list and never writes. +The project prompt supplies item ids and the declared Prompt Syntax surface. Report +state with `@agency:items.state`, create with `@agency:items.add`, and remove an +incorrect row with `@agency:items.retire`. Never address an existing row by title: +paraphrases were how the old checkbox contract created near-duplicates. Full contract: +[`docs/task-manager.md`](docs/task-manager.md#the-project-session-contract). +[`wt-tools`](crates/wt-tools) reads the list and never writes. - **Close in the same turn the work ships.** Shipped means merged and released. - **Read the title before striking it.** `wt-tools search-items ` prints it as stored. diff --git a/AgencyZero.md b/AgencyZero.md index 55d3bd6..49f70da 100644 --- a/AgencyZero.md +++ b/AgencyZero.md @@ -3,21 +3,18 @@ 3. Ship through a pull request. 4. Merge only after review passes, or the owner overrides. 5. No em dashes. -6. Track the work as items, in your reply, as it happens: - `- [ ]` opens, `- [~]` plans, `- [/]` starts, `- [>] title (#35)` ships. - `- [?]` when you are stuck on an answer only the owner can give. - Never `- [x]`: you cannot confirm your own fix works. The owner closes it. - Quoted or fenced checklists are never read as items, so quote freely. -7. To retire a row, or to write into another project, ask the host directly: - `` … checkbox lines … ``. - Project by name or by id, omit it to mean this one. Prose can open, plan, - start and ship a row here; only a directive can delete or reach elsewhere. -8. Report item state by id, on its own line, as it happens: +6. Track work through the declared Prompt Syntax surface, in your reply, as it + happens. Prose, checkboxes, quotes, fences and URLs are inert. +7. Report item state by id, on its own line: `` `` `` removes one that should not be there. The turn's prompt lists the open items and their ids, and answers back with what each directive did. Never address a row by its title. +8. A pull request is state only when authored as state: + ``. + Also paste the full URL in prose so the owner can open it. A run started from + an item ships with `items.state(..., status: "shipped", pr: "")`. 9. Ask before installing anything: a download, a global cache, a browser, a toolchain. A repo doc recommending it is not permission. 10. Persisted data goes in a WorkTable table. Not a JSON file beside the store, diff --git a/Cargo.lock b/Cargo.lock index 0649d9c..d7ee745 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -142,25 +142,25 @@ checksum = "f2032f911046de80f0a198e0901378627c33f59ea0ac00e363d481118bd70a53" [[package]] name = "az-agent" -version = "0.1.55" +version = "0.1.56" dependencies = [ "az-core", ] [[package]] name = "az-agent-proxy" -version = "0.1.55" +version = "0.1.56" dependencies = [ "az-core", ] [[package]] name = "az-core" -version = "0.1.55" +version = "0.1.56" [[package]] name = "az-gui" -version = "0.1.55" +version = "0.1.56" dependencies = [ "agent-abstraction", "az-core", @@ -183,7 +183,7 @@ dependencies = [ [[package]] name = "az-mcp-proxy" -version = "0.1.55" +version = "0.1.56" dependencies = [ "az-core", ] @@ -5551,7 +5551,7 @@ dependencies = [ [[package]] name = "wt-migrate" -version = "0.1.55" +version = "0.1.56" dependencies = [ "derive_more 2.1.1", "eyre", @@ -5565,7 +5565,7 @@ dependencies = [ [[package]] name = "wt-tools" -version = "0.1.55" +version = "0.1.56" dependencies = [ "derive_more 2.1.1", "dirs", diff --git a/Cargo.toml b/Cargo.toml index 2c6f3fd..d2b7724 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -11,7 +11,7 @@ members = [ ] [workspace.package] -version = "0.1.55" +version = "0.1.56" edition = "2024" publish = false diff --git a/apps/gui/frontend/src/features/settings/SettingsTab.tsx b/apps/gui/frontend/src/features/settings/SettingsTab.tsx index 6386b80..9aba3fc 100644 --- a/apps/gui/frontend/src/features/settings/SettingsTab.tsx +++ b/apps/gui/frontend/src/features/settings/SettingsTab.tsx @@ -304,7 +304,7 @@ export function SettingsTab(): JSX.Element { on its own line, outside fenced or quoted content", namespace: "agency", - verbs: &[ - "items.state", - "items.add", - "items.retire", - "items.inject", - "pr.link", - ], + verbs: &["items.state", "items.add", "items.retire", "pr.link"], reserved: &["status:finished", "status:canceled"], bound: "any project in this installation's store, named by id or by name; \ - no reach outside it, and no other namespace is live", + Home Task Manager may create a named project inside that store through \ + items.add; no reach outside it, and no other namespace is live", }; /// The statuses an agent may set. @@ -97,11 +92,24 @@ pub enum Directive { /// module exists to remove. ItemAdd { handle: Option, + /// Another project in this installation, by id or exact name. + /// + /// Used by Home's task manager so it speaks the same item language as + /// an ordinary project instead of switching to a JSONL dialect. + project: Option, title: String, status: String, }, - /// Attach a pull request to an item. - PrLink { number: String, item: String }, + /// Track a pull request, and optionally attach it to an item. + /// + /// A URL creates the PR row. A number is enough only when an item is also + /// named, because a bare number says nothing about which repository owns + /// it. Keeping both forms lets an already tracked PR be attached cheaply. + PrLink { + url: Option, + number: Option, + item: Option, + }, /// Remove a row, by id. /// /// The cleanup verb, and the reason it exists is that the old one deleted @@ -207,6 +215,9 @@ pub fn parse(line: &str) -> Option { handle: arg(args, "ref") .map(str::to_string) .filter(|handle| !handle.is_empty()), + project: arg(args, "project") + .map(str::to_string) + .filter(|project| !project.is_empty()), title: title.to_string(), status: arg(args, "status") .unwrap_or("new") @@ -219,10 +230,19 @@ pub fn parse(line: &str) -> Option { return (!id.is_empty()).then(|| Directive::ItemRetire { id: id.to_string() }); } if verb.eq_ignore_ascii_case("@agency:pr.link") { - let number = arg(args, "number")?.trim_start_matches('#').to_string(); - let item = arg(args, "item")?.to_string(); - return (!number.is_empty() && !item.is_empty()) - .then_some(Directive::PrLink { number, item }); + let url = arg(args, "url") + .map(str::to_string) + .filter(|url| !url.is_empty()); + let number = arg(args, "number") + .map(|number| number.trim_start_matches('#').to_string()) + .filter(|number| !number.is_empty()); + let item = arg(args, "item") + .map(str::to_string) + .filter(|item| !item.is_empty()); + // A URL can be tracked without an item. A number alone cannot: it has + // no repository and therefore cannot identify a pull request row. + return (url.is_some() || (number.is_some() && item.is_some())) + .then_some(Directive::PrLink { url, number, item }); } None } @@ -279,12 +299,50 @@ mod tests { parse(r#""#), Some(Directive::ItemAdd { handle: Some("t1".into()), + project: None, title: "Wrap it, then ship it".into(), status: "new".into(), }) ); } + #[test] + fn an_item_add_may_name_another_project() { + assert_eq!( + parse( + r#""# + ), + Some(Directive::ItemAdd { + handle: Some("t1".into()), + project: Some("Prompt Syntax".into()), + title: "Unify the surface".into(), + status: "planning".into(), + }) + ); + } + + #[test] + fn a_pr_url_may_be_tracked_with_or_without_an_item() { + let url = "https://github.com/pathscale/agencyzero/pull/76"; + assert_eq!( + parse(&format!(r#""#)), + Some(Directive::PrLink { + url: Some(url.into()), + number: None, + item: None, + }) + ); + assert_eq!( + parse(r#""#), + Some(Directive::PrLink { + url: None, + number: Some("76".into()), + item: Some("item-a3f9".into()), + }) + ); + assert!(parse(r#""#).is_none()); + } + /// Casual capitalisation compiles to the canonical verb; a confusable does /// not bind at all, because the fold is ASCII. #[test] @@ -297,6 +355,8 @@ mod tests { fn anything_that_is_not_ours_is_not_a_directive() { assert!(parse("").is_none()); assert!(parse(r#""#).is_none()); + assert!(parse(r#""#).is_none()); + assert!(parse("- [ ] A checklist is display text").is_none()); assert!(parse("Mention @agency:items.state in a sentence").is_none()); // Missing the fields it exists to carry. assert!(parse(r#""#).is_none()); diff --git a/apps/gui/src/projects.rs b/apps/gui/src/projects.rs index 2ee2ab0..5db4105 100644 --- a/apps/gui/src/projects.rs +++ b/apps/gui/src/projects.rs @@ -304,6 +304,15 @@ fn agent_wire_name(agent: Agent) -> &'static str { } } +/// Whether the next provider text event starts a new visible block. +/// +/// Codex `exec --json` emits each completed commentary or final message as a +/// whole `Event::Text`. Claude and Copilot may emit adjacent deltas of one +/// message, so their consecutive text events must stay adjacent. +fn needs_text_break(agent: Agent, streamed_any: bool, last_was_text: bool) -> bool { + streamed_any && (!last_was_text || agent == Agent::Codex) +} + pub(crate) fn id(prefix: &str) -> String { format!("{prefix}-{}", uuid::Uuid::new_v4()) } @@ -333,262 +342,6 @@ fn name_from_prompt(prompt: &str) -> String { } } -/// The most items one reply may contribute. -/// -/// A bound rather than trust: a long plan should not turn the Items panel into -/// a wall, and a malformed reply should not be able to write a thousand rows. -const MAX_ITEMS_PER_REPLY: usize = 20; - -/// Work items found in the agent's reply. -/// -/// **Markdown checkboxes only**: `- [ ] title` and `- [x] title`. Deliberately -/// not bullets, numbered lists or headings: a checkbox is unambiguously a task, -/// whereas a bulleted list is just as often prose. Reading structure into -/// ordinary paragraphs would write items the agent never proposed, and an -/// invented to-do in someone's workspace is worse than an empty panel. -/// -/// Returns `(title, status)` in the order they appear, with `[x]` mapping onto -/// the `finished` status the panel already renders struck through. -/// One checkbox line, read. -#[derive(Debug, PartialEq, Eq)] -struct Checked { - /// The match key, and never carries the reference. See the `reference` - /// column on `ProjectItem` for why the two are kept apart. - title: String, - status: String, - /// A pull request or issue the line pointed at, without the `#`. - reference: Option, - /// Where the enclosing directive routed this row, when it routed it - /// anywhere. A project name as a person would write it, or a `proj-` id. - /// Never read off the line itself: which project a row belongs to is - /// something the host was asked to do, not something the row says about - /// itself. - project: Option, - /// Whether the line came from inside an authoring segment. - /// - /// A reply is model-generated text, so by default it is inert: it may - /// propose rows, and may not destroy them. Only text the agent deliberately - /// framed as a directive carries the destructive half of the vocabulary. - authored: bool, -} - -/// The one host verb that opens an authoring segment. -/// -/// Domain-qualified on purpose. `@project:ui` would name a *feature* of the -/// app and read like a field; `@agency:items.inject` names the application, the -/// surface, and the operation, so a reader can see it is the host being asked -/// to act rather than a value being passed. Anything the app is asked to *do* -/// is written this way, and the checkbox lines inside carry only what the row -/// *is*. -const INJECT_VERB: &str = "@agency:items.inject"; - -/// What an authoring segment routes its lines to. -#[derive(Debug, PartialEq, Eq)] -struct Segment { - /// The project named in the directive's arguments, if any. Absent means - /// the session's own project, which is the overwhelmingly common case. - project: Option, -} - -/// Read one `key: value` out of a directive's argument list. -/// -/// Quotes optional, since a project name is prose and a person writing one -/// should not have to think about which half of the pair needs them. -fn directive_arg<'a>(args: &'a str, key: &str) -> Option<&'a str> { - args.split(',').find_map(|pair| { - let (found, value) = pair.split_once(':')?; - (found.trim() == key).then(|| value.trim().trim_matches(|c| c == '"' || c == '\'').trim()) - }) -} - -/// Recognize ``. -/// -/// The span is Prompt Syntax's own tag, and the verb inside it is what makes -/// this segment ours: a `` carrying some other directive is not an item -/// segment, and its contents stay inert. Naming the verb rather than trusting -/// the tag is what keeps that true as the tag grows other uses. -fn segment_open(line: &str) -> Option { - let inner = line.trim().strip_prefix("')?.trim(); - let (verb, args) = match inner.split_once('(') { - Some((verb, args)) => (verb.trim(), Some(args.strip_suffix(')')?)), - None => (inner, None), - }; - - /* - * Canonical is lowercase; a capitalized spelling is casual input that - * compiles to it. Folded as ASCII on purpose, and this is the one place - * that rule differs from how project names are matched: a name is prose - * and folds by Unicode, whereas a verb is an identifier with an ASCII - * canonical form. The difference is what makes a homoglyph inert. A verb - * written with a Cyrillic `а` is not this verb, does not open a segment, - * and cannot quietly become one. - */ - if !verb.eq_ignore_ascii_case(INJECT_VERB) { - return None; - } - Some(Segment { - project: args - .and_then(|args| directive_arg(args, "project")) - .filter(|named| !named.is_empty()) - .map(str::to_string), - }) -} - -/// Split a trailing `(#35)` off a checkbox line. -/// -/// Written as a suffix because that is how a person writes it, and because the -/// alternative is a second field in a format whose whole appeal is that it is -/// one line of markdown. Only a trailing group of digits counts: a title that -/// happens to contain `(#3)` mid-sentence keeps it. -fn split_reference(line: &str) -> (&str, Option<&str>) { - let Some(open) = line.rfind("(#") else { - return (line, None); - }; - let rest = &line[open + 2..]; - let Some(digits) = rest.strip_suffix(')') else { - return (line, None); - }; - if digits.is_empty() || !digits.chars().all(|c| c.is_ascii_digit()) { - return (line, None); - } - (line[..open].trim_end(), Some(digits)) -} - -/// Read one checkbox line, or nothing. -fn checkbox(line: &str) -> Option { - let line = line.trim_start(); - // A list marker, then the box. `1.` and `1)` count: an agent writing - // an ordered checklist means the same thing. - let rest = line - .strip_prefix("- ") - .or_else(|| line.strip_prefix("* ")) - .or_else(|| line.strip_prefix("+ ")) - .or_else(|| { - let digits = line.trim_start_matches(|c: char| c.is_ascii_digit()); - (digits.len() < line.len()) - .then(|| { - digits - .strip_prefix(". ") - .or_else(|| digits.strip_prefix(") ")) - }) - .flatten() - })? - .trim_start(); - - /* - * `[ ]` proposes, `[~]` plans, `[/]` starts, `[>]` ships, `[x]` - * closes, `[-]` removes. - * - * `[ ]` opens a row as `new`: proposed, and nobody has decided - * anything about it yet. `[~]` is the phase before work, where the - * shape of the thing is still being argued about, which a list that - * jumped from proposed to in-progress could not show at all. - * - * The last two are this contract's own. Markdown has no "strike - * this row" checkbox, and a project session needs one: an obsolete - * item is not a *finished* item, and before `[-]` existed an agent - * working a backlog could add rows but never retire one, so the - * list only grew. `[-]` never creates, since removing something - * that does not exist is already true. - * - * `[>]` is the answer to a fix that was reported as done and was - * not. An agent can say it shipped something; it cannot say the - * thing works, because it is not the one looking at the screen. - * So it moves a row to `shipped` naming the pull request, and the - * row waits there for the owner. A copy bug was called fixed three - * times in one evening, and under the old vocabulary the row would - * have been deleted after the first. - */ - let (marker, title) = [ - ("new", "[ ] "), - ("planning", "[~] "), - ("active", "[/] "), - /* - * `[?]` is started and stuck on something only the owner can answer. - * - * Without it the agent's only honest options were to leave the row - * `active`, which claims work is happening while nothing is, or to - * write the question into the transcript, where it scrolls away. The - * list could not answer "which of these are waiting on me", which is - * the question the owner asks it most. - */ - ("questions", "[?] "), - ("shipped", "[>] "), - ("deleted", "[-] "), - ("finished", "[x] "), - ("finished", "[X] "), - ] - .into_iter() - .find_map(|(marker, checkbox)| rest.strip_prefix(checkbox).map(|title| (marker, title)))?; - - let (title, reference) = split_reference(title.trim()); - (!title.is_empty()).then(|| Checked { - title: truncate_on_char_boundary(title, 120), - status: marker.to_string(), - reference: reference.map(str::to_string), - // Both filled by the reader, from the directive around the line. - project: None, - authored: false, - }) -} - -/// Read a reply as a sequence of lines with a provenance for each. -/// -/// Two rules, and they are the same rule twice. Fenced text is content: an -/// agent quoting a file that happens to contain a checklist is reporting, not -/// asking, and reading those lines as directives writes rows nobody proposed. -/// Unfenced prose is content too, but weakly, because the checkbox vocabulary -/// grew up there and an agent narrating its own work is genuinely proposing -/// rows. -/// -/// So an ordinary line may still open, plan, start and ship a row in the -/// session's own project, and may not destroy one and may not reach another -/// project. Both of those need ``, which is the -/// agent saying "this is a directive" rather than the app guessing. -fn items_from_reply(reply: &str) -> Vec { - let mut read = Vec::new(); - let mut fenced = false; - let mut segment: Option = None; - - for line in reply.lines() { - let trimmed = line.trim(); - if trimmed.starts_with("```") || trimmed.starts_with("~~~") { - fenced = !fenced; - continue; - } - if fenced { - continue; - } - if let Some(open) = segment_open(trimmed) { - segment = Some(open); - continue; - } - if trimmed == "" { - segment = None; - continue; - } - let Some(mut item) = checkbox(line) else { - continue; - }; - match &segment { - Some(open) => { - item.project.clone_from(&open.project); - item.authored = true; - } - // The destructive half of the vocabulary needs a directive. A - // reply is model-generated text, and text can be quoted, echoed - // back or pasted from a file: none of that may remove a row. - None if item.status == "deleted" => continue, - None => {} - } - read.push(item); - if read.len() == MAX_ITEMS_PER_REPLY { - break; - } - } - read -} - /// What a task row reads as, built from the tool's own arguments. /// /// This has to happen in Rust: `ToolCall::input` arrives in the *agent's* shape, @@ -628,12 +381,6 @@ fn tool_label(name: &str, input: &serde_json::Value) -> String { truncate_on_char_boundary(&label.replace('\n', " "), 160) } -/// Cut to `max` characters, never mid-character. See -/// [`truncate_on_char_boundary`]; re-exported for `tasks.rs`. -pub fn clip(text: &str, max: usize) -> String { - truncate_on_char_boundary(text, max) -} - /// Cut to `max` characters, never mid-character. /// /// Byte slicing a multi-byte character panics, and a tool argument is arbitrary @@ -843,6 +590,35 @@ pub async fn set_item_status( ); return Err(format!("not an item status: {status}")); } + let delete_completed = status == "finished" + && state + .tables + .kv_get(crate::settings::KEY) + .and_then(|raw| serde_json::from_str::(&raw).ok()) + .unwrap_or_default() + .completed_items + == "delete"; + if delete_completed { + let mut row = state + .tables + .project_item + .select(id.clone()) + .ok_or_else(|| format!("no item {id}"))?; + state + .tables + .project_item + .delete(id.clone()) + .await + .map_err(|error| error.to_string())?; + let _ = app.emit( + "item:deleted", + serde_json::json!({ "id": id, "projectId": row.project_id }), + ); + // Preserve the command's return shape for callers that await it even + // though the event removes the row from the live store. + row.status = status; + return Ok(ProjectItemDto::from(row)); + } state .tables .project_item @@ -1048,199 +824,6 @@ impl From<&TaskLogEntryDto> for TaskLogRow { } } -/// Turn the task manager's JSONL block into item rows. -/// -/// Separate from the checkbox scan the ordinary projects use: this reply is -/// answering a contract rather than writing prose, so a line that does not -/// parse is a broken contract worth reporting rather than a sentence to skip. -/// -/// The count of rejected lines goes to the I/O panel. A model that has drifted -/// off the format produces a short list and no error anywhere else, which is -/// exactly the failure that looks like the feature not working. -async fn write_tasks_from_reply( - app: &AppHandle, - io: &AgentIo, - tables: &crate::db::tables::Tables, - project_id: &str, - reply: &str, -) { - let harvest = crate::tasks::harvest(reply); - - note_io( - app, - io, - project_id, - "gui", - "harvest", - format!( - "{} task(s) parsed from the reply, {} line(s) rejected", - harvest.tasks.len(), - harvest.rejected - ), - ); - crate::log!( - crate::log::Level::Info, - "tasks", - "{project_id}: harvested {} task(s), rejected {}", - harvest.tasks.len(), - harvest.rejected - ); - - if harvest.tasks.is_empty() { - return; - } - - /* - * Harvested tasks land on real projects: the `project` value names one, - * matched case-insensitively against what exists, created bare when - * nothing matches. This replaces the first design — items parked on the - * task manager's own project with the name folded into the title — which - * asked the user to re-type every line the model had already structured. - * - * A created project is bare on purpose: a row and a tab, no first message - * and no agent run. The task manager organises; it does not start work. - */ - let all_projects: Vec = tables.project.select_all().execute().unwrap_or_default(); - let mut project_ids: std::collections::HashMap = all_projects - .iter() - .map(|row| (row.name.trim().to_lowercase(), row.id.clone())) - .collect(); - let mut order = u32::try_from(all_projects.len()).unwrap_or(0); - let mut created: Vec = Vec::new(); - let mut placed = 0usize; - - let mut removed = 0usize; - for task in harvest.tasks { - let key = task.project.trim().to_lowercase(); - - /* - * The one destructive verb, and it only ever removes an exact match: - * a delete against a project or title that does not exist is a no-op, - * never a fuzzy guess. Absence from the output changes nothing — the - * contract says so to the model, and this code says it to the store. - */ - if task.status == "deleted" { - let Some(target_id) = project_ids.get(&key).cloned() else { - continue; - }; - let existing: Vec = tables - .project_item - .select_by_project_id(target_id) - .execute() - .unwrap_or_default(); - for row in existing { - if row.title.to_lowercase() == task.item.to_lowercase() { - match tables.project_item.delete(row.id.clone()).await { - Ok(()) => { - removed += 1; - let _ = app.emit( - "item:deleted", - serde_json::json!({ "id": row.id, "projectId": row.project_id }), - ); - } - Err(error) => crate::log!( - crate::log::Level::Error, - "tasks", - "{project_id}: could not delete a task: {error}" - ), - } - } - } - continue; - } - - let target_id = match project_ids.get(&key) { - Some(found) => found.clone(), - None => { - let row = ProjectRow { - id: id("proj"), - name: task.project.trim().to_string(), - status: "active".into(), - position: order, - dirs: "[]".into(), - pinned: false, - moderator_enabled: false, - forked_from: String::new(), - last_activity_at: now(), - }; - if let Err(error) = tables.project.insert(row.clone()) { - crate::log!( - crate::log::Level::Error, - "tasks", - "could not create project {:?}: {error}", - task.project - ); - continue; - } - order += 1; - created.push(row.name.clone()); - let dto = with_session(ProjectDto::from(row.clone()), tables); - let _ = app.emit("project:created", &dto); - project_ids.insert(key, row.id.clone()); - row.id - } - }; - - // Appended after what is there, duplicates skipped: a later prompt - // restating the list must not stack the same line up or disturb rows - // the user added themselves. - let existing: Vec = tables - .project_item - .select_by_project_id(target_id.clone()) - .execute() - .unwrap_or_default(); - if existing - .iter() - .any(|row| row.title.to_lowercase() == task.item.to_lowercase()) - { - continue; - } - let row = ProjectItemRow { - id: id("item"), - project_id: target_id, - title: task.item, - status: task.status, - position: u32::try_from(existing.len()).unwrap_or(0), - reference: String::new(), - }; - match tables.project_item.insert(row.clone()) { - Ok(_) => { - placed += 1; - let _ = app.emit("item:created", ProjectItemDto::from(row)); - } - Err(error) => crate::log!( - crate::log::Level::Error, - "tasks", - "{project_id}: could not write a task: {error}" - ), - } - } - - // Said where the harvest count already lives, so "where did my tasks go" - // is answerable from the same panel that reported them parsed. - let mut summary = format!("{placed} item(s) placed"); - if removed > 0 { - summary.push_str(&format!(", {removed} deleted")); - } - if !created.is_empty() { - summary.push_str(&format!("; created project(s): {}", created.join(", "))); - } - note_io(app, io, project_id, "gui", "harvest", summary); -} - -/// Turn any checklist in the reply into item rows, appended after what is there. -/// -/// Appends rather than replaces: the panel's items are the user's list too, and -/// a later turn restating the plan must not delete the rows they added or the -/// ones they already ticked off. Duplicate titles are skipped so an agent that -/// repeats its checklist each turn does not stack the same line up. -/// -/// A line lands on the session's own project unless a directive around it says -/// otherwise. Every session can write anywhere for the same reason the task -/// manager always could: the app owns the store, so which project a row belongs -/// to is something the host can be asked to decide, not a consequence of which -/// window the reply came from. Work spills across projects constantly, and the -/// alternative is telling the user to go and re-type it somewhere else. /// What this project's list looks like right now, for the prompt. /// /// The agent was being asked to maintain a list it had never been shown. Every @@ -1272,7 +855,9 @@ fn state_snapshot( .unwrap_or_default(); let mut out = String::new(); - if items.is_empty() { + if project_id == crate::tasks::TASK_MANAGER_ID { + out.push_str("Home Task Manager uses this same authoring surface for every mutation."); + } else if items.is_empty() { out.push_str("This project has no open items."); } else { out.push_str("Open items in this project. Answer with the id, never the title:\n"); @@ -1332,9 +917,11 @@ fn state_snapshot( out.push_str( "\nSay so with a directive on its own line, as it happens rather than at the end:\n\ \", status: \"active\")>\n\ - \", status: \"shipped\", pr: 66)>\n\ + \", status: \"shipped\", pr: \"https://github.com/owner/repo/pull/66\")>\n\ \", status: \"planning\")>\n\ - \")> removes a row that should not be there\n\ + \", ref: \"t2\", title: \"\")>\n\ + \")>\n\ + \")>\n\ {declared}\n\ Statuses you may set: new, planning, active, questions, shipped. `questions` \ means you are stopped on something only the owner can answer. `finished` and \ @@ -1358,9 +945,15 @@ async fn apply_directive( ) -> crate::directives::Outcome { use crate::directives::{Directive, Outcome}; + /* + * IDs are installation-wide, and the declared surface explicitly reaches + * any project in this store. Reading the whole id set is what makes the + * Task Manager able to speak the same state/retire/link verbs as a project + * tab instead of needing title-based JSONL mutations of its own. + */ let rows: Vec = tables .project_item - .select_by_project_id(project_id.to_string()) + .select_all() .execute() .unwrap_or_default(); @@ -1384,7 +977,27 @@ async fn apply_directive( }; } }; - if let Some(number) = pr.as_deref() + let target_project = rows + .iter() + .find(|row| row.id == resolved) + .map(|row| row.project_id.as_str()) + .unwrap_or(project_id); + let pr_number = match pr.as_deref() { + Some(url) if url.starts_with("https://github.com/") => { + match crate::prs::record_url(app, tables, target_project, url) { + Ok(number) => Some(number.to_string()), + Err(code) => { + return Outcome::Refused { + what: format!("items.state({resolved}) pull request"), + code, + }; + } + } + } + Some(number) => Some(number.to_string()), + None => None, + }; + if let Some(number) = pr_number.as_deref() && let Err(error) = tables .project_item .update_reference_by_id( @@ -1414,7 +1027,7 @@ async fn apply_directive( if let Some(updated) = tables.project_item.select(resolved.clone()) { let _ = app.emit("item:updated", ProjectItemDto::from(updated)); } - let said = match pr { + let said = match pr_number { Some(number) => format!("{resolved} -> {status} (#{number})"), None => format!("{resolved} -> {status}"), }; @@ -1428,18 +1041,77 @@ async fn apply_directive( } Directive::ItemAdd { handle, + project, title, status, } => { - let status = if crate::directives::settable(&status) { - status - } else { - "new".to_string() + if !crate::directives::settable(&status) { + return Outcome::Refused { + what: format!("items.add({title:?} -> {status})"), + code: "STATUS_NOT_YOURS".into(), + }; + } + let target_project = match project.as_deref() { + Some(named) => { + let projects: Vec = + tables.project.select_all().execute().unwrap_or_default(); + match resolve_project(&projects, named) { + Some(found) => found, + None if project_id == crate::tasks::TASK_MANAGER_ID => { + /* + * Home has always been able to turn a named task + * group into a bare project. Keeping that ability + * on the explicit `project:` argument preserves + * the feature without treating prose as a command. + */ + let row = ProjectRow { + id: id("proj"), + name: named.trim().to_string(), + status: "active".into(), + position: u32::try_from(projects.len()).unwrap_or(0), + dirs: "[]".into(), + pinned: false, + moderator_enabled: false, + forked_from: String::new(), + last_activity_at: now(), + }; + if let Err(error) = tables.project.insert(row.clone()) { + return Outcome::Refused { + what: format!("items.add({title:?}) project"), + code: format!("WRITE_FAILED: {error}"), + }; + } + let _ = app.emit( + "project:created", + with_session(ProjectDto::from(row.clone()), tables), + ); + row.id + } + None => { + return Outcome::Refused { + what: format!("items.add({title:?}) project {named:?}"), + code: "ENTITY_NOT_FOUND".into(), + }; + } + } + } + None if project_id == crate::tasks::TASK_MANAGER_ID => { + return Outcome::Refused { + what: format!("items.add({title:?}) project"), + code: "ENTITY_NOT_FOUND".into(), + }; + } + None => project_id.to_string(), }; + let target_rows: Vec<&ProjectItemRow> = rows + .iter() + .filter(|row| row.project_id == target_project) + .collect(); + // The same title twice is the agent restating itself, not a second // item. It answers with the existing id so the next reference is // by id, which is how a restatement stops becoming a duplicate. - if let Some(existing) = rows + if let Some(existing) = target_rows .iter() .find(|row| row.title.eq_ignore_ascii_case(title.trim())) { @@ -1450,10 +1122,10 @@ async fn apply_directive( } let row = ProjectItemRow { id: id("item"), - project_id: project_id.to_string(), + project_id: target_project, title: truncate_on_char_boundary(title.trim(), 120), status, - position: u32::try_from(rows.len()).unwrap_or(0), + position: u32::try_from(target_rows.len()).unwrap_or(0), reference: String::new(), }; match tables.project_item.insert(row.clone()) { @@ -1487,11 +1159,16 @@ async fn apply_directive( .find(|row| row.id == resolved) .map(|row| row.title.clone()) .unwrap_or_default(); + let target_project = rows + .iter() + .find(|row| row.id == resolved) + .map(|row| row.project_id.clone()) + .unwrap_or_else(|| project_id.to_string()); match tables.project_item.delete(resolved.clone()).await { Ok(()) => { let _ = app.emit( "item:deleted", - serde_json::json!({ "id": resolved, "projectId": project_id }), + serde_json::json!({ "id": resolved, "projectId": target_project }), ); Outcome::Done(format!("{resolved} retired {title:?}")) } @@ -1501,16 +1178,56 @@ async fn apply_directive( }, } } - Directive::PrLink { number, item } => { - let known: Vec<&str> = rows.iter().map(|row| row.id.as_str()).collect(); - let resolved = match crate::directives::resolve(&known, &item) { - Ok(found) => found.to_string(), - Err(code) => { - return Outcome::Refused { - what: format!("pr.link(item: {item})"), - code, + Directive::PrLink { url, number, item } => { + let linked = match item.as_deref() { + Some(item) => { + let known: Vec<&str> = rows.iter().map(|row| row.id.as_str()).collect(); + let resolved = match crate::directives::resolve(&known, item) { + Ok(found) => found.to_string(), + Err(code) => { + return Outcome::Refused { + what: format!("pr.link(item: {item})"), + code, + }; + } }; + let target_project = rows + .iter() + .find(|row| row.id == resolved) + .map(|row| row.project_id.clone()) + .unwrap_or_else(|| project_id.to_string()); + Some((resolved, target_project)) } + None => None, + }; + let target_project = linked + .as_ref() + .map(|(_, project)| project.as_str()) + .unwrap_or(project_id); + let tracked = match url.as_deref() { + Some(url) => match crate::prs::record_url(app, tables, target_project, url) { + Ok(found) => Some(found.to_string()), + Err(code) => { + return Outcome::Refused { + what: "pr.link(url)".into(), + code, + }; + } + }, + None => None, + }; + let number = tracked.or(number); + let Some((resolved, _)) = linked else { + return Outcome::Done(format!( + "pull request #{} tracked", + number.unwrap_or_else(|| "?".into()) + )); + }; + let Some(number) = number else { + return Outcome::Refused { + what: format!("pr.link({resolved})"), + code: "ENTITY_NOT_FOUND".into(), + }; }; match tables .project_item @@ -1537,82 +1254,52 @@ async fn apply_directive( } } -/// Read every directive out of a stretch of reply text and carry them out. +/// Return an authored PS line, preserving fence state across streamed chunks. /// -/// Fenced text is skipped for the same reason the checkbox scan skips it: an -/// agent quoting an example of the syntax is showing it, not issuing it. -async fn apply_directives( +/// The declared segment has to occupy its line. Markdown blockquotes, indented +/// code, and fenced code are quoted content, so PS-shaped text inside them is +/// inert. Keeping `fenced` outside this function is essential for streaming: +/// an opening fence, its contents, and its closer usually arrive as separate +/// calls. +fn authored_directive_line<'a>(line: &'a str, fenced: &mut bool) -> Option<&'a str> { + let line = line.trim_end_matches(['\r', '\n']); + let trimmed = line.trim(); + if trimmed.starts_with("```") || trimmed.starts_with("~~~") { + *fenced = !*fenced; + return None; + } + if *fenced || line.starts_with(" ") || line.starts_with('\t') || trimmed.starts_with('>') { + return None; + } + Some(trimmed) +} + +/// Read every directive out of a stretch of reply text and carry it out. +async fn apply_directives_with_state( app: &AppHandle, tables: &crate::db::tables::Tables, project_id: &str, text: &str, + fenced: &mut bool, ) -> Vec { let mut done = Vec::new(); - let mut fenced = false; for line in text.lines() { - let trimmed = line.trim(); - if trimmed.starts_with("```") || trimmed.starts_with("~~~") { - fenced = !fenced; - continue; - } - if fenced { - continue; - } - if let Some(directive) = crate::directives::parse(trimmed) { + if let Some(directive) = + authored_directive_line(line, fenced).and_then(crate::directives::parse) + { done.push(apply_directive(app, tables, project_id, directive).await); } } done } -async fn write_items_from_reply( +async fn apply_directives( app: &AppHandle, tables: &crate::db::tables::Tables, project_id: &str, - reply: &str, -) { - let proposed = items_from_reply(reply); - if proposed.is_empty() { - return; - } - - /* - * A line may say where it goes. Grouped rather than written one at a time - * so that each project is read once and its positions stay contiguous, - * and kept in the order the reply wrote them. - */ - let mut targets: Vec<(String, Vec)> = Vec::new(); - let mut projects: Option> = None; - for item in proposed { - let target = match item.project.as_deref() { - None => project_id.to_string(), - Some(named) => { - let rows = projects.get_or_insert_with(|| { - tables.project.select_all().execute().unwrap_or_default() - }); - match resolve_project(rows, named) { - Some(found) => found, - None => { - crate::log!( - crate::log::Level::Error, - "items", - "{project_id}: no project matches {named:?}, so {:?} was not written", - item.title - ); - continue; - } - } - } - }; - match targets.iter_mut().find(|(id, _)| *id == target) { - Some((_, group)) => group.push(item), - None => targets.push((target, vec![item])), - } - } - - for (target, group) in targets { - write_items_into(app, tables, &target, group).await; - } + text: &str, +) -> Vec { + apply_directives_with_state(app, tables, project_id, text, &mut false).await } /// Find the project a line named, by id or by name, case-insensitively. @@ -1636,171 +1323,6 @@ fn resolve_project(rows: &[ProjectRow], named: &str) -> Option { .map(|row| row.id.clone()) } -/// Write one project's worth of checkbox lines. -async fn write_items_into( - app: &AppHandle, - tables: &crate::db::tables::Tables, - project_id: &str, - proposed: Vec, -) { - let existing: Vec = tables - .project_item - .select_by_project_id(project_id.to_string()) - .execute() - .unwrap_or_default(); - let by_title: std::collections::HashMap = existing - .iter() - .map(|row| (row.title.to_lowercase(), row)) - .collect(); - let mut next = u32::try_from(existing.len()).unwrap_or(0); - - // Settings decide what "done" does to an existing row; read once per reply. - let delete_completed = tables - .kv_get(crate::settings::KEY) - .and_then(|raw| serde_json::from_str::(&raw).ok()) - .unwrap_or_default() - .completed_items - == "delete"; - - for Checked { - title, - status, - reference, - // Already spent: grouping the reply is what chose `project_id`. - project: _, - authored, - } in proposed - { - /* - * A checkbox line naming an item that already exists is a status - * report, not a proposal: `- [x] Fix the picker` from the session - * that just fixed it marks the row finished (or deletes it, per - * Settings). This is how a run closes out the item it was started - * from without any special verb. - */ - if let Some(row) = by_title.get(&title.to_lowercase()) { - if row.status == status { - continue; - } - /* - * A proposal never moves a row that already exists. - * - * `- [ ]` means "this should be done", which is already true of - * every row on the list, so re-listing the backlog in a reply must - * not knock a row being worked on back to the start. The contract - * has always said a proposal leaves an existing title alone; the - * code updated it anyway, which was survivable while `[ ]` meant - * `pending` and is not now that it means `new`. - */ - if status == "new" { - continue; - } - /* - * `[-]` removes outright, whatever Settings says about completed - * items: an obsolete row is not a finished one, and the setting - * governs what "done" means, not what "gone" means. `[x]` defers - * to the setting as before. - */ - /* - * `[-]` cannot reach here from ordinary prose, the reader drops - * it. `[x]` can, and under "delete completed items" it destroys a - * row just as thoroughly, so outside a directive it is honoured as - * the status it names and nothing more. The setting still governs - * what "done" means; it does not lend that power to a quoted line. - */ - if status == "deleted" || (status == "finished" && delete_completed && authored) { - let (row_id, row_project) = (row.id.clone(), row.project_id.clone()); - match tables.project_item.delete(row_id.clone()).await { - Ok(()) => { - let _ = app.emit( - "item:deleted", - serde_json::json!({ "id": row_id, "projectId": row_project }), - ); - } - Err(error) => crate::log!( - crate::log::Level::Error, - "items", - "{project_id}: could not delete the item: {error}" - ), - } - continue; - } - let row_id = row.id.clone(); - /* - * The reference is recorded before the status, so a row that - * reaches `shipped` always names where it went. A shipped row with - * no pull request on it is the state this whole verb exists to - * make impossible: it reads exactly like the "done" that was not. - */ - if let Some(number) = reference.as_deref() - && row.reference != number - && let Err(error) = tables - .project_item - .update_reference_by_id( - ItemReferenceByIdQuery { - reference: number.to_string(), - }, - row_id.clone(), - ) - .await - { - crate::log!( - crate::log::Level::Error, - "items", - "{project_id}: could not record the item's reference: {error}" - ); - } - match tables - .project_item - .update_status_by_id(ItemStatusByIdQuery { status }, row_id.clone()) - .await - { - Ok(()) => { - if let Some(updated) = tables.project_item.select(row_id) { - let _ = app.emit("item:updated", ProjectItemDto::from(updated)); - } - } - Err(error) => crate::log!( - crate::log::Level::Error, - "items", - "{project_id}: could not update the item's status: {error}" - ), - } - continue; - } - // `[-]` for a title that does not exist is already true — removing - // nothing must not create something. - if status == "deleted" { - continue; - } - let row = ProjectItemRow { - id: id("item"), - project_id: project_id.to_string(), - title, - status, - position: next, - reference: reference.unwrap_or_default(), - }; - match tables.project_item.insert(row.clone()) { - Ok(_) => { - next += 1; - let _ = app.emit("item:created", ProjectItemDto::from(row)); - } - Err(error) => crate::log!( - crate::log::Level::Error, - "items", - "{project_id}: could not write an item: {error}" - ), - } - } - crate::log!( - crate::log::Level::Info, - "items", - "{project_id}: {} item(s) after the reply", - next - ); -} - /// Milliseconds between two RFC 3339 stamps, or `None` if either will not parse. /// /// `None` rather than zero: an unparseable stamp means the duration is unknown, @@ -2521,7 +2043,7 @@ fn page_task_log( } } -/// The live project and task lists, for the task manager's eyes. +/// The live project and item ids, for the task manager's eyes. /// /// Bounded: a store with hundreds of tasks must not turn every prompt into a /// novel, so the block is cut at a ceiling with an honest marker. The cut is @@ -2536,11 +2058,11 @@ fn task_manager_snapshot(tables: &crate::db::tables::Tables) -> String { } let mut block = String::from( - "\n\n---\nCurrent projects and tasks, live from the store. To remove one, \ - emit its line with status \"deleted\"; to add or restate, emit it normally.\n", + "\n\n---\nCurrent projects and open items, live from the store. Address an \ + existing row by its item id, never by retyping its title.\n", ); 'outer: for project in &projects { - let header = format!("# {}\n", project.name); + let header = format!("# {} · {}\n", project.id, project.name); if block.len() + header.len() > CEILING { block.push_str("… (list truncated)\n"); break; @@ -2551,10 +2073,13 @@ fn task_manager_snapshot(tables: &crate::db::tables::Tables) -> String { .project_item .select_by_project_id(project.id.clone()) .execute() - .unwrap_or_default(); + .unwrap_or_default() + .into_iter() + .filter(|item| item.status != "finished" && item.status != "canceled") + .collect(); items.sort_by_key(|row| row.position); for item in &items { - let line = format!("- [{}] {}\n", item.status, item.title); + let line = format!(" {} · {} · {}\n", item.id, item.status, item.title); if block.len() + line.len() > CEILING { block.push_str("… (list truncated)\n"); break 'outer; @@ -4465,6 +3990,13 @@ async fn drive_run( ), ); + // The previous receipt has now been included in this request. Start a new + // one before streamed directives arrive so this turn cannot inherit old + // outcomes and so a live add keeps its newly assigned id in the receipt. + if let Ok(mut kept) = receipts.lock() { + kept.remove(&project_id); + } + let mut run = match agent_abstraction::stream(&request) { Ok(run) => run, Err(error) => { @@ -4508,7 +4040,7 @@ async fn drive_run( */ let mut streamed_text = String::new(); /* - * How much of `streamed_text` has been scanned for checkbox lines. + * How much of `streamed_text` has been scanned for PS directive lines. * * Items used to be written only from the finished reply, which made it * impossible to open one and then work on it in the same turn: the row did @@ -4516,7 +4048,8 @@ async fn drive_run( * asked to follow the procedure could not follow it, and the list only ever * described work that was already over. */ - let mut items_scanned_to = 0usize; + let mut directives_scanned_to = 0usize; + let mut directives_fenced = false; /* * The checkpoint clock. The reply is flushed to `kv` on the first delta @@ -4735,7 +4268,16 @@ async fn drive_run( ); } Event::Text(delta) => { - let delta = if streamed_any && !last_was_text { + /* + * Codex emits each completed commentary/final message as a + * separate Text event, not as deltas of one message. Joining + * two of them byte-for-byte glued a directive to the prose + * before it (`sentence.`), turning a valid authored + * segment into inert text. Other providers may genuinely + * stream adjacent deltas, so only Codex gets a block break at + * every Text boundary. + */ + let delta = if needs_text_break(agent, streamed_any, last_was_text) { format!("\n\n{delta}") } else { delta @@ -4748,50 +4290,49 @@ async fn drive_run( serde_json::json!({ "projectId": project_id, "delta": delta }), ); /* - * Checkbox lines take effect as they are written. + * Directives take effect as their complete line arrives. * * Only whole lines: the scan stops at the last newline seen, so - * a title still being streamed is never read as a shorter one. - * Without that, "Fix the copy bug" would land as "Fix the" and - * the real title would arrive later as a second row. + * a directive still being streamed is never read as a shorter + * one. * - * The end-of-turn pass still runs and is the backstop for a - * final line with no newline after it. It cannot double-apply: - * a row already at the status the line asks for is skipped. + * The end-of-turn pass handles only the unscanned tail, as a + * backstop for a final line with no newline after it. */ - if !is_task_manager { - while let Some(at) = streamed_text[items_scanned_to..].find('\n') { - let end = items_scanned_to + at + 1; - let line = streamed_text[items_scanned_to..end].to_string(); - items_scanned_to = end; - if !items_from_reply(&line).is_empty() { - /* - * Applied as the line completes rather than at the - * end of the turn. A state report is only useful - * while the work is happening: a panel that - * catches up when the run finishes has told you - * nothing you could not see from the transcript. - */ - let done = apply_directives(&app, &tables, &project_id, &line).await; - if !done.is_empty() { - note_io( - &app, - &io, - &project_id, - "received", - "directive", - done.iter() - .map(crate::directives::Outcome::line) - .collect::>() - .join("; "), - ); - if let Ok(mut kept) = receipts.lock() { - kept.entry(project_id.clone()) - .or_default() - .extend(done.iter().map(crate::directives::Outcome::line)); - } - } - write_items_from_reply(&app, &tables, &project_id, &line).await; + while let Some(at) = streamed_text[directives_scanned_to..].find('\n') { + let end = directives_scanned_to + at + 1; + let line = streamed_text[directives_scanned_to..end].to_string(); + directives_scanned_to = end; + /* + * Never gate PS parsing on another grammar. The old code + * called `apply_directives` only after a checkbox matched, + * so a standalone `items.state` line was guaranteed to be + * skipped until the turn ended. + */ + let done = apply_directives_with_state( + &app, + &tables, + &project_id, + &line, + &mut directives_fenced, + ) + .await; + if !done.is_empty() { + note_io( + &app, + &io, + &project_id, + "received", + "directive", + done.iter() + .map(crate::directives::Outcome::line) + .collect::>() + .join("; "), + ); + if let Ok(mut kept) = receipts.lock() { + kept.entry(project_id.clone()) + .or_default() + .extend(done.iter().map(crate::directives::Outcome::line)); } } } @@ -5209,10 +4750,11 @@ async fn drive_run( * tool calls vanished the moment a run finished. The terminal * text remains the fallback for a run that never streamed. */ - let body = if streamed_text.trim().is_empty() { - outcome.text.clone() - } else { + let used_streamed_body = !streamed_text.trim().is_empty(); + let body = if used_streamed_body { streamed_text + } else { + outcome.text.clone() }; let stop = match &outcome.stop { Stop::Completed => "completed".to_string(), @@ -5394,37 +4936,48 @@ async fn drive_run( } } - if is_task_manager { - // Parsed from the same body the transcript stores, so what the - // harvester saw and what the user reads can never disagree. - write_tasks_from_reply(&app, &io, &tables, &project_id, &body).await; + /* + * One reverse-channel parser for Home and project tabs. The + * mid-stream path has already applied every complete line. + * Only the unscanned tail is handled here, which catches a final + * directive without a newline while preserving the exact receipt + * from a live add instead of replaying it as "already open". + */ + let tail_at = if used_streamed_body { + directives_scanned_to.min(body.len()) } else { - /* - * The receipt starts empty for this turn, so what the agent is - * told next turn is what happened in the last one rather than - * everything since launch. The mid-stream path has already - * applied most of these; re-running them is free, because a - * state already set is a no-op and an id that resolved once - * resolves again. - */ - if let Ok(mut kept) = receipts.lock() { - kept.remove(&project_id); - } - let done = apply_directives(&app, &tables, &project_id, &body).await; + 0 + }; + let done = if used_streamed_body { + apply_directives_with_state( + &app, + &tables, + &project_id, + &body[tail_at..], + &mut directives_fenced, + ) + .await + } else { + apply_directives(&app, &tables, &project_id, &body).await + }; + if !done.is_empty() { + note_io( + &app, + &io, + &project_id, + "received", + "directive", + done.iter() + .map(crate::directives::Outcome::line) + .collect::>() + .join("; "), + ); if let Ok(mut kept) = receipts.lock() { - let lines: Vec = - done.iter().map(crate::directives::Outcome::line).collect(); - if lines.is_empty() { - kept.remove(&project_id); - } else { - kept.insert(project_id.clone(), lines); - } + kept.entry(project_id.clone()) + .or_default() + .extend(done.iter().map(crate::directives::Outcome::line)); } - write_items_from_reply(&app, &tables, &project_id, &body).await; } - // Any PR the reply mentions becomes a chip; `gh` fills it in - // off-path moments later. - crate::prs::harvest_prs(&app, &tables, &project_id, &body, item_id.as_deref()); emit_run_stopped( &app, &project_id, @@ -6099,97 +5652,6 @@ mod tests { assert_eq!(truncate_to_bytes("short", 8_000), "short"); } - /// Checkboxes are tasks. Bullets and prose are not, and reading items out of - /// them would write to-dos the agent never proposed. - #[test] - fn only_checkboxes_become_items() { - let reply = "Here is the plan:\n\ - - [ ] Port the model into az-core\n\ - * [x] Decide the store\n\ - 1. [ ] Pick the id scheme\n\ - - Just a bullet, not a task\n\ - 2. An ordinary numbered line\n\ - Some prose about [x] brackets in a sentence."; - - let read = items_from_reply(reply); - let seen: Vec<(&str, &str)> = read - .iter() - .map(|item| (item.title.as_str(), item.status.as_str())) - .collect(); - assert_eq!( - seen, - vec![ - ("Port the model into az-core", "new"), - ("Decide the store", "finished"), - ("Pick the id scheme", "new"), - ] - ); - } - - /* - * The state that only exists because "done" was not. - * - * An agent can say it shipped something; it cannot say the thing works, - * because it is not the one looking at the screen. A copy bug was reported - * fixed three times in one evening, and under the old vocabulary its row - * would have been deleted after the first. - */ - #[test] - fn a_row_can_be_started_and_shipped_without_being_finished() { - let read = items_from_reply( - "- [/] Fix copy in the prompt area\n- [>] Add a shipped state (#35)\n", - ); - - assert_eq!(read[0].status, "active"); - assert_eq!(read[0].reference, None); - assert_eq!(read[1].status, "shipped"); - assert_eq!(read[1].reference.as_deref(), Some("35")); - } - - /* - * The ladder a row climbs, and the two rungs that were missing. - * - * `new` is proposed and untriaged; `planning` is the phase before work, - * where the shape is still being argued about. A list that could only say - * proposed or in-progress had nowhere to put either. - */ - #[test] - fn a_row_can_be_new_and_then_planned() { - let read = items_from_reply("- [ ] Decide the memory key\n- [~] Decide the memory key\n"); - - assert_eq!(read[0].status, "new"); - assert_eq!(read[1].status, "planning"); - } - - /// The reference is its own field, so the title stays the match key: a row - /// shipped as `(#35)` still answers to the line that closes it. - #[test] - fn a_reference_never_becomes_part_of_the_title() { - let read = items_from_reply("- [>] Add a shipped state (#35)\n"); - assert_eq!(read[0].title, "Add a shipped state"); - - let closing = items_from_reply("- [x] Add a shipped state\n"); - assert_eq!(closing[0].title, read[0].title, "the same row is meant"); - } - - /// Only a trailing group of digits is a reference. A title that mentions - /// one mid-sentence keeps every character it was written with. - #[test] - fn only_a_trailing_number_reads_as_a_reference() { - assert_eq!( - split_reference("Fix the (#3) case in the parser"), - ("Fix the (#3) case in the parser", None) - ); - assert_eq!(split_reference("Ship it (#12)"), ("Ship it", Some("12"))); - assert_eq!(split_reference("Not a ref (#)"), ("Not a ref (#)", None)); - assert_eq!( - split_reference("Not a ref (#abc)"), - ("Not a ref (#abc)", None) - ); - } - - /// Only the two fields resolution looks at matter here; the rest is what an - /// empty project is. fn project_row(id: &str, name: &str) -> ProjectRow { ProjectRow { id: id.into(), @@ -6204,155 +5666,6 @@ mod tests { } } - /// The directive routes; the line stays a line. A title is data and must - /// not carry a word of the instruction that placed it. - #[test] - fn a_directive_routes_the_lines_it_encloses() { - let read = items_from_reply( - "\n\ - - [ ] Audit the token counter\n\ - - [>] Ship the parser (#39)\n\ - \n", - ); - assert_eq!(read.len(), 2); - assert_eq!(read[0].title, "Audit the token counter"); - assert_eq!(read[0].project.as_deref(), Some("Other project")); - assert!(read[0].authored); - // The reference is a fact about the row, so it stays on the row. - assert_eq!(read[1].reference.as_deref(), Some("39")); - assert_eq!(read[1].project.as_deref(), Some("Other project")); - - // Closed, so the next line is ordinary prose again and lands here. - let after = items_from_reply( - "\n\ - - [ ] There\n\ - \n\ - - [ ] Here\n", - ); - assert_eq!(after[1].project, None); - assert!(!after[1].authored); - } - - /// An id is accepted for the same reason a name is: a session that knows - /// where a row belongs may not know what that project is called. Quotes - /// are optional, because a project name is prose. - #[test] - fn a_directive_takes_an_id_or_a_bare_name() { - let by_id = items_from_reply( - "\n\ - - [ ] Audit\n\ - \n", - ); - assert_eq!( - by_id[0].project.as_deref(), - Some("proj-846b5542-fe2d-4d60-a296-7c10e1119562") - ); - - // No argument means here, which is the overwhelmingly common case and - // is still worth writing when the verbs need to be honoured. - let here = items_from_reply("\n- [-] Retire this\n\n"); - assert_eq!(here[0].project, None); - assert_eq!(here[0].status, "deleted"); - } - - /// The verb is what makes the segment ours. A span carrying some other - /// directive is somebody else's, and its contents stay content. - #[test] - fn only_our_verb_opens_a_segment() { - assert_eq!( - segment_open(""), - Some(Segment { - project: Some("ui".into()) - }) - ); - assert_eq!( - segment_open(""), - Some(Segment { project: None }) - ); - assert_eq!(segment_open(""), None); - assert_eq!( - segment_open(""), - None - ); - assert_eq!( - segment_open("Mention @agency:items.inject in a sentence"), - None - ); - } - - /// Lowercase is canonical, and a capitalized spelling is casual input that - /// compiles to it. - /// - /// The fold is ASCII, which is the security half of the same decision. The - /// second case is the canonical verb with a Cyrillic `а` in `agency`: a - /// confusable that bound silently would be how a directive gets past a - /// reader who can see no difference at all. - #[test] - fn a_capitalized_verb_is_an_alias_and_a_confusable_is_not() { - assert_eq!( - segment_open(""), - Some(Segment { - project: Some("ui".into()) - }) - ); - assert_eq!(segment_open(""), None); - } - - /// The bug this whole shape exists to close. An agent quoting a file is - /// reporting, not instructing, and a checklist inside that quote must not - /// touch the store. Proved rather than asserted: this exact reply used to - /// delete a row and reach into another project. - #[test] - fn quoted_content_is_never_a_directive() { - let read = items_from_reply( - "Here is what the README says:\n\ - \n\ - ```markdown\n\ - - [-] Ship the parser\n\ - - [ ] Something nobody asked for\n\ - ```\n\ - \n\ - That is the file, unchanged.\n", - ); - assert!(read.is_empty(), "read {read:?} out of quoted content"); - - // A directive inside a fence is quoted too, and quoting a directive - // is not issuing one. - let fenced_directive = items_from_reply( - "An example of the syntax:\n\ - \n\ - ```\n\ - \n\ - - [-] Delete everything\n\ - \n\ - ```\n", - ); - assert!( - read.is_empty(), - "read {fenced_directive:?} out of a fenced example" - ); - } - - /// Unfenced prose keeps the vocabulary it grew up with, minus the half - /// that destroys. An agent narrating its own work is genuinely proposing - /// rows; it just cannot remove one without saying so deliberately. - #[test] - fn prose_may_proffer_a_row_but_never_remove_one() { - let read = items_from_reply( - "- [ ] Open one\n- [~] Plan one\n- [/] Start one\n- [>] Ship one (#1)\n- [-] Remove one\n", - ); - assert_eq!( - read.iter() - .map(|item| item.status.as_str()) - .collect::>(), - ["new", "planning", "active", "shipped"], - "a bare `[-]` is not a directive" - ); - assert!(read.iter().all(|item| !item.authored)); - } - - /// Resolution never guesses. An unknown name returns nothing, and the - /// caller reports it, rather than a project appearing out of a typo. #[test] fn a_project_resolves_by_id_or_by_name_and_never_by_guess() { let rows = vec![ @@ -6368,66 +5681,6 @@ mod tests { assert_eq!(resolve_project(&[], "AgencyZero"), None); } - #[test] - fn a_reply_with_no_checklist_writes_no_items() { - assert!(items_from_reply("I read the file and it looks fine.").is_empty()); - assert!(items_from_reply("").is_empty()); - // An empty title is not an item. - assert!(items_from_reply("- [ ] ").is_empty()); - } - - /// A malformed or enormous reply must not be able to fill the panel. - #[test] - fn the_item_count_from_one_reply_is_bounded() { - let many = (0..100) - .map(|n| format!("- [ ] item {n}")) - .collect::>() - .join("\n"); - assert_eq!(items_from_reply(&many).len(), MAX_ITEMS_PER_REPLY); - } - - /// `[?]` is the state an agent needs when it is stuck on the owner. - /// - /// Without it the honest options were to leave the row `active`, which - /// claims work is happening while nothing is, or to put the question in the - /// transcript, where it scrolls away. Not destructive, so prose may say it: - /// an agent that stops mid-answer must be able to record that plainly. - #[test] - fn a_question_box_marks_a_row_as_waiting_on_the_owner() { - let read = items_from_reply("- [?] Which runner should macOS use\n"); - assert_eq!(read[0].status, "questions"); - assert_eq!(read[0].title, "Which runner should macOS use"); - assert!(!read[0].authored); - } - - /// The third box: `[-]` marks a row for removal, so an agent can retire an - /// obsolete item. The write path deletes on match and refuses to create - /// from it. - /// - /// It needs a directive around it. That is the one behaviour this shape - /// took away, and deliberately: `[-]` was the only verb in the vocabulary - /// that could destroy something, sitting in text that anything upstream - /// could have written. - #[test] - fn a_struck_box_parses_as_deleted_inside_a_directive() { - assert_eq!( - items_from_reply( - "\n\ - - [-] Verify release 0.1.4 upgrade path\n\ - " - ) - .into_iter() - .map(|item| (item.title, item.status)) - .collect::>(), - vec![( - "Verify release 0.1.4 upgrade path".to_string(), - "deleted".to_string() - )] - ); - // The box needs its title, same as the others. - assert!(items_from_reply("\n- [-] \n").is_empty()); - } - /// `ok` is a tri-state the column cannot hold, and the distinction between /// "the agent said it failed" and "the agent did not say" is the whole /// reason it is nullable. It has to survive the round trip. @@ -6596,6 +5849,44 @@ mod tests { assert!(BUSY_WITH_RUN_ALREADY.contains("already active")); } + #[test] + fn codex_message_boundaries_cannot_glue_a_directive_to_prose() { + assert!(needs_text_break(Agent::Codex, true, true)); + assert!(needs_text_break(Agent::Codex, true, false)); + assert!(!needs_text_break(Agent::Codex, false, false)); + + // Adjacent events from the other providers may be deltas of one + // message. Tool or thinking events still establish a block break. + assert!(!needs_text_break(Agent::Claude, true, true)); + assert!(needs_text_break(Agent::Claude, true, false)); + assert!(!needs_text_break(Agent::Copilot, true, true)); + } + + #[test] + fn streamed_fences_and_markdown_quotes_keep_ps_inert() { + let directive = r#""#; + let mut fenced = false; + + assert_eq!(authored_directive_line("```text\n", &mut fenced), None); + assert!(fenced); + assert_eq!(authored_directive_line(directive, &mut fenced), None); + assert_eq!(authored_directive_line("```\n", &mut fenced), None); + assert!(!fenced); + + assert_eq!( + authored_directive_line(&format!("> {directive}"), &mut fenced), + None + ); + assert_eq!( + authored_directive_line(&format!(" {directive}"), &mut fenced), + None + ); + assert_eq!( + authored_directive_line(directive, &mut fenced), + Some(directive) + ); + } + /// An agent that reported nothing must not look like a free turn. #[test] fn an_unreported_usage_sums_to_zero_rather_than_inventing_one() { diff --git a/apps/gui/src/prs.rs b/apps/gui/src/prs.rs index 28c84ca..8b5e884 100644 --- a/apps/gui/src/prs.rs +++ b/apps/gui/src/prs.rs @@ -1,10 +1,11 @@ //! Pull requests cut during runs, tracked as chips over the composer. //! -//! The transcript is the source: any GitHub PR URL in an agent's reply becomes -//! a row (once per project), and `gh` — when installed — fills in what the -//! chip shows: state, branch, +adds −dels, and the CI rollup. No `gh`, no -//! problem: the chip still exists with what the URL alone says, which is the -//! repo and the number. Everything else reads `unknown` rather than a guess. +//! An authored Prompt Syntax directive is the source: `@agency:pr.link` or an +//! item's `pr:` field turns a GitHub PR URL into a row (once per project), and +//! `gh`, when installed, fills in what the chip shows: state, branch, +adds +//! −dels, and the CI rollup. A URL in prose is inert. No `gh`, no problem: the +//! chip still exists with what the directive says, and everything else reads +//! `unknown` rather than a guess. use serde::Serialize; use tauri::{AppHandle, Emitter, Manager, State}; @@ -86,81 +87,60 @@ fn pr_urls(text: &str) -> Vec<(String, String, u32)> { found } -/// Record any PRs a reply mentions and start a background refresh for each. +/// Record the one GitHub PR URL an authored directive names. /// -/// Called from the run's completion path, after the reply row is safe. Rows -/// are born `unknown` and honest; `gh` upgrades them moments later when it is -/// installed and authenticated. +/// URLs in prose are inert. The old completion hook scanned every reply and +/// wrote a row for anything URL-shaped, which made ordinary model text an +/// undeclared authoring surface. The caller now reaches this function only +/// after `` or an item's `pr:` field has parsed. /// -/// `started_from` is the item the run began on, when it began on one. A pull -/// request opened during that run belongs to that item, and the app knows it -/// without being told: the association is a fact about the run, not something -/// the agent has to remember to report. Only a newly recorded pull request -/// attaches, and only to a row that names none yet, so nothing already linked -/// is ever overwritten. -pub fn harvest_prs( +/// Rows are born `unknown` and honest; `gh` upgrades them moments later when +/// it is installed and authenticated. The number is returned so the same +/// directive can attach it to an item without parsing the URL twice. +pub fn record_url( app: &AppHandle, tables: &Tables, project_id: &str, - reply: &str, - started_from: Option<&str>, -) { - let mentioned = pr_urls(reply); - if mentioned.is_empty() { - return; - } + authored_url: &str, +) -> Result { + let Some((url, repo, number)) = pr_urls(authored_url).into_iter().next() else { + return Err("ENTITY_NOT_FOUND: not a GitHub pull request URL".into()); + }; let existing: Vec = tables .pull_request .select_by_project_id(project_id.to_string()) .execute() .unwrap_or_default(); - for (url, repo, number) in mentioned { - let known = existing.iter().find(|row| row.url == url); - let id = match known { - Some(row) => row.id.clone(), - None => { - let row = PullRequestRow { - id: crate::projects::id("pr"), - project_id: project_id.to_string(), - url: url.clone(), - repo, - number, - branch: String::new(), - state: "unknown".into(), - additions: 0, - deletions: 0, - ci: "unknown".into(), - dismissed: false, - updated_at: crate::projects::now(), - }; - let id = row.id.clone(); - let opened = row.number; - match tables.pull_request.insert(row.clone()) { - Ok(_) => { - let _ = app.emit("pr:updated", PullRequestDto::from(row)); - if let Some(item) = started_from { - attach(app, tables, item, opened); - } - } - Err(error) => { - crate::log!( - crate::log::Level::Error, - "prs", - "{project_id}: could not record {url}: {error}" - ); - continue; - } - } - id - } + if !existing.iter().any(|row| row.url == url) { + let row = PullRequestRow { + id: crate::projects::id("pr"), + project_id: project_id.to_string(), + url: url.clone(), + repo, + number, + branch: String::new(), + state: "unknown".into(), + additions: 0, + deletions: 0, + ci: "unknown".into(), + dismissed: false, + updated_at: crate::projects::now(), }; - // A mention of a known PR is still news: merged since last time, CI - // done. One batched refresh covers every row this reply touched. - let _ = id; + tables.pull_request.insert(row.clone()).map_err(|error| { + crate::log!( + crate::log::Level::Error, + "prs", + "{project_id}: could not record {url}: {error}" + ); + format!("WRITE_FAILED: {error}") + })?; + let _ = app.emit("pr:updated", PullRequestDto::from(row)); } - // One query for the project, after every mention in this reply is recorded. + + // A known PR is still news: it may have merged since the last refresh. refresh_project(app.clone(), project_id.to_string()); + Ok(number) } /// The `owner/name` this working directory pushes to, if any. @@ -555,55 +535,12 @@ pub fn refresh_pull_request(app: AppHandle, id: String) { } } -/// Point an item at the pull request its run produced. -/// -/// Nothing happens if the row already names one: a run that mentions a second -/// pull request has not changed which one the item shipped as, and quietly -/// repointing it would lose the first. -fn attach(app: &AppHandle, tables: &Tables, item_id: &str, number: u32) { - let Some(row) = tables.project_item.select(item_id.to_string()) else { - return; - }; - if !row.reference.is_empty() { - return; - } - let app = app.clone(); - // Just this table's handle: `Tables` is a bag of `Arc`s and the write is - // one row on one of them. - let items = tables.project_item.clone(); - let id = item_id.to_string(); - tauri::async_runtime::spawn(async move { - if let Err(error) = items - .update_reference_by_id( - crate::db::schema::project_item::ReferenceByIdQuery { - reference: number.to_string(), - }, - id.clone(), - ) - .await - { - crate::log!( - crate::log::Level::Error, - "prs", - "could not attach #{number} to {id}: {error}" - ); - return; - } - if let Some(updated) = items.select(id) { - let _ = app.emit( - "item:updated", - crate::projects::ProjectItemDto::from(updated), - ); - } - }); -} - #[cfg(test)] mod tests { use super::pr_urls; #[test] - fn urls_are_found_in_prose_and_markdown() { + fn authored_values_may_contain_markdown_link_syntax() { let text = "Opened [PR #16](https://github.com/pathscale/agencyzero/pull/16) and \ https://github.com/pathscale/agencyzero/pull/17. Done."; let found = pr_urls(text); diff --git a/apps/gui/src/tasks.rs b/apps/gui/src/tasks.rs index cdd2442..f5a8f2c 100644 --- a/apps/gui/src/tasks.rs +++ b/apps/gui/src/tasks.rs @@ -1,33 +1,18 @@ -//! The Home task manager: one long-running conversation that keeps the project -//! and item lists in order. +//! Home's Task Manager: one long-running conversation that maintains the same +//! project items an ordinary project tab does. //! -//! # Why it is a project +//! # One reverse-channel language //! -//! Home needs a transcript, a session to resume, a model, a cost and a raw I/O -//! trail — every one of which a project already has. Inventing a parallel set of -//! those for one screen would mean two of everything, so Home *is* a project, -//! reserved under a fixed id and hidden from the project lists. +//! Home used to switch the agent into a private `AZ-TASKS-BEGIN` JSONL format. +//! Project tabs used Prompt Syntax directives, and PRs were found by scanning +//! prose for URLs. That made an agent learn three unrelated mutation channels +//! and made ordinary output executable in two of them. //! -//! # The output contract -//! -//! The reply has to become rows. Asking a model for prose and then parsing it is -//! how you get a to-do list that is subtly wrong; asking for JSONL and refusing -//! anything else is how you get one that is either right or visibly empty. -//! -//! So the user's own words go out unchanged, with [`OUTPUT_CONTRACT`] appended. -//! One JSON object per line, each naming a project and an item. Anything that -//! does not parse is skipped and counted rather than guessed at — a task list -//! that quietly invents entries is worse than one that comes back short. -//! -//! # What it cannot do yet -//! -//! It reads what you paste and what the agent can reach through its own tools in -//! the workspace directory. **It cannot read the WorkTable store**: those files -//! are rkyv, a binary layout with no text form, so an agent pointed at them sees -//! bytes. Giving it real access to its own data means a query tool — see -//! `docs/task-manager.md`. - -use serde::{Deserialize, Serialize}; +//! Home now uses the declared Prompt Syntax surface too. The live snapshot +//! gives every existing row's id, so updates and removals use `items.state` +//! and `items.retire`; new rows use `items.add(project: ...)`. The Task Manager +//! keeps its one extra capability: when that explicit `project:` argument does +//! not resolve, the host creates a bare project before adding the item. /// The reserved project id for Home's conversation. /// @@ -35,342 +20,50 @@ use serde::{Deserialize, Serialize}; /// prefixed differently from `proj-` so it can never collide with a real one. pub const TASK_MANAGER_ID: &str = "home-task-manager"; -/// Opens the machine-readable block. Only lines between the markers are -/// authoritative; see [`harvest`] for what happens without them. -pub const TASKS_BEGIN: &str = "AZ-TASKS-BEGIN"; -/// Closes the machine-readable block. -pub const TASKS_END: &str = "AZ-TASKS-END"; - -/// The most tasks one reply may mutate. The live lists are dozens of rows; -/// a reply proposing hundreds is a malfunction, not a plan. -const MAX_TASKS: usize = 100; - -/// Appended to whatever the user types. +/// Appended to the user's Task Manager prompt. /// -/// Deliberately explicit about the failure mode: a model told only "return -/// JSONL" will still wrap it in prose or a fence often enough to matter, and -/// every one of those is a dropped line. +/// The general surface declaration is already in the system prompt. This +/// narrows it to Home's job and states the one context-specific capability: +/// an authored `items.add(project: ...)` may create the named project. pub const OUTPUT_CONTRACT: &str = "\n\n\ ---\n\ -When you have finished, end your reply with a machine-readable block so this \ -application can store what you produced.\n\n\ -The block must be wrapped in exactly these two marker lines, each alone on \ -its own line:\n\n\ -AZ-TASKS-BEGIN\n\ -{\"project\": \"\", \"item\": \"\", \"status\": \"pending\"}\n\ -AZ-TASKS-END\n\n\ -Emit one JSON object per line between the markers, nothing else in the block, \ -no markdown fence, no commentary between lines.\n\n\ +Maintain the project list using the declared Prompt Syntax authoring surface. \ +There is no second Task Manager output format.\n\n\ +Use these same single-line directives used in an ordinary project:\n\ +\", ref: \"t1\", title: \"\", status: \"new\")>\n\ +\", status: \"active\")>\n\ +\")>\n\ +\")>\n\n\ Rules:\n\ -- `status` is one of pending, active, finished, deleted.\n\ -- `deleted` removes the existing task whose project and item match. This is \ -the only way to remove one: omitting a task from your output never deletes \ -it, so never re-emit a list hoping the absences take effect. Deletions are \ -honoured only inside the markers.\n\ -- Only the marked block is read as instructions. When *discussing* a task or \ -quoting an example, never place it between marker lines — and never write the \ -marker lines anywhere except around your real block.\n\ -- Use exactly the three fields shown. A line with extra fields is rejected.\n\ -- One task per line. Do not number them.\n\ -- Keep `item` under 120 characters and specific enough to act on.\n\ -- Group related tasks by repeating the same `project` value.\n\ -- If you have no tasks to record, emit no block at all rather than an empty \ -one or an explanatory line.\n\ -- Put the block last, after any prose you want to write."; - -/// One task the agent proposed. -/// -/// `deny_unknown_fields` because these lines are promoted to database -/// mutations: JSON quoted from a README, a log or an issue almost always -/// carries extra fields, and rejecting it is the cheapest way to tell the -/// contract's shape apart from the world's. -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -#[serde(deny_unknown_fields)] -pub struct ProposedTask { - pub project: String, - pub item: String, - #[serde(default = "default_status")] - pub status: String, -} - -fn default_status() -> String { - "pending".to_string() -} - -/// What a reply yielded, and what it cost us to read it. -#[derive(Debug, Default, PartialEq)] -pub struct Harvest { - pub tasks: Vec, - /// Lines that looked like JSON and were not usable. Reported rather than - /// hidden: a contract the model keeps drifting from is worth knowing about. - pub rejected: usize, -} - -/// Pull the task block out of a reply. -/// -/// The marked block is the authority: when `AZ-TASKS-BEGIN` appears, only -/// lines between it and `AZ-TASKS-END` are read, so a task the model merely -/// *quotes* in its prose — from a README, an example, a discussion of the -/// format — cannot mutate anything. A `BEGIN` whose `END` never arrives runs -/// to the end of the reply, because a forgotten closer should not discard a -/// real block. -/// -/// Without any marker the whole reply is scanned as before — models move or -/// forget delimiters often enough that refusing the reply outright would lose -/// whole harvests — but that lenient path is additive only: a `deleted` line -/// outside the markers is refused and counted, never applied. A stray quoted -/// line can at worst add a row someone deletes; it can no longer destroy one. -/// -/// A line that starts like JSON and does not parse counts as rejected. Prose -/// is ignored entirely — it is not an error for a reply to contain sentences. -#[must_use] -pub fn harvest(reply: &str) -> Harvest { - let mut out = Harvest::default(); - - let marked: Vec<&str> = { - let mut lines = Vec::new(); - let mut inside = false; - for line in reply.lines() { - let bare = line.trim().trim_matches('`'); - if bare == TASKS_BEGIN { - inside = true; - } else if bare == TASKS_END { - inside = false; - } else if inside { - lines.push(line); - } - } - lines - }; - - if marked.is_empty() { - scan(reply.lines(), false, &mut out); - } else { - scan(marked.into_iter(), true, &mut out); - } - out -} - -/// One pass over candidate lines. `allow_delete` is what separates the marked -/// block from the lenient whole-reply fallback. -fn scan<'a>(lines: impl Iterator, allow_delete: bool, out: &mut Harvest) { - for line in lines { - let line = line.trim().trim_start_matches("```json").trim_matches('`'); - if !line.starts_with('{') { - continue; - } - - match serde_json::from_str::(line) { - Ok(task) if !task.project.trim().is_empty() && !task.item.trim().is_empty() => { - let status = normalize_status(&task.status); - // Destructive words need the marked block; see `harvest`. - // A reply proposing more than MAX_TASKS is a malfunction and - // the excess is refused rather than trusted. - if (status == "deleted" && !allow_delete) || out.tasks.len() >= MAX_TASKS { - out.rejected += 1; - continue; - } - out.tasks.push(ProposedTask { - project: crate::projects::clip(task.project.trim(), 80), - item: crate::projects::clip(task.item.trim(), 120), - status, - }); - } - // Parsed but useless, or did not parse at all. Both are the model - // not keeping the contract, and both are worth counting. - _ => out.rejected += 1, - } - } -} - -/// Anything unrecognized becomes `pending` rather than being dropped. -/// -/// A task whose status we cannot read is still a task; refusing it would lose -/// work over a spelling. `deleted` is the one destructive word and is matched -/// strictly for that reason: "remove"-adjacent spellings fall through to -/// `pending`, because guessing at a deletion is worse than adding a stray row. -fn normalize_status(raw: &str) -> String { - match raw.trim().to_ascii_lowercase().as_str() { - "active" | "in_progress" | "in-progress" | "doing" => "active", - "finished" | "done" | "complete" | "completed" => "finished", - "deleted" => "deleted", - _ => "pending", - } - .to_string() -} +- Existing rows are addressed by the item id in the live snapshot, never by title.\n\ +- `items.add` must name `project`. Here only, if that exact project id or name \ +does not exist, the application creates a bare project before adding the row.\n\ +- Statuses are new, planning, active, questions, or shipped. `finished` and \ +`canceled` remain reserved to the owner, exactly as in a project tab.\n\ +- Omission changes nothing. Retire only an item the user explicitly asked to remove.\n\ +- Keep titles under 120 characters. Use one directive per mutation.\n\ +- Put each directive on its own line, outside fences and quotes. Prose may appear \ +before, but never inside or on the same line as a directive.\n\ +- If there is no mutation to record, emit no directive."; #[cfg(test)] mod tests { use super::*; - /// `deleted` is the one destructive word, so it passes through exactly and - /// nothing else is allowed to drift into it: a "remove" that became a - /// deletion by fuzzy matching would delete work over a spelling. - #[test] - fn deleted_passes_and_nothing_drifts_into_it() { - let explicit = harvest( - "AZ-TASKS-BEGIN\n{\"project\": \"p\", \"item\": \"t\", \"status\": \"deleted\"}\nAZ-TASKS-END", - ); - assert_eq!(explicit.tasks[0].status, "deleted"); - - let fuzzy = harvest( - "AZ-TASKS-BEGIN\n{\"project\": \"p\", \"item\": \"t\", \"status\": \"remove\"}\nAZ-TASKS-END", - ); - assert_eq!(fuzzy.tasks[0].status, "pending"); - } - - /// The confused-deputy fix: a deletion the model merely *mentions* in - /// prose — quoting the format, an example, someone else's text — must not - /// remove anything. Destructive words need the marked block. - #[test] - fn a_deleted_line_outside_the_markers_is_refused() { - let got = harvest(r#"{"project": "p", "item": "t", "status": "deleted"}"#); - assert!(got.tasks.is_empty()); - assert_eq!(got.rejected, 1, "refused visibly, not dropped silently"); - } - - /// When a marked block exists, it is the whole authority: a plausible - /// line quoted in the prose around it is ignored, not harvested. - #[test] - fn json_outside_a_marked_block_is_ignored() { - let reply = "For example {\"project\": \"Quoted\", \"item\": \"never store this\"} is the shape.\n\ - AZ-TASKS-BEGIN\n\ - {\"project\": \"Real\", \"item\": \"store this\"}\n\ - AZ-TASKS-END\n\ - And {\"project\": \"Also quoted\", \"item\": \"nor this\"} afterwards."; - - let got = harvest(reply); - - assert_eq!(got.tasks.len(), 1); - assert_eq!(got.tasks[0].project, "Real"); - assert_eq!(got.rejected, 0, "quoted prose is not contract drift"); - } - - /// A forgotten closing marker must not discard the real block. - #[test] - fn a_block_missing_its_end_marker_runs_to_the_end() { - let reply = "AZ-TASKS-BEGIN\n{\"project\": \"P\", \"item\": \"t\"}"; - assert_eq!(harvest(reply).tasks.len(), 1); - } - - /// Extra fields are the signature of JSON quoted from somewhere else — - /// an issue, a log, a fixture. The contract's shape is exact. - #[test] - fn a_line_with_extra_fields_is_rejected() { - let got = harvest( - "AZ-TASKS-BEGIN\n\ - {\"project\": \"P\", \"item\": \"t\", \"status\": \"pending\", \"id\": 7}\n\ - AZ-TASKS-END", - ); - assert!(got.tasks.is_empty()); - assert_eq!(got.rejected, 1); - } - - /// A reply proposing hundreds of mutations is a malfunction; the excess - /// is refused and counted rather than trusted. - #[test] - fn a_reply_cannot_mutate_more_than_the_cap() { - let mut reply = String::from("AZ-TASKS-BEGIN\n"); - for index in 0..120 { - reply.push_str(&format!( - "{{\"project\": \"P\", \"item\": \"task {index}\"}}\n" - )); - } - reply.push_str("AZ-TASKS-END"); - - let got = harvest(&reply); - - assert_eq!(got.tasks.len(), 100); - assert_eq!(got.rejected, 20); - } - - #[test] - fn the_contract_names_the_delete_rule() { - assert!(OUTPUT_CONTRACT.contains("deleted")); - assert!(OUTPUT_CONTRACT.contains("never deletes")); - } - - #[test] - fn a_jsonl_block_after_prose_is_found() { - let reply = "Here is what I found in the repo.\n\n\ - {\"project\": \"WorkTable\", \"item\": \"Bench before the rewrite\", \"status\": \"pending\"}\n\ - {\"project\": \"WorkTable\", \"item\": \"Ship 0.9.3\", \"status\": \"active\"}\n"; - - let got = harvest(reply); - - assert_eq!(got.tasks.len(), 2); - assert_eq!(got.rejected, 0); - assert_eq!(got.tasks[0].project, "WorkTable"); - assert_eq!(got.tasks[1].status, "active"); - } - - /// Models fence the block whatever the instructions say. Losing every task - /// to three backticks would be the most common possible failure. - #[test] - fn a_fenced_block_still_parses() { - let reply = - "```json\n{\"project\": \"P\", \"item\": \"Do a thing\", \"status\": \"pending\"}\n```"; - assert_eq!(harvest(reply).tasks.len(), 1); - } - - /// Prose is not an error. Only something shaped like JSON that fails counts. - #[test] - fn prose_is_ignored_but_broken_json_is_counted() { - let reply = "I could not find anything to do.\nNothing here is a task.\n\ - {\"project\": \"P\", \"item\":}\n"; - - let got = harvest(reply); - - assert!(got.tasks.is_empty()); - assert_eq!( - got.rejected, 1, - "the malformed line is reported, not hidden" - ); - } - - /// An empty field is not a task. Writing it would put a blank row in the - /// panel that nobody can act on or explain. - #[test] - fn an_empty_field_is_rejected_rather_than_stored() { - let reply = "{\"project\": \"\", \"item\": \"x\"}\n{\"project\": \"P\", \"item\": \" \"}"; - - let got = harvest(reply); - - assert!(got.tasks.is_empty()); - assert_eq!(got.rejected, 2); - } - - #[test] - fn a_missing_status_defaults_to_pending() { - let got = harvest("{\"project\": \"P\", \"item\": \"Do a thing\"}"); - assert_eq!(got.tasks[0].status, "pending"); - } - - /// A status we do not recognize must not lose the task. - #[test] - fn an_unknown_status_becomes_pending_rather_than_dropping_the_task() { - let got = harvest("{\"project\": \"P\", \"item\": \"x\", \"status\": \"whenever\"}"); - assert_eq!(got.tasks.len(), 1); - assert_eq!(got.tasks[0].status, "pending"); - } - #[test] - fn statuses_are_normalized_from_the_words_models_actually_use() { - let got = harvest( - "{\"project\": \"P\", \"item\": \"a\", \"status\": \"in_progress\"}\n\ - {\"project\": \"P\", \"item\": \"b\", \"status\": \"DONE\"}", - ); - assert_eq!(got.tasks[0].status, "active"); - assert_eq!(got.tasks[1].status, "finished"); + fn home_teaches_the_same_ps_surface_as_project_tabs() { + assert!(OUTPUT_CONTRACT.contains("@agency:items.add")); + assert!(OUTPUT_CONTRACT.contains("@agency:items.state")); + assert!(OUTPUT_CONTRACT.contains("@agency:items.retire")); + assert!(OUTPUT_CONTRACT.contains("@agency:pr.link")); + assert!(OUTPUT_CONTRACT.contains("project:")); + assert!(!OUTPUT_CONTRACT.contains("AZ-TASKS")); + assert!(!OUTPUT_CONTRACT.contains("JSONL")); } - /// The contract has to say the things models get wrong, or it is decoration. #[test] - fn the_contract_forbids_the_usual_failures() { - assert!(OUTPUT_CONTRACT.contains("no markdown fence")); - assert!(OUTPUT_CONTRACT.contains("one JSON object per line")); - assert!(OUTPUT_CONTRACT.contains("emit no block at all")); - assert!(OUTPUT_CONTRACT.contains(TASKS_BEGIN)); - assert!(OUTPUT_CONTRACT.contains(TASKS_END)); + fn owner_only_end_states_stay_owner_only_on_home() { + assert!(OUTPUT_CONTRACT.contains("reserved to the owner")); + assert!(OUTPUT_CONTRACT.contains("shipped")); } } diff --git a/docs/gui-wiring-plan.md b/docs/gui-wiring-plan.md index 2790e5b..da0a411 100644 --- a/docs/gui-wiring-plan.md +++ b/docs/gui-wiring-plan.md @@ -148,14 +148,10 @@ being a list of user messages. `IMPLEMENTED` rather than faked, so the frontend keeps serving it from fixtures and greys it out instead of appearing to stop something it cannot. - [x] `clear_task_log(project_id)`. -- [ ] **Items from the agent's reply — checkboxes only.** A finished turn's reply is - scanned for markdown checkboxes (`- [ ]` / `- [x]`, ordered forms included) and each - becomes a `ProjectItem`, appended and deduplicated by title so a restated plan does - not stack up or delete what the user added. Deliberately *not* bullets, numbered - lists or headings: a checkbox is unambiguously a task, a bullet is as often prose, - and inventing to-dos nobody proposed is worse than an empty panel. **Still open:** - an agent that writes its plan as prose contributes nothing, so the cheap second - call that the naming design uses (below) is the obvious next step. +- [x] **One authored PS surface for items and pull requests.** Standalone + `@agency:items.*` and `@agency:pr.link` directives are the only mutations + read from an agent reply. Checkboxes, prose, quoted examples, fenced + examples, and bare PR URLs remain display text. - [ ] **Streaming needs a frontend pass too.** `TranscriptPane` renders finished messages only. Adding a `message:delta` event means either extending `AppEvents` or reusing `useStreamingBuffer` from `@pathscale/ui`. Decide which before emitting deltas. diff --git a/docs/ps-capability.yaml b/docs/ps-capability.yaml index 331c032..195acf6 100644 --- a/docs/ps-capability.yaml +++ b/docs/ps-capability.yaml @@ -18,7 +18,6 @@ standing_promotion: - agency:items.state - agency:items.add - agency:items.retire - - agency:items.inject - agency:pr.link # 13.2.2 Verbs and values reserved to principals above the agent. An agent may @@ -27,12 +26,25 @@ standing_promotion: - status:finished - status:canceled - # 13.2.3 The capability bound. `items.inject` takes a project, so this surface - # can write outside the project a turn came from. Declared here rather than - # left implicit in what the verb happens to do. + # 13.2.3 The capability bound. `items.add(project: ...)` can write outside + # the project a turn came from, and item ids resolve installation-wide. + # Home's Task Manager may also create a bare project when an explicit add + # names one that does not exist. Declared here rather than left implicit. bound: >- any project in this installation's store, named by id or by name; - no reach outside it, and no other namespace is live + Home Task Manager may create a named project inside that store through + items.add; no reach outside it, and no other namespace is live + + arguments: + agency:items.state: [id, status, pr] + agency:items.add: [project, ref, title, status] + agency:items.retire: [id] + agency:pr.link: [url, number, item] + + semantics: + pr_urls_in_prose: inert + task_manager_private_formats: none + task_manager_creates_project_with: agency:items.add # 13.2.4 Trace obligations. Every directive ends in exactly one recorded # outcome, an unresolvable reference is ENTITY_NOT_FOUND rather than a silent diff --git a/docs/task-manager.md b/docs/task-manager.md index c8389dc..39a14e2 100644 --- a/docs/task-manager.md +++ b/docs/task-manager.md @@ -17,85 +17,62 @@ collide with a real project. ## The output contract -The reply has to become rows in WorkTable. Asking a model for prose and parsing -it afterwards is how you get a to-do list that is subtly wrong. Asking for JSONL -and refusing anything else is how you get one that is either right or visibly -empty. +The reply has to become rows in WorkTable. Home once asked for an +`AZ-TASKS-BEGIN` JSONL block, while project tabs asked for checkboxes and PRs +were found by scanning prose for URLs. That was three reverse-channel +languages, two of which treated ordinary model output as executable. -The user's own words go out unchanged, with `OUTPUT_CONTRACT` appended: +Home now uses the same declared Prompt Syntax surface as every project tab: ``` -AZ-TASKS-BEGIN -{"project": "", "item": "", "status": "pending"} -AZ-TASKS-END +", ref: "t1", title: "", status: "new")> +", status: "active")> +")> ``` -The marked block is the authority. When `AZ-TASKS-BEGIN` appears, `harvest()` -reads only the lines between the markers — a task the model merely *quotes* in -its prose (an example, a README, a discussion of the format) cannot mutate -anything. Lines carry exactly the three fields (`deny_unknown_fields`, the -signature of JSON quoted from somewhere else), and one reply may mutate at -most 100 tasks. +The `` line is the authority. Prose, quoted material, fenced examples, +and URLs outside one of those lines are inert. Each directive ends in a receipt +on the next turn, including typed failures for an unknown or ambiguous id. -Models still move, fence, or forget delimiters, so a reply with no marker at -all falls back to scanning every line — but that lenient path is additive -only: a `deleted` outside the markers is refused and counted, never applied. -A stray quoted line can at worst add a row someone deletes; it can no longer -destroy one. A line is taken when it parses as an object with a non-empty -`project` and `item`. +The Task Manager keeps one context-specific capability from the old contract. +An explicit `items.add(project: ...)` that names no existing project creates a +bare project, then adds the item. The project argument is mandatory on Home; +omitting it is a surfaced `ENTITY_NOT_FOUND`, never an item hidden under the +reserved `home-task-manager` id. ## Deleting, and why absence never deletes -`status: "deleted"` removes the existing task whose project and item match, -exactly. That is the only remove verb: a model asked to "delete X" will -happily re-emit the whole list without X, and treating absence as deletion -would wipe rows the user added by hand every time the model abbreviated. So -the harvester appends and deletes only what is named, and the contract tells -the model so. +`items.retire(id: ...)` removes exactly one existing row. It is the only remove +verb available to the agent. A model asked to "delete X" will happily re-emit +the whole list without X, and treating absence as deletion would wipe rows the +user added by hand every time the model abbreviated. Omission therefore changes +nothing. To make bulk edits exact ("delete everything about cleaning junk"), every -prompt carries a live snapshot of the current projects and tasks, built from -the tables and bounded at ~6KB with an honest truncation marker. The model -sees what exists; each deletion is an explicit, auditable line in the I/O -panel. +prompt carries a live snapshot of current project and item ids, built from the +tables and bounded at about 6KB with an honest truncation marker. The model sees +what exists; each deletion names the stable id that will be removed. The write path stays in the GUI on purpose. wt-tools is read-only by construction — that is what makes it safe to run beside the GUI on a single-writer store — so it will never grow a delete. wt-tools is the eyes; -the harvest contract is the hands; the GUI is the only writer. - -Three decisions worth keeping: - -- **A line that looks like JSON and does not parse is counted, not hidden.** The - count goes to the Agent I/O panel as a `harvest` entry. A model that has - drifted off the format otherwise produces a short list and no error anywhere, - which looks exactly like the feature not working. -- **An unrecognised `status` becomes `pending` rather than dropping the task.** - Losing work over a spelling is worse than a wrong column. -- **Tasks are written as items on the task manager's own project**, carrying the - proposed project name in the title. Creating real projects from a model's - output unasked is not something it should do on its own initiative; promoting - a line to a project is a decision for a person. - -## The project-session contract: three checkboxes - -Ordinary project conversations (everything that is not the Home task manager) -speak a simpler, checklist-shaped contract, parsed from any reply: - -- `- [ ] ` **proposes** a new pending item. An existing title is left - alone — a proposal is not permission to clobber a row the user owns. -- `- [x] <title>` **closes** the existing item with that exact title. What - closing means is the Settings choice (*Completed items*): mark resolved, or - delete the row. The run-this-item prompt teaches this line, so a run started - from an item can end by closing it. -- `- [-] <title>` **removes** the existing item outright, whatever the - Settings say — an obsolete row is not a finished one. It never creates: - striking something that does not exist is already true. - -Titles match case-insensitively and exactly; a paraphrased title is a new -proposal, which is the append-only safety working as intended. `[x]` and `[-]` -landed in 0.1.6 and 0.1.10 respectively — an agent reading older sources will -find an append-only path and wrongly conclude the list can only grow. +the declared authoring surface is the hands; the GUI is the only writer. + +## The project-session contract + +Ordinary project conversations use the same directives. The prompt supplies +the current ids and the declared closed verb set: + +- `items.add` creates a row. Its `ref` is a temporary handle echoed beside the + real id in the next-turn receipt. +- `items.state` moves an existing id through `new`, `planning`, `active`, + `questions`, or `shipped`. +- `items.retire` removes exactly the named id. +- `pr.link` records an authored GitHub PR URL and may attach its number to an + item. A URL in prose is display text only. + +`finished` and `canceled` are reserved to the owner on Home and project tabs. +Checkboxes, prose, quoted examples, fenced examples, and bare PR URLs are inert. ## Can the agent read the WorkTable store?