Skip to content

Propose macdoc docx workflow CLI - #94

Closed
kiki830621 wants to merge 1 commit into
mainfrom
codex/macdoc-docx-workflow-spec
Closed

kiki830621 wants to merge 1 commit into
mainfrom
codex/macdoc-docx-workflow-spec

Conversation

@kiki830621

Copy link
Copy Markdown
Member

Summary

Validation

  • spectra analyze macdoc-docx-workflow-cli --json
  • spectra validate macdoc-docx-workflow-cli
  • git diff --check -- openspec/changes/macdoc-docx-workflow-cli

Refs #92

@kiki830621

kiki830621 commented May 25, 2026 •

Copy link
Copy Markdown
Member Author

Verify Report — PR #94

Engine

6-AI ensemble: 5 general-purpose Agents (Claude reviewers) + Codex (gpt-5.5 xhigh). Initial spawn rate-limited; full Recovery Protocol Step 2.5b retry with FULL context re-paste succeeded — all 5/5 findings present + non-empty. No Process Gaps.

Aggregate

NEEDS WORK — 0 P0 blockers (no production code regression), but multiple SPEC GAPS that conflict with the merge intent of locking #92's design direction. Recommend either:

  • (A) Block merge; revise spec to address spec gaps before archive (proper IDD discipline)
  • (B) Merge as Phase 1 minimum + file 4-5 follow-up Spectra changes / issues for the gaps (pragmatic if Phase 1 scope is intentional)

Scope coverage


#92 — feat: dxedit declarative docx edit CLI + library

Requirements coverage (per Codex + Requirements + Logic agreement):

#92 Requirement Status Notes
Declarative docx edit plan + manifest FULLY proposal/spec/tasks consistent
Avoid MCP transport, direct ooxml-swift FULLY DocxWorkflowLib boundary clear
Library + CLI dual surface FULLY (PARTIAL per DA) DocxWorkflowLib defined; import contract / external consumer policy not locked
Standalone dxedit binary NOT (intentional re-scope) Documented in proposal §Non-Goals + design.md
Replicable pipelines FULLY build/patch/apply 3-workflow split covers use cases
YAML/CJK-friendly manifest PARTIALLY JSON-first. YAML deferred. No documented "lossless JSON↔YAML mapping commitment".
Apply steps: image/text/paragraph FULLY each has spec Requirement + Scenario
Apply step: replace_text_batch NOT First-class operation missing — issue use case 2 (batch fix-up) lacks dedicated step
Verify harness: libxml2_valid NOT Spec only has containsText/notContainsText/replacementCount/readbackSucceeds
Verify harness: byte_preserved_parts NOT design.md Risks delegates to "existing OOXML preservation tests" — silent contract drop
Verify harness: expected_images / bookmarks NOT Issue sketch lists these; spec has zero coverage
Plan dry-run FULLY spec no-output Scenario explicit
Diff FULLY uses ooxml-swift compare API
Archive-first (issue Phase 3) NOT Silently dropped — no non-goal/trade-off note
che-word-mcp integration tests PARTIALLY DocxWorkflowLib enables it, but no spec Scenario or task line for E2E

Findings (merged + deduplicated across 6 sources)

