mirror of
https://github.com/lllin000/PaperForge.git
synced 2026-07-22 06:50:53 +00:00
docs(49-module-hardening): complete phase 49 — add summary, verification, state update
This commit is contained in:
Research Assistant
parent
377c8c7b44
commit
1315bbdde1
5 changed files with 277 additions and 29 deletions
|
|
@ -32,21 +32,21 @@
|
|||
|
||||
**Root cause:** The Textual TUI setup wizard (`paperforge setup` without `--headless`) is broken (NameError crash), unreachable from either real install workflow (plugin settings tab uses `--headless`, AI agents use `--headless`), and adds ~1200 lines of Textual-dependent code to maintain.
|
||||
|
||||
- [ ] **DEPR-01**: `paperforge setup` (bare, no `--headless`) prints a help message redirecting to `--headless` or the plugin settings tab. Textual TUI classes removed from `setup_wizard.py`: `WelcomeStep`, `DirOverviewStep`, `VaultStep`, `PlatformStep`, `DeployStep`, `DoneStep`, `SetupWizardApp`, `ContentSwitcher`, `StepScreen`, and all TUI-only import paths. `headless_setup()` and shared utilities (`EnvChecker`, `AGENT_CONFIGS`, `_copy_file_incremental`, `_merge_env_incremental`) preserved.
|
||||
- [ ] **DEPR-02**: Three documentation files updated: `docs/setup-guide.md`, `docs/INSTALLATION.md`, `README.md` — all bare `paperforge setup` references changed to `paperforge setup --headless`.
|
||||
- [ ] **DEPR-03**: Post-install instruction text in `setup_wizard.py:1306-1307` and `headless_setup` completion message updated to reflect headless-only workflow. `--non-interactive` removed from CLI options. `textual` removed from project optional dependencies.
|
||||
- [x] **DEPR-01**: `paperforge setup` (bare, no `--headless`) prints a help message redirecting to `--headless` or the plugin settings tab. Textual TUI classes removed from `setup_wizard.py`: `WelcomeStep`, `DirOverviewStep`, `VaultStep`, `PlatformStep`, `DeployStep`, `DoneStep`, `SetupWizardApp`, `ContentSwitcher`, `StepScreen`, and all TUI-only import paths. `headless_setup()` and shared utilities (`EnvChecker`, `AGENT_CONFIGS`, `_copy_file_incremental`, `_merge_env_incremental`) preserved.
|
||||
- [x] **DEPR-02**: Three documentation files updated: `docs/setup-guide.md`, `docs/INSTALLATION.md`, `README.md` — all bare `paperforge setup` references changed to `paperforge setup --headless`.
|
||||
- [x] **DEPR-03**: Post-install instruction text in `setup_wizard.py:1306-1307` and `headless_setup` completion message updated to reflect headless-only workflow. `--non-interactive` removed from CLI options. `textual` removed from project optional dependencies.
|
||||
|
||||
### HARDEN — Module Hardening (7 requirements)
|
||||
|
||||
**Root cause:** New modules (`discussion.py`, `asset_state.py`) and the Obsidian plugin were built quickly during v1.6-v1.8 and lack safety hardening.
|
||||
|
||||
- [ ] **HARDEN-01**: `discussion.py:281-314` file-level locking around JSON and MD read-modify-write cycles. Concurrent `/pf-paper` and `/pf-deep` calls for the same paper do not silently drop sessions.
|
||||
- [ ] **HARDEN-02**: `discussion.py:170-171` Markdown special characters (`*`, `#`, `[`, `_`, `` ` ``) escaped in QA question/answer fields. Individual QA dict keys validated before building session.
|
||||
- [ ] **HARDEN-03**: `discussion.py:40` hardcoded CST (UTC+8) replaced with UTC. All timestamps use `datetime.now(timezone.utc)`.
|
||||
- [ ] **HARDEN-04**: `main.js:2116` PaddleOCR API key passed via environment variable, not command-line argument. `spawn()` with `env: { PADDLEOCR_API_TOKEN: ... }`.
|
||||
- [ ] **HARDEN-05**: `main.js:1815` `innerHTML` assignment replaced with `createEl()` DOM API for directory tree rendering. No XSS vector from user-configured directory names.
|
||||
- [ ] **HARDEN-06**: `asset_state.py:225-226` workspace integrity checks performed before returning `"/pf-deep"`. Checks currently at lines 233-240 moved before line 226.
|
||||
- [ ] **HARDEN-07**: `status.py:687-690` `lifecycle_level_counts`, `health_aggregate`, `maturity_distribution` return empty dicts `{}` when no canonical index exists (not `null`/`None`). Downstream JSON parsers do not crash on field access.
|
||||
- [x] **HARDEN-01**: `discussion.py:281-314` file-level locking around JSON and MD read-modify-write cycles. Concurrent `/pf-paper` and `/pf-deep` calls for the same paper do not silently drop sessions.
|
||||
- [x] **HARDEN-02**: `discussion.py:170-171` Markdown special characters (`*`, `#`, `[`, `_`, `` ` ``) escaped in QA question/answer fields. Individual QA dict keys validated before building session.
|
||||
- [x] **HARDEN-03**: `discussion.py:40` hardcoded CST (UTC+8) replaced with UTC. All timestamps use `datetime.now(timezone.utc)`.
|
||||
- [x] **HARDEN-04**: `main.js:2116` PaddleOCR API key passed via environment variable, not command-line argument. `spawn()` with `env: { PADDLEOCR_API_TOKEN: ... }`.
|
||||
- [x] **HARDEN-05**: `main.js:1815` `innerHTML` assignment replaced with `createEl()` DOM API for directory tree rendering. No XSS vector from user-configured directory names.
|
||||
- [x] **HARDEN-06**: `asset_state.py:225-226` workspace integrity checks performed before returning `"/pf-deep"`. Checks currently at lines 233-240 moved before line 226.
|
||||
- [x] **HARDEN-07**: `status.py:687-690` `lifecycle_level_counts`, `health_aggregate`, `maturity_distribution` return empty dicts `{}` when no canonical index exists (not `null`/`None`). Downstream JSON parsers do not crash on field access.
|
||||
|
||||
### REPAIR — Repair Blind Spots (4 requirements)
|
||||
|
||||
|
|
@ -83,16 +83,16 @@
|
|||
| LEGACY-05 | Phase 47 | Complete |
|
||||
| LEGACY-06 | Phase 47 | Complete |
|
||||
| LEGACY-07 | Phase 47 | Complete |
|
||||
| DEPR-01 | Phase 48 | Pending |
|
||||
| DEPR-02 | Phase 48 | Pending |
|
||||
| DEPR-03 | Phase 48 | Pending |
|
||||
| HARDEN-01 | Phase 49 | Pending |
|
||||
| HARDEN-02 | Phase 49 | Pending |
|
||||
| HARDEN-03 | Phase 49 | Pending |
|
||||
| HARDEN-04 | Phase 49 | Pending |
|
||||
| HARDEN-05 | Phase 49 | Pending |
|
||||
| HARDEN-06 | Phase 49 | Pending |
|
||||
| HARDEN-07 | Phase 49 | Pending |
|
||||
| DEPR-01 | Phase 48 | Complete |
|
||||
| DEPR-02 | Phase 48 | Complete |
|
||||
| DEPR-03 | Phase 48 | Complete |
|
||||
| HARDEN-01 | Phase 49 | Complete |
|
||||
| HARDEN-02 | Phase 49 | Complete |
|
||||
| HARDEN-03 | Phase 49 | Complete |
|
||||
| HARDEN-04 | Phase 49 | Complete |
|
||||
| HARDEN-05 | Phase 49 | Complete |
|
||||
| HARDEN-06 | Phase 49 | Complete |
|
||||
| HARDEN-07 | Phase 49 | Complete |
|
||||
| REPAIR-01 | Phase 50 | Pending |
|
||||
| REPAIR-02 | Phase 50 | Pending |
|
||||
| REPAIR-03 | Phase 50 | Pending |
|
||||
|
|
|
|||
|
|
@ -245,7 +245,7 @@ Plans:
|
|||
| 46. Index Path Resolution | v1.11 | 2/2 | Complete | 2026-05-07 |
|
||||
| 47. Library-Records Deprecation Cleanup | v1.11 | 1/2 | In Progress| |
|
||||
| 48. Textual TUI Removal | v1.11 | 3/2 | Complete | 2026-05-07 |
|
||||
| 49. Module Hardening | v1.11 | 0/0 | Not started | - |
|
||||
| 49. Module Hardening | v1.11 | 1/3 | In Progress| |
|
||||
| 50. Repair Blind Spots | v1.11 | 0/0 | Not started | - |
|
||||
|
||||
---
|
||||
|
|
|
|||
|
|
@ -3,14 +3,14 @@ gsd_state_version: 1.0
|
|||
milestone: v1.11
|
||||
milestone_name: Merge Gate — v1.9 Ripple Remediation
|
||||
status: completed
|
||||
stopped_at: Completed 48 - Textual TUI Removal (48-001, 48-002)
|
||||
last_updated: "2026-05-07T11:44:47.723Z"
|
||||
last_activity: 2026-05-07 — Phase 48 executed (DEPR-01 through DEPR-03)
|
||||
stopped_at: Completed 49 - Module Hardening (HARDEN-01 through HARDEN-07)
|
||||
last_updated: "2026-05-07T12:07:44.635Z"
|
||||
last_activity: 2026-05-07 — Phase 48 executed (48-001, 48-002)
|
||||
progress:
|
||||
total_phases: 9
|
||||
completed_phases: 0
|
||||
total_plans: 6
|
||||
completed_plans: 7
|
||||
completed_phases: 1
|
||||
total_plans: 9
|
||||
completed_plans: 10
|
||||
percent: 20
|
||||
---
|
||||
|
||||
|
|
@ -54,6 +54,7 @@ Progress: [████████░░] 20%
|
|||
*Updated after each plan completion*
|
||||
| Phase 47-library-records-deprecation-cleanup P001-002 | 18min | 4 tasks | 16 files |
|
||||
| Phase 48-textual-tui-removal P001-002 | 15min | 5 tasks | 6 files |
|
||||
| Phase 49-module-hardening P001-003 | 8 | 6 tasks | 7 files |
|
||||
|
||||
## Accumulated Context
|
||||
|
||||
|
|
@ -81,6 +82,6 @@ Progress: [████████░░] 20%
|
|||
|
||||
## Session Continuity
|
||||
|
||||
Last session: 2026-05-07T11:44:47.723Z
|
||||
Stopped at: Completed 48 - Textual TUI Removal (DEPR-01 through DEPR-03)
|
||||
Last session: 2026-05-07T12:07:44.632Z
|
||||
Stopped at: Completed 49 - Module Hardening (HARDEN-01 through HARDEN-07)
|
||||
Resume file: None
|
||||
|
|
|
|||
157
.planning/phases/49-module-hardening/49-SUMMARY.md
Normal file
157
.planning/phases/49-module-hardening/49-SUMMARY.md
Normal file
|
|
@ -0,0 +1,157 @@
|
|||
---
|
||||
phase: 49-module-hardening
|
||||
plan: 001-003 (combined)
|
||||
wave: 1
|
||||
subsystem: worker, plugin
|
||||
tags: hardening, security, safety
|
||||
provides:
|
||||
- HARDEN-01: Cross-process file locking in discussion.py
|
||||
- HARDEN-02: Markdown escaping in QA text fields
|
||||
- HARDEN-03: UTC timestamps (not CST/UTC+8)
|
||||
- HARDEN-04: API key via environment variable (not CLI arg)
|
||||
- HARDEN-05: createEl() DOM API (no innerHTML XSS vector)
|
||||
- HARDEN-06: Workspace integrity checks before /pf-deep recommendation
|
||||
- HARDEN-07: Empty dicts {} instead of None for index aggregates
|
||||
affects:
|
||||
- discussion.py: read-modify-write now locked, timestamps UTC, MD escaped
|
||||
- main.js: spawn uses env var, dir tree uses createEl()
|
||||
- asset_state.py: next_step ordering corrected
|
||||
- status.py: JSON output safe for dict consumers
|
||||
tech-stack:
|
||||
added:
|
||||
- filelock library (existing dependency, now used by discussion.py)
|
||||
patterns:
|
||||
- filelock.FileLock for cross-process synchronization
|
||||
- datetime.now(timezone.utc) for canonical timestamps
|
||||
- PADDLEOCR_API_TOKEN env var for API key transport
|
||||
- createEl() obsidian DOM API for XSS-safe rendering
|
||||
key-files:
|
||||
created: []
|
||||
modified:
|
||||
- paperforge/worker/discussion.py
|
||||
- paperforge/plugin/main.js
|
||||
- paperforge/worker/asset_state.py
|
||||
- paperforge/worker/status.py
|
||||
- tests/test_discussion.py
|
||||
- tests/test_asset_state.py
|
||||
- tests/test_status.py
|
||||
decisions:
|
||||
- "filelock.FileLock used over fcntl/msvcrt for cross-platform portability (same pattern as asset_index.py)"
|
||||
- "CST constant name preserved but now assigned timezone.utc — minimal diff, no downstream impact"
|
||||
- "Both JSON and MD writes wrapped in single lock scope for read-modify-write consistency"
|
||||
metrics:
|
||||
duration: 8 min
|
||||
completed_date: "2026-05-07"
|
||||
---
|
||||
|
||||
# Phase 49 Module Hardening: Summary
|
||||
|
||||
**One-liner:** Production-grade safety guards across discussion.py (locking, escaping, UTC), main.js (env var API key, createEl() DOM), and asset_state/status (reordered checks, empty-safe dicts).
|
||||
|
||||
## Plans Executed
|
||||
|
||||
### Plan 49-001: discussion.py hardening (HARDEN-01, HARDEN-02, HARDEN-03)
|
||||
|
||||
| Task | Name | Commit | Files |
|
||||
|------|------|--------|-------|
|
||||
| 1 | UTC timestamps + Markdown escaping | `5a69874` | discussion.py, test_discussion.py |
|
||||
| 2 | File locking | `5a69874` | discussion.py, test_discussion.py |
|
||||
|
||||
### Plan 49-002: main.js hardening (HARDEN-04, HARDEN-05)
|
||||
|
||||
| Task | Name | Commit | Files |
|
||||
|------|------|--------|-------|
|
||||
| 1 | API key via env var | `8b9b366` | main.js |
|
||||
| 2 | createEl() not innerHTML | `8b9b366` | main.js |
|
||||
|
||||
### Plan 49-003: asset_state.py + status.py hardening (HARDEN-06, HARDEN-07)
|
||||
|
||||
| Task | Name | Commit | Files |
|
||||
|------|------|--------|-------|
|
||||
| 1 | Workspace integrity check order | `4e816f9` | asset_state.py, test_asset_state.py |
|
||||
| 2 | Empty dicts not null | `4e816f9` | status.py, test_status.py |
|
||||
|
||||
## Verification Results
|
||||
|
||||
```
|
||||
test_discussion.py ........ 12/12 passed (5 new tests)
|
||||
test_asset_state.py ....... 28/28 passed (2 new + 1 updated)
|
||||
test_status.py ............ 6/6 passed (1 assertion updated)
|
||||
Full suite ................ 485/489 passed, 2 skipped, 2 pre-existing failures
|
||||
```
|
||||
|
||||
- **2 pre-existing failures** in `test_ocr_state_machine.py` (unrelated — test OCR state transitions, not modified by any plan)
|
||||
- **0 regressions** from our changes
|
||||
- **JS verification:** `--paddleocr-key` absent, `PADDLEOCR_API_TOKEN` present, `innerHTML` eliminated from dir tree, `createEl()` chain present
|
||||
|
||||
## Deviations from Plan
|
||||
|
||||
None — all three plans executed exactly as written.
|
||||
|
||||
## Verification Details
|
||||
|
||||
### HARDEN-01: File locking
|
||||
- `import filelock` present in discussion.py ✅
|
||||
- `filelock.FileLock` used with `.json.lock` suffix ✅
|
||||
- Lock timeout = 10s ✅
|
||||
- Both JSON and MD writes inside same lock scope ✅
|
||||
- `test_file_lock_prevents_concurrent_write` passes ✅
|
||||
- `test_lock_timeout_returns_error` passes ✅
|
||||
|
||||
### HARDEN-02: Markdown escaping
|
||||
- `_escape_md()` helper escapes `*`, `#`, `[`, `]`, `_`, `` ` `` via regex ✅
|
||||
- Applied to both question and answer in `_build_md_session()` ✅
|
||||
- `test_markdown_escaping` passes ✅
|
||||
- `test_markdown_escaping_cjk` passes ✅
|
||||
- Idempotent (does not double-escape) via regex pattern ✅
|
||||
|
||||
### HARDEN-03: UTC timestamps
|
||||
- `_CST = timezone.utc` (replaced `timedelta(hours=8)`) ✅
|
||||
- `_now_iso()` docstring updated ✅
|
||||
- `test_utc_timestamp` passes (ends with `+00:00`) ✅
|
||||
- No `timedelta(hours=8)` remains in discussion.py ✅
|
||||
|
||||
### HARDEN-04: API key via env var
|
||||
- CLI arg `--paddleocr-key` removed from setupArgs ✅
|
||||
- `PADDLEOCR_API_TOKEN` present in spawn options ✅
|
||||
- `env: { ...process.env, PADDLEOCR_API_TOKEN: ... }` pattern ✅
|
||||
- API key not visible in process listing ✅
|
||||
|
||||
### HARDEN-05: createEl() DOM API
|
||||
- `tree.innerHTML = ...` replaced with `tree.createEl()` chain ✅
|
||||
- `textContent` used instead of innerHTML for user-supplied values ✅
|
||||
- XSS vector from directory names eliminated ✅
|
||||
|
||||
### HARDEN-06: Workspace integrity before /pf-deep
|
||||
- note_path check moved before /pf-deep check ✅
|
||||
- workspace_paths check moved before /pf-deep check ✅
|
||||
- Docstring updated to reflect new ordering ✅
|
||||
- `test_missing_note_blocks_pf_deep` returns "sync" ✅
|
||||
- `test_missing_workspace_path_blocks_pf_deep` returns "sync" ✅
|
||||
|
||||
### HARDEN-07: Empty dicts not null
|
||||
- `lifecycle_level_counts = {}` (was `None`) ✅
|
||||
- `health_aggregate = {}` (was `None`) ✅
|
||||
- `maturity_distribution = {}` (was `None`) ✅
|
||||
- Comment updated ✅
|
||||
- `test_status_json_fallback_when_index_missing` asserts `== {}` ✅
|
||||
|
||||
## Known Stubs
|
||||
|
||||
None identified.
|
||||
|
||||
## Self-Check: PASSED
|
||||
|
||||
- [x] All modified files verified: discussion.py, main.js, asset_state.py, status.py, 3 test files
|
||||
- [x] All 3 commits exist in git log
|
||||
- [x] Tests pass for all modified modules
|
||||
- [x] Full suite: no regressions (2 pre-existing failures unrelated)
|
||||
- [x] JS verification clean
|
||||
|
||||
## Self-Check
|
||||
|
||||
- [x] All modified files verified: discussion.py, main.js, asset_state.py, status.py, 3 test files
|
||||
- [x] All 3 commits exist in git log
|
||||
- [x] Tests pass for all modified modules
|
||||
- [x] Full suite: no regressions (2 pre-existing failures unrelated)
|
||||
- [x] JS verification clean
|
||||
90
.planning/phases/49-module-hardening/49-VERIFICATION.md
Normal file
90
.planning/phases/49-module-hardening/49-VERIFICATION.md
Normal file
|
|
@ -0,0 +1,90 @@
|
|||
---
|
||||
phase: 49-module-hardening
|
||||
plans: 001, 002, 003
|
||||
requirements: HARDEN-01 through HARDEN-07
|
||||
status: PASSED
|
||||
---
|
||||
|
||||
# Phase 49: Module Hardening — Verification Report
|
||||
|
||||
## Summary
|
||||
|
||||
**Result: PASSED** — 7/7 HARDEN requirements verified. All automated checks green.
|
||||
|
||||
| Requirement | Status | Evidence |
|
||||
|-------------|--------|----------|
|
||||
| HARDEN-01 | PASS | filelock.FileLock wraps JSON+MD writes; 2 tests pass |
|
||||
| HARDEN-02 | PASS | _escape_md() escapes 6 special chars; 2 tests pass |
|
||||
| HARDEN-03 | PASS | timezone.utc replaces CST; 1 test passes |
|
||||
| HARDEN-04 | PASS | --paddleocr-key removed, PADDLEOCR_API_TOKEN in env |
|
||||
| HARDEN-05 | PASS | innerHTML removed, createEl() chain present |
|
||||
| HARDEN-06 | PASS | note_path/workspace_paths checked before /pf-deep |
|
||||
| HARDEN-07 | PASS | {} instead of None for 3 dict fields |
|
||||
|
||||
## Automated Test Results
|
||||
|
||||
### Plan 49-001: discussion.py
|
||||
```
|
||||
tests/test_discussion.py::TestRecordSession::test_create_both_files PASSED
|
||||
tests/test_discussion.py::TestRecordSession::test_append_second_session PASSED
|
||||
tests/test_discussion.py::TestRecordSession::test_utc_timestamp PASSED [NEW]
|
||||
tests/test_discussion.py::TestRecordSession::test_markdown_escaping PASSED [NEW]
|
||||
tests/test_discussion.py::TestRecordSession::test_markdown_escaping_cjk PASSED [NEW]
|
||||
tests/test_discussion.py::TestRecordSession::test_file_lock_prevents_concurrent_write PASSED [NEW]
|
||||
tests/test_discussion.py::TestRecordSession::test_lock_timeout_returns_error PASSED [NEW]
|
||||
tests/test_discussion.py::TestRecordSession::test_missing_vault PASSED
|
||||
tests/test_discussion.py::TestRecordSession::test_unknown_key PASSED
|
||||
tests/test_discussion.py::TestRecordSession::test_cjk_encoding PASSED
|
||||
tests/test_discussion.py::TestRecordSession::test_atomic_write_no_partial PASSED
|
||||
tests/test_discussion.py::TestRecordSession::test_cli_invocation PASSED
|
||||
```
|
||||
**12/12 passed** (7 existing + 5 new)
|
||||
|
||||
### Plan 49-002: main.js
|
||||
```
|
||||
JS static verification:
|
||||
--paddleocr-key present: false [OK]
|
||||
PADDLEOCR_API_TOKEN present: true [OK]
|
||||
innerHTML (dir tree): absent [OK]
|
||||
createEl dir-tree present: true [OK]
|
||||
```
|
||||
**4/4 checks passed** (syntax-only verification)
|
||||
|
||||
### Plan 49-003: asset_state.py + status.py
|
||||
```
|
||||
tests/test_asset_state.py ... 28/28 passed (2 new, 1 updated)
|
||||
tests/test_status.py ......... 6/6 passed (1 assertion updated)
|
||||
```
|
||||
**34/34 passed**
|
||||
|
||||
### Full Regression Suite
|
||||
```
|
||||
485 passed, 2 skipped, 2 failed
|
||||
```
|
||||
The 2 failures are pre-existing in `test_ocr_state_machine.py` (unrelated):
|
||||
- `test_retry_exhaustion_becomes_error` — expects "error" but OCR returns "blocked"
|
||||
- `test_full_cycle_from_pending_to_done` — expects "done" but OCR returns "queued"
|
||||
|
||||
**0 regressions from Phase 49 changes.**
|
||||
|
||||
## Verification Commands Executed
|
||||
|
||||
```bash
|
||||
python -m pytest tests/test_discussion.py -v --tb=short
|
||||
python -m pytest tests/test_asset_state.py -v --tb=short
|
||||
python -m pytest tests/test_status.py -v --tb=short
|
||||
python -m pytest tests/ -q --tb=short
|
||||
node -e "const fs=require('fs');const c=fs.readFileSync('paperforge/plugin/main.js','utf-8');..."
|
||||
```
|
||||
|
||||
## Manual Verification Notes
|
||||
|
||||
- **main.js**: Syntax verified via Node.js require. Full Obsidian plugin runtime verification requires user testing in Obsidian.
|
||||
- **No linting violations** observed (ruff check passed for Python files).
|
||||
|
||||
## Gap Items
|
||||
|
||||
| Gap | Severity | Note |
|
||||
|-----|----------|------|
|
||||
| OCR state machine pre-existing failures | Low | Unrelated to Phase 49; existed before changes |
|
||||
| main.js runtime not tested | Low | Cannot automate Obsidian plugin testing in CI |
|
||||
Loading…
Reference in a new issue