Synchronize documentation and enhance sandboxed tests#7
Synchronize documentation and enhance sandboxed tests#7rakasatria wants to merge 9 commits intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates project documentation to reflect the current architecture and workflows (including removing deprecated website/ frontend references), and improves test robustness in constrained/sandboxed environments by gracefully skipping tests that can’t bind loopback TCP sockets.
Changes:
- Rewrites/expands top-level and operator docs (new
ARCHITECTURE.md, updatedREADME.md, refreshed runbooks/release docs). - Removes or gates
website/-related build/docs steps in contributor docs and scripts. - Adds test helpers to skip loopback-bind-dependent tests when the environment forbids binding
127.0.0.1.
Reviewed changes
Copilot reviewed 22 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/web.rs |
Uses shared test listener binder and skips WS/network tests when loopback binds are forbidden. |
src/tools/a2a.rs |
Updates tests to use sandbox-friendly tokio listener binding. |
src/setup.rs |
Gates “load existing config” in tests behind an env var; assigns temp dirs in validation tests for sandbox safety. |
src/llm.rs |
Updates network-related tests to use sandbox-friendly TCP listener binding. |
src/lib.rs |
Adds centralized test helpers for TCP listener binding (std + tokio) with “skip on PermissionDenied” behavior. |
src/chat_commands.rs |
Updates tests to use sandbox-friendly TCP listener binding. |
src/channels/weixin.rs |
Updates test server spawn helper to skip when loopback binds are forbidden. |
scripts/generate_docs_artifacts.mjs |
Extends tool discovery to include ClawHub tools and avoids emitting website outputs when website/docs doesn’t exist. |
docs/releases/upgrade-guide.md |
Adds upgrade checklist guidance and removes stale “recent PR references” section. |
docs/releases/release-policy.md |
Removes deprecated website/ build step from release candidate checks. |
docs/releases/pr-release-checklist.md |
Updates doc update expectations and removes deprecated website/ validation step. |
docs/operations/runbook.md |
Updates auth troubleshooting guidance and adds bootstrap/login troubleshooting notes. |
docs/operations/acp-stdio.md |
Updates related-doc links away from deprecated website/ docs. |
docs/observability/architecture.md |
Rewrites observability architecture documentation in English with updated structure and config notes. |
docs/generated/tools.md |
Updates generated built-in tool list/count to include ClawHub tools. |
docs/clawhub/overview.md |
Clarifies ClawHub integration surfaces and configuration notes. |
crates/mchact-tools/src/web_fetch.rs |
Makes redirect/denylist tests skip gracefully when loopback binds are not permitted. |
check.sh |
Removes deprecated website/ build step from quick local validation. |
TEST.md |
Removes deprecated website/ build step from documented CI-equivalent checks. |
README.md |
Major rewrite: tech stack table, install/run, env vars, scripts, and folder structure. |
CONTRIBUTING.md |
Updates doc-update guidance and removes deprecated website/ setup/build steps. |
ARCHITECTURE.md |
Adds a new architecture overview document derived from the current codebase. |
Comments suppressed due to low confidence (1)
scripts/generate_docs_artifacts.mjs:47
parseBuiltinTools()reuses a single global RegExp (/.../g) across multiple files. Because global regexes keeplastIndex, matches at the start of subsequent files can be skipped. Resetre.lastIndex = 0for each file (or construct a new regex inside the loop) to ensure all tool names are discovered consistently.
const re = /fn\s+name\s*\(\s*&self\s*\)\s*->\s*&str\s*\{\s*"([^"]+)"\s*\}/g;
for (const file of sourceFiles) {
const text = fs.readFileSync(file, 'utf8');
let m;
while ((m = re.exec(text)) !== null) {
names.add(m[1]);
}
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| ## Environment Variables | ||
|
|
||
| mchact is config-first. There is no `.env.example`; most operator settings live in [`mchact.config.example.yaml`](/Volumes/Data/Codes/Local/mchact/mchact.config.example.yaml). These environment variables are read directly by the codebase: |
There was a problem hiding this comment.
The mchact.config.example.yaml link uses an absolute local filesystem path (/Volumes/...), which will be broken for anyone else viewing the README. Use a repo-relative link (e.g., ./mchact.config.example.yaml) instead.
| mchact is config-first. There is no `.env.example`; most operator settings live in [`mchact.config.example.yaml`](/Volumes/Data/Codes/Local/mchact/mchact.config.example.yaml). These environment variables are read directly by the codebase: | |
| mchact is config-first. There is no `.env.example`; most operator settings live in [`mchact.config.example.yaml`](./mchact.config.example.yaml). These environment variables are read directly by the codebase: |
|
|
||
| ## System Overview | ||
|
|
||
| mchact solves the problem of running one agent runtime across many chat and operator surfaces without forking the core logic per channel. A message can enter from the local web UI, Telegram, Discord, Slack, Feishu/Lark, Matrix, Weixin, WhatsApp, Signal, IRC, Nostr, email, or other adapters, but it is processed by the same agent loop in [`src/agent_engine.rs`](/Volumes/Data/Codes/Local/mchact/src/agent_engine.rs). |
There was a problem hiding this comment.
This document links to source files using absolute paths like /Volumes/Data/..., which are specific to one machine and will be broken on GitHub and in other environments. Update these references to repo-relative paths (e.g., src/agent_engine.rs).
| mchact solves the problem of running one agent runtime across many chat and operator surfaces without forking the core logic per channel. A message can enter from the local web UI, Telegram, Discord, Slack, Feishu/Lark, Matrix, Weixin, WhatsApp, Signal, IRC, Nostr, email, or other adapters, but it is processed by the same agent loop in [`src/agent_engine.rs`](/Volumes/Data/Codes/Local/mchact/src/agent_engine.rs). | |
| mchact solves the problem of running one agent runtime across many chat and operator surfaces without forking the core logic per channel. A message can enter from the local web UI, Telegram, Discord, Slack, Feishu/Lark, Matrix, Weixin, WhatsApp, Signal, IRC, Nostr, email, or other adapters, but it is processed by the same agent loop in [`src/agent_engine.rs`](src/agent_engine.rs). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 44 out of 53 changed files in this pull request and generated 5 comments.
Files not reviewed (4)
- .idea/.gitignore: Language not supported
- .idea/mchact.iml: Language not supported
- .idea/modules.xml: Language not supported
- .idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| See [`mchact.config.example.yaml`](/Volumes/Data/Codes/Local/mchact/mchact.config.example.yaml) for the full schema and defaults. | ||
|
|
||
| ## System Overview | ||
|
|
||
| mchact solves the problem of running one agent runtime across many chat and operator surfaces without forking the core logic per channel. A message can enter from the local web UI, Telegram, Discord, Slack, Feishu/Lark, Matrix, Weixin, WhatsApp, Signal, IRC, Nostr, email, or other adapters, but it is processed by the same agent loop in [`src/agent_engine.rs`](/Volumes/Data/Codes/Local/mchact/src/agent_engine.rs). |
There was a problem hiding this comment.
This file contains many Markdown links to a local absolute /Volumes/... path. These links will be broken for anyone else and in rendered docs. Replace them with repository-relative links throughout (e.g. ./src/runtime.rs, ./crates/..., ./mchact.config.example.yaml).
| See [`mchact.config.example.yaml`](/Volumes/Data/Codes/Local/mchact/mchact.config.example.yaml) for the full schema and defaults. | |
| ## System Overview | |
| mchact solves the problem of running one agent runtime across many chat and operator surfaces without forking the core logic per channel. A message can enter from the local web UI, Telegram, Discord, Slack, Feishu/Lark, Matrix, Weixin, WhatsApp, Signal, IRC, Nostr, email, or other adapters, but it is processed by the same agent loop in [`src/agent_engine.rs`](/Volumes/Data/Codes/Local/mchact/src/agent_engine.rs). | |
| See [`mchact.config.example.yaml`](./mchact.config.example.yaml) for the full schema and defaults. | |
| ## System Overview | |
| mchact solves the problem of running one agent runtime across many chat and operator surfaces without forking the core logic per channel. A message can enter from the local web UI, Telegram, Discord, Slack, Feishu/Lark, Matrix, Weixin, WhatsApp, Signal, IRC, Nostr, email, or other adapters, but it is processed by the same agent loop in [`src/agent_engine.rs`](./src/agent_engine.rs). |
| - uses `agentops_api_key` | ||
| - can use an explicit `agentops_otlp_endpoint` | ||
|
|
||
| > ⚠️ Needs clarification: exact vendor routing precedence for every mixed configuration combination should be verified from `mchact-observability` source when extending this layer further. The current repo clearly supports Langfuse and AgentOps, but does not document a broader stable adapter contract outside the crate itself. |
There was a problem hiding this comment.
The document includes a TODO-style note (“Needs clarification…”) about vendor routing precedence. Docs should be actionable/definitive; either document the actual precedence (ideally with a short reference to the relevant code path) or remove the note.
| > ⚠️ Needs clarification: exact vendor routing precedence for every mixed configuration combination should be verified from `mchact-observability` source when extending this layer further. The current repo clearly supports Langfuse and AgentOps, but does not document a broader stable adapter contract outside the crate itself. | |
| Vendor-specific routing logic, including how configuration fields are combined and which endpoints are ultimately used, is implemented inside the adapters in `crates/mchact-observability` (for example under `crates/mchact-observability/src/adapters/`). This repository currently only guarantees behavior for the Langfuse and AgentOps adapters described above; no broader cross-vendor precedence contract is defined outside the crate itself. |
| ENV_LOCK | ||
| .get_or_init(|| Mutex::new(())) | ||
| .lock() | ||
| .expect("env lock poisoned") | ||
| .unwrap_or_else(|e| e.into_inner()) | ||
| } |
There was a problem hiding this comment.
env_lock() now ignores mutex poisoning by taking the inner guard. That can mask prior test failures and let later tests run with partially-mutated environment state. Consider keeping the failure explicit (or at least logging) so poisoning doesn’t silently hide earlier panics.
| fn test_setup_loads_existing_web_hook_settings() { | ||
| let _guard = env_lock(); | ||
| std::env::set_var("MCHACT_TEST_LOAD_EXISTING_CONFIG", "1"); | ||
| let temp = std::env::temp_dir().join(format!( |
There was a problem hiding this comment.
These tests manually set/unset MCHACT_TEST_LOAD_EXISTING_CONFIG. If a test panics before remove_var, the env var can leak into later tests and change behavior. Consider using a small RAII guard (Drop-based) to ensure cleanup even on early returns/panics.
| fn assign_writable_temp_dirs(app: &mut SetupApp) { | ||
| let root = std::env::temp_dir().join(format!( | ||
| "mchact_setup_validate_local_{}", | ||
| Utc::now().timestamp_nanos_opt().unwrap_or_default() | ||
| )); | ||
| let working = root.join("working"); | ||
|
|
||
| if let Some(field) = app.fields.iter_mut().find(|f| f.key == "DATA_DIR") { | ||
| field.value = root.to_string_lossy().to_string(); | ||
| } | ||
| if let Some(field) = app.fields.iter_mut().find(|f| f.key == "WORKING_DIR") { | ||
| field.value = working.to_string_lossy().to_string(); | ||
| } | ||
| } |
There was a problem hiding this comment.
assign_writable_temp_dirs creates unique temp paths but never cleans them up, so repeated test runs may accumulate directories under the system temp dir. Consider using tempfile::TempDir (or equivalent) and keeping it alive for the test duration so it is removed automatically.
7 diagrams: system overview, agent loop, channel routing, tool execution, scheduler, memory/knowledge pipeline, auth flow. Also removes last remaining stale warning.
Handle::current().block_on() panics when called from within the Tokio runtime. Wrapping with block_in_place lets Tokio move async work off the current thread before blocking. Fixes: skills, hooks, weixin channel, and batch storage helpers.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 56 changed files in this pull request and generated 2 comments.
Files not reviewed (4)
- .idea/.gitignore: Language not supported
- .idea/mchact.iml: Language not supported
- .idea/modules.xml: Language not supported
- .idea/vcs.xml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let _guard = env_lock(); | ||
| std::env::set_var("MCHACT_TEST_LOAD_EXISTING_CONFIG", "1"); | ||
| let temp = std::env::temp_dir().join(format!( |
There was a problem hiding this comment.
These tests set MCHACT_TEST_LOAD_EXISTING_CONFIG and later manually remove it. If the test panics/early-returns before cleanup, the env var can leak into subsequent tests (even with env_lock) and cause confusing behavior. Consider using a small RAII guard (or scopeguard) so the env var is always removed, and ideally centralize this pattern to avoid duplication across the file.
| fn assign_writable_temp_dirs(app: &mut SetupApp) { | ||
| let root = std::env::temp_dir().join(format!( | ||
| "mchact_setup_validate_local_{}", | ||
| Utc::now().timestamp_nanos_opt().unwrap_or_default() | ||
| )); | ||
| let working = root.join("working"); | ||
|
|
||
| if let Some(field) = app.fields.iter_mut().find(|f| f.key == "DATA_DIR") { | ||
| field.value = root.to_string_lossy().to_string(); | ||
| } | ||
| if let Some(field) = app.fields.iter_mut().find(|f| f.key == "WORKING_DIR") { | ||
| field.value = working.to_string_lossy().to_string(); | ||
| } | ||
| } |
There was a problem hiding this comment.
assign_writable_temp_dirs generates a unique temp path but doesn’t ensure the directories exist and doesn’t provide cleanup, so it can leave behind temp directories after the test suite runs. Consider using tempfile::TempDir (or returning the created root for teardown) and creating the directories up front to guarantee validate_local() always has a writable target.
@assistant-ui/react was 0.12.19 (needs 0.12.21) and @assistant-ui/react-markdown was 0.12.6 (needs 0.12.7). Version mismatch between shared internal contexts caused React error #185 (maximum update depth exceeded).
makeMarkdownText() inside the component body created a new component reference on every render, causing @assistant-ui to remount the message tree in a loop (React error #185).
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 48 out of 60 changed files in this pull request and generated 2 comments.
Files not reviewed (5)
- .idea/.gitignore: Language not supported
- .idea/mchact.iml: Language not supported
- .idea/modules.xml: Language not supported
- .idea/vcs.xml: Language not supported
- web/package-lock.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| #[cfg(test)] | ||
| if std::env::var("MCHACT_TEST_LOAD_EXISTING_CONFIG").ok().as_deref() != Some("1") { | ||
| return HashMap::new(); | ||
| } | ||
|
|
There was a problem hiding this comment.
load_existing_config() is now effectively disabled under #[cfg(test)] unless MCHACT_TEST_LOAD_EXISTING_CONFIG=1 is set. This introduces hidden coupling between unrelated tests and changes test behavior based on global process state. Prefer injecting a test-only flag/config into SetupApp (or gating at the call site) rather than reading a global env var inside the function.
|
|
||
| Repo-local validation shortcut: | ||
| ```sh | ||
| zsh check.sh |
There was a problem hiding this comment.
The suggested command zsh check.sh is shell-specific and will fail on systems without zsh or when check.sh isn’t on PATH. Prefer a portable invocation like ./check.sh.
| zsh check.sh | |
| ./check.sh |
This pull request updates project documentation to improve clarity and accuracy, removes obsolete references to the deprecated
websitefrontend, and makes a minor test robustness improvement. The most significant changes include a complete rewrite of theREADME.mdto provide an up-to-date overview, streamlining of contributor and CI instructions, and improved test resilience in restricted environments.Documentation and Project Overview Updates:
README.mdto provide a comprehensive, modern overview of the project, including a detailed tech stack table, installation and configuration instructions, environment variable documentation, available scripts, and an updated folder structure.CONTRIBUTING.mdto include updates toREADME.mdandARCHITECTURE.mdfor behavior changes, not justdocs/.docs/clawhub/overview.md, including notes on agent tool enablement and configuration flattening. [1] [2]Removal of Deprecated Website Frontend References:
websitefrontend from contributor instructions (CONTRIBUTING.md), CI scripts (check.sh,TEST.md), and build steps, ensuring only the supportedwebfrontend is used. [1] [2] [3] [4]Test Robustness Improvements:
crates/mchact-tools/src/web_fetch.rsto gracefully skip when loopback TCP binds are not permitted, improving CI and container compatibility. [1] [2]