Skip to content

docs(adr-013): Phase 0a findings — close Q1/11/12/13, split Phase 4#291

Merged
samxu01 merged 1 commit intomainfrom
docs/adr-013-phase-0a-findings
May 4, 2026
Merged

docs(adr-013): Phase 0a findings — close Q1/11/12/13, split Phase 4#291
samxu01 merged 1 commit intomainfrom
docs/adr-013-phase-0a-findings

Conversation

@samxu01
Copy link
Copy Markdown
Contributor

@samxu01 samxu01 commented May 4, 2026

Summary

Records the verdicts from the Phase 0a research spike, closing four open questions from ADR-013 and finalizing the phasing.

Per Invariant 8, Phase 0b code cannot land while any of (i)/(ii)/(iii) remain in placeholder state. This PR resolves all three.

Verdicts

(i) SHA256SUMS publication — ✅ Available

iOfficeAI/OfficeCLI v1.0.70 publishes `SHA256SUMS` (710 bytes) directly on the GitHub release. Phase 0b uses the upstream artifact verification path; no committed-constant fallback needed. Linux x64 binary confirmed at 32,156,584 bytes — matches the Part 2 size estimate.

(ii) Per-agent skill install scoping — ❌ Deeper backend change required

Not a parameter add. Three surfaces all need work:

# Surface What's wrong
1 `backend/routes/skills.ts:279-307` Stores agent-scoped metadata but drops `agentName` when calling sync hook
2 `backend/services/agentProvisionerService.ts:424-549` `syncOpenClawSkillsLocal` queries ALL pod-active skills, ignores per-agent scope. `buildScopedSkillKey` helper exists but is unused in the sync path
3 `${WORKSPACE_ROOT}/${accountId}/skills/` `accountId` uses `instanceId` only for openclaw (collision-prone); import bypasses `resolveOpenClawAccountId`

Implication: Phase 4 splits firmly into 4a (frontend Configure drawer + inspector tabs) + 4b (backend per-agent install scoping) as the v5.4 contingency anticipated. PR estimate finalized at ~7 PRs.

(iii) Workspace-boundary helper — ⚠️ Helper exists but not exported, plus surprise

The validation helper `toRelativeWorkspacePath` exists at `_external/clawdbot/src/agents/path-policy.ts:105-116`. High quality — rejects `..`, resolves symlinks, cross-platform.

Two issues:

  1. Not exported from plugin-sdk → ~10-line refactor absorbs into Phase 0b
  2. `acpx_run` itself has zero path validation today (bigger gap than Invariant 4 assumed)

Recommendation: accept the `acpx_run` gap. ADR-005 Stage 3 is retiring `acpx_run` in favor of A2A-via-DM (now first-class via shipped `commonly_open_dm` + `agent-dm` pod type). Hardening a deprecated surface is misallocated effort.

Open questions resolved

New open question

  • Add additional frontend tests #14 — Should `acpx_run` be hardened with `toRelativeWorkspacePath` for its remaining lifetime? Recommendation: defer / accept gap. Track ADR-005 Stage 3 deprecation timeline; revisit if it slips past 90 days.

Naming-note update

Updated §"Relationship to ADR-005" to reflect that `commonly_open_dm` (extension tool) + `agent-dm` pod type both shipped (per CLAUDE.md update). A2A-via-DM is now two shipped primitives, not just the wrapper-agent pattern from ADR-005.

What this enables

Phase 0b can now scope cleanly:

  • Dockerfile uses upstream `SHA256SUMS` (no placeholder lookup)
  • `commonly_attach_file` imports the helper from plugin-sdk (after the small export PR lands)
  • Phase 4 splits 4a/4b are explicit, not contingent

Test plan

  • Reviewers confirm the (ii) finding accurately reflects the code paths cited
  • Reviewers confirm the (iii) recommendation to defer `acpx_run` hardening
  • Sign-off on the Phase 4 split into 4a + 4b

Companion

ADR-013 PR #287 (merged) — this is the follow-up that closes its open questions.

🤖 Generated with Claude Code

All three load-bearing prereqs ran as a research spike. Verdicts:

(i) SHA256SUMS — ✅ Available. iOfficeAI/OfficeCLI v1.0.70 publishes
    the artifact at github.com/.../releases/download/v1.0.70/SHA256SUMS
    (710 bytes). Use upstream verification path.

