199 lines
14 KiB
Markdown
199 lines
14 KiB
Markdown
# M3: Import and Indexing Engine Implementation Plan
|
|
|
|
> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking.
|
|
|
|
**Goal:** Build scanning, validation, hashing, verified-copy primitives plus background worker, progress, and failure capture for managed imports.
|
|
|
|
**Architecture:** Three phases: (1) scan/copy primitives as reusable services in `src/Indexer/`; (2) repository layer for M4 #28 persistence; (3) background worker consuming `import_jobs`. Filesystem behind interfaces for testability.
|
|
|
|
**Tech Stack:** PHP 8.3+, PostgreSQL via Phinx, PSR-3 logging, PSR-4 `MeshVault\`, strict types, PSR-12. No new runtime deps.
|
|
|
|
**Spec:** `notes/backlog-m3-m4.json` issues 20-27 plus 28, and `notes/Roadmap.md`.
|
|
|
|
## Global Constraints
|
|
|
|
- PHP 8.3+, strict_types=1, PSR-12, four spaces, LF, UTF-8 no BOM.
|
|
- New classes `final readonly` where possible; constructor injection.
|
|
- PDO repositories, no ORM.
|
|
- Streams only; never load whole files into memory.
|
|
- `realpath()` checks; reject symlinks, `..`, dest outside library root.
|
|
- SHA-256 streaming, 128 KiB default buffer.
|
|
- Collision suffix `_NNN` before extension.
|
|
- Free-space safety margin 5% default.
|
|
- Library root from validated `AppConfig::libraryPath`.
|
|
- Error codes: `SCAN_EMPTY`, `SCAN_READ_ERROR`, `HASH_MISMATCH`, `DISK_FULL`, `COLLISION`, `SYMLINK_ESCAPE`, `PATH_TRAVERSAL`, `STL_INVALID`, `COPY_FAILED`, `DB_ERROR`.
|
|
- Atomic worker claim on `import_jobs`.
|
|
- Unit plus integration tests; 2+ failure-path tests per feature.
|
|
|
|
## Review Focus
|
|
|
|
1. Path safety vs traversal, symlinks, absolute escapes.
|
|
2. Streaming hash equality with `hash_file()`; mid-read changes.
|
|
3. Atomic publish temp-then-rename; no partial completes.
|
|
4. Worker idempotency on retry.
|
|
5. Disk-full pre-check plus mid-copy cleanup.
|
|
|
|
## Task 1.1 actual scope/status
|
|
|
|
- [x] Implemented only filesystem abstraction and path safety: `FilesystemInterface`,
|
|
`LocalFilesystem`, `PathSafety`, `PathSafetyException`, and unit tests for both services.
|
|
Native-style I/O failures return false; resource returns are PHPDoc and caller-owned.
|
|
Source checks allow existing external sources; destinations stay within the library
|
|
and may have missing suffixes. Relative paths, traversal, ancestor/dangling symlinks,
|
|
root-prefix siblings, and ambiguous Windows names are checked with structured reasons.
|
|
- Validation on local PHP 8.3.35: initial RED 22 missing-class assertion failures;
|
|
Windows alias regression also observed RED before its fix. Final focused suite:
|
|
23 tests, 102 assertions, 5 symlink skips (host cannot create them). `composer test`:
|
|
foundation checks pass; 110 tests, 604 assertions, 7 skips total. New PHP files pass
|
|
PSR-12; full `composer lint` still reports three unrelated existing errors in
|
|
`db/seeds/DefaultDataSeed.php` and `tests/Integration/MigrationTest.php` (not changed).
|
|
- Drive-relative review fix: `ensureSourceSafe()` now rejects any uppercase/lowercase
|
|
drive prefix without a following slash as `INVALID_PATH` before link inspection;
|
|
constructor roots inherit the same check. Existing relative/destination behavior is unchanged.
|
|
Regression RED: all 5 new cases failed with expected missing-rejection failures,
|
|
including realpath-confirmed existing source/root paths with upper/lowercase drives.
|
|
GREEN on Windows/PHP 8.3.35: focused `tests/Unit/Indexer` suite passed with 28 tests,
|
|
531 assertions, 5 symlink skips. `composer test` passed foundation checks and 115 tests,
|
|
1033 assertions, 7 skips. Scoped PHPCS using `phpcs.xml.dist` passed for
|
|
`src/Indexer/Filesystem/PathSafety.php` and `tests/Unit/Indexer/PathSafetyTest.php`.
|
|
Coverage also checks all 52 drive-letter variants with bare, file, and dot-relative suffixes.
|
|
- Limitations: checks are point-in-time, not race-proof; callers must control directory
|
|
mutation around I/O. Symlink behavior needs execution on a host permitting links;
|
|
no Linux runtime validation performed. No later M3 tasks, commits, or delegation.
|
|
|
|
## Task 1.2 actual scope/status
|
|
|
|
- [x] Implemented recursive `FileEnumerator`, `FileEnumeratorInterface`, and separate
|
|
PSR-4 `ScanResult`, `ScannedFile`, `ScanError` readonly DTOs. Manifest entries expose
|
|
canonical absolute `sourcePath`, safe forward-slash `relativePath`, `size`, and
|
|
`mimeType`; totals include only successful entries. Default allowlist is
|
|
stl/jpg/jpeg/png/gif/pdf/txt/md, matched case-insensitively; an explicit empty
|
|
allowlist selects nothing. Empty directories return an empty result without errors.
|
|
- Safety ruling: the global rejection of **all** symlinks overrides any older
|
|
follow-in-root-link wording. File, directory, dangling and ancestor links are
|
|
reported as `SYMLINK_ESCAPE`, even if the link has a disallowed extension, and
|
|
are never traversed. `PathSafety` checks run before canonicalization; the review fix
|
|
below tightens POSIX source spelling. Traversal retains `PATH_TRAVERSAL`. Invalid/missing/non-directory
|
|
roots and per-entry readability, resolution, stat, MIME and directory-read failures
|
|
produce actionable structured `SCAN_READ_ERROR` errors; independent siblings continue.
|
|
- Minimal filesystem API addition: `readDirectory(): array|false` returns entry names
|
|
without dot entries, backed by suppressed native `scandir()` failures. Existing
|
|
metadata calls and injected filesystem doubles cover deterministic I/O failures;
|
|
no new runtime dependencies or container changes. Iterative traversal avoids call
|
|
stack recursion; root depth is zero and the configurable maximum is 128 by default.
|
|
Exceeding it reports an error for that subtree without losing siblings.
|
|
- TDD/validation on Windows, PHP 8.3.35 (commands run from repository root):
|
|
- RED: `php vendor/phpunit/phpunit/phpunit tests/Unit/Indexer/FileEnumeratorTest.php`
|
|
produced 15 expected missing-FileEnumerator assertion failures before implementation.
|
|
- GREEN, same command: 15 tests, 143 assertions, 4 symlink privilege skips.
|
|
- Added direct adapter regression coverage for empty/nested listings, missing paths
|
|
and non-directory reads; enumeration tests cover mixed-case nested defaults,
|
|
disallowed extensions, custom/empty allowlists, invalid roots, six injected
|
|
per-entry failure modes, root failures, sibling continuation and depth limits.
|
|
- `php vendor/squizlabs/php_codesniffer/bin/phpcbf --standard=phpcs.xml.dist src/Indexer tests/Unit/Indexer/FileEnumeratorTest.php tests/Unit/Indexer/LocalFilesystemTest.php`
|
|
fixed generated CRLF and multiline formatting; no behavioral changes.
|
|
- `php vendor/phpunit/phpunit/phpunit tests/Unit/Indexer`: 44 tests, 679 assertions,
|
|
9 skips; success.
|
|
- `composer test`: foundation checks pass; 131 tests, 1181 assertions, 11 skips;
|
|
success (existing HTTP failure-path tests emit expected error log records).
|
|
- `php vendor/squizlabs/php_codesniffer/bin/phpcs --standard=phpcs.xml.dist src/Indexer tests/Unit/Indexer/FileEnumeratorTest.php tests/Unit/Indexer/LocalFilesystemTest.php`:
|
|
passes with no findings. Full-repository lint's previously documented unrelated
|
|
failures were not addressed.
|
|
- Review fixes (task 1.2):
|
|
- Policy ruling: POSIX source paths containing any backslash are rejected as
|
|
`INVALID_PATH` before normalization, including constructor library roots. Windows
|
|
backslash separators remain supported; relative/destination normalization is unchanged.
|
|
`FileEnumerator` now uses the canonical path returned by safety checks for injected
|
|
filesystem operations, retaining the adapter realpath call and its failure handling.
|
|
- Safety failure messages retain reason tokens and scan codes, but add corrective
|
|
guidance for missing, empty/NUL, traversal and symlink paths. Tests now require
|
|
directory-selection guidance rather than merely the phrase "source root".
|
|
- Added five tests: source/constructor platform policy, POSIX backslash-named ancestor
|
|
symlink with a separate slash-spelled decoy tree, traversal root/entry codes, zero-byte
|
|
metadata, and negative-size/empty-MIME sibling continuation. Existing failure doubles
|
|
continue to pass, including injected realpath failures.
|
|
- RED: Windows focused suite failed on missing-root guidance (49 tests, 688 assertions,
|
|
1 failure, 10 skips). A local WSL/PHP 8.5.4 standalone probe using real directories
|
|
and a backslash-named ancestor symlink failed source and constructor rejection and
|
|
demonstrated one accepted target file before the fix. After the fix all three probe
|
|
checks passed, with no accepted files. Probe fixtures were cleaned up; the ignored
|
|
`.phpunit.cache/posix-review-probe.php` remains because shell cleanup was blocked.
|
|
- GREEN on Windows/PHP 8.3.35:
|
|
`php vendor/phpunit/phpunit/phpunit tests/Unit/Indexer`: 49 tests, 722 assertions,
|
|
10 skips (nine host symlink-permission skips and one POSIX-only regression).
|
|
`composer test`: foundation checks pass; 136 tests, 1224 assertions, 12 skips
|
|
(the ten above plus two existing integration skips); expected HTTP error logs only.
|
|
`php vendor/squizlabs/php_codesniffer/bin/phpcs --standard=phpcs.xml.dist src/Indexer tests/Unit/Indexer`:
|
|
exit 0, no findings.
|
|
- Linux PHPUnit was attempted via WSL but cannot run because its PHP lacks dom,
|
|
mbstring and xmlwriter. No extensions were installed; the standalone native POSIX
|
|
RED/GREEN probe is not a substitute for a full Linux suite.
|
|
- Limitations: symlink PHPUnit cases require a link-capable host; Windows skips are
|
|
detailed above. Path checks are point-in-time, not race-proof, so callers must
|
|
control source-tree mutation. No file payload is loaded by the enumerator or opened
|
|
through `readStream`; native fileinfo may inspect content for MIME detection.
|
|
Directory name lists and the returned manifest are materialized in memory (not file
|
|
contents). Scan order is not a public guarantee. No free-space, hashing, copying,
|
|
persistence or worker work; no commits/merges/pushes. Task 1.1 source safety is tightened
|
|
only by the POSIX backslash policy above.
|
|
|
|
## Tasks 1.3-3.4 and delivery layer: actual scope/status
|
|
|
|
- [x] **1.3 SpaceChecker** (`src/Indexer/SpaceChecker.php`) — 5% default margin, injectable
|
|
free-space probe, `DISK_FULL` / `SPACE_CHECK_FAILED` reasons.
|
|
- [x] **1.4 StreamingHasher** (`src/Indexer/StreamingHasher.php`) — SHA-256 in 128 KiB chunks,
|
|
validates the source through `PathSafety` before and after the read.
|
|
- [x] **1.5 StlValidator** (`src/Indexer/StlValidator.php`) — binary and ASCII STL, facet-count
|
|
and terminator checks, `STL_INVALID` / `SCAN_READ_ERROR`.
|
|
- [x] **1.6/1.7 VerifiedCopier** (`src/Indexer/VerifiedCopier.php`) — temp copy, size and hash
|
|
verification, link-based publication, `_NNN` collision suffix, cleanup on failure.
|
|
- [x] **1.8 ImportService** (`src/Indexer/ImportService.php`) — scan, free-space preflight, then
|
|
one atomic manifest+job write. Scan errors become terminal `Failed` items.
|
|
- [x] **Phase 2 repositories** — `CatalogRepositoryInterface`, `ImportJobRepositoryInterface`,
|
|
`PdoRepository`, `PdoCatalogRepository`, `PdoImportJobRepository`. No schema changes.
|
|
- [x] **Phase 3 worker** (`src/Indexer/ImportWorker.php`) — single advisory `flock` over the
|
|
library, `resumeInterrupted()` under that lock, atomic claim, plan-before-publish durability,
|
|
per-item failure isolation, graceful-stop callback that leaves the job `Pending`.
|
|
- [x] **Delivery layer** — `src/Console/Console.php` (argument parsing, exit codes),
|
|
`ConsoleServices`, `ContainerConsoleServices`, `ImportWorkerFactory`, `LazyConsoleServices`,
|
|
`bin/meshvault`, container wiring in `config/container.php`, and `import` / `worker` /
|
|
`worker-once` subcommands in `dev.sh` and `dev.ps1`.
|
|
|
|
### Rulings
|
|
|
|
- **Ruling: no `/import` HTTP route in M3** — the backlog's HTTP surface has no authentication
|
|
yet, and any route accepting a source path would expose arbitrary server filesystem reads to
|
|
unauthenticated callers. Deferred to M6 behind auth. Cost if wrong: the UI trigger lands one
|
|
milestone later; the CLI covers the workflow meanwhile.
|
|
- **Ruling: CLI exit codes are 0 success / 1 usage / 2 runtime** — matches the existing wrappers'
|
|
use of exit 2 for refusal. Cost if wrong: scripted callers need remapping.
|
|
- **Ruling: services are constructed lazily in `bin/meshvault`** — `help` and usage errors must
|
|
not require a database or library directory. Cost if wrong: a misconfiguration surfaces one
|
|
call later, still as exit 2.
|
|
- **Ruling: grouping is by first directory component below the source root**, never inferred from
|
|
file contents. Cost if wrong: flat source trees produce one model per source directory.
|
|
|
|
### Validation (Windows, PHP 8.3.35)
|
|
|
|
- `composer test`: foundation checks pass; **285 tests, 1796 assertions, 18 skips**, no failures.
|
|
- `composer lint`: **exit 0, no findings repository-wide.** The two long-standing errors in
|
|
`db/seeds/DefaultDataSeed.php` and `tests/Integration/MigrationTest.php` were trailing-newline
|
|
violations and are now fixed; no logic in either file changed.
|
|
- `vendor\bin\phpunit tests/Unit/Console`: 22 tests, 71 assertions, 0 skips.
|
|
- `vendor\bin\phpunit --filter TaskRunner tests/Integration`: 1 test, 71 assertions — asserts the
|
|
wrappers forward `import`, `worker` and `worker-once` to `bin/meshvault` with arguments intact.
|
|
- Live CLI exit-code probe: `help`=0, `bogus`=1, no args=1, `import` (no path)=1,
|
|
`worker --sleep=x`=1, `worker-once extra`=1, `import C:\nope` (unconfigured env)=2.
|
|
|
|
### Limitations
|
|
|
|
- **PostgreSQL was not exercised.** Integration tests run against SQLite built from the Phinx
|
|
migrations; `FOR UPDATE SKIP LOCKED` is driver-detected but unverified on a live server.
|
|
`tests/Integration/MigrationTest.php` still skips without DB credentials.
|
|
- **Signals are POSIX-only.** `bin/meshvault` installs `SIGTERM`/`SIGINT` handlers when `pcntl`
|
|
is available; on Windows the worker stops only at `--max-iterations`.
|
|
- Graceful stop is verified between items, not mid-copy.
|
|
- Symlink unit tests skip on hosts that cannot create links (11 skips here).
|
|
- Nothing is committed, merged or pushed; all work is uncommitted in the working tree.
|
|
|