Feat/build your own x skills - #84
Merged
Merged
Conversation
…access anh Tam's direct request mid-session 2026-07-24: yana-ai chat's Ollama provider (confirmed live via a real ollama_chat test against qwen2.5-coder:14b) is pure conversation only by design -- no file read, no repo visibility (src/chat/mod.rs's module doc explains why: nothing executes, so there's nothing for Yana AI's hook system to guard). He wants it to actually read the repo, Claude-Code-like -- chose this (extend yana-ai chat directly) over the alternative (point Cursor's agent mode at Ollama and reuse the existing core/adapters/cursor/ hook, whose Ollama-agent support couldn't be confirmed from Cursor's docs). Per this repo's own D7/ADS v1 rule (Specification-first Development, established by anh Tam himself this session), this is not something to silently code -- it's a real scope-expansion question for Program J (Universal Capability Runtime), whose Scope section currently names exactly 4 AI-tool clients (Claude/Cursor/Gemini/Codex) sourced from VISION-2.4.md's roadmap. Grepped VISION-2.4.md directly: no mention of yana-ai chat or a local-model client anywhere in the original 30- capability roadmap. Adding it would be real scope growth (M=5, not 4), not filling in content that was already implied -- different in kind from how the other 3/4 Open Questions got answered this session (pure re-reading of existing source docs, no new scope). Recorded as a new "Input bổ sung" section with a second Open Question (does this belong in Program J's scope or is it a separate Program/ Program D sub-goal) -- explicitly not decided by this agent, flagged for anh Tam. Also recorded upfront, before any Architecture discussion: this is a real security-posture change, not a small one -- yana-ai chat's current zero-tool-calling design is exactly why it sits outside the hook system's reach; giving it real file/exec access means it must start complying with the same guard rails every other agentic surface in this repo already does (04-sandbox-isolation-law.md, agent-excessive-agency-law.md, agent-tool-poisoning-guard.md, execution-environment.md's banned runtime functions) -- a weaker local model doesn't mean weaker consequences from what it's allowed to run. Docs-only, no code touched -- matches the D7 gate (Specification phase only until anh Tam resolves the two open questions). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
anh Tam's direct decision: "program J đi, gộp vào cho gọn" -- confirms yana-ai chat + local Ollama belongs inside Program J's scope rather than a separate Program, accepting the real scope expansion from M=4 (the roadmap-named Claude/Cursor/Gemini/Codex) to M=5. Updated: Scope section records the decision with a note that this 5th client is qualitatively different from the other 4 (a Yana AI-owned binary, not an external editor/CLI needing a translator script) -- flagged as a preliminary observation, not an architecture decision, since Phase 3 for the whole Program is still blocked on the earlier, still-unresolved question (MCP Server replacing vs. extending the translator-per-engine pattern). Open Questions: the yana-ai-chat scope question is marked answered; the whitelist-relationship question from the original roadmap remains the one real open item. Capability List's AI Adapter Layer row gets a cross-reference note rather than a duplicate new row, since this is a new client for an existing capability, not a new capability itself. README.md's Program table synced to match. Still not touching Architecture/Modules/Interfaces/code -- Phase 3 stays blocked until the MCP-Server-vs-translator-pattern question is resolved, per this repo's own D7/ADS v1 gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…aces translator pattern anh Tam's decision via AskUserQuestion, choosing between two presented options: MCP Server fully replaces the existing translator-per-engine pattern (core/adapters/cursor/before-shell-execution.js and future translators), rather than the lower-risk incremental option (MCP added as a discovery-only layer, translator kept for real-time hook enforcement) that was presented as the recommendation. Recorded the risk that was surfaced before the decision, not to contest it -- it's anh Tam's call, not something this agent infers -- but so it isn't lost: the current translator-per-engine path is proven, live security enforcement (guard-destructive.sh via Cursor); full replacement means that enforcement path needs to be rewritten and re-validated through MCP, which is inherently request/response and needs to be confirmed suitable for a PreToolUse hook's fast-block-or-fail requirement before implementation, not assumed. Both architecture questions that were blocking Phase 3 (this one, and the yana-ai-chat scope question resolved in commit 252da15) are now answered -- Phase 3 Architecture is unblocked for the whole Program. Docs-only, no code -- Phase 3 itself is diagrams/design per ADS v1, still not an implementation gate. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…kills, never actually existed Found while working through Program J's last Open Question: agent-tool- poisoning-guard.md, owasp-llm-top10/SKILL.md, agent-attack-surface/ SKILL.md, and deusdata--codebase-memory-mcp/SKILL.md all reference core/config/mcp-whitelist.json as if it already existed. Confirmed via `find`/`grep` that it never did -- the same "rule describes unbuilt infrastructure" pattern already fixed multiple times this session for 50-financial-deadman-switch-law.md, 54-bft-consensus-law.md, 56-circuit-breaker-law.md, and 62-sovereign-overlord-gate-law.md, and for the yana-router reference inside 9router-gateway/SKILL.md. anh Tam's direct instruction on hearing this: "không có thì tạo" (if it doesn't exist, create it). Created following the exact schema agent-tool-poisoning-guard.md already documents (policy/unknown_server_action/approved_mcp_servers), not inventing new structure. Seeded with one real, grounded entry -- ollama, confirmed live and in actual use this session (Program J's local-model- client work) -- rather than a placeholder list. Explicitly disclosed, not silently implied: no hook or script currently reads this file (grepped core/gates/sovereign-interceptor.js and core/hooks/*.sh, found no reference). The file now exists and matches its documented schema, but wiring an actual enforcement point is separate work, tracked in PROGRAM-J-SKELETON.md's Open Questions section as unresolved, not claimed done here. Also updates PROGRAM-J-SKELETON.md and README.md: Program J's Open Question 1 is now answered (the file's absence, not a real "extend vs build" choice, was the actual blocker), bringing Program J's Open Question count to 0. Phase 3 Architecture is unblocked but not yet started -- flagged as a stopping point for this session given its length, not started speculatively. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nded in real code
Per ADS v1's own Phase 3 definition ("ve kien truc, so do luong... khong
code"), drafted two flow diagrams for the MCP-Server-replaces-translator
decision from commit f6efc81:
1. Real-time hook enforcement (the security-critical path) -- traced
through src/guard/mod.rs directly before drawing anything. Found
check_command(command: &str) -> Option<&'static str> (line 691) is
already a pure function with no I/O, and its own comment says it was
extracted specifically so it "can be called once per MCP candidate" --
the MCP integration point already exists, unused. Also found the real
blocker: dispatch() (line 99) calls std::process::exit() directly,
fine for one-shot CLI use but fatal if called as-is from a long-running
MCP server process -- the correct integration point is check_command()
itself (needs a pub bump), not dispatch()/cmd_destructive(). Recorded
as a concrete Phase 9 Implementation Plan dependency, not glossed over.
2. Capability discovery (the genuinely new thing MCP adds beyond what the
translator pattern could do) -- tools/list against a runtime-read
registry instead of a hardcoded per-engine list.
Also recorded, as a flagged assumption needing anh Tam's confirmation
rather than a silent decision: Claude Code's own native PreToolUse/
PostToolUse hooks (.claude/settings.json, calling core/hooks/*.sh
directly) are a different mechanism from the translator-per-engine
pattern (Cursor's before-shell-execution.js and friends) and are assumed
OUT of this replacement's scope -- Claude Code was never using a
translator to begin with.
Modules table and README.md's Program table synced. Phase 4 (Workflow)
and Interfaces (message schema) intentionally left as TODO -- Phase 3 is
scoped to flow diagrams only per ADS v1, not full interface contracts.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…kening enforcement anh Tam's answer to the Claude Code scope question: "neu chuyen duoc thi cu, khong thi neu van ho tro thi dung nhu cu la duoc" (convert if feasible, otherwise keep using it as-is). Worked through the actual feasibility question rather than assuming an answer either way, because it's a real safety question, not just a technical one: Claude Code's PreToolUse hook is a mandatory interception mechanism (runs before every Bash/Write/Edit call regardless of what the model wants), while an MCP tool is something the model voluntarily chooses to call. Naively "moving hook enforcement to MCP" would mean turning a mandatory check into a tool the model could be prompt-injected into skipping -- a real security regression, not a neutral refactor. The reconciliation: don't touch the interception mechanism itself (.claude/settings.json's PreToolUse stays exactly as mandatory as it is today, the model has no visibility into or control over it) -- only change what the hook script does internally, from calling guard-destructive.sh directly to calling the new MCP Server (the same thin-bridge pattern core/adapters/cursor/before-shell-execution.js already uses, just with Claude Code's hook runner as the caller instead of Cursor). This unifies the judgment source across all 5 MCP clients without weakening what makes the Claude Code path safe today. Conclusion: Claude Code is now in scope for the MCP replacement, no exception. Updated the Ranh gioi scope section, Modules table, and the Status header/README.md table to match -- no more "assumption pending confirmation" language since anh Tam settled it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… not invented schema Fetched the actual MCP specification (modelcontextprotocol.io/ specification/2025-06-18/server/tools) before writing anything -- JSON-RPC 2.0, tools/list and tools/call message shapes taken verbatim from the published spec, not guessed from training-data recall (this session's trust-score flag requires double evidence for claims; a wrong schema here would be worse than no schema). Interfaces: defines the check_command tool (name/description/inputSchema/ outputSchema) and concrete tools/call request/response examples for both allow and deny outcomes. The one thing flagged as load-bearing, not a footnote: MCP's spec itself splits errors into two channels (JSON-RPC Protocol Errors vs. isError:true Tool Execution Errors) that guard- destructive.sh/check_command() currently has no equivalent of -- every unverifiable case there is just "deny," never a third "error, not sure" state. Both MCP error channels MUST map to deny on the client side, no exception, or the MCP transition would silently reintroduce a fail-open path the current design goes out of its way to avoid (see the fail-closed philosophy documented in core/adapters/cursor/before-shell-execution.js, read directly in the Phase 3 commit). Phase 4 Workflow: expands Phase 3's flow diagram into a full pipeline with startup, persistent connection (not per-request spawn, unlike the current spawnSync-per-call translator), all 4 result branches including the two failure paths, and audit logging (routed through the existing audit-hardening-policy.md chain, not a new parallel log per MCP's own "log tool usage for audit purposes" requirement). Three items explicitly left undecided for Phase 9 Implementation Plan or anh Tam's call: the MCP server's CLI subcommand name, the client-side timeout value (the current 15s was tuned for spawning bash; in-process calls should be much faster but that needs Phase 12 Benchmark data, not a guess), and how the server process gets started in a working session. Status header and README.md's table synced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Found while honestly scoring Program J against ADS v1's Readiness Matrix (Phase 5): none of today's Program J decisions (MCP replaces translator, Claude Code in scope without weakening enforcement, mcp-whitelist.json created) had been written to L1 atomic memory, despite memory-persistence-law.md's explicit requirement that architecture decisions affecting future work MUST be persisted before relying on them in a future session -- conversational context is ephemeral, L1 is not. add-fact.sh is interactive (prompts per field), not scriptable in this session -- followed the same manual-write pattern already established by fact-20260710-220852 and fact-20260710-220901 (matching schema, ID convention, confidence: unverified per fact-confidence-rule.md's "manual promotion only" constraint) rather than inventing a new convention. Recorded only the single highest-impact decision (MCP replaces translator-per-engine, including Claude Code, without weakening its enforcement mechanism) as one atomic fact -- not every sub-decision from today's session, per L1's own "atomic" naming. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ed from implementation Honest, evidence-based scoring against all 10 Readiness Matrix categories (ADS-v1.md only names the 10 categories, doesn't define a rubric per category -- flagged the two genuinely ambiguous ones, Notebook and Context, as "cach hieu chua chac" rather than confidently scoring something undefined): Ready (5): Repository, Knowledge, Runtime, Governance, Security -- each with concrete evidence, not just asserted. Ambiguous interpretation (2, counted at half weight): Notebook, Context. Not ready (2): Benchmark (no numbers anywhere, not even a rough estimate -- the timeout question from Phase 4's Workflow section is exactly this gap), Cost (never addressed in Phase 0-4 despite being thematically connected to this session's earlier "reduce Claude quota" discussion -- the two were never actually connected into real analysis). Tally: 5 + 2*0.5 + 0 = 6/10 = 60%, below ADS v1's own 80% bar -- "Readiness < 80% -> Block, chi duoc Research/ADR/Design, khong code." This is the expected, correct outcome at this stage (Phase 0-4 just finished, Phase 6 ADR / Phase 7 Research / Phase 8 Design Review haven't happened yet) -- not a failure, the gate doing its job. Rewrote the Status header to compress the accumulated per-decision history from today's session into one current-state summary, since it had grown into a long chronological log that the body sections (Scope, Architecture, Interfaces, Workflow, Readiness Matrix) already cover in full -- the detail isn't lost, just no longer duplicated at the top. README.md's Program table synced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…l-model delegation failed twice Benchmark: measured the CURRENT translator pattern directly (5 runs, node + spawnSync + core/adapters/cursor/before-shell-execution.js against a benign `ls -la`) rather than leaving the Readiness gap as an unmeasured guess: 178-310ms per call, ~220ms average. Compared against already-measured numbers from BENCHMARK.md (2026-07-23): Rust binary startup ~22-24ms, yana-rt guard token-budget dispatch (in-process, but carries ADR-008 lock overhead) ~65ms. check_command() has neither process-spawn nor lock overhead, so a persistent in-process MCP server should beat the current 220ms baseline by at least an order of magnitude -- stated as a directional estimate, not a real number, since the MCP server doesn't exist yet to measure. Cost: tried delegating a first-draft cost-factor brainstorm to two local Ollama models before writing it myself, per this session's local-model- delegation practice. Both failed -- qwen2.5-coder:14b gave an off-topic, truncated answer; huihui_ai/qwen3.5-abliterated:9b ran past 120s (backgrounded) and came back with an empty, invalid-JSON result. Wrote the analysis directly instead of forcing a bad delegation result into the doc. Flagged the most important point in it: the highest real cost here isn't money, it's the consequence of getting the MCP-error-to-deny mapping wrong (documented in Interfaces) -- a safety regression, not a budget line item, so this category needs engineering review more than financial estimation. Readiness Matrix score updated 60% -> 70% (Benchmark and Cost move from Not-ready to Partial). Still below ADS v1's 80% bar, still correctly blocked from Phase 10 Implementation -- recorded plainly that the honest path to 80% is going through Phase 6-8, not padding the score. Status header and README.md's table synced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…cision Phase 6 of ADS v1 for Program J. Follows ADS v1's own specified ADR template (Decision/Problem/Alternatives/Tradeoffs/Reason/Consequence), not the freeform Status/Context/Decision shape ADR-006 through ADR-009 used, since ADS v1 (established 2026-07-24) is now this repo's standing process for Program-scoped decisions. Distills PROGRAM-J-SKELETON.md's Phase 0-5 content into the formal ADR shape -- no new decisions introduced, every claim traces back to that doc, the real code read directly during this session (core/adapters/ cursor/before-shell-execution.js, src/guard/mod.rs), or the L1 fact already recorded (fact-20260724-233122). Records both alternatives that were actually on the table (the lower-risk incremental option this agent recommended via AskUserQuestion, and the full-replacement option anh Tam chose), not just the winner, per the Alternatives/Tradeoffs sections ADS v1's template requires. States plainly in its own Status section that writing this ADR does not clear Program J for Phase 10 Implementation -- Readiness is 70%, below ADS v1's 80% bar, and Phase 7 (Research) / Phase 8 (Design Review) still need to happen first. An ADR records a decision; it isn't a readiness gate by itself. Updated PROGRAM-J-SKELETON.md's ADR section (was a TODO placeholder) and its Status header, plus README.md's Program table, to point at the new file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…-config install path
Fetched real sources before writing anything (github.com/modelcontextprotocol/
rust-sdk, code.claude.com/docs/en/mcp), per ADS v1 Phase 7's own scope
("Khong viet code. Nguon: GitHub... Documentation... MCP...").
Three findings, each with a direct Phase 9 implication:
1. rmcp is the official Anthropic-maintained Rust MCP SDK
(cargo add rmcp --features server), actively developed, macro-based
tool definitions, supports both stdio and Streamable HTTP in
long-running multi-client mode. Means Phase 9 doesn't need to hand-roll
JSON-RPC -- lower risk of getting the error-mapping requirement
(Interfaces, Phase 1) wrong through a homegrown protocol
implementation.
2. Claude Code's local MCP server connection uses CLAUDE_PROJECT_DIR in
the spawned server's environment -- the exact same convention already
used by core/adapters/cursor/before-shell-execution.js (read directly
in the Phase 3 commit). No new convention needed.
3. The one with real install-experience impact: Claude Code plugins can
declare mcpServers directly in plugin.json using ${CLAUDE_PLUGIN_ROOT},
auto-connecting at session startup with zero manual .mcp.json setup.
Yana AI already ships as a Claude Code plugin (plugin.json,
marketplace.json exist) -- this means the MCP Server addition doesn't
need a new install step beyond an entry in plugin.json, matching the
existing "npx yana-ai-install wires the hooks (60 seconds)" philosophy
README.md already advertises.
First drafted a condensed summary via qwen2.5-coder:14b (local Ollama) --
this time asked it to reformat/condense already-fetched real source
material rather than reason from scratch (the task shape that failed
twice earlier today for Cost analysis). Result was accurate and usable,
unlike those two attempts -- used as the starting structure, then
expanded with the full detail and Program-J-specific implications above
before this commit.
Explicitly flagged as NOT yet researched: how Cursor, Codex, and Gemini
(3 of Program J's 5 clients) connect to local MCP servers -- only Claude
Code's mechanism is confirmed. Left as an open Phase 8 dependency, not
assumed to work the same way.
Status header rewritten again (was growing into a long chronological
log) -- README.md's table synced.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…endency caution Scored against ADS v1's own 9-item checklist (Architecture/Naming/ Dependency/Duplicate/Security/Maintainability/Performance/Scalability/ Governance), each with cited evidence rather than a rubber stamp. Dependency: fetched crates.io's real API data for rmcp before writing anything -- 17.1M total downloads, Apache-2.0 (matches Yana AI's own license), last published 2026-07-23 (yesterday), easily clears dependency-vetting-law.md's 8-criteria bar. Found one real thing worth flagging: crates.io's max_version is 3.0.0-beta.1, but 2.2.0 is the actual latest stable release -- Phase 9 should pin the stable line for a dependency sitting on a security-critical path, not the beta. Scalability: read check_command()'s signature directly rather than guessing -- `fn check_command(command: &str) -> Option<&'static str>` has no shared mutable state, no locks, no I/O, so it's safe to call concurrently from multiple client connections without added synchronization. A real finding from the code, not an assumption. Naming: genuinely unresolved -- the MCP server's CLI subcommand name was never decided (Workflow section already flagged this placeholder). Left as a real gap for Phase 9, not silently picked here. Zero items scored Fail. Explicitly noted: Design Review passing does not raise the Readiness Matrix score by itself -- they're different gates. Readiness stays at 70%, still below ADS v1's 80% implementation bar. Also re-flagged, still unresolved from Phase 7: how Cursor/Codex/Gemini (3 of Program J's 5 clients) connect to local MCP servers was never researched, only Claude Code's mechanism was. Left as an open gap rather than assumed identical. Status header and README.md's table synced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…name proposed from precedent Read src/main.rs's existing Commands enum before proposing a name for the new MCP server subcommand, rather than inventing one -- found Chat (line 162) is the one existing top-level command with flat flags and no nested action enum, unlike the Task/Guard/Bus-style noun+action pattern every other command uses. Proposed `yana-rt mcp` on that precedent, since the MCP server is "run as a mode" like chat, not a CRUD resource like task/ guard. Treated this as a naming/housekeeping call following existing convention, not an architecture decision needing anh Tam's sign-off -- flagged the reasoning so it's not an unlabeled guess either. Five stages per ADS v1's own Phase 9 template (Research -> Prototype -> Alpha -> Beta -> Stable): - Research: the still-open Cursor/Codex/Gemini MCP connection gap from Phase 7/8, plus a check_command()-to-rmcp spike with no client swapped yet. - Prototype: yana-rt mcp exposing exactly one tool, tested against a real MCP client, both Interfaces error-mapping branches (Protocol Error, isError:true) verified to resolve to deny -- stated as a hard gate before Alpha, not optional. - Alpha: Cursor only (it already has a translator to compare against), running in parallel with the old path, real Benchmark data gathered against the 220ms baseline before touching anything else. - Beta: remaining clients, including wiring actual enforcement for core/config/mcp-whitelist.json (created earlier today, still has no reader -- this is where that gap gets closed, not left open forever). - Stable: all 5 clients on MCP; the guard-destructive.sh/before-shell- execution.js deprecate-or-keep question (raised, not answered, in Phase 8's Maintainability row) gets decided here, not earlier. Explicit boundary stated: this plan stages the work, it does not itself authorize Phase 10 code -- Readiness is still 70%, still below ADS v1's 80% bar. Status header condensed again and README.md's table synced. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…config confirmed Fetched real docs for each of the 3 remaining clients separately (per this session's own established discipline: "khong gia dinh giong nhau"), not assumed identical to Claude Code: - Cursor: .cursor/mcp.json, same mcpServers JSON schema as Claude Code (cursor.com/docs/context/mcp) - Gemini CLI: .gemini/settings.json, same mcpServers JSON schema again (github.com/google-gemini/gemini-cli docs) - Codex CLI: .codex/config.toml, genuinely different -- TOML, not JSON, [mcp_servers.<name>] table syntax (learn.chatgpt.com/docs/extend/mcp) Net finding for Phase 9: 3 of 4 clients (Cursor, Gemini, and Claude Code when not using the plugin.json auto-connect path) share one identical JSON schema -- an install script only needs one TOML branch (Codex) as the real special case, not four separate formats. This repo already has untracked .cursor/, .codex/, .gemini/ directories from earlier session work -- the right locations for these config files once Phase 10 starts. Marks Roadmap's Research stage as done; the remaining Research-stage item (the rmcp + check_command spike) is unchanged, tracked separately. Status header and README.md's table updated to note anh Tam authorized proceeding past the Readiness gate for a small Prototype spike next. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…atency data anh Tam authorized proceeding past the 70% Readiness gate for a small Research/Prototype spike, per ADS v1's own exception path for a human override. Not Phase 10 implementation -- an isolated, disposable prototype validating the architecture before committing to it for real. New: src/mcp.rs, a minimal MCP server (rmcp 2.2.0, pinned to the stable line per Phase 8's own finding, not crates.io's 3.0.0-beta.1 max_version) exposing exactly one tool, check_command, over stdio. Calls crate::guard::check_command() directly, in-process -- the same pure judgment function core/hooks/guard-destructive.sh mirrors, unchanged. src/guard/mod.rs: check_command() becomes pub (was module-private) so src/mcp.rs can call it without going through dispatch()/cmd_destructive() (both route to std::process::exit(), fatal if called from a long-running server process). Zero logic changes -- confirmed by diff, verified in review. src/main.rs + Cargo.toml: new Commands::Mcp and rmcp/tokio dependencies, all gated behind a new `mcp` Cargo feature, separate from default `cli`. This is the crate's first tokio/async dependency (confirmed via docs.rs before adding) -- kept isolated so the spike doesn't change the default build's footprint. Verified at the binary level (strings scan), not just by reading #[cfg] attributes: tokio/rmcp symbols are absent from a default-feature build. Live end-to-end test this session (real Python MCP client, full initialize/tools-list/tools-call handshake, not simulated): benign command -> allow, `rm -rf` -> deny with the real reason text, and the exact `python3 -c` inline-script bypass fixed earlier this session -> deny with the identical reason the bash guard would give -- proving check_command() is genuinely reused, not reimplemented. Measured latency: p50 0.134ms, avg 0.154ms per call (50 runs, post-warmup) versus the previously-measured 178-310ms translator baseline -- roughly 1400-1600x faster once the server is already running. cargo test --features cli (mcp NOT enabled) still 183/183 + 63/63, single-threaded, confirming zero regression to the existing default build. One pre-existing test flake found and left alone, out of scope: guard::blast_paths::tests::absolute_path_under_repo_root_is_protected races on a shared YANA_REPO_ROOT env var under parallel test execution (passes reliably in isolation) -- not touched by this change, not introduced by it. Per 54-bft-consensus-law.md's spirit (src/guard/mod.rs is core-lock-pinned security-critical code, same category as core/hooks/** even though the law's literal trigger table doesn't name src/), dispatched both security-team/security-auditor.md and code-auditor.md before this commit. No Safety-severity findings. Three Style/Performance-tier notes from code-auditor, explicitly scoped to "fix before Phase 10, not now": the tool_router field is currently dead code (rmcp's default #[tool_handler] expansion calls the associated fn, not the cached instance -- confirmed by reading rmcp-macros' generated code, not assumed), the `mcp` feature doesn't declare its real dependency on `cli` (works today only because main.rs is itself gated on `cli`), and tokio's "full" feature set is wider than a stdio-only server needs. Applied the one cheap fix (security-auditor's and code-auditor's own wording note on the check_command doc comment: "both call process::exit()" corrected to "both route to process::exit()", since only dispatch() calls it directly). core/config/core-lock.json regenerated per 67-core-integrity-lock-law.md's protocol (diff reviewed -- only src/guard/mod.rs's hash changed -- regenerated via update-core-lock.sh, never hand-edited). Readiness Matrix impact (docs/programs/PROGRAM-J-SKELETON.md): Benchmark row moves from Partial to Ready on this real data, taking the overall score from 70% to 85% -- above ADS v1's 80% bar. Stated plainly in the doc: this is the result of one small spike, not a completed Phase 10, and whether to treat Program J as formally past the Readiness gate is anh Tam's call to make, not this agent's to declare. Also closed the Research-stage gap flagged in the previous two commits: fetched real docs for Cursor (.cursor/mcp.json), Gemini CLI (.gemini/settings.json), and Codex CLI (.codex/config.toml) separately, per this session's own discipline of not assuming they match Claude Code's mechanism. Finding: 3 of 4 clients share one identical mcpServers JSON schema; only Codex differs (TOML). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Still root-causing the E403 on the actual npm publish PUT after confirming (2026-07-24/25) that OIDC token exchange and provenance signing both succeed -- environment/permissions/id-token are all verified present in this file, branch is confirmed main (workflow_dispatch's own if: condition doesn't gate on ref at all), so the failure is somewhere in npm-side Trusted Publisher matching, not this workflow's config as written. Adds one diagnostic step printing GITHUB_REPOSITORY/REF/WORKFLOW/JOB/ EVENT_NAME/ACTOR right before the publish step, to hand-compare against npmjs.com's Trusted Publisher entry for anything logs don't otherwise surface (e.g. GITHUB_WORKFLOW is the workflow's `name:` field, not its filename -- worth confirming npm expects the filename, not the display name, since this repo's workflow name is "Publish to npm + PyPI + crates.io", not "publish.yml"). Explicitly temporary -- remove once the 403 is root-caused. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
src/yana_ai/rt.py resolved the yana-rt binary via shutil.which with no self-check, the same bug class fixed in scripts/yana-rt-wrapper.js on 2026-07-08/09 after a real incident (self-invocation through PATH, 100% CPU, 116C, forced shutdown) -- but that fix was never ported to this file, so every PyPI release to date carries the same unpatched recursion risk. Adds the same guard pattern as the JS fix: a hard re-entry env var checked before resolving any candidate, plus a realpath self-check applied to every candidate including the explicit override, so the wrapper's own path pasted into the override cannot re-arm the recursion either. Verified with 5 direct logic tests against the new helper functions (self-reference rejection, nonexistent path, empty input, real executable, exit code 1 on re-entry) -- all pass. README.md: corrected an inaccurate warning that claimed pip was unaffected; only cargo install yana-rt is structurally immune (no wrapper to recurse through). npm and PyPI both currently carry the bug in their latest published release. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Ports the last four English-only README updates into the three translations: the yana-rt self-recursion warning (npm/PyPI affected, cargo unaffected), the demo GIF, the debunked-1256x benchmark rewrite, and the new yana-ai chat subcommand. Also corrects a pre-existing 57-vs-58 hooks mismatch in the safety-architecture tree diagram that predates this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds `yana-ai skill-quality show|promote`, a per-skill outcome ledger built entirely from two signals Yana AI already produces: which skill/agent a task's session invoked (audit-chain.log) and eval judge's PASS/FAIL verdict. No new hook, no new LLM call, no new dependency. Idea borrowed from HKUDS/OpenSpace's quality-from-real-tasks and provisional/trusted promotion model, reimplemented from scratch with no dependency on that project or its cloud. Promotion always requires an explicit human command; demotion on a fresh FAIL streak is automatic, since revoking standing is safe to run unattended in a way granting it isn't. Incorporates a follow-up round from external code review: narrows skill/task attribution to the task's own work window instead of an unbounded "now" (mitigates, doesn't fully close, cross-task attribution — full fix needs task-id tagging in audit-log.sh itself, out of scope here), and switches timestamp comparison from raw string ordering to parsed RFC3339 instants. 262 tests pass (199 unit + 63 integration), 0 regressions. Verified end-to-end against a real task, a real Skill tool-call entry in audit-chain.log, and a local Ollama judge call. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
npm publishing has been blocked for weeks by an issue confirmed to be account-level, not package- or config-specific: the same 403 reproduces across multiple packages under the same npm account, through both CI's OIDC trusted publishing and a fresh manual browser login, while npm's own `access list` reports read-write on every one of them. Reported to npm support repeatedly with no resolution. Rather than keep chasing an unresponsive support channel, all four READMEs now say so plainly and point new installs at `pip install yana-ai` or `cargo install yana-rt` for the current version. Also updates the existing yana-rt self-recursion known-issue callout, which referenced this same npm block as "tracked, in progress" — no longer accurate now that active pursuit has stopped. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Review or Edit in CodeSandboxOpen the branch in Web Editor • VS Code • Insiders |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (20)
📝 WalkthroughWalkthroughPR bổ sung MCP stdio server và feature build tương ứng, công khai guard contract, thêm hệ thống theo dõi chất lượng skill, bảo vệ wrapper ChangesTheo dõi chất lượng skill
MCP guard runtime
Bảo vệ runtime và tài liệu phát hành
Chẩn đoán publish npm
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
yana | 0cdb881 | Commit Preview URL Branch Preview URL |
Jul 26 2026, 06:23 AM |
…-skills # Conflicts: # README.md # src/main.rs
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (1)
src/skill_quality.rs (1)
175-179: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueĐọc streaming thay vì nạp cả
audit-chain.logvào RAM.
audit-chain.logđược ghi trên mỗi tool call và không có pruning (module doc đã thừa nhận), nênread_to_stringsẽ tải toàn bộ file mỗi lầneval judgechạy.BufReader::new(File::open(..))+.lines()giữ nguyên logic mà bộ nhớ không phụ thuộc kích thước log.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/skill_quality.rs` around lines 175 - 179, Update the audit log reading in the function containing audit_log_path and the found collection to stream lines with BufReader over File::open instead of loading the entire file via fs::read_to_string. Preserve filtering of blank lines and the existing processing logic, handling file-open or line-read failures consistently with the current fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/config/mcp-whitelist.json`:
- Around line 7-16: Wire the MCP whitelist configuration into the runtime server
authorization path so “deny-by-default” is actually enforced. Use
approved_mcp_servers to allow only matching name, origin, and permissions; block
and log unknown or unapproved servers according to unknown_server_action. If no
runtime consumer exists in this change, explicitly document that this file is
configuration-only and defer enforcement rather than implying it is active.
In `@docs/programs/PROGRAM-J-SKELETON.md`:
- Around line 655-656: Update the Security status entry in the Phase 1-4 table
so it is not marked “✅ Ready” while client-side fail-closed handling remains
unimplemented and untested. Mark it Partial/Blocked or clearly separate
server-spike readiness from end-to-end enforcement, while preserving the
existing security details and Alpha prerequisite.
- Line 158: Update the fenced code blocks at the indicated documentation
sections to specify the text language, changing their opening fences to ```text
for diagrams and pipelines so the document passes markdownlint MD040.
In `@docs/programs/README.md`:
- Line 62: Replace Program J’s Draft status with a distinct ADS-aligned
post-readiness status indicating that the spike is complete and Phase 10 awaits
owner approval. Apply the same status wording and interpretation in
docs/programs/README.md at line 62 and docs/programs/PROGRAM-J-SKELETON.md at
lines 3-7 so the overview and header/status table remain consistent.
In `@README.ko.md`:
- Around line 13-14: Synchronize the hooks and skills counts with the verified
canonical source: update the summary metrics at README.ko.md lines 13-14,
README.vi.md lines 13-14, and README.zh.md lines 13-14, then update the
corresponding architecture-tree counts at README.ko.md line 261, README.vi.md
line 262, and README.zh.md line 261 so every value matches.
In `@README.md`:
- Around line 134-142: Update the installation guidance in README.md lines
134-142 to remove pip as a current-version option and recommend cargo or source
installation until PyPI is verified to contain the fix; update README.ko.md
lines 134-135, README.vi.md lines 134-135, and README.zh.md lines 134-135
likewise by removing pip and clearly distinguishing the source version from the
published npm version.
In `@src/mcp.rs`:
- Around line 55-61: Đồng bộ phản hồi tool với hợp đồng outputSchema đã công bố:
tại src/mcp.rs dòng 55-61, dùng CallToolResult::structured cho dữ liệu
allow/deny thay vì chỉ trả ContentBlock::text, đồng thời giữ wire format khớp
schema. Cập nhật các phần outputSchema/structuredContent tương ứng tại
docs/programs/PROGRAM-J-SKELETON.md dòng 245-267 và 281-305 nếu implementation
không thể đáp ứng schema, rồi bổ sung test wire-format cho hợp đồng thực tế.
In `@src/skill_quality.rs`:
- Around line 271-327: Refactor cmd_show into two focused helpers: show_one for
the named-skill detail path and show_table for the aggregate listing path,
leaving cmd_show as a short dispatcher while preserving all output and JSON
behavior. Also reduce skill_quality.rs below 300 lines by moving the mod tests
section into an appropriate tests/ file or child module, without changing test
coverage.
- Line 291: Update the promotion hint in the skill-quality output to use an
existing yana-rt subcommand, or add the necessary dispatch route before
retaining the yana-ai command; ensure the printed command is executable through
the bin/yana wrapper.
- Around line 109-120: Update load_store and save_store to preserve ledger data
on corruption and avoid panics during eval judge. Make save_store return a
Result and write through a temporary file followed by an atomic rename,
propagating filesystem or serialization errors instead of using expect; update
its caller to handle the returned error. In load_store, distinguish missing
files from read/parse failures and do not replace a corrupted existing store
with SkillQualityStore::default().
---
Nitpick comments:
In `@src/skill_quality.rs`:
- Around line 175-179: Update the audit log reading in the function containing
audit_log_path and the found collection to stream lines with BufReader over
File::open instead of loading the entire file via fs::read_to_string. Preserve
filtering of blank lines and the existing processing logic, handling file-open
or line-read failures consistently with the current fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ffffbd11-8991-475f-ac82-f68b8831b595
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.github/workflows/publish.yml.gitignoreCargo.tomlREADME.ko.mdREADME.mdREADME.vi.mdREADME.zh.mdcore/config/core-lock.jsoncore/config/mcp-whitelist.jsondocs/adr/ADR-010-mcp-server-replaces-translator-per-engine.mddocs/programs/PROGRAM-J-SKELETON.mddocs/programs/README.mdmemory/L1_atomic/INDEX.mdmemory/L1_atomic/fact-20260724-233122.mdsrc/guard/mod.rssrc/main.rssrc/mcp.rssrc/skill_quality.rssrc/task.rssrc/yana_ai/rt.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 10
🧹 Nitpick comments (1)
src/skill_quality.rs (1)
175-179: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueĐọc streaming thay vì nạp cả
audit-chain.logvào RAM.
audit-chain.logđược ghi trên mỗi tool call và không có pruning (module doc đã thừa nhận), nênread_to_stringsẽ tải toàn bộ file mỗi lầneval judgechạy.BufReader::new(File::open(..))+.lines()giữ nguyên logic mà bộ nhớ không phụ thuộc kích thước log.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/skill_quality.rs` around lines 175 - 179, Update the audit log reading in the function containing audit_log_path and the found collection to stream lines with BufReader over File::open instead of loading the entire file via fs::read_to_string. Preserve filtering of blank lines and the existing processing logic, handling file-open or line-read failures consistently with the current fallback behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/config/mcp-whitelist.json`:
- Around line 7-16: Wire the MCP whitelist configuration into the runtime server
authorization path so “deny-by-default” is actually enforced. Use
approved_mcp_servers to allow only matching name, origin, and permissions; block
and log unknown or unapproved servers according to unknown_server_action. If no
runtime consumer exists in this change, explicitly document that this file is
configuration-only and defer enforcement rather than implying it is active.
In `@docs/programs/PROGRAM-J-SKELETON.md`:
- Around line 655-656: Update the Security status entry in the Phase 1-4 table
so it is not marked “✅ Ready” while client-side fail-closed handling remains
unimplemented and untested. Mark it Partial/Blocked or clearly separate
server-spike readiness from end-to-end enforcement, while preserving the
existing security details and Alpha prerequisite.
- Line 158: Update the fenced code blocks at the indicated documentation
sections to specify the text language, changing their opening fences to ```text
for diagrams and pipelines so the document passes markdownlint MD040.
In `@docs/programs/README.md`:
- Line 62: Replace Program J’s Draft status with a distinct ADS-aligned
post-readiness status indicating that the spike is complete and Phase 10 awaits
owner approval. Apply the same status wording and interpretation in
docs/programs/README.md at line 62 and docs/programs/PROGRAM-J-SKELETON.md at
lines 3-7 so the overview and header/status table remain consistent.
In `@README.ko.md`:
- Around line 13-14: Synchronize the hooks and skills counts with the verified
canonical source: update the summary metrics at README.ko.md lines 13-14,
README.vi.md lines 13-14, and README.zh.md lines 13-14, then update the
corresponding architecture-tree counts at README.ko.md line 261, README.vi.md
line 262, and README.zh.md line 261 so every value matches.
In `@README.md`:
- Around line 134-142: Update the installation guidance in README.md lines
134-142 to remove pip as a current-version option and recommend cargo or source
installation until PyPI is verified to contain the fix; update README.ko.md
lines 134-135, README.vi.md lines 134-135, and README.zh.md lines 134-135
likewise by removing pip and clearly distinguishing the source version from the
published npm version.
In `@src/mcp.rs`:
- Around line 55-61: Đồng bộ phản hồi tool với hợp đồng outputSchema đã công bố:
tại src/mcp.rs dòng 55-61, dùng CallToolResult::structured cho dữ liệu
allow/deny thay vì chỉ trả ContentBlock::text, đồng thời giữ wire format khớp
schema. Cập nhật các phần outputSchema/structuredContent tương ứng tại
docs/programs/PROGRAM-J-SKELETON.md dòng 245-267 và 281-305 nếu implementation
không thể đáp ứng schema, rồi bổ sung test wire-format cho hợp đồng thực tế.
In `@src/skill_quality.rs`:
- Around line 271-327: Refactor cmd_show into two focused helpers: show_one for
the named-skill detail path and show_table for the aggregate listing path,
leaving cmd_show as a short dispatcher while preserving all output and JSON
behavior. Also reduce skill_quality.rs below 300 lines by moving the mod tests
section into an appropriate tests/ file or child module, without changing test
coverage.
- Line 291: Update the promotion hint in the skill-quality output to use an
existing yana-rt subcommand, or add the necessary dispatch route before
retaining the yana-ai command; ensure the printed command is executable through
the bin/yana wrapper.
- Around line 109-120: Update load_store and save_store to preserve ledger data
on corruption and avoid panics during eval judge. Make save_store return a
Result and write through a temporary file followed by an atomic rename,
propagating filesystem or serialization errors instead of using expect; update
its caller to handle the returned error. In load_store, distinguish missing
files from read/parse failures and do not replace a corrupted existing store
with SkillQualityStore::default().
---
Nitpick comments:
In `@src/skill_quality.rs`:
- Around line 175-179: Update the audit log reading in the function containing
audit_log_path and the found collection to stream lines with BufReader over
File::open instead of loading the entire file via fs::read_to_string. Preserve
filtering of blank lines and the existing processing logic, handling file-open
or line-read failures consistently with the current fallback behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ffffbd11-8991-475f-ac82-f68b8831b595
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (20)
.github/workflows/publish.yml.gitignoreCargo.tomlREADME.ko.mdREADME.mdREADME.vi.mdREADME.zh.mdcore/config/core-lock.jsoncore/config/mcp-whitelist.jsondocs/adr/ADR-010-mcp-server-replaces-translator-per-engine.mddocs/programs/PROGRAM-J-SKELETON.mddocs/programs/README.mdmemory/L1_atomic/INDEX.mdmemory/L1_atomic/fact-20260724-233122.mdsrc/guard/mod.rssrc/main.rssrc/mcp.rssrc/skill_quality.rssrc/task.rssrc/yana_ai/rt.py
🛑 Comments failed to post (10)
core/config/mcp-whitelist.json (1)
7-16: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Whitelist deny-by-default chưa có điểm enforcement.
File này chỉ mô tả policy;
_metaxác nhận chưa có runtime consumer. Vì vậy MCP server/unknown server hiện chưa thực sự bị block-and-log. Nếu PR chỉ thêm schema thì cần ghi rõ đây là cấu hình chưa enforce; nếu đây là acceptance criterion, phải wire enforcement trước release.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/config/mcp-whitelist.json` around lines 7 - 16, Wire the MCP whitelist configuration into the runtime server authorization path so “deny-by-default” is actually enforced. Use approved_mcp_servers to allow only matching name, origin, and permissions; block and log unknown or unapproved servers according to unknown_server_action. If no runtime consumer exists in this change, explicitly document that this file is configuration-only and defer enforcement rather than implying it is active.docs/programs/PROGRAM-J-SKELETON.md (2)
158-158: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Thêm language cho các fenced code block.
markdownlint-cli2báo MD040 tại Line 158, Line 185 và Line 341. Dùng```textcho các sơ đồ/pipeline để tài liệu vượt lint.Also applies to: 185-185, 341-341
🧰 Tools
🪛 markdownlint-cli2 (0.23.0)
[warning] 158-158: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/programs/PROGRAM-J-SKELETON.md` at line 158, Update the fenced code blocks at the indicated documentation sections to specify the text language, changing their opening fences to ```text for diagrams and pipelines so the document passes markdownlint MD040.Source: Linters/SAST tools
655-656: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Không đánh dấu Security là Ready khi fail-closed phía client còn chưa hoàn tất.
Line 604-609 xác nhận mapping Protocol Error và
isError:truesang deny vẫn chưa được implement/test, đồng thời ghi đây là điều kiện bắt buộc trước Alpha. Vì vậy trạng tháiSecurity | ✅ Readyhiện gây hiểu nhầm; hãy đổi thành Partial/Blocked hoặc tách rõ “server spike đã sẵn sàng” khỏi “end-to-end enforcement”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/programs/PROGRAM-J-SKELETON.md` around lines 655 - 656, Update the Security status entry in the Phase 1-4 table so it is not marked “✅ Ready” while client-side fail-closed handling remains unimplemented and untested. Mark it Partial/Blocked or clearly separate server-spike readiness from end-to-end enforcement, while preserving the existing security details and Alpha prerequisite.docs/programs/README.md (1)
62-62: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Đồng bộ trạng thái Program J với vocabulary ADS.
Cả hai tài liệu đều dùng
Draftcho trạng thái Phase 0-9 đã hoàn tất, trái với định nghĩaDraftlà Specification chưa đầy đủ.
docs/programs/README.md#L62: đổi sang trạng thái post-readiness phù hợp hoặc bổ sung trạng thái riêng cho “đã spike, chờ owner mở khóa Phase 10”.docs/programs/PROGRAM-J-SKELETON.md#L3-L7: dùng cùng trạng thái với bảng tổng quan, tránh để header và status table diễn giải khác nhau.📍 Affects 2 files
docs/programs/README.md#L62-L62(this comment)docs/programs/PROGRAM-J-SKELETON.md#L3-L7🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/programs/README.md` at line 62, Replace Program J’s Draft status with a distinct ADS-aligned post-readiness status indicating that the spike is complete and Phase 10 awaits owner approval. Apply the same status wording and interpretation in docs/programs/README.md at line 62 and docs/programs/PROGRAM-J-SKELETON.md at lines 3-7 so the overview and header/status table remain consistent.README.ko.md (1)
13-14: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Đồng bộ các số liệu hooks và skills.
Mỗi bản dịch hiện hiển thị 61 hooks/2.025 skills ở phần đầu, nhưng cây kiến trúc lại ghi 57 hooks/2.016 skills. Ít nhất một tập số liệu là sai.
README.ko.md#L13-L14: dùng số liệu đã xác minh từ nguồn chuẩn.README.ko.md#L261-L261: cập nhật hooks và skills trong cây để khớp nguồn chuẩn.README.vi.md#L13-L14: dùng số liệu đã xác minh từ nguồn chuẩn.README.vi.md#L262-L262: cập nhật hooks và skills trong cây để khớp nguồn chuẩn.README.zh.md#L13-L14: dùng số liệu đã xác minh từ nguồn chuẩn.README.zh.md#L261-L261: cập nhật hooks và skills trong cây để khớp nguồn chuẩn.📍 Affects 3 files
README.ko.md#L13-L14(this comment)README.ko.md#L261-L261README.vi.md#L13-L14README.vi.md#L262-L262README.zh.md#L13-L14README.zh.md#L261-L261🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.ko.md` around lines 13 - 14, Synchronize the hooks and skills counts with the verified canonical source: update the summary metrics at README.ko.md lines 13-14, README.vi.md lines 13-14, and README.zh.md lines 13-14, then update the corresponding architecture-tree counts at README.ko.md line 261, README.vi.md line 262, and README.zh.md line 261 so every value matches.README.md (1)
134-142: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Không hướng người dùng sang
pipcho đến khi PyPI chứa bản vá.Các cảnh báo phía trên nói bản PyPI đã sửa chưa được phát hành, nhưng các dòng này lại giới thiệu
pip install yana-ailà đường cài “bản hiện hành”. PyPI hiện vẫn liệt kê bản 0.42.3, phát hành ngày 29-06-2026. (pypi.org)
README.md#L134-L142: chỉ đề xuấtcargohoặc cài từ mã nguồn cho đến khi xác minh PyPI đã phát hành bản vá.README.ko.md#L134-L135: bỏ đề xuấtpip; đồng thời phân biệt rõ phiên bản mã nguồn với phiên bản npm đã phát hành.README.vi.md#L134-L135: bỏ đề xuấtpip; đồng thời phân biệt rõ phiên bản mã nguồn với phiên bản npm đã phát hành.README.zh.md#L134-L135: bỏ đề xuấtpip; đồng thời phân biệt rõ phiên bản mã nguồn với phiên bản npm đã phát hành.📍 Affects 4 files
README.md#L134-L142(this comment)README.ko.md#L134-L135README.vi.md#L134-L135README.zh.md#L134-L135🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@README.md` around lines 134 - 142, Update the installation guidance in README.md lines 134-142 to remove pip as a current-version option and recommend cargo or source installation until PyPI is verified to contain the fix; update README.ko.md lines 134-135, README.vi.md lines 134-135, and README.zh.md lines 134-135 likewise by removing pip and clearly distinguishing the source version from the published npm version.src/mcp.rs (1)
55-61: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== files ==" git ls-files | rg '(^|/)(src/mcp\.rs|docs/programs/(PROGRAM-J-SKELETON|README)\.md|Cargo\.toml|Cargo\.lock)$' || true echo "== src/mcp.rs excerpt ==" if [ -f src/mcp.rs ]; then nl -ba src/mcp.rs | sed -n '1,120p' fi echo "== docs excerpts ==" if [ -f docs/programs/PROGRAM-J-SKELETON.md ]; then nl -ba docs/programs/PROGRAM-J-SKELETON.md | sed -n '230,315p' fi echo "== rmcp dependency versions ==" rg -n "rmcp|version = \"|name = \"rm" Cargo.toml Cargo.lock 2>/dev/null | head -80 || true echo "== git diff stat/name-only ==" git diff --stat || true git diff --name-only || trueRepository: yanacuti1121/Yana-AI
Length of output: 328
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== src/mcp.rs excerpt ==" if [ -f src/mcp.rs ]; then awk '{printf "%6d\t%s\n", NR, $0}' src/mcp.rs | sed -n '1,120p' fi echo "== docs excerpts ==" if [ -f docs/programs/PROGRAM-J-SKELETON.md ]; then awk '{printf "%6d\t%s\n", NR, $0}' docs/programs/PROGRAM-J-SKELETON.md | sed -n '230,315p' fi echo "== dependency/version references ==" rg -n "rmcp|version = \"|name = \"rm" Cargo.toml Cargo.lock 2>/dev/null | head -80 || true echo "== current branch/merged diff stat ==" git diff --stat || true git diff --name-only || true echo "== references to outputSchema/structuredContent/text content ==" rg -n "outputSchema|structuredContent|ContentBlock::text|to_string\\(\\)|check_command" --glob '!target/**' --glob '!node_modules/**' .Repository: yanacuti1121/Yana-AI
Length of output: 50378
🌐 Web query:
Model Context Protocol 2025-06-18 tools call result structuredContent content text outputSchema💡 Result:
In the Model Context Protocol (MCP) as of the 2025-06-18 specification, structured tool output is managed via the
outputSchemaproperty in theTooldefinition and thestructuredContentfield in theCallToolResultresponse [1][2][3]. Key technical details regarding these fields include: - outputSchema: This is an optional JSON Schema object defined within a tool's metadata [1][2]. It specifies the expected structure of the tool's output [4][1]. If provided, servers must ensure that thestructuredContentreturned in a tool call conforms to this schema [1][2]. - structuredContent: When a tool generates structured data, it is returned as a JSON object in this field [1][2]. This field is specifically for programmatic consumption (e.g., code generation or type-safe integrations) and is distinct from the unstructuredcontentfield used for LLM interaction [5][6]. - content: This field contains an array of content blocks (e.g.,TextContent) [1][2]. For backwards compatibility, when a tool returnsstructuredContent, it is recommended (SHOULD) that the server also return a serialized, human-readable version of that JSON within aTextContentblock in thecontentfield [5][1]. This mechanism allows tools to provide both a machine-readable format for reliable data extraction and a human-readable format to maintain compatibility with older clients and LLMs [5][6][7].Citations:
- 1: https://modelcontextprotocol.io/specification/2025-06-18/server/tools
- 2: https://github.com/modelcontextprotocol/modelcontextprotocol/blob/main/schema/2025-06-18/schema.ts
- 3: https://modelcontextprotocol.io/specification/2025-06-18/changelog
- 4: https://modelcontextprotocol.io/specification/draft/server/tools
- 5: modelcontextprotocol/modelcontextprotocol#1563
- 6: modelcontextprotocol/modelcontextprotocol#1624
- 7: https://devblogs.microsoft.com/dotnet/mcp-csharp-sdk-2025-06-18-update/
🌐 Web query:
rmcp 2.2.0 CallToolResult::success structured_content API💡 Result:
In the
rmcpRust SDK (version 2.2.0 and later),CallToolResultis a core data structure used to represent the outcome of a tool execution within the Model Context Protocol [1][2]. TheCallToolResultstruct is defined in thermcp::modelmodule [1] and typically includes the following fields [1]: - content: A vector of content items (e.g., text, images, or other supported types) returned by the tool [1]. - structured_content: An optional field (usingserde_json::Value) for structured data returns [1]. - is_error: An optional boolean flag to indicate if the tool execution encountered an error [1]. - meta: An optional metadata field for additional context [1]. To create a successful result, the SDK provides theCallToolResult::successmethod [1]. The signature is: pub fn success(content: Vec) -> Self This method creates aCallToolResultwith the provided unstructured content vector [1]. If your implementation requires structured content, you may also use theCallToolResult::structuredmethod, which allows you to pass aserde_json::Valuedirectly to represent the structured return [1]. Thermcpcrate serves as the official Rust SDK for the Model Context Protocol [2][3]. Version 2.2.0, released in early 2026, includes several updates such as improved authentication support (S256 PKCE), conformance suite fixes, and alignment with the late-2025 MCP specification [4][5][6]. Detailed documentation for the current version can be found on docs.rs [3][7].Citations:
- 1: https://docs.rs/rmcp/latest/rmcp/model/struct.CallToolResult.html?search=
- 2: https://github.com/modelcontextprotocol/rust-sdk
- 3: https://docs.rs/rmcp/latest/rmcp/index.html
- 4: hyper-mcp-rs/hyper-mcp-proxy#12
- 5: hyperpolymath/echidna#315
- 6: dinglebear-ai/synapse#37
- 7: https://docs.rs/rmcp
🌐 Web query:
MCP servers should support both structuredContent and content text when outputSchema is defined💡 Result:
In the Model Context Protocol (MCP), when an outputSchema is defined for a tool, servers are expected to support and utilize both structuredContent and content fields, though they serve different, complementary purposes [1][2][3]. StructuredContent is intended for machine-oriented output [1]. When an outputSchema is provided, servers must populate structuredContent with data that conforms to that schema [2][4]. This field facilitates programmatic tool use, type-safe orchestration, and strict validation [1][2]. Content is intended for model-oriented output [1]. It is used to provide a human-readable representation of the tool's result, which is optimized for readability and token efficiency by language models [1][5]. For backwards compatibility and to ensure a high-quality experience for both automated systems and conversational agents, servers should provide this human-readable summary in the content field even when structuredContent is present [1][2]. In practice, the two fields are intended to be semantically equivalent, offering the same information through different presentations [1]. While some client implementations may have encountered issues where one field overrides or obscures the other (e.g., in certain editor integrations) [6], the protocol specification emphasizes the importance of using both to satisfy different audience needs—structured data for the application's internal logic and text content for the model's understanding [1][2][5].
Citations:
- 1: modelcontextprotocol/modelcontextprotocol#1624
- 2: https://modelcontextprotocol.io/specification/draft/server/tools
- 3: https://github.com/modelcontextprotocol/typescript-sdk/blob/main/docs/servers/tools.md
- 4: https://modelcontextprotocol.io/specification/2025-11-25/server/tools
- 5: https://sunpeak.ai/docs/mcp-apps/server/build-server
- 6: microsoft/vscode#290063
Đồng bộ response MCP với
outputSchemađã công bố.
docs/programs/PROGRAM-J-SKELETON.md#L245công bốoutputSchema, nhưngsrc/mcp.rs#L55-L61chỉ trả text content;src/mcp.rs#L59-L61cũng không dùng APICallToolResult::structuredcho dữ liệu đã mô tả schema. Cập nhật implementation/tool response hoặc bỏoutputSchema/structuredContenttrong docs rồi thêm test wire-format theo contract thực tế.📍 Affects 2 files
src/mcp.rs#L55-L61(this comment)docs/programs/PROGRAM-J-SKELETON.md#L245-L267docs/programs/PROGRAM-J-SKELETON.md#L281-L305🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/mcp.rs` around lines 55 - 61, Đồng bộ phản hồi tool với hợp đồng outputSchema đã công bố: tại src/mcp.rs dòng 55-61, dùng CallToolResult::structured cho dữ liệu allow/deny thay vì chỉ trả ContentBlock::text, đồng thời giữ wire format khớp schema. Cập nhật các phần outputSchema/structuredContent tương ứng tại docs/programs/PROGRAM-J-SKELETON.md dòng 245-267 và 281-305 nếu implementation không thể đáp ứng schema, rồi bổ sung test wire-format cho hợp đồng thực tế.src/skill_quality.rs (3)
109-120: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Ledger có thể bị mất sạch dữ liệu khi file hỏng, và
save_storepanic giữa luồngeval judge.Hai điểm ghép lại thành một rủi ro thật:
load_storenuốt mọi lỗi đọc/parse thànhSkillQualityStore::default(). Nếuskill_quality.jsonbị cắt dở (chínhfs::writebên dưới truncate rồi mới ghi, nên crash/ngắt nguồn giữa chừng là kịch bản có thật), lầnrecord_outcomekế tiếp sẽ ghi đè store rỗng — mất toàn bộ lịch sử outcome và trạng tháiTrusted.save_storedùng.expect("write failed"); nó được gọi từtask::cmd_eval_judgesau khi task store đã lưu, nên lỗi ghi ledger sẽ panic và làm hỏng cả lệnh judge dù verdict đã hợp lệ.Ghi atomic (tmp + rename) và không im lặng ghi đè khi parse thất bại sẽ xử lý cả hai.
🛡️ Đề xuất: ghi atomic + không ghi đè khi store hỏng
-fn load_store() -> SkillQualityStore { - let path = store_path(); - if !path.exists() { return SkillQualityStore::default(); } - serde_json::from_str(&fs::read_to_string(&path).unwrap_or_default()).unwrap_or_default() -} +fn load_store() -> SkillQualityStore { + let path = store_path(); + if !path.exists() { return SkillQualityStore::default(); } + let raw = fs::read_to_string(&path).unwrap_or_default(); + match serde_json::from_str(&raw) { + Ok(store) => store, + Err(e) => { + eprintln!("error: {} không đọc được ({e}) — dừng lại thay vì ghi đè lịch sử", path.display()); + std::process::exit(1); + } + } +} -fn save_store(store: &SkillQualityStore) { - let path = store_path(); - if let Some(p) = path.parent() { fs::create_dir_all(p).ok(); } - fs::write(&path, serde_json::to_string_pretty(store).expect("serialize failed")) - .expect("write failed"); -} +fn save_store(store: &SkillQualityStore) { + let path = store_path(); + if let Some(p) = path.parent() { fs::create_dir_all(p).ok(); } + let tmp = path.with_extension("json.tmp"); + let body = serde_json::to_string_pretty(store).expect("serialize failed"); + if let Err(e) = fs::write(&tmp, body).and_then(|_| fs::rename(&tmp, &path)) { + eprintln!("warn: không ghi được skill-quality ledger: {e}"); + } +}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.fn load_store() -> SkillQualityStore { let path = store_path(); if !path.exists() { return SkillQualityStore::default(); } let raw = fs::read_to_string(&path).unwrap_or_default(); match serde_json::from_str(&raw) { Ok(store) => store, Err(e) => { eprintln!("error: {} không đọc được ({e}) — dừng lại thay vì ghi đè lịch sử", path.display()); std::process::exit(1); } } } fn save_store(store: &SkillQualityStore) { let path = store_path(); if let Some(p) = path.parent() { fs::create_dir_all(p).ok(); } let tmp = path.with_extension("json.tmp"); let body = serde_json::to_string_pretty(store).expect("serialize failed"); if let Err(e) = fs::write(&tmp, body).and_then(|_| fs::rename(&tmp, &path)) { eprintln!("warn: không ghi được skill-quality ledger: {e}"); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/skill_quality.rs` around lines 109 - 120, Update load_store and save_store to preserve ledger data on corruption and avoid panics during eval judge. Make save_store return a Result and write through a temporary file followed by an atomic rename, propagating filesystem or serialization errors instead of using expect; update its caller to handle the returned error. In load_store, distinguish missing files from read/parse failures and do not replace a corrupted existing store with SkillQualityStore::default().
271-327: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
cmd_showdài 57 dòng và file đạt 486 dòng — vượt giới hạn trong coding guidelines.Tách nhánh chi tiết một skill và nhánh bảng tổng hợp thành hai hàm nhỏ (
show_one,show_table) là đủ để đưacmd_showvề dưới 50 dòng. File cũng vượt giới hạn 300 dòng; có thể tách phầnmod tests(dòng 351-486) sangtests/hoặc tách module con cho phần CLI.As per coding guidelines: "Function length must not exceed 50 lines" và "File length must not exceed 300 lines".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/skill_quality.rs` around lines 271 - 327, Refactor cmd_show into two focused helpers: show_one for the named-skill detail path and show_table for the aggregate listing path, leaving cmd_show as a short dispatcher while preserving all output and JSON behavior. Also reduce skill_quality.rs below 300 lines by moving the mod tests section into an appropriate tests/ file or child module, without changing test coverage.Source: Coding guidelines
291-291: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Kiểm tra wrapper yana-ai có route subcommand skill-quality tới yana-rt không fd -t f 'yana' bin 2>/dev/null rg -n 'skill-quality|skill_quality' --hidden -g '!target/**' | head -50 rg -n 'yana-rt|exec .*yana' bin 2>/dev/null | head -30Repository: yanacuti1121/Yana-AI
Length of output: 167
🏁 Script executed:
#!/bin/bash set -euo pipefail echo "== bin/yana ==" wc -l bin/yana cat -n bin/yana echo echo "== relevant source files ==" fd -t f 'skill_quality|task\.rs|main\.rs' src .github bin 2>/dev/null || true echo echo "== all yana-ai/yana-rt references in tracked files ==" rg -n "yana-ai|yana-rt|skill-quality|skill_quality" --hidden -g '!target/**' --glob '!**/target/**' 2>/dev/null | head -200Repository: yanacuti1121/Yana-AI
Length of output: 31037
Không cần đổi các gợi ý lệnh sang
yana-rt.Wrapper
bin/yanakhông routeyana-ai skill-quality ...; command này không hiện trong dispatch table và sẽ trả “Unknown command”. Hãy đổi hướng dẫn thành một subcommandyana-rtcó thật, hoặc thêm route phù hợp trước.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/skill_quality.rs` at line 291, Update the promotion hint in the skill-quality output to use an existing yana-rt subcommand, or add the necessary dispatch route before retaining the yana-ai command; ensure the printed command is executable through the bin/yana wrapper.
This was referenced Jul 30, 2026
This was referenced Aug 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Tính năng mới
yana-ai chatvà khả năng tìm kiếm vault.Cải tiến
yana-rt, giảm nguy cơ CPU tăng cao.Tài liệu