(ii) Per-agent skill scoping — ❌ Deeper backend change required.
     Three surfaces all need work:
       1. backend/routes/skills.ts:279-307 — store agent metadata
          but drop agentName when calling sync hook
       2. backend/services/agentProvisionerService.ts:424-549 —
          syncOpenClawSkillsLocal queries ALL pod skills, ignores
          per-agent scope. buildScopedSkillKey exists but is unused
          in the sync path
       3. ${WORKSPACE_ROOT}/${accountId}/skills/ — accountId uses
          instanceId only for openclaw (collision-prone); import
          path bypasses resolveOpenClawAccountId
     Phase 4 splits into 4a (frontend) + 4b (backend). PR estimate
     finalized at ~7.

(iii) Workspace-boundary helper — ⚠️ Helper at path-policy.ts:105
      is high quality (rejects .., resolves symlinks, cross-platform)
      but NOT exported from plugin-sdk. ~10-line export refactor
      absorbs into Phase 0b. Surprise: acpx_run has zero path
      validation today. Recommendation: defer hardening — ADR-005
      Stage 3 is retiring acpx_run anyway. Track deprecation timeline;
      revisit if it slips past 90 days.

Closes open questions #1 (yes, import does push to PVC; the per-agent
scoping inside that path is what's missing), #11, #12, #13. Adds
new question #14 (acpx_run hardening — defer).

Updated §"Relationship to ADR-005" naming note to reflect that
commonly_open_dm + agent-dm pod type both shipped (per CLAUDE.md
update). A2A-via-DM is now two shipped primitives, not just the
wrapper-agent pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@samxu01 samxu01 merged commit b97c960 into main May 4, 2026
8 checks passed
samxu01 added a commit that referenced this pull request May 4, 2026
…294)

Closes Phase 0b of ADR-013 by wiring up the kernel verb, toolchain, dev
preset bundles, auto-import, and a locally-bundled OfficeCLI skill so
the four production dev agents (Theo/Nova/Pixel/Ops) can produce real
deliverables and attach them to chat after a single reprovision-all.

What ships in one merge:

1. **Submodule bump** _external/clawdbot → 4bf74dfc5 on rebase-2026.3.29
   (openclaw#2 merge commit). Adds the `commonly_attach_file` extension
   tool, the `uploadFile()` client method, the `toRelativeWorkspacePath`
   plugin-sdk export for path-traversal protection, and the
   `OPENCLAW_INSTALL_DOC_TOOLCHAIN` Dockerfile build-arg gating the
   ~170MB document toolchain (pinned OfficeCLI 1.0.70 + pandoc +
   texlive-xetex + markitdown + pypdf, with build-time self-test).

2. **deploy-dev.yml** passes `--build-arg OPENCLAW_INSTALL_DOC_TOOLCHAIN=1`
   to the clawdbot-gateway docker build so dev gateways get the toolchain.

3. **defaultSkills** for the four production dev presets (dev-pm/Theo,
   backend-engineer/Nova, frontend-engineer/Pixel, devops-engineer/Ops):
   github + officecli + pandic-office + markdown-converter + pdf, plus
   tmux for the three engineers. These four presets previously shipped
   with `defaultSkills: []` despite the install pipeline being fully
   wired — a quiet correctness bug ADR-013 (#287) flagged.

4. **backend/services/presetSkillsAutoImport.ts** (~210 lines, new) —
   resolves each defaultSkills entry to SKILL.md content (local bundle
   first, then catalog index → fetched via the existing
   fetchSkillContentFromSource), then upserts each as a PodAsset via
   the existing PodAssetService.upsertImportedSkillAsset. Idempotent
   across reprovisions. Pure reuse of the same pipeline the manual
   POST /api/skills/import route uses — no new install plumbing.

5. **provision.ts + reprovision.ts** invoke applyPresetDefaultSkills
   after provisionAgentRuntime returns and BEFORE syncOpenClawSkills
   runs. The new PodAssets land just in time for the existing gateway
   sync to pick them up and write their SKILL.md files to
   /workspace/<accountId>/skills/<id>/ on the gateway PVC.

6. **commonly-bundled-skills/officecli/** — local bundle of the
   upstream Apache-2.0 SKILL.md + LICENSE. Bundled because the upstream
   awesome-agent-skills catalog was last synced 2026-02-05 and OfficeCLI
   was created 2026-03-15. README.md documents the bundling convention
   so future skills follow the same pattern; when upstream gains the
   skill, the local bundle can be deleted.

After Deploy Dev + reprovision-all on dev:
- Theo/Nova/Pixel/Ops each have their bundle synced to PVC
- Agent runs `pandoc input.md -o report.pdf` (or officecli for slides)
- Agent calls commonly_attach_file({podId, filePath, message: '...'})
- Pill renders with preview in chat

Companion: ADR-013 (#287) merged. Phase 0a findings (#291) still open.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant