Merge pull request #3 from Molecule-AI/chore/structural-cleanup
chore: structural cleanup — dead dirs, moves, gitignore
This commit is contained in:
commit
5e70a8607a
@ -1,172 +0,0 @@
|
||||
---
|
||||
name: code-review
|
||||
description: "Review code for best practices, modularity, scalability, abstraction, test coverage, redundancy, hardcoded values, type safety, performance, naming, API design, async patterns, config/env sync, template consistency, and documentation alignment. Generates detailed report with issues and recommendations."
|
||||
---
|
||||
|
||||
# Code Review
|
||||
|
||||
Perform a comprehensive code review of recent changes or specified files to ensure quality standards.
|
||||
|
||||
## Review Criteria
|
||||
|
||||
### 1. Best Practices
|
||||
- Follows TypeScript strict mode conventions
|
||||
- Proper error handling (try/catch, error types, no silent failures)
|
||||
- No hardcoded values (use environment variables or constants)
|
||||
- Proper logging with appropriate log levels
|
||||
- Security best practices (input validation, no SQL injection, XSS prevention)
|
||||
- No console.log in production code (use logger)
|
||||
|
||||
### 2. Modularity
|
||||
- Single responsibility principle (each function/class does one thing)
|
||||
- Functions are small and focused (< 50 lines ideally)
|
||||
- No code duplication (DRY principle)
|
||||
- Clear separation of concerns (routes, services, utilities)
|
||||
|
||||
### 3. Scalability
|
||||
- Efficient database queries (proper indexing, no N+1 queries)
|
||||
- Connection pooling used correctly
|
||||
- Async operations handled properly
|
||||
- No blocking operations in hot paths
|
||||
|
||||
### 4. Abstraction
|
||||
- Interfaces/types defined for all public APIs
|
||||
- Implementation details hidden behind abstractions
|
||||
- Adapter pattern used for external services (LLM, database)
|
||||
- Configuration externalized (not hardcoded)
|
||||
|
||||
### 5. Test Coverage
|
||||
- Unit tests exist for all utility functions and service functions
|
||||
- Service layer has integration tests
|
||||
- Edge cases are covered
|
||||
- Test files go in `tests/unit/` or `tests/integration/`, named `*.test.ts`
|
||||
- All exported functions have at least one test
|
||||
|
||||
### 6. No Redundancy
|
||||
- No duplicate code blocks (extract to shared functions/utilities)
|
||||
- No repeated logic across files (consolidate into services)
|
||||
- No redundant imports or unused variables
|
||||
- No copy-pasted code with minor variations (use parameters/generics)
|
||||
- No redundant API calls (cache or batch where appropriate)
|
||||
- No repeated validation logic (create reusable validators)
|
||||
- No duplicate helper logic in test files (extract shared test utilities)
|
||||
|
||||
### 7. No Hardcoded Values
|
||||
- No hardcoded URLs, API endpoints, or hostnames (use env vars)
|
||||
- No hardcoded credentials, keys, or secrets (use env vars)
|
||||
- No magic numbers without named constants
|
||||
- No hardcoded file paths (use configuration or path utilities)
|
||||
- No hardcoded timeouts/limits (externalize to config)
|
||||
- No hardcoded error messages (use constants or i18n)
|
||||
- No hardcoded feature flags (use configuration system)
|
||||
- No hardcoded tenant/user IDs in business logic
|
||||
|
||||
### 8. Type Safety
|
||||
- No usage of `any` type (use `unknown` or proper types)
|
||||
- Proper null/undefined handling (optional chaining, nullish coalescing)
|
||||
- Generic types used appropriately
|
||||
- Return types explicitly declared for public functions
|
||||
- No type assertions (`as`) without validation
|
||||
|
||||
### 9. Performance
|
||||
- No memory leaks (cleanup subscriptions, timers, event listeners)
|
||||
- Proper memoization for expensive computations
|
||||
- Lazy loading for heavy components/modules
|
||||
- Efficient data structures for the use case
|
||||
- No synchronous operations blocking the event loop
|
||||
- Batch API calls where possible (e.g., single `messages.modify` with multiple label IDs)
|
||||
|
||||
### 10. Naming & Readability
|
||||
- Descriptive variable/function names (no `x`, `temp`, `data`)
|
||||
- Consistent naming conventions (camelCase, PascalCase)
|
||||
- No misleading names (function does what name suggests)
|
||||
- Boolean variables prefixed appropriately (`is`, `has`, `should`)
|
||||
- No excessive abbreviations
|
||||
- Code is self-documenting where possible
|
||||
|
||||
### 11. API Design
|
||||
- Consistent response formats across endpoints
|
||||
- Proper HTTP status codes used
|
||||
- Input validation at API boundaries
|
||||
- Proper error response structure
|
||||
- RESTful conventions followed
|
||||
- API versioning considered for breaking changes
|
||||
|
||||
### 12. Async & Concurrency
|
||||
- No unhandled promise rejections
|
||||
- Proper race condition handling
|
||||
- Concurrent operations use Promise.all where appropriate
|
||||
- No floating promises (missing await)
|
||||
- Proper cleanup on component unmount/request abort
|
||||
- AbortController used for cancellable operations
|
||||
|
||||
### 13. Dependency Management
|
||||
- No unused dependencies in package.json
|
||||
- No deprecated packages
|
||||
- Security vulnerabilities addressed (npm audit)
|
||||
- Peer dependency conflicts resolved
|
||||
- Dependencies pinned to specific versions where needed
|
||||
|
||||
### 14. Environment & Configuration Sync
|
||||
- Every env var used in `src/config/env.ts` is documented in `.env.example`
|
||||
- Every env var in `.env.example` is defined in the Zod schema (`src/config/env.ts`)
|
||||
- Default values match between `.env.example` comments and Zod `.default()` calls
|
||||
- Conditional requirements are documented (e.g., "only required when LLM_PROVIDER=openai")
|
||||
- No env vars referenced directly via `process.env` outside of `src/config/env.ts` and `src/lib/logger.ts`
|
||||
- `docker-compose.yml` service ports/URLs align with `.env.example` defaults
|
||||
- `Dockerfile` exposes the correct `PORT` matching `.env.example`
|
||||
- `docs/railway-deployment.md` env var list matches the Zod schema
|
||||
|
||||
### 15. Template & Documentation Consistency
|
||||
- Email templates in `docs/templates/` have all `{{variable}}` placeholders documented in their "Available Variables" table
|
||||
- Template variable sources match actual database columns and service outputs
|
||||
- Classification categories in `docs/classification-design.md` match the `EmailCategory` type in `src/types/email.ts`
|
||||
- Confidence thresholds in docs match the actual thresholds implemented in code
|
||||
- Sub-types in docs match the template trigger conditions
|
||||
- Gmail label names in code (`GmailLabel` const) match labels documented in architecture docs
|
||||
- API endpoint schemas in `docs/api-spec.md` match actual route handler request/response types
|
||||
- Error handling strategies in `docs/error-handling.md` match actual retry/error class behavior (e.g., `isRetryable` flags)
|
||||
|
||||
### 16. Error Messages & UX
|
||||
- User-friendly error messages (no technical jargon)
|
||||
- Loading states for async operations
|
||||
- Empty states handled gracefully
|
||||
- Graceful degradation when features fail
|
||||
- Confirmation for destructive actions
|
||||
- Success feedback for completed actions
|
||||
- Error boundaries to prevent full app crashes
|
||||
- Proper form validation with clear feedback
|
||||
|
||||
## Output Format
|
||||
|
||||
```markdown
|
||||
## Code Review Report
|
||||
|
||||
### Files Reviewed
|
||||
- List of files
|
||||
|
||||
### Issues Found
|
||||
|
||||
#### 🔴 Critical
|
||||
- [file:line] Description - Recommendation
|
||||
|
||||
#### 🟡 Warning
|
||||
- [file:line] Description - Recommendation
|
||||
|
||||
#### 🔵 Suggestions
|
||||
- [file:line] Description - Recommendation
|
||||
|
||||
### Config & Template Sync
|
||||
- .env.example ↔ env.ts schema: [in sync / N mismatches]
|
||||
- docs/classification-design.md ↔ src/types/email.ts: [in sync / N mismatches]
|
||||
- docs/templates/ ↔ template variables: [in sync / N mismatches]
|
||||
- docs/error-handling.md ↔ src/lib/errors.ts: [in sync / N mismatches]
|
||||
|
||||
### Test Coverage
|
||||
- Files missing tests
|
||||
- Coverage gaps
|
||||
|
||||
### Summary
|
||||
- Total issues count
|
||||
- Action items
|
||||
```
|
||||
@ -1,60 +0,0 @@
|
||||
---
|
||||
name: update-docs
|
||||
description: "Review recent edits and update all documentation including architecture docs, API specs, and edit history. Creates missing docs for new implementations."
|
||||
---
|
||||
|
||||
# Update Documentation
|
||||
|
||||
Review recent code changes and update ALL relevant documentation in the `/docs` folder.
|
||||
|
||||
## Steps
|
||||
|
||||
1. **Read today's edit history**
|
||||
|
||||
- Check `docs/edit-history/` for the current date's session file
|
||||
- Identify all files that were modified
|
||||
|
||||
2. **Analyze changes**
|
||||
|
||||
- Read the modified files to understand what changed
|
||||
- Categorize changes: new features, bug fixes, architecture changes, API changes, config changes
|
||||
|
||||
3. **Update edit-history session file**
|
||||
|
||||
- Add a summary section at the top describing what was accomplished
|
||||
- Group related changes under descriptive headings
|
||||
- Add any missing context about why changes were made
|
||||
|
||||
4. **Update AGENTS.md if needed**
|
||||
|
||||
- New commands or scripts added
|
||||
- Architecture or key modules changed
|
||||
- New environment variables required
|
||||
- New routes or endpoints added
|
||||
|
||||
5. **Update docs/README.md if needed**
|
||||
|
||||
- New features or capabilities
|
||||
- Changed setup instructions
|
||||
- Updated project overview
|
||||
|
||||
6. **Update docs/ files**
|
||||
Review and update all architecture documentation to match current implementation
|
||||
|
||||
**For each doc:**
|
||||
|
||||
- Check if documented features match actual code implementation
|
||||
- Update outdated sections to reflect current code
|
||||
- Add NEW sections for features that are implemented but not documented
|
||||
- Remove or mark deprecated features that no longer exist
|
||||
- Ensure code examples match actual implementation
|
||||
|
||||
7. **Create new docs if needed**
|
||||
|
||||
- If a significant new feature or module was added but has no documentation, create appropriate documentation
|
||||
- Follow existing documentation style and structure
|
||||
|
||||
8. **Report summary**
|
||||
- List all documentation files updated
|
||||
- Note any new documentation files created
|
||||
- Summarize key changes documented
|
||||
6
.gitignore
vendored
6
.gitignore
vendored
@ -83,6 +83,12 @@ redis_data/
|
||||
# Workspace instance configs (auto-generated by provisioner, not templates)
|
||||
workspace-configs-templates/ws-*
|
||||
|
||||
# Local dev cruft — provisioner writes here at runtime; templates live at repo root
|
||||
platform/workspace-configs-templates/
|
||||
|
||||
# Codex/Gemini agent skill cache (local only, not authoritative)
|
||||
.agents/
|
||||
|
||||
# Workspace runtime markers (written by agent containers, not committed)
|
||||
.initial_prompt_done
|
||||
|
||||
|
||||
4
PLAN.md
4
PLAN.md
@ -139,7 +139,7 @@ for the full code audit.
|
||||
|
||||
- [x] **30.8 Remote-agent SDK + docs** — `sdk/python/molecule_agent/`
|
||||
thin client: register → pull secrets → run A2A loop → poll state →
|
||||
heartbeat. Working `examples/remote-agent/` a new user can run on a
|
||||
heartbeat. Working `sdk/python/examples/remote-agent/` a new user can run on a
|
||||
laptop. Remove the three feature flags. Remote workspaces become GA.
|
||||
|
||||
### Out of scope for Phase 30
|
||||
@ -155,7 +155,7 @@ for the full code audit.
|
||||
|
||||
### Success criteria
|
||||
|
||||
- `examples/remote-agent/` boots on a laptop disconnected from the
|
||||
- `sdk/python/examples/remote-agent/` boots on a laptop disconnected from the
|
||||
platform's LAN, registers, receives a task from parent PM via A2A,
|
||||
returns a result, appears on the canvas.
|
||||
- `tests/e2e/test_federation.sh` spawns a second platform instance +
|
||||
|
||||
25
docs/README.md
Normal file
25
docs/README.md
Normal file
@ -0,0 +1,25 @@
|
||||
# docs/
|
||||
|
||||
This directory serves two purposes:
|
||||
|
||||
1. **Markdown content** — everything under `architecture/`, `agent-runtime/`, `api-protocol/`, `development/`, `frontend/`, `plugins/`, `product/`, etc. This is what agents and humans read.
|
||||
2. **VitePress site** — `.vitepress/config.ts`, `package.json`, `package-lock.json`. These drive the rendered documentation site.
|
||||
|
||||
## Local preview
|
||||
|
||||
```bash
|
||||
cd docs
|
||||
npm install
|
||||
npm run dev # preview on http://localhost:5173
|
||||
npm run build # static build to docs/.vitepress/dist/
|
||||
```
|
||||
|
||||
## Conventions
|
||||
|
||||
- New top-level docs must be linked from `PLAN.md`, `README.md`, and `CLAUDE.md` — otherwise agents can't find them (see `.claude/` memory `feedback_cross_reference_docs.md`).
|
||||
- `edit-history/YYYY-MM-DD.md` is append-only log of significant changes; don't rewrite history.
|
||||
- `archive/` holds one-shot analyses and retired docs — kept for context but not maintained.
|
||||
|
||||
## Why site tooling lives here (not in `docs-site/`)
|
||||
|
||||
VitePress expects its config at `<root>/.vitepress/config.ts` where `<root>` is also the content directory. Splitting tooling into a sibling `docs-site/` would require a non-trivial `srcDir` shim and break relative links in `.vitepress/config.ts`. Keeping both together is the pragmatic choice; this README is the tradeoff ledger.
|
||||
@ -1083,7 +1083,7 @@ describe("Phase 30 remote-agent tools", () => {
|
||||
expect(body.workspace_name).toBe("remote-1");
|
||||
expect(body.setup_command).toContain("WORKSPACE_ID=ws-ext");
|
||||
expect(body.setup_command).toContain("PLATFORM_URL=");
|
||||
expect(body.setup_command).toContain("molecule-agent");
|
||||
expect(body.setup_command).toContain("molecule_agent");
|
||||
});
|
||||
|
||||
test("handleCheckRemoteAgentFreshness fresh when heartbeat is recent", async () => {
|
||||
|
||||
@ -94,8 +94,13 @@ export async function handleGetRemoteAgentSetupCommand(params: {
|
||||
``,
|
||||
`WORKSPACE_ID=${w.id} \\`,
|
||||
`PLATFORM_URL=${targetUrl} \\`,
|
||||
`python3 -m examples.remote-agent.run`,
|
||||
`python3 -c "from molecule_agent import RemoteAgentClient; \\`,
|
||||
` c = RemoteAgentClient.register_from_env(); \\`,
|
||||
` c.pull_secrets(); \\`,
|
||||
` c.run_heartbeat_loop()"`,
|
||||
``,
|
||||
`# For a richer demo (logging, graceful shutdown) see`,
|
||||
`# sdk/python/examples/remote-agent/run.py in the molecule-monorepo checkout.`,
|
||||
`# The agent will register, mint its bearer token (cached at`,
|
||||
`# ~/.molecule/${w.id}/.auth_token), pull secrets, then heartbeat.`,
|
||||
].join("\n");
|
||||
|
||||
@ -270,7 +270,7 @@ func Setup(hub *ws.Hub, broadcaster *events.Broadcaster, prov *provisioner.Provi
|
||||
}
|
||||
|
||||
func findPluginsDir(configsDir string) string {
|
||||
// configsDir-relative is most reliable (avoids empty platform/plugins/)
|
||||
// configsDir-relative is most reliable; plugins live at repo-root plugins/
|
||||
candidates := []string{
|
||||
filepath.Join(configsDir, "..", "plugins"),
|
||||
"../plugins",
|
||||
|
||||
@ -27,7 +27,7 @@ curl -s -X POST http://localhost:8080/workspaces/<UUID>/secrets \
|
||||
|
||||
# 3. Run the demo from any machine that can reach the platform:
|
||||
WORKSPACE_ID=<UUID> PLATFORM_URL=http://localhost:8080 \
|
||||
python3 examples/remote-agent/run.py
|
||||
python3 sdk/python/examples/remote-agent/run.py
|
||||
```
|
||||
|
||||
You should see log lines for each of the three phases, and then
|
||||
@ -44,7 +44,7 @@ print(f"loop exited: {terminal}")
|
||||
```
|
||||
|
||||
A runnable demo with full setup walkthrough lives at
|
||||
[`examples/remote-agent/`](../../../examples/remote-agent).
|
||||
[`sdk/python/examples/remote-agent/`](../../examples/remote-agent).
|
||||
|
||||
## What the SDK gives you
|
||||
|
||||
@ -93,5 +93,5 @@ the security benefits of bearer auth until both sides upgrade.
|
||||
|
||||
- [`molecule_plugin`](../molecule_plugin) — the *other* SDK in this
|
||||
package, for plugin authors. Different audience.
|
||||
- [`examples/remote-agent/run.py`](../../../examples/remote-agent/run.py)
|
||||
- [`sdk/python/examples/remote-agent/run.py`](../../examples/remote-agent/run.py)
|
||||
— the runnable demo that proves all of the above end-to-end.
|
||||
|
||||
@ -20,7 +20,7 @@ Intended usage::
|
||||
env = client.pull_secrets() # decrypted secrets dict
|
||||
client.run_heartbeat_loop() # background heartbeat + state-poll
|
||||
|
||||
See ``examples/remote-agent/`` for a runnable demo.
|
||||
See ``sdk/python/examples/remote-agent/`` for a runnable demo.
|
||||
|
||||
Design notes:
|
||||
* **No async.** The SDK uses blocking ``requests`` so a remote agent author
|
||||
|
||||
24
tests/README.md
Normal file
24
tests/README.md
Normal file
@ -0,0 +1,24 @@
|
||||
# Tests
|
||||
|
||||
This repo uses the standard monorepo testing convention: **unit tests live with their package, cross-component E2E tests live here.**
|
||||
|
||||
## Where to find tests
|
||||
|
||||
| Scope | Location |
|
||||
|---|---|
|
||||
| Go unit + integration (platform, CLI, handlers) | `platform/**/*_test.go` — run with `cd platform && go test -race ./...` |
|
||||
| TypeScript unit (canvas components, hooks, store) | `canvas/src/**/__tests__/` — run with `cd canvas && npm test -- --run` |
|
||||
| TypeScript unit (MCP server handlers) | `mcp-server/src/__tests__/` — run with `cd mcp-server && npx jest` |
|
||||
| Python unit (workspace runtime, adapters) | `workspace-template/tests/` — run with `cd workspace-template && python3 -m pytest` |
|
||||
| Python unit (SDK: plugin + remote agent) | `sdk/python/tests/` — run with `cd sdk/python && python3 -m pytest` |
|
||||
| **Cross-component E2E** (spans platform + runtime + HTTP) | `tests/e2e/` ← **you are here** |
|
||||
|
||||
## Why split this way
|
||||
|
||||
- **Go** requires co-located `_test.go` files to access unexported symbols.
|
||||
- **Per-package test commands** keep the inner loop fast — changing canvas doesn't re-run Go tests.
|
||||
- **`tests/e2e/`** covers scenarios that no single package owns: a full workspace lifecycle, A2A across two provisioned agents, delegation chains, bundle round-trips.
|
||||
|
||||
## Running E2E
|
||||
|
||||
Every E2E script here assumes the platform is running at `localhost:8080` and (where noted) provisioned agents are online. See the header comment of each `.sh` for specifics.
|
||||
Loading…
Reference in New Issue
Block a user