feat: persistent analysis workspaces - #2
Conversation
![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: 4e320e2e5d
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| PRIMARY KEY (space_id, user_id) | ||
| ); | ||
|
|
||
| CREATE INDEX IF NOT EXISTS idx_project_spaces_owner_updated ON project_spaces(owner_id, updated_at); |
There was a problem hiding this comment.
On every existing v1 database, project_spaces lacks updated_at; init_platform_db() executes this schema before migrate_platform_schema() adds the column, so this index statement raises sqlite3.OperationalError: no such column: updated_at and prevents the application from starting. Create this index in the additive migration after the ALTER TABLE, rather than in the pre-migration schema script.
AGENTS.md reference: database/AGENTS.md:L11-L16
Useful? React with 👍 / 👎.
| # Canvas, notes, and task text must never be copied to Trace/tool-call | ||
| # storage. Keep only enough metadata to make the invocation auditable. |
There was a problem hiding this comment.
When a configured generic HTTP agent quotes any supplied Canvas, note, or task text in its response, this helper sanitizes only the request: call_generic_http() still writes response_json unchanged to tool_calls, and query() subsequently stores the same normalized response in the Trace. That violates the documented guarantee that Trace retains only workspace version/count summaries, so the response persistence path must also prevent echoed workspace bodies from being recorded.
Useful? React with 👍 / 👎.
| source_type = updates.get("source_type", current.get("source_type")) | ||
| source_id = updates.get("source_id", current.get("source_id")) | ||
| _validate_source(user, source_type, source_id) |
There was a problem hiding this comment.
When an editor can access a shared workspace note but not its linked native resource, the read path intentionally redacts the source reference, yet an unrelated PATCH of the note title or body reuses the hidden stored source and fails _validate_source() with 404. The identical pattern also prevents toggling or editing a shared task linked by another member; preserve the existing reference without revalidation unless the PATCH explicitly changes source_type or source_id.
Useful? React with 👍 / 👎.
| const description=window.prompt('用一句话说明这个工作空间要解决什么问题（可选）') || ''; | ||
| try{ | ||
| const created=await api('/api/workspaces',{method:'POST',body:JSON.stringify({name:normalized,description:description.trim()})}); | ||
| activeWorkspaceId=created.id; |
There was a problem hiding this comment.
The create endpoint returns the workspace as {space: {...}, role, ...}, so created.id is always undefined. The UI consequently stores an invalid active-workspace value and relies on the subsequent list ordering to choose a workspace; when multiple spaces share the second-resolution timestamps used for ordering, creating one can reopen a different space instead. Select created.space.id from the actual response shape.
Useful? React with 👍 / 👎.
| return | ||
|
|
||
| _upsert_by_id("project_spaces", "space_demo", {"id": "space_demo", "name": "独立数据智能体演示空间", "owner_id": "u_admin", "description": "面向销售、客户服务、营销和经营分析的独立演示空间", "status": "active", "created_at": t}) | ||
| _upsert_by_id("project_spaces", "space_demo", {"id": "space_demo", "name": "独立数据智能体演示空间", "owner_id": "u_admin", "description": "面向销售、客户服务、营销和经营分析的独立演示空间", "status": "active", "created_at": t, "updated_at": t}) |
There was a problem hiding this comment.
In the default demo/HF configuration, seed_platform() runs on every startup and _upsert_by_id() overwrites every field supplied here for an existing space_demo. Any user rename, description edit, archive state, and updated_at value is therefore silently reset after a restart, contradicting the new persistent-workspace behavior and moving this fixture back to the top of recency ordering. Seed this row only when absent, or avoid overwriting user-mutable fields.
AGENTS.md reference: database/AGENTS.md:L15-L18
Useful? React with 👍 / 👎.
| current = con.execute("SELECT * FROM workspace_canvases WHERE space_id=?", [space_id]).fetchone() | ||
| current_version = int(current["version"]) if current else 0 | ||
| if current_version != payload["expected_version"]: | ||
| raise HTTPException(status_code=409, detail="Canvas version conflict") |
There was a problem hiding this comment.
When two editors save the same Canvas version concurrently, both deferred SQLite transactions can read the same version before either writes; the losing transaction then fails while upgrading its stale read snapshot with database is locked rather than returning the intended 409 conflict. Use an atomic conditional update such as WHERE version=? and map a zero-row update to 409, or acquire a write transaction before reading.
Useful? React with 👍 / 👎.
| function workspaceEditable(detail=activeWorkspaceDetail){ | ||
| return ['owner','editor'].includes(detail?.role || detail?.space?.role || ''); | ||
| } |
There was a problem hiding this comment.
For an archived workspace whose user is still an owner or editor, this helper returns true and the page continues to expose Canvas save, task, note, and resource mutation controls. Every such action is rejected by the backend's writable=True check with 409, leaving the UI in a misleading editable state; include the workspace's active status in this predicate.
Useful? React with 👍 / 👎.
| | Runtime URL | `https://blueskyxn-data-agent-panel-hfs.hf.space` | | ||
| | Runtime mode | Private Docker Space / Pattern A / repo root | |
There was a problem hiding this comment.
This new tracked environment register explicitly identifies the deployment as a private Space and records its concrete runtime hostname. That exposes a private deployment endpoint in repository history despite the repository rule prohibiting committed private hosts; use a placeholder or keep operational endpoint inventory outside tracked source.
AGENTS.md reference: AGENTS.md:L86-L86
Useful? React with 👍 / 👎.
| if(type==='report') return `openReportCommand('${jsArg(id)}')`; | ||
| if(type==='trace') return `showPage('audit');setTimeout(()=>openAuditTrace('${jsArg(id)}','summary',null),120)`; | ||
| if(type==='dataset') return `openDatasetCommand('${jsArg(id)}')`; | ||
| if(type==='analysis_task') return `showPage('analysis');setTimeout(()=>{const i=document.getElementById('analysisTaskId');if(i){i.value='${jsArg(id)}';loadLastAnalysisTask(null)}},120)`; |
There was a problem hiding this comment.
For an analysis_task resource, this action looks for an element with ID analysisTaskId, but no such element exists anywhere in the workbench; the guarded block therefore never calls loadLastAnalysisTask(). Clicking “打开” only navigates to the analysis page without loading the linked task, so assign the resource ID to lastAnalysisTaskId before invoking the loader.
Useful? React with 👍 / 👎.
| let contextPackPresets = loadContextPackPresets(); | ||
| let activeContextPackPresetId = ''; | ||
| let workspaces = []; | ||
| let activeWorkspaceId = localStorage.getItem('dap_active_workspace_id') || ''; |
There was a problem hiding this comment.
The active workspace is stored under one browser-global key and is not cleared on logout. On a shared browser, if user A logs out while ?page=chat is active and user B logs in, bootstrap skips refreshWorkspaces(), so B's next query unconditionally submits A's workspace ID and fails with 404 until the workspace page is visited. Clear this value on logout or persist and validate it per authenticated user before adding it to chat context.
Useful? React with 👍 / 👎.
1 participant
Summary
Verification
PASS standard is hfs-dev
PASS schema_version is 1 or 2
PASS pattern is valid
PASS runtime_mode is valid
PASS space_root_mode is valid
PASS hfs_dir exists
PASS HFS README.md exists
PASS HFS Dockerfile exists
PASS HFS README declares sdk: docker
PASS Pattern A uses repo-root space_root_mode
PASS Pattern A hfs_dir is project root
PASS release_pin_required is true
PASS v2 release pins use structured [[release_pins]]
PASS release_pins are non-empty structured tables
PASS release_pins[1] has name
PASS release_pins[1] has valid type
PASS release_pins[1] has source
PASS release_pins[1].required_for_release is boolean
PASS release_pins[1].dev_mutable_default_allowed is boolean
PASS release_pins name is unique: PYTHON_BASE_IMAGE
PASS release_pins include required release inputs
PASS self-contained release pins include base image digest
PASS required file exists: README.md
PASS required file exists: README.hf-space.md
PASS required file exists: Dockerfile
PASS required file exists: hf_entrypoint.sh
PASS required file exists: requirements.txt
PASS required file exists: docs/huggingface-spaces.md
PASS required file exists: scripts/hf_space_smoke.sh
PASS required file exists: scripts/static_check.py
PASS required file exists: hfs-dev.toml
PASS HFS alignment manifest and static contract checks
$ git diff --check
$ bash -n scripts/build_hf_local.sh
$ bash -n scripts/hf_space_smoke.sh
$ bash -n scripts/install_codex_cli.sh
$ bash -n scripts/run_hf_local.sh
$ bash -n hf_entrypoint.sh
$ bash -n run_dev.sh
$ /opt/homebrew/bin/node --check apps/web/static/app.js
$ /opt/homebrew/bin/node --check services/codex_sdk_bridge/run_task.mjs
$ /Library/Frameworks/Python.framework/Versions/3.11/bin/python3 scripts/check_hfs_alignment.py .
Static checks passed