# Severity Finding Source Action
1 HIGH Verify check set in spec.md (4 checks) does not match issue #92 manifest sketch (libxml2_valid, byte_preserved_parts, expected_images, expected_bookmarks_min). design.md Risks delegates byte-preservation to "existing OOXML preservation tests" — but that's an implementation deflection, not a spec contract. agents:requirements+logic+codex+devils-advocate Blocking for spec lock
2 HIGH replace_text_batch is missing as a first-class operation. Issue use case 2 ("batch fix-up") is the second motivation in the issue body. spec/tasks Phase 1 only lists replaceText singular — needs explicit decision: ordered loop of single-replace vs dedicated batch step. agents:logic+codex+devils-advocate Blocking for spec lock
3 HIGH Manifest schema underspecified for implementation: missing field names (insertImageAfterText fields), option names (--overwrite vs --force), all-matches behaviour, path resolution rule (manifest-relative or cwd-relative), replacementCount step binding. agents:codex+logic Blocking
4 HIGH Adversarial input handling completely absent from spec. 9 security gaps: path traversal in baseline/output/path/image-path fields; YAML/JSON DoS (no manifest/steps/string size limits); {{name}} placeholder unicode normalization + recursive expansion; OOXML readback XXE / zip-bomb / zip-slip trust model; output atomicity + symlink reject + file mode; image source sanitization + size cap + MIME validation; literal-vs-regex declaration for containsText; stdin/--output - behaviour. agents:security Blocking for production-grade spec lock
5 HIGH che-word-mcp integration testing is the second motivation in issue #92 (lines 8, 86). DocxWorkflowLib enables it, but no spec Scenario or task explicitly covers shell-out from che-word-mcp tests to DocxWorkflowLib. agents:requirements+codex In-scope gap
6 HIGH archive-first (issue Phase 3 auto-snapshot baseline) silently removed. PR introduces separate-output + overwrite protection as alternative but doesn't document whether archive-first is: deferred / replaced / cancelled. agents:codex+devils-advocate Blocking — explicit non-goal decision needed
7 MEDIUM JSON-first manifest decision (over YAML) is rationalized in Current Status but Current Status was written same-day by the PR author (Codex), not third-party-reviewed. design.md missing "lossless JSON↔YAML representability commitment" for future YAML support. agents:devils-advocate Document
8 MEDIUM Build scope inconsistency: proposal/design says build covers sections/paragraphs/tables/images/equations; spec/tasks Phase 1 only lists sections/paragraphs/tables/text-runs. Internal drift. agents:codex Reconcile within spec
9 MEDIUM Re-scope (standalone dxedit → macdoc docx) is documented but loses concrete future reversibility: separate Homebrew tap, independent release cadence, independent issue tracker. design.md mentions "future wrapper" but treats as escape hatch fiction. agents:devils-advocate Document trade-off
10 MEDIUM Synthetic fixture policy mentioned in PR summary but is negative-space-only ("don't commit real document files"). Missing positive Scenario: what makes a fixture "synthetic" (programmatic WordBuilder API generation? size cap? library helper?). agents:devils-advocate+logic Add positive spec
11 MEDIUM Capability naming docx-workflow-cli includes -cli (product surface), whereas other capabilities in repo name the capability (bib-apa-to-html, mdocx-grammar). build subcommand name overlaps with convert --to docx — no documented disambiguation. agents:devils-advocate+regression Rename or document
12 LOW cli-design/error-and-output.md requires Traditional Chinese error messages. Spec has zero examples of expected error message language. agents:logic Phase 1 implementation note
13 LOW che-word-mcp#61 anchor dependency not reflected in design.md Risks. agents:logic Document
14 LOW Active Spectra change word-aligned-state-sync (reshaping ooxml-swift internals) not cross-referenced from design.md. DocxWorkflowLib's import path through the typed-view API needs explicit acknowledgement. agents:regression Cross-link
15 LOW archive-first semantic clash with macdoc's existing archive-first plugin (PreToolUse hooks on archived/). Naming collision risk if not disambiguated. agents:regression Naming clarity

Scope Check

No scope creep — diff is pure spec proposal under one directory. Re-scoping from standalone dxedit → integrated macdoc docx IS documented (proposal.md §Non-Goals + design.md + Current Status), but DA points out the sign-off chain is single-author (Codex same-day self-sign), not third-party reviewed.

Security

