Consolidate enterprise and agent skill upload surfaces#588
Open
chenjiayi8 wants to merge 174 commits into
Open
Consolidate enterprise and agent skill upload surfaces#588chenjiayi8 wants to merge 174 commits into
chenjiayi8 wants to merge 174 commits into
Conversation
- entrypoint.sh: add clawith user to docker socket group for container mgmt - agent_seeder.py: support SKIP_DEFAULT_AGENTS env var - nginx.conf: fix proxy_pass to use consultant_backend hostname
SQLAlchemy 2.0 models for tracking workspace deployments (static + container) and user/health-check bug reports with auto-fix circuit breaker fields. Also adds WORKSPACE_STATIC_DIR, WORKSPACE_CONF_DIR, WORKSPACE_GATEWAY_CONTAINER config settings.
…ecks - workspace_tools.py: slug validation, deploy_static, request_container_deploy, approve/reject pipeline, undeploy, bug report tools, gateway reload - workspace_index.py: auto-generated HTML index page at /workspace/ - workspace_health.py: periodic health checks with parallel execution and circuit breaker (max 3 auto-fixes per 24h window)
- POST /api/workspace/projects/{slug}/report-bug (public, rate-limited, honeypot)
- POST /api/workspace/projects/{slug}/approve (builds Docker image, starts container)
- POST /api/workspace/projects/{slug}/reject
- GET /api/workspace/projects (list all projects)
Add tool definitions (AGENT_TOOLS), dispatch wiring (execute_tool), and DB seeding (BUILTIN_TOOLS) for: request_build, list_build_requests, deploy_static, request_container_deploy, list_workspace_projects, undeploy_project, get_bug_reports, resolve_bug, report_workspace_bug.
Register workspace models for auto-creation, workspace API routers, and health check background task in the FastAPI lifespan.
* feat: add mobile responsive layout shell Add hamburger menu, slide-over sidebar, and full-width main content for viewports <=768px. Desktop layout unchanged. * fix: remove unused isMobile state, close sidebar on account settings click Address code review: simplify closeMobileMenu, add closeMobileMenu to account settings handler so sidebar dismisses on mobile. * feat: make Dashboard responsive for mobile Stats grid goes 2x2, agent table becomes card list, activity column hidden on narrow viewports. * feat: make Chat page responsive for mobile Wider bubbles, smaller avatars, compact input area, dvh-based height for mobile viewport. * feat: make Plaza responsive for mobile Two-column layout stacks vertically, remove avatar indent on narrow viewports, sidebar moves below feed. * fix: address final code review issues - Move display:grid from agent-row-header inline to CSS class so media query display:none works on mobile - Move plaza-sidebar width/position from inline to CSS class, remove !important from media query overrides
- Make agent detail tabs horizontally scrollable on mobile - Stack chat session sidebar above chat area on mobile instead of side-by-side columns
Gateway tools (category=mcp-gateway) are platform-level and should be visible on all agent tool pages like builtin tools. Previously, all MCP tools were hidden unless the agent already had an explicit AgentTool assignment, creating a chicken-and-egg problem where tools couldn't be assigned from the UI.
This reverts commit 105b59f.
CAPTCHA solving via 2Captcha can take up to 180s. The previous timeouts (30s streamable HTTP, 60s SSE) caused "Both transports failed" errors when the gateway was still waiting for 2Captcha to respond.
Supports the skill autocomplete & role system feature. The is_hidden flag allows persisting injected role context messages that are sent to the LLM but not displayed to the user.
- Add _resolve_skill_content() and _inject_skill_if_matched() helpers for parsing /skill:sub prefixes and injecting hidden context messages - Add _persist_and_inject_skill() to save hidden messages to DB - Add type-based dispatch: "edit" messages rebuild conversation from DB and re-invoke LLM; normal messages follow existing path - Add /role:clear handling to remove all hidden skill messages - Track hidden message indices via websocket._hidden_indices for filtering on clear
…back, onPaste forwarding
…ebsocket The config module exports get_settings(), not a settings instance.
Keep an existing-but-invalid SS_CONFIG_FILE as a non-fatal stop for ss-local startup instead of falling through to env-based proxy configuration, preserving the pre-refactor precedence contract while still handling mis-mounted directories safely.\n\nConstraint: PR #21 must stay narrowly targeted to startup safety without changing proxy source precedence\nRejected: falling back to SS_SERVER/SS_PASSWORD when a bad config path exists | changes runtime behavior beyond the production fix described by the PR\nConfidence: high\nScope-risk: narrow\nDirective: treat invalid proxy config files as a graceful skip unless product requirements explicitly change the precedence rules\nTested: python -m pytest tests/test_ss_local_proxy_startup.py -q\nTested: python -m ruff check app/main.py tests/test_ss_local_proxy_startup.py\nTested: python -m compileall app/main.py tests/test_ss_local_proxy_startup.py\nNot-tested: live startup in Docker or production deployment
Constraint: Onboarding greetings in web chat can resolve through the finish tool without emitting streamed chunks or completed non-finish tool callbacks. Rejected: Persisting onboarding only from stream/tool callbacks | finish-only replies bypass those hooks and leave the session stuck in greeting mode. Confidence: high Scope-risk: narrow Directive: Keep onboarding phase advancement aligned with actual successful reply completion, even when the model exits through finish. Tested: PYTHONPATH=. pytest tests/test_websocket_retry.py tests/test_onboarding.py -q; python -m ruff check backend/app/api/websocket.py backend/tests/test_websocket_retry.py Not-tested: Full end-to-end browser reproduction against a live websocket session
Constraint: Invite-login must work whether /tenants/join returns a new token or mutates the existing user record in place Rejected: Backend-only token changes | Higher coupling and unnecessary for the stale frontend-state bug Confidence: high Scope-risk: narrow Directive: Refresh authenticated user state after successful invite joins before routing to protected pages Tested: node --test frontend/tests/finalizeInvitationJoin.test.mjs; cd frontend && npm run build Not-tested: Full browser repro against a live frontend session
Constraint: Support both agent filesystem and registry-backed skills with one validation model Rejected: Duplicating archive parsing in files.py and skills.py | Would drift and break diff consistency Confidence: high Scope-risk: narrow Directive: Keep root SKILL.md enforcement and diff generation in this shared service Tested: pytest backend/tests/test_skill_archive_import.py -q Not-tested: API wiring and frontend flow
…ports Constraint: Preserve backend import behavior while matching the task's shared archive inspection contract Rejected: Leaving prefix stripping based on partial nesting | Misclassifies mixed root archives and drops directory segments Confidence: high Scope-risk: narrow Directive: Only strip a root folder when every archive file lives under the same top-level directory after path normalization Tested: cd backend && python -m pytest tests/test_skill_archive_import.py -q Not-tested: API wiring and frontend flow
Constraint: Archive path validation must treat normalized Windows paths as invalid absolute inputs across upload backends Rejected: Relying on pathlib absolute checks alone | Does not catch drive-letter and UNC-style paths after slash normalization Confidence: high Scope-risk: narrow Directive: Keep slash-normalized path validation explicit for Unix, Windows drive-letter, UNC, and traversal cases Tested: cd backend && python -m pytest tests/test_skill_archive_import.py -q Not-tested: API wiring and frontend flow
Constraint: Keep the active agent skill surface on the legacy AgentDetail route working without changing existing ZIP import behavior Rejected: Reusing extract-zip for strict folder uploads | It cannot enforce SKILL.md or diff confirmation cleanly Confidence: medium Scope-risk: moderate Directive: Preserve preview digest checks so apply cannot drift from the reviewed archive Tested: pytest backend/tests/test_files_api.py -q Not-tested: frontend integration and browser UX
Constraint: Preserve the strict preview/apply contract while matching filesystem reality for pre-created target directories Rejected: Inferring update mode from manifest contents alone | Empty folders are still existing replacement targets Confidence: high Scope-risk: narrow Directive: Keep confirmation and mode decisions keyed to target folder existence, not whether files are already present Tested: cd backend && python -m pytest tests/test_files_api.py -q Not-tested: frontend integration and browser UX
Constraint: Keep the preview/apply workflow forward-compatible so later frontend steps can pass an additional preview state token without changing the strict archive review flow Rejected: Trusting only the uploaded archive digest | It cannot detect files added or removed in the target folder after preview Confidence: high Scope-risk: narrow Directive: Preserve target-state digest checks so apply only runs against the exact folder state the user reviewed Tested: cd backend && python -m pytest tests/test_files_api.py -q Not-tested: frontend integration and browser UX
Constraint: Reuse tenant-scoped skill access rules while preserving registry semantics for presets Rejected: Storing uploaded folders only on disk for enterprise skills | Would bypass the existing registry model and preset import flow Confidence: medium Scope-risk: moderate Directive: Keep registry updates transactional and preserve explicit confirmation for destructive syncs Tested: pytest backend/tests/test_skills_api.py -q Not-tested: frontend integration and real DB transaction edge cases
Constraint: Keep the follow-up fix inside the assigned test scope while validating the spec's replacement semantics Rejected: Leaving the update test at mode-only assertions | Would not prove stale registry rows are removed and rewritten exactly Confidence: high Scope-risk: narrow Directive: Keep Task 3 coverage focused on observable registry sync effects, not just happy-path status fields Tested: cd backend && python -m pytest tests/test_skills_api.py -q Not-tested: frontend integration and real DB transaction edge cases
Constraint: Keep the repo dependency set unchanged while still producing ZIP uploads from local folders Rejected: Adding JSZip or fflate immediately | Repo guidance prefers no new dependencies unless explicitly requested Confidence: medium Scope-risk: moderate Directive: Keep the ZIP utility narrowly scoped to skill-folder uploads rather than general archive creation Tested: cd frontend && npm run build Not-tested: Browser folder selection and drag-drop behavior
Constraint: The active route still renders frontend/src/pages/AgentDetail.tsx, so new UI behavior must land there first Rejected: Implementing only frontend/src/pages/agent-detail/tabs/SkillsTab.tsx | That path is not the active route today Confidence: high Scope-risk: moderate Directive: Keep the shared modal generic so enterprise can reuse it without duplicating diff-confirmation logic Tested: cd frontend && npm run build Not-tested: Browser drag-drop flow and API round-trip behavior
Constraint: Task 6 needs to reuse the same modal without agent-specific copy coupling or unsafe async state reuse Rejected: Relying on the latest resolved preview promise | Late responses can restore stale preview payloads after folder, target, or modal state changes Confidence: high Scope-risk: narrow Directive: Keep invalidation-driven async guards aligned with any future modal reuse so preview and apply always operate on the same snapshot Tested: cd frontend && npm run build Not-tested: Browser folder-picker interaction timing and live API race behavior
Constraint: Keep agent and enterprise upload semantics aligned while preserving separate storage backends Rejected: A second enterprise-only modal implementation | Would duplicate diff preview and drift from the agent flow Confidence: medium Scope-risk: moderate Directive: Reuse the shared modal and keep warning copy aligned between agent and enterprise flows Tested: pytest backend/tests/test_skill_archive_import.py -q; pytest backend/tests/test_files_api.py -q; pytest backend/tests/test_skills_api.py -q; cd frontend && npm run build Not-tested: End-to-end browser interaction against a running app
Constraint: The implementation plan's final verification names pytest commands from the repo root, so backend tests must import `app` without requiring a manual `cd backend` workaround. Rejected: Relying on implicit operator knowledge to `cd backend` first | Leaves the documented verification steps inaccurate on this branch Confidence: high Scope-risk: narrow Directive: Keep backend test imports runnable from the repository root while preserving the existing backend package layout Tested: pytest backend/tests/test_skill_archive_import.py -q; pytest backend/tests/test_files_api.py -q; pytest backend/tests/test_skills_api.py -q Not-tested: Full backend test suite from repo root
Constraint: The PR promises preview/apply drift protection and metadata-preserving exact-sync across agent and enterprise uploads Rejected: Leaving enterprise drift tokens optional and lossy archive decoding in place | Would allow preview bypasses and silent file corruption Confidence: high Scope-risk: narrow Directive: Keep folder-upload archives UTF-8 text-only until the skill storage model can preserve binary assets end-to-end Tested: python -m pytest backend/tests/test_skill_archive_import.py backend/tests/test_files_api.py backend/tests/test_skills_api.py -q; cd frontend && npm run build Not-tested: Browser-driven end-to-end upload interaction against a running app
Constraint: Review surfaced folder-name persistence bugs and UTF-8 ZIP filename corruption that would break valid skill uploads Rejected: Leaving folder names unchecked or trusting legacy ZIP default encoding | Produces unreachable registry rows and mangled non-ASCII paths Confidence: high Scope-risk: narrow Directive: Reuse the strict folder-name contract across agent and enterprise uploads, and preserve UTF-8 filename flags in any future ZIP writer changes Tested: python -m pytest backend/tests/test_skill_archive_import.py backend/tests/test_files_api.py backend/tests/test_skills_api.py backend/tests/test_skill_folder_zip_compat.py -q; cd frontend && npm run build Not-tested: Browser-driven end-to-end upload interaction against a running app
…s or size limits Constraint: Global unique indexes and the registry's 500KB storage budget must still hold for the new folder-upload path Rejected: Relying on scoped preview lookups or late commit failures | Leaves hidden conflicts, name collisions, and oversize uploads to explode after preview Confidence: high Scope-risk: narrow Directive: Keep registry upload previews aligned with the real database uniqueness boundary, and preserve deterministic 409/413 errors before destructive apply work Tested: python -m pytest backend/tests/test_skill_archive_import.py backend/tests/test_files_api.py backend/tests/test_skills_api.py backend/tests/test_skill_folder_zip_compat.py -q; cd frontend && npm run build Not-tested: Browser-driven end-to-end upload interaction against a running app
Constraint: Task 1 required pure helpers and failing-first bundled tests without touching page wiring Rejected: wiring pages now | out of assigned scope for this task Confidence: high Scope-risk: narrow Directive: Keep these helpers pure until the UI surfaces are migrated in follow-up tasks Tested: node --test frontend/tests/skillsActionItems.test.mjs; node --test frontend/tests/skillUploadSurfaceAdapters.test.mjs Not-tested: page-level integration wiring in EnterpriseSettings, SkillsTab, and AgentDetail
Constraint: Spec requires exact exported helper names for follow-up task integration Rejected: keeping surface-adapter suffixes | diverges from approved Task 1 contract Confidence: high Scope-risk: narrow Directive: Keep test and helper export names aligned with the task contract before wiring consumers Tested: node --test frontend/tests/skillsActionItems.test.mjs; node --test frontend/tests/skillUploadSurfaceAdapters.test.mjs Not-tested: page-level integration wiring remains intentionally untouched
Constraint: Task 1 bundled node tests import esbuild directly and must not rely on transitive deps Rejected: relying on Vite transitive esbuild | brittle and outside the test contract Confidence: high Scope-risk: narrow Directive: Keep direct test-runtime dependencies explicit in frontend/package.json Tested: node --test frontend/tests/skillsActionItems.test.mjs; node --test frontend/tests/skillUploadSurfaceAdapters.test.mjs; cd frontend && npm run build Not-tested: unrelated audit vulnerability remediation
Constraint: Task 1 tests must run in standard frontend and CI verification without changing helper logic Rejected: custom one-off CI commands only | would leave local npm test incomplete Confidence: high Scope-risk: narrow Directive: Keep frontend Node-based regression tests reachable via npm test and CI before build Tested: cd frontend && npm test; cd frontend && npm run build Not-tested: GitHub Actions execution itself beyond equivalent local commands
Constraint: Preserve existing enterprise settings, URL import, ClawHub, and folder upload behavior while introducing the shared seam Rejected: Keep the inline SkillsTab action cluster | would continue duplicating action order and upload wiring Confidence: high Scope-risk: narrow Directive: Keep enterprise action ordering sourced from shared skillsActionItems and upload behavior sourced from the shared adapter Tested: cd frontend && npm test; cd frontend && npm run build Not-tested: Manual browser verification of the enterprise settings tab UI
Constraint: Keep the shared action bar presentational while proving the enterprise tab stays wired to the shared action and upload seams Rejected: Leave the action config as Record<string, ...> | too loose to protect the shared seam Confidence: high Scope-risk: narrow Directive: Keep enterprise seam coverage in the existing skillsActionItems test file so shared ordering and source wiring stay locked together Tested: cd frontend && npm test; cd frontend && npm run build Not-tested: Manual browser verification of the enterprise skills tab layout
Constraint: Task 3 ownership limited changes to AgentDetail and targeted tests while preserving agent-only modal flows Rejected: Keeping the bespoke action cluster and URL import UI | would diverge from the canonical shared agent action set Confidence: high Scope-risk: narrow Directive: Keep agent skill action ordering sourced from getAgentSkillActionIds() and reuse the shared upload adapter for future surface changes Tested: cd frontend && npm test; cd frontend && npm run build Not-tested: Manual browser interaction of the Agent Detail skills tab
Constraint: Task 4 required removing the stale inline enterprise SkillsTab while preserving existing route usage. Rejected: Keeping the inline duplicate and only asserting route usage | would preserve dead divergent code and fail the consolidation goal Confidence: high Scope-risk: narrow Directive: Keep EnterpriseSettings importing the canonical tab file instead of reintroducing local SkillsTab logic. Tested: cd frontend && npm test; cd frontend && npm run build Not-tested: Manual browser verification of the /enterprise skills tab UI
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2a74d5bae0
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| ): | ||
| """Delete a file.""" | ||
| await _require_agent_file_delete_access(db, current_user, agent_id) | ||
| await check_agent_access(db, current_user, agent_id) |
There was a problem hiding this comment.
Replace the generic access check here with a manage/admin-only guard. check_agent_access grants both manage and use access, so this change allows any user who can merely use a company agent to delete files/directories via /agents/{id}/files/content, including shared skill content and other workspace data. The previous behavior explicitly required manager/admin privileges for destructive deletes, so this is a real permission regression for non-managers.
Useful? React with 👍 / 👎.
Constraint: Chromium only opens a true folder picker when the directory attributes are present on the rendered file input itself Rejected: set webkitdirectory in useEffect | still falls back to file-only selection in the live modal Confidence: high Scope-risk: narrow Directive: Keep directory-selection attributes rendered on the hidden file input and retain enterprise upload-folder wiring coverage Tested: cd frontend && node --test tests/skillsUploadFolderRegression.test.mjs; cd frontend && npm test; cd frontend && npm run build Not-tested: authenticated live-browser interaction against ai-company.growatt-support.com/enterprise
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e7479b2803
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Comment on lines
+208
to
+209
| if entry.name == ".openclaw": | ||
| continue |
There was a problem hiding this comment.
Hiding .openclaw from directory listings here is not sufficient because the new protection is not applied in preview_file, so a caller can still request /api/agents/{id}/files/preview?path=.openclaw/openclaw.json directly and read protected workspace secrets. This commit introduces .openclaw as protected data, so the same _ensure_not_protected_workspace_path check needs to be enforced on preview (and any other content-serving endpoints) to avoid a direct-read bypass.
Useful? React with 👍 / 👎.
Comment on lines
+961
to
+963
| target_dir = (skills_root / target_path).resolve() | ||
| if not str(target_dir).startswith(str(skills_root)): | ||
| raise HTTPException(status_code=403, detail="Path traversal not allowed") |
There was a problem hiding this comment.
The traversal guard uses str(target_dir).startswith(str(skills_root)), which can be bypassed by sibling paths that share the same prefix (for example target_path=../skills_backup resolves outside .../skills but still passes the string-prefix test). That allows extraction outside the intended skills root. Use a real path containment check (e.g., Path.is_relative_to/relative_to) for both target_dir and out_path validation.
Useful? React with 👍 / 👎.
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
/enterpriseskills through the canonical tab component and remove the stale inline enterprise implementationWhat changed
frontend/src/components/skills/skillsActionItems.tsfrontend/src/components/skills/skillUploadSurfaceAdapters.tsfrontend/src/components/skills/SkillsActionBar.tsxSkillsTab()fromfrontend/src/pages/EnterpriseSettings.tsxnpm testbefore buildVerification
cd frontend && npm testcd frontend && npm run buildNotes