From c3be79ead97269ad4e2c25410f04c782423bc1b8 Mon Sep 17 00:00:00 2001 From: Test User Date: Wed, 3 Jun 2026 20:16:27 +0200 Subject: [PATCH 1/2] fix(shared-learning): guard promote_to_l2 against L0 -- return error instead of silent no-op Refs #2049 - SharedLearningStore::promote_to_l2 now returns StoreError::InvalidInput when the learning is not at L1, with message "Learning is at {level}; promote to L1 first using --to l1" - SharedLearningStore::promote_to_l1 now returns StoreError::InvalidInput when the learning is not at L0 (prevents silent re-promote no-op) - CLI 'learn shared promote --to l1' now calls store.promote_to_l1 instead of unconditionally returning error; fixes wrong message "learnings start at L1" (they start at L0) - Test suite: fix 5 pre-existing tests that called promote_to_l2 without first calling promote_to_l1 (masked the bug); fix shared_import assertion from L1 to L0 (accurate to code behaviour); add 3 regression tests for #2049 (promote_l0_directly_to_l2_fails, promote_l0_to_l1_via_store_succeeds, promote_l1_to_l1_again_fails) - All 12 shared_learning_cli_tests pass with --features shared-learning --- crates/terraphim_agent/src/main.rs | 8 +- .../src/shared_learning/store.rs | 12 +++ .../tests/shared_learning_cli_tests.rs | 99 ++++++++++++++++++- 3 files changed, 113 insertions(+), 6 deletions(-) diff --git a/crates/terraphim_agent/src/main.rs b/crates/terraphim_agent/src/main.rs index 291d18a42..1e82b827d 100644 --- a/crates/terraphim_agent/src/main.rs +++ b/crates/terraphim_agent/src/main.rs @@ -4013,9 +4013,11 @@ async fn run_shared_learning_command( println!("Promoted learning {} to L3 (Human-Approved).", id); } TrustLevel::L1 => { - return Err(anyhow::anyhow!( - "Cannot promote to L1 -- learnings start at L1. Use l2 or l3." - )); + store + .promote_to_l1(&id) + .await + .map_err(|e| anyhow::anyhow!("{}", e))?; + println!("Promoted learning {} to L1 (Unverified).", id); } TrustLevel::L0 => { return Err(anyhow::anyhow!( diff --git a/crates/terraphim_agent/src/shared_learning/store.rs b/crates/terraphim_agent/src/shared_learning/store.rs index 7a16ff664..557180c72 100644 --- a/crates/terraphim_agent/src/shared_learning/store.rs +++ b/crates/terraphim_agent/src/shared_learning/store.rs @@ -333,6 +333,12 @@ impl SharedLearningStore { let learning = index .get_mut(id) .ok_or_else(|| StoreError::NotFound(id.to_string()))?; + if learning.trust_level != TrustLevel::L0 { + return Err(StoreError::InvalidInput(format!( + "Learning {} is at {}; it can only be promoted to L1 from L0", + id, learning.trust_level + ))); + } learning.promote_to_l1(); let updated = learning.clone(); drop(index); @@ -347,6 +353,12 @@ impl SharedLearningStore { let learning = index .get_mut(id) .ok_or_else(|| StoreError::NotFound(id.to_string()))?; + if learning.trust_level != TrustLevel::L1 { + return Err(StoreError::InvalidInput(format!( + "Learning {} is at {}; promote to L1 first using --to l1", + id, learning.trust_level + ))); + } learning.promote_to_l2(); let updated = learning.clone(); drop(index); diff --git a/crates/terraphim_agent/tests/shared_learning_cli_tests.rs b/crates/terraphim_agent/tests/shared_learning_cli_tests.rs index bc26b2637..3425703bf 100644 --- a/crates/terraphim_agent/tests/shared_learning_cli_tests.rs +++ b/crates/terraphim_agent/tests/shared_learning_cli_tests.rs @@ -44,7 +44,9 @@ async fn shared_list_with_trust_level_filter() { SharedLearningSource::Manual, "test-agent".to_string(), ); + let l1_id = l1.id.clone(); store.insert(l1).await.expect("insert l1"); + store.promote_to_l1(&l1_id).await.expect("promote l1 to L1"); let mut l2 = SharedLearning::new( "L2 learning".to_string(), @@ -52,6 +54,7 @@ async fn shared_list_with_trust_level_filter() { SharedLearningSource::Manual, "test-agent".to_string(), ); + l2.promote_to_l1(); l2.promote_to_l2(); store.insert(l2).await.expect("insert l2"); @@ -92,6 +95,7 @@ async fn shared_promote_l1_to_l2() { let id = learning.id.clone(); store.insert(learning).await.expect("insert"); + store.promote_to_l1(&id).await.expect("promote to l1"); store.promote_to_l2(&id).await.expect("promote to l2"); let fetched = store.get(&id).await.expect("get after promote"); @@ -122,7 +126,7 @@ async fn shared_promote_to_l3() { async fn shared_stats_counts() { let store = create_store().await; - // Insert 2 L1, 1 L2 + // Insert 2 L1, 1 L2 (each starts at L0 and must be explicitly promoted) for i in 0..2 { let l = SharedLearning::new( format!("L1 item {}", i), @@ -130,7 +134,9 @@ async fn shared_stats_counts() { SharedLearningSource::Manual, "agent".to_string(), ); + let lid = l.id.clone(); store.insert(l).await.expect("insert l1"); + store.promote_to_l1(&lid).await.expect("promote to L1"); } let mut l2 = SharedLearning::new( @@ -139,6 +145,7 @@ async fn shared_stats_counts() { SharedLearningSource::Manual, "agent".to_string(), ); + l2.promote_to_l1(); l2.promote_to_l2(); store.insert(l2).await.expect("insert l2"); @@ -190,7 +197,7 @@ async fn shared_import_creates_l1_entries() { let all = store.list_all().await.expect("list_all"); assert_eq!(all.len(), 1); - assert_eq!(all[0].trust_level, TrustLevel::L1); + assert_eq!(all[0].trust_level, TrustLevel::L0); assert_eq!(all[0].source_agent, "cli-import"); assert!(all[0].original_command.is_some()); assert!(all[0].error_context.is_some()); @@ -236,7 +243,8 @@ async fn shared_store_survives_restart() { let id = learning.id.clone(); store.insert(learning).await.expect("insert"); - // 3. Promote it to L2 + // 3. Promote it to L2 (must go through L1 first) + store.promote_to_l1(&id).await.expect("promote to l1"); store.promote_to_l2(&id).await.expect("promote to l2"); // 4. Drop the store (simulating process exit) @@ -264,6 +272,91 @@ async fn shared_store_survives_restart() { assert_eq!(retrieved.quality.agent_names.len(), 2); } +/// Regression test for #2049: promoting an L0 learning directly to L2 must +/// return an error rather than silently no-opping and printing false success. +#[tokio::test] +async fn promote_l0_directly_to_l2_fails() { + let store = create_store().await; + + let learning = SharedLearning::new( + "L0 learning".to_string(), + "content".to_string(), + SharedLearningSource::Manual, + "test-agent".to_string(), + ); + let id = learning.id.clone(); + store.insert(learning).await.expect("insert"); + + // Verify the learning is at L0 + let fetched = store.get(&id).await.expect("get"); + assert_eq!(fetched.trust_level, TrustLevel::L0); + + // Promoting directly to L2 from L0 must fail with a descriptive error + let result = store.promote_to_l2(&id).await; + assert!( + result.is_err(), + "promote_to_l2 from L0 must fail, not silently no-op" + ); + let err_msg = result.unwrap_err().to_string(); + assert!( + err_msg.contains("Extracted") || err_msg.contains("L0"), + "error must mention current level: {}", + err_msg + ); + assert!( + err_msg.contains("l1"), + "error must instruct user to promote to L1 first: {}", + err_msg + ); + + // Learning must still be at L0 (unchanged) + let still_l0 = store.get(&id).await.expect("get after failed promote"); + assert_eq!(still_l0.trust_level, TrustLevel::L0); +} + +/// Regression test for #2049: promoting from L0 to L1 via --to l1 must succeed. +#[tokio::test] +async fn promote_l0_to_l1_via_store_succeeds() { + let store = create_store().await; + + let learning = SharedLearning::new( + "L0 to L1".to_string(), + "content".to_string(), + SharedLearningSource::Manual, + "test-agent".to_string(), + ); + let id = learning.id.clone(); + store.insert(learning).await.expect("insert"); + + store + .promote_to_l1(&id) + .await + .expect("promote L0 -> L1 should succeed"); + + let fetched = store.get(&id).await.expect("get after promote"); + assert_eq!(fetched.trust_level, TrustLevel::L1); + assert!(fetched.promoted_at.is_some()); +} + +/// Promoting an already-L1 learning to L1 again must fail (not silent no-op). +#[tokio::test] +async fn promote_l1_to_l1_again_fails() { + let store = create_store().await; + + let learning = SharedLearning::new( + "L1 again".to_string(), + "content".to_string(), + SharedLearningSource::Manual, + "test-agent".to_string(), + ); + let id = learning.id.clone(); + store.insert(learning).await.expect("insert"); + store.promote_to_l1(&id).await.expect("initial L0 -> L1"); + + let result = store.promote_to_l1(&id).await; + assert!(result.is_err(), "re-promoting L1 to L1 must fail"); +} + #[tokio::test] async fn shared_store_dedups_on_restart() { let temp_dir = tempfile::tempdir().unwrap(); From 4f05e9a01f69fd9361610d4a7ac6d75c62592d28 Mon Sep 17 00:00:00 2001 From: Test User Date: Wed, 3 Jun 2026 20:19:12 +0200 Subject: [PATCH 2/2] docs(handover): session handover 2026-06-03 issue #2049 complete Refs #2049 --- HANDOVER.md | 188 ++++++++++------------------------------------------ 1 file changed, 35 insertions(+), 153 deletions(-) diff --git a/HANDOVER.md b/HANDOVER.md index e4084ceb0..2e1a23c57 100644 --- a/HANDOVER.md +++ b/HANDOVER.md @@ -1,169 +1,51 @@ -# Handover: 2026-04-17 -- FFF Epic #222 Complete + Rust 1.95 Clippy Fixes +# Handover: 2026-06-03 -- Echo / implementation-swarm-A: #2049 shared-learning promote guard -**Branch**: main (clean, up to date with origin at `0cae8f77f`) -**Previous Handover**: 2026-04-16 - Sprint Planning + 4 Feature PRs Merged + Issue Housekeeping -**Full handover**: `.docs/handover-2026-04-17.md` +**Branch**: task/2049-shared-learning-promote-guard +**Base**: origin/main (GitHub monorepo, 58 crates, commit 98fa93b32) +**Commit**: c3be79ead +**GitHub PR**: https://github.com/terraphim/terraphim-ai/pull/897 +**Gitea Issue**: #2049 +**Gitea Comment**: https://git.terraphim.cloud/terraphim/terraphim-ai/issues/2049#issuecomment-36299 ## Session Summary -Full sprint planning, research, design, implementation, verification, validation, and merge cycle. Closed 33 stale issues, migrated 8 Odilo issues to dedicated repo, implemented 4 features across independent workstreams, verified with right-side-of-V agents, and updated documentation on both mdbook and terraphim.ai website. +Implemented fix for issue #2049: `learn shared promote --to l2` from L0 silently no-opped and reported false success. Root cause: `SharedLearningStore::promote_to_l2` called `SharedLearning::promote_to_l2()` without checking the current trust level. `SharedLearning::promote_to_l2()` only promotes from L1 (guards with `if trust_level == L1`) — when called on L0, it was a silent no-op, but the store returned `Ok(())` and the CLI printed "Promoted". ## What Was Done -### Sprint Planning (Disciplined Research + Design) - -- Evaluated all open issues across GitHub (82) and Gitea (30+) -- Cross-checked against Odilo project at `~/projects/zestic-ai/odilo/` -- discovered duplicate issue tracking -- Created sprint plan: `plans/sprint-2026-04-16-terraphim-ai-design.md` -- Created listener dispatch design: `plans/design-listener-shell-dispatch.md` -- User decisions captured via AskUserQuestion: Odilo = active client work, TinyClaw = rethink as Gitea listener, sprint focus = all workstreams equally - -### 7 PRs Merged (4 feature + 3 dependabot) - -| PR | Commit | Feature | Tests | -|----|--------|---------|-------| -| #814 | `993e4e0` | Parser upstream: MatchStrategy, iterative normalize, configurable SectionConfig | 26 | -| #815 | `9396e35` | Learn compile: corrections-to-thesaurus feedback loop | 9 | -| #816 | `9fd985d` | Automata evaluation framework: precision/recall/F1 | 14 | -| #817 | `9196d1f` | Listener shell dispatch: execute subcommands from @adf mentions | 31 | -| #811 | `d607a82` | Dependabot: actions/github-script 7->9 | -- | -| #812 | `574ff84` | Dependabot: docker/build-push-action 5->7 | -- | -| #813 | `4aff773` | Dependabot: docker/metadata-action 5->6 | -- | - -### 33 Issues Closed +### Fix (3 files changed, 113 insertions, 6 deletions) -| Category | Count | Issues | -|----------|-------|--------| -| GitHub (implemented) | 12 | #694, #695, #703, #599, #692, #697, #784-787, #730, #638 | -| GitHub (wontfix) | 2 | #562, #585 | -| Gitea (Odilo migrated) | 8 | #561, #565-571 | -| Gitea (ADF remediation) | 10 | #461-463, #468, #490, #494, #497, #499, #501, #504, #506, #507 | -| Gitea (desktop repo) | 1 | #490 | +**`crates/terraphim_agent/src/shared_learning/store.rs`** +- `promote_to_l1`: added guard — returns `StoreError::InvalidInput` if `trust_level != TrustLevel::L0` +- `promote_to_l2`: added guard — returns `StoreError::InvalidInput("Learning {id} is at {level}; promote to L1 first using --to l1")` if `trust_level != TrustLevel::L1` -### 4 Issues Created (terraphim-ai) - -| # | Title | -|---|-------| -| #575 | feat(markdown-parser): upstream MatchStrategy, iterative normalize, configurable SectionConfig | -| #576 | feat(automata): ground-truth evaluation framework | -| #577 | feat(listener): shell dispatch bridge | -| Gitea only | 8 issues migrated to zestic-ai/odilo (#23-30) | - -### CI Fix - -- `cfee683` -- `cargo fmt` fix for terraphim-markdown-parser (Rust 2024 edition import ordering) -- `db0809f` -- Removed desktop npm from dependabot config (desktop is separate repo) - -### Documentation - -- `docs/src/evaluation-framework.md` -- Automata evaluation API docs -- `docs/src/listener-dispatch.md` -- Gitea dispatch security + config docs -- `docs/src/learning-compile.md` -- Corrections feedback loop docs -- `docs/src/SUMMARY.md` -- Added Agent Capabilities section -- `terraphim.ai/content/capabilities/evaluation.md` -- Website capability page -- `terraphim.ai/content/capabilities/listener-dispatch.md` -- Website capability page -- `terraphim.ai/content/capabilities/terraphim-agent.md` -- Updated with new features +**`crates/terraphim_agent/src/main.rs`** +- `TrustLevel::L1` CLI branch: now calls `store.promote_to_l1(&id).await` and prints success, instead of returning a static error with wrong message ("learnings start at L1") -### Verification & Validation - -All 4 PRs went through right-side-of-V: -- **Verification**: 3 parallel agents verified implementation against design specs -- **Validation**: 1 agent validated against user requirements -- **Result**: GO for all 4 PRs. 80 new tests, 0 regressions, 0 clippy warnings on new code -- **Minor defects** (all Low): D1 (textbook_default deviation), D817-1 (missing wiremock integration tests), D-1 (export_corrections_as_kg deferred), D-2 (cache invalidation deferred), D-3 (evaluate CLI deferred) - -## New Files - -### Feature code -- `crates/terraphim_agent/src/learnings/compile.rs` -- corrections-to-thesaurus compiler (384 lines, 9 tests) -- `crates/terraphim_agent/src/shell_dispatch.rs` -- shell dispatch bridge (666 lines, 31 tests) -- `crates/terraphim_automata/src/evaluation.rs` -- evaluation framework (613 lines, 14 tests) - -### Modified -- `crates/terraphim-markdown-parser/src/heading.rs` -- MatchStrategy enum, pattern/match_strategy fields -- `crates/terraphim-markdown-parser/src/lib.rs` -- iterative normalize_markdown, pub(crate) collect_text_content -- `crates/terraphim_agent/src/listener.rs` -- DispatchConfig, shell dispatch wiring in process_comment -- `crates/terraphim_agent/src/main.rs` -- LearnSub::Compile variant, mod shell_dispatch - -### Plans -- `plans/sprint-2026-04-16-terraphim-ai-design.md` -- 2-week sprint plan -- `plans/design-listener-shell-dispatch.md` -- shell dispatch design - -## New CLI Surface - -``` -terraphim-agent learn compile --output FILE [--merge-with FILE] -``` - -## New Library API - -```rust -// Evaluation framework -terraphim_automata::evaluation::evaluate(ground_truth, thesaurus) -> EvaluationResult -terraphim_automata::evaluation::load_ground_truth(path) -> Vec - -// Learning compile -learnings::compile::compile_corrections_to_thesaurus(dir) -> Thesaurus -learnings::compile::merge_thesauruses(curated, compiled) -> Thesaurus -learnings::compile::write_thesaurus_json(thesaurus, path) - -// Shell dispatch (internal to listener) -shell_dispatch::parse_dispatch_command(context, extra_allowed) -shell_dispatch::execute_dispatch(config, subcommand, args) -shell_dispatch::format_dispatch_result(result, agent, session, event) -``` - -## What's Working - -- All 80 new tests pass across 4 features -- Workspace tests green -- Both remotes (origin + gitea) in sync -- terraphim.ai website updated with new capabilities -- Listener dispatch has 3-layer security: allowlist + metachar rejection + CommandGuard - -## What's Deferred (tracked as Low-severity defects) - -1. `export_corrections_as_kg()` -- Logseq markdown export from corrections -2. Cache invalidation for SQLite thesaurus when corrections change -3. `evaluate` CLI subcommand (library API exists, no CLI wrapper yet) -4. Wiremock integration tests for dispatch-through-listener path -5. W5b (#727): Wire agent_evolution into orchestrator -- not started this sprint -6. W4 (#553/#608): Community content -- not started this sprint +**`crates/terraphim_agent/tests/shared_learning_cli_tests.rs`** +- Fixed 5 pre-existing broken tests that called `promote_to_l2` from L0 (masked the root-cause bug) +- Fixed `shared_import_creates_l1_entries` assertion from `TrustLevel::L1` to `TrustLevel::L0` (import does not promote) +- Added 3 regression tests: `promote_l0_directly_to_l2_fails`, `promote_l0_to_l1_via_store_succeeds`, `promote_l1_to_l1_again_fails` -## Odilo Migration - -8 issues moved from terraphim/terraphim-ai to zestic-ai/odilo: -- Epic (#561 -> odilo#23), Stages 3-6, Stage 0b, Multi-language, ADR-040 -- Also moved: Stage 0b -> terraphim-ai#576 (reframed as generic eval framework) -- Also moved: Stage 1 parser work -> terraphim-ai#575 (upstream to canonical crate) -- Review comment posted on odilo#26 (quality gate -- may belong in terraphim-ai) +### Results -## Known Issues +- `cargo test -p terraphim_agent --test shared_learning_cli_tests --features shared-learning`: 12/12 PASS +- `cargo clippy -p terraphim_agent --features shared-learning -- -D warnings`: CLEAN -1. `procedure.rs` has 4 pre-existing dead_code warnings (TRIVIAL_COMMANDS, is_trivial_command, from_session_commands, save_with_dedup) -- blocked by `clippy -D warnings` in CI but only when running without `repl-sessions` feature -2. Gitea has many stale task branches from ADF bot agents -- cleanup opportunity -3. `cross_mode_consistency_test` still has known pre-existing failures (server vs CLI) - -## Test Commands +## Critical Lessons Learned -```bash -# Sprint features -cargo test -p terraphim-markdown-parser # 26 tests -cargo test -p terraphim_agent --bin terraphim-agent -- compile # 9 tests -cargo test -p terraphim_automata -- evaluation # 14 tests -cargo test -p terraphim_agent --bin terraphim-agent -- shell_dispatch # 31 tests +- **gitea/main vs origin/main divergence**: This worktree uses `gitea/main` (polyrepo, 15 crates). Issues referencing `terraphim_types`, `terraphim_agent`, etc. require `origin/main` (GitHub, 58 crates). Always branch from `origin/main` for issues that reference these crates. +- **TrustLevel Display**: `TrustLevel::L0` displays as `"Extracted"` (not `"L0"`). When asserting on error messages that include `{trust_level}`, check for `"Extracted"` or `"L0"` (both), not just `"L0"`. +- **Store guard location**: Add guards to STORE methods (not struct methods). The struct's `promote_to_l2()` is also called in the auto-promote path (`record_effective`), which already checks `trust_level == TrustLevel::L1` before calling it. Guarding the struct method would break nothing, but guarding the store method is the right boundary. +- **Wiki REST API returns 405**: Skip wiki creation, use Gitea issue comment for session record. -# Full workspace -cargo test --workspace --lib -cargo clippy --workspace --all-targets -``` - -## Sprint Plan Remaining (Week 2) - -Per `plans/sprint-2026-04-16-terraphim-ai-design.md`: +## What Is Next -| Day | Task | Status | -|-----|------|--------| -| Day 7 | Deploy Gitea listener, reframe TinyClaw | Not started | -| Day 8-9 | Wire agent_evolution into orchestrator (#727) | Not started | -| Day 10 | Community content (#553/#608), sprint retro | Not started | +- The GitHub PR #897 is open for review and merge +- Gitea issue #2049 remains open (close on merge per workflow) +- Related issue #2046 ("5 SharedLearningStore tests fail") is effectively resolved by this PR — the 5 tests are now fixed. Consider closing #2046 after #2049 merges. +- The `terraphim_orchestrator/src/lib.rs` format drift (issue #2044) is pre-existing and out of scope for this PR + +## State + +Branch is pushed. PR is created. Issue has handover comment. Working tree is clean. No uncommitted work.