This is a spec-level security review (not code). The proposal does not contain an adversarial-input handling section. 9 gaps documented. Severity HIGH (#4 above) because Phase 1 implementation could ship without trust-boundary enforcement if the spec doesn't require it.

Process Gaps

Initial spawn rate-limited (transient API throttle). Recovery Protocol Step 2.5b (retry with FULL context re-paste) successful — 5/5 reviewer findings produced. No coordinator self-review fallback needed.


Recommendation

Do NOT merge as-is if the goal is "lock the spec for #92".

This proposal addresses ~50% of issue #92's surface and would force significant Phase 2+ Spectra changes. Two paths:

Path A (proper IDD): Block merge. Send Codex back with a revision request covering:

  1. Verify hooks: add libxml2_valid / byte_preserved_parts / expected_images / expected_bookmarks_min as first-class Spec Requirements (or explicit Non-Goal with rationale)
  2. replace_text_batch: add first-class step OR add Scenario showing ordered single-replace covers use case 2
  3. Manifest schema: complete field-level specification (option names, path resolution, all-matches behaviour)
  4. archive-first: explicit Non-Goal decision with link to separate Spectra change for future
  5. che-word-mcp integration: add spec Scenario + task line
  6. Adversarial input section addressing the 9 security gaps

Path B (pragmatic): Merge as Phase 1 minimum (with explicit acknowledgement that it's not the full #92). File 4 follow-up issues / Spectra changes:

Path B sacrifices spec discipline (locks half-spec) for momentum. Path A is the correct IDD discipline but blocks the PR.

@kiki830621

Copy link
Copy Markdown
Member Author

Blocked pending spec revision (6 HIGH gaps).

Full verify report: #94 (comment)

Required revisions before merge:

  1. Verify hooks — Add libxml2_valid, byte_preserved_parts, expected_images, expected_bookmarks_min as first-class spec Requirements (or document each as explicit Non-Goal with rationale). Currently spec only covers 4 text-level checks while issue feat: dxedit — declarative docx edit CLI + library (manifest-driven, ooxml-swift direct, peer to che-word-mcp MCP front-end) #92 manifest sketch enumerates 8 verification dimensions.

  2. replace_text_batch — Add as first-class operation in spec, OR add explicit Scenario showing ordered single-replace covers use case 2 ("batch fix-up"). Currently the second motivation in issue body has no dedicated spec coverage.

  3. Manifest schema completeness — Lock down: insertImageAfterText field names, all-matches behaviour name, replacementCount step binding, path resolution rule (manifest-relative vs cwd-relative), --overwrite vs --force option naming.

  4. archive-first decision — Explicit Non-Goal entry stating whether issue feat: dxedit — declarative docx edit CLI + library (manifest-driven, ooxml-swift direct, peer to che-word-mcp MCP front-end) #92 Phase 3 archive-first is: deferred (link to follow-up Spectra change), replaced (by separate-output + overwrite protection — needs justification), or cancelled.

  5. che-word-mcp integration test E2E — Add spec Scenario + task line. Issue feat: dxedit — declarative docx edit CLI + library (manifest-driven, ooxml-swift direct, peer to che-word-mcp MCP front-end) #92 lists integration testing as the second core motivation (lines 8, 86 of issue body); DocxWorkflowLib enables it but no spec Scenario locks the contract.

  6. Adversarial input handling — Add spec section covering: path traversal (baseline/output/image-path); manifest size limits (DoS); {{name}} placeholder unicode normalization + recursive expansion guard; OOXML readback trust model (XXE / zip-bomb / zip-slip); output atomicity + symlink reject + file mode; image source MIME / size cap; literal-vs-regex declaration for containsText. Currently spec has zero adversarial-input section.

Additional medium-priority items (resolve in same revision if low cost):

  • Document lossless JSON↔YAML representability commitment (since YAML deferred)
  • Reconcile build scope drift (proposal/design lists images+equations; spec/tasks Phase 1 lists only sections/paragraphs/tables/text-runs)
  • Capability rename (docx-workflow-cli → naming aligned with repo convention like bib-apa-to-html, removing -cli product surface from capability name)
  • Cross-reference active word-aligned-state-sync change in design.md Risks
  • Disambiguate archive-first semantic from existing macdoc archive-first plugin

The current revision sign-off chain is single-author (Codex same-day Current Status edit); requesting third-party review before merging this back to lock the design direction for #92.

@kiki830621

Copy link
Copy Markdown
Member Author

Closing — Superseded by ooxml-edit-isomorphism-foundation (#99 merged in PR #106)

Per #99 ADR-009, this proposal is reframed as a Layer 3 (DSL frontend) front-end of the architectural foundation. The current spec has known gaps (see verify report findings) that the foundation's locked contract addresses naturally.

The re-framing work is tracked at #102 — Codex (or other authors) can open a new PR citing ooxml-edit-algebra capability spec + foundation design.md ADRs as inputs. The Codex work in this PR remains accessible in repo history (git show codex/<branch>) for reference when authoring the re-frame.

Issue #92 (original umbrella) was closed as absorbed into #99 architecture; #102 is the operational follow-up.

Refs #99 #92 #102

@kiki830621 kiki830621 closed this May 25, 2026
kiki830621 added a commit that referenced this pull request Sep 29, 2026
Pins the Word↔Swift edit-isomorphism contract as the macdoc OOXML
toolchain's core architectural contract via:
- New capability spec 'ooxml-edit-algebra' (8 Requirements)
- design.md with 9 ADRs (canonical-identity, Edit-as-first-class,
  two-layer algebra, module split, naming, Word UI ground truth,
  conformance suite, lens migration path deferred, downstream rerouting)
- tasks.md with hybrid scope: Edit type elevation + property-based
  functor tests on 3-5 representative OOXMLEdit cases only;
  downstream migrations explicitly deferred to follow-up Spectra changes

Cross-references active 'word-aligned-state-sync' change in design.md
Relationship section. Coordinates downstream PRs #94/#95/#96/#97/#98
that 6-AI verify identified as blocked on spec ambiguity — they will
be reframed as front-ends to this foundation per ADR-009.

Refs #99
kiki830621 added a commit that referenced this pull request Sep 29, 2026
…-§10)

Shipped (apply phase, 11/35 tasks):
- §8.1 Cross-reference to ooxml-edit-isomorphism-foundation in
  word-aligned-state-sync/design.md Relationship section
- §8.2 Follow-up issue #101 (word-builder-swift lens migration)
- §8.3 Follow-up issue PsychQuant/che-word-mcp#162 (MCP boundary refactor)
- §8.4 Follow-up issue #102 (PR #94 dxedit re-frame)
- §8.5 Follow-up issue #103 (PR #96 R-wordbuilder re-frame)
- §8.6 Follow-up issue #104 (PR #95 pptx-mcp re-frame)
- §9.1 .github/PULL_REQUEST_TEMPLATE.md with CD-diagram requirement
- §9.2 EditAlgebra/README.md documenting CD discipline + worked examples
- §10.1 spectra validate green
- §10.3 docs/structural-editing-paradigm.md cross-reference
- §10.4 docs/lossless-conversion.md cross-reference

Deferred to Phase 2 Spectra change (§1-§7 + §10.2 Swift implementation,
23/35 tasks): Edit protocol code, OOXMLEdit/WordEdit enums, Document.apply
API, property-based functor tests. Deferral rationale documented in
tasks.md ASSUMPTION block; new follow-up Spectra change
'ooxml-edit-algebra-implementation' opens after this foundation archives.

Decision-pinning (this change) + runtime implementation (Phase 2) serve
different review modes and benefit from being separate changes.

Refs #99
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