Repository navigation
Conversation
…sample Add the second CAO blog post (memory + multi-intent orchestration, with the human-gate relay-by-mechanism principle), and update examples/aidlc-portfolio so it backs the post's claims: - memory: replace the hand-built flat-file SHA-CAS protocol with CAO's native memory layer (memory_store/memory_recall); durable learnings are promoted under supervisor review, per-space project.md/team.md stay local scratch. - human gate: bind question-answer resolution to a verified human-surface approval (--approval-id) instead of a self-asserted --answered-by string. bun run check: 28 pass / 0 fail. Docusaurus build: clean.
gutosantos82
left a comment
There was a problem hiding this comment.
PR Review: #936 — blog: Running AI-DLC on CAO (memory + multi-intent orchestration)
Summary
Adds the second CAO blog post (running several AI-DLC intents on CAO via native memory and per-intent worktrees) and rewrites examples/aidlc-portfolio to back it: the hand-built flat-file SHA-CAS memory protocol is removed in favour of memory_store/memory_recall, and question answer now requires a packet-bound, single-use --approval-id instead of a free-text --answered-by. The TypeScript changes are logically sound, the blog's claims about CAO's real API (memory tools, scopes, worktree isolation, v2.5.3) check out, and the test rewrite tracks the behaviour change well. However, the migration of the sample is incomplete: three sample files not in the diff (SKILL.md, README.md, runner-message.md) still instruct agents to run commands this PR deletes and still describe the removed single-writer/CAS model — so the PR's stated goal ("updated to back the post instead of contradicting it") is not yet met. Recommend Request changes for the stale sample docs and a softer framing of the human-gate guarantee; the rest is ready.
Blocking (must fix before merge)
- [consistency][correctness] examples/aidlc-portfolio/skills/aidlc-portfolio/SKILL.md:271,282,299,308,312,318,320 — SKILL.md is the operating manual the supervisor loads and executes, and it was not updated. It still tells the agent to run
question answer … --answered-by "<human>"(now fails with "--approval-id is required"),learning approveandlearning reconcile(removed → "unknown command"),memory refresh/memory inspect/memory clean(wholememorynamespace removed fromparseCommand→ "unexpected argument"), and says "dispatch checkreturns canonical baseline revisions for project.md/team.md; include them in the runner assignment" (that field no longer exists). An agent following SKILL.md hits hard failures, and the sample directly contradicts both the post and the code.bun run checkcannot catch this (prose), which is why the green check doesn't cover it. Rewrite the Shared Memory / Human Gate / Learning sections to the--approval-id+learning propose|promote|rejectflow. 🆕
Important (should fix)
- [consistency][conventions] examples/aidlc-portfolio/README.md:99,218,223,237 — still describes the removed model: "a single-writer proposal model for shared AI-DLC memory", "Treat project and team memory as single-writer shared state", "only the supervisor can serialize an approved update into canonical memory", and Human Control Point #4 "review … before the utility performs the canonical write".
promoteLearningProposalno longer performs any canonical file write. Update to the CAO-memory promotion model described in the post. 🆕 - [consistency] examples/aidlc-portfolio/skills/aidlc-portfolio/assets/templates/runner-message.md:11-12,40 — dangling placeholders
{{project_memory_revision}}/{{team_memory_revision}}that nothing populates any more (thememoryfield was removed fromDispatchReadiness), plus "Do not approve or directly merge project.md/team.md changes" referencing the removed approve flow. 🆕 - [security][correctness] docusaurus/blog/…/index.md, references/coordination-rules.md §4, tests/portfolio.test.ts:441 — the "enforced by mechanism, not prompt" claim is overstated for the sample as shipped. The approval record is a plain JSON file in
portfolio/approvals/in the same tree the agent already writes to with its ordinary filesystem tools; nothing stops a supervisor or runner from writing{"source":"human-surface","packet":"<id>","answeredBy":"Portfolio owner",…}directly and then callingquestion answer --approval-id <id>. The boundary is only "no agent-facing subcommand mints an approval", not "the agent cannot mint one". The code comments onrecordHumanApprovalare honest that it stands in for CAO's real human-approval surface, but the post ("the orchestrator cannot speak for you even if a prompt drifts"), coordination-rules #4 ("even an agent that ignored it cannot resolve a packet without a human-surface approval") and the test comment ("a forged 'answered-by' is impossible") present it as the boundary itself. Reframe: the sample models the protocol shape (packet-bound, single-use, identity taken from the record — all genuinely implemented and a real improvement over--answered-by); real enforcement requires the approval record to live outside the agent's write reach or be signed with a key the agent lacks. Low actual risk (example code), medium as an accuracy issue in a post whose headline rule this is. 🆕
Nits (optional)
- [conventions] commit message —
blog:is not a Conventional Commits type; the prior post useddocs(blog):(01c179a). Since this commit also changes the example's public CLI,docs(blog):with the example change in the body, or a split, would match repo convention. - [conventions] CHANGELOG.md — the blog needs no entry, but removing/renaming example CLI commands (
learning approve/reconcile,memory *→learning promote,--answered-by→--approval-id) is user-facing; precedent (#845/#892) adds[Unreleased]entries for example changes. Advisory. - [correctness][consistency] learning-proposal.schema.json:48-51, lib.ts:217 —
baseRevisionis still defined (schema + optional interface field) though nothing reads or writes it and the references now say "It carries no baseline revision";lifecycle.test.ts:155still sets it. Drop it. - [correctness] lib.ts
promoteLearningProposal—destination(project|team) remains schema-required but promotion hardcodesscope="project"/memoryType="feedback"regardless, so the field is now inert. Either honour it or document that it is informational. - [tests] portfolio.ts arg parsing — all approval tests call the library directly; nothing asserts at the CLI layer that
question answererrors without--approval-idor ignores--answered-by. A one-line CLI test would lock the flag removal. - [tests] tests/portfolio.test.ts "dispatch check enforces catalog dependencies…" — sits near bun's 5 s default timeout (multiple git spawns + harness verify, ~4.5 s). Under load it timed out once mid-
withLock, leaving.portfolio.lockheld and cascading "portfolio is locked" failures (27 pass / 1 fail; clean on rerun and with--timeout 20000). Consider a higher per-test timeout for the git-heavy cases. - [conventions] blog index.md — "first post in this series" links to the blog index rather than the memory-design post (2026-10-08). Works, but imprecise.
Tests
Coverage tracks the behaviour change well. All three removed tests map to removed code (the cleanWorktreeMemory error path, the SHA-CAS serialization flow, the idempotent-approval rule), and the one live behaviour that flipped — session completion no longer blocked by worktree memory — is positively re-covered. The --approval-id mechanism is tested for the happy path and the negative paths that matter: unknown/forged id, id bound to a different packet, replay (single-use), and altered question text with a valid approval. The learning-proposal schema change (status promoted, promotedAt/promotedBy/memory{scope,memoryType,key}, optional baseRevision, malformed rejection, re-promote refused) is asserted. Gaps are minor: no CLI-layer test for the flag change, and no test that additionalProperties:false now rejects an old-format on-disk proposal. The claimed 28 tests is exact (15 + 5 + 4 + 4 across the four suites).
Verification
Dynamic verification did not return. Incidental evidence from the correctness reviewer: bun run check in the worktree reported 27 pass / 1 fail on a first run (the dispatch-check test timing out at 5 s under load, see Nits) and 28/28 on rerun and with --timeout 20000; the CI job "AI-DLC Portfolio Example" passes at this head. The failing CI jobs (Unit Tests 3.10/3.11, CAO MCP Apps) are pre-existing main breakage — test/test_dependency_security.py::test_real_inventory_regression_has_a_required_scanner_in_ci fails on an aquasecurity/setup-trivy pin-hash mismatch from a Dependabot bump, which #935 is repairing; this PR touches no Python or workflow files. Verified against the worktree: the stale SKILL.md commands and runner-message placeholders listed above are present at this head; pyproject.toml at this head is 2.5.3 and v2.5.3 is the latest GitHub release, so the post's version pin is correct (one reviewer's claim that it was inconsistent came from the stale main checkout and is discarded).
Verdict
Request changes — the code and post are sound, but SKILL.md, README.md and runner-message.md still document the removed --answered-by/learning approve|reconcile/memory */baseline-revision protocol, so agents following the sample hit failures and the sample contradicts the post; fix those (and soften the "enforced by mechanism" claim to what the file-based stand-in actually guarantees) and this is ready.
Per PR #936 review: SKILL.md, README.md, and runner-message.md still documented the removed --answered-by / learning approve|reconcile / memory inspect|refresh|clean / canonical-baseline-revision protocol, so an agent following SKILL.md would hit hard CLI failures and the sample contradicted both the post and the shipped code. The two profile files (aidlc_portfolio_supervisor.md, aidlc_runner.md) and references/shared-memory.md were already correct; coordination-rules.md needed only the overstated security claim fixed, not the mechanics. - SKILL.md: rewrite Human Question Relay to --approval-id, and Shared Memory to the learning propose/list/promote/reject + memory_store flow (drop learning approve/reconcile, memory inspect/clean/refresh, and the canonical-baseline-revision instruction). - README.md: drop "single-writer proposal model" / "canonical write" framing; describe proposal review + promotion into CAO memory. - runner-message.md: remove dangling {{project_memory_revision}} / {{team_memory_revision}} placeholders (nothing populates them) and the stale "do not approve/merge project.md/team.md" line. - coordination-rules.md + blog post + portfolio.test.ts comment: soften "enforced by mechanism, cannot speak for you" to what the sample's file-based approval record actually guarantees (no agent-facing subcommand mints one; a real write-access boundary still needs CAO's real human-approval surface or a signed record). - Drop the dead `baseRevision` field (schema, lib.ts interface, lifecycle.test.ts fixture) and document that `destination` is informational-only at promotion time (always scope=project, memoryType=feedback). Verified: `tsc --noEmit` clean, `npm run build` (docusaurus) clean. bun is not available in this environment so `bun run check` could not be re-run here; no test behavior changed, only comments/docs/schema and one dead optional field.
|
Red CI isn't from this PR. Traced it: all three Checked Nothing to fix on my end here — this is main's to repair. Will rebase once it's green. |
Second post in the CAO blog series — this one is about running AI-DLC on CAO.
The angle is for an AI-DLC user: you already drive one intent through the lifecycle, here's what CAO adds when you want to run several of them. Two things, nothing more — I deliberately kept it tight:
project.md/team.mdscoped to one intent/worktree. CAO's native memory layer (memory_store/memory_recall) lets what one intent learns reach the next and survive a restart. Amplify (promote durable learnings) or replace (portfolio-level knowledge lives in CAO memory).aidlc/tree stops being a shared contention point.Plus the one rule I care about most: wherever CAO sits between AI-DLC and the human, the orchestrator must relay the human-gate questions, never answer them — and that should be enforced by mechanism, not prompt.
I dropped the cao_workflow angle on purpose. Reproducing AI-DLC's hook-driven determinism in the workflow engine is a real, separate problem and it was muddying the post.
Also updates the sample
examples/aidlc-portfoliois updated to back the post instead of contradicting it:question answernow takes a verified--approval-idinstead of a self-asserted--answered-bystring.Verification:
bun run checkin the sample: 28 pass / 0 failNote the post is dated/pinned to CAO
v2.5.3. Still polishing the prose, but the structure and the sample are solid. Keen to hear what you think.