refactor(harness): route workflow persistence through repositories
This commit is contained in:
@@ -1,65 +1,56 @@
|
||||
# Task 3 report — reconnect SSE on same-session Resume
|
||||
# Task 3 report — workflow repository migration
|
||||
|
||||
## Status
|
||||
## RED
|
||||
|
||||
Complete. A successful Resume of the currently active session now replaces its existing
|
||||
`EventSource` connection. Resuming a different session continues to reconnect through the
|
||||
session ID change only, without a generation-driven second connection.
|
||||
- `harness/tests/test_session_repository_workflow.py` initially failed at collection:
|
||||
`persist_verified_finalization` did not exist.
|
||||
- The new gate test initially failed because `write_cte_sql` and `write_final_sql`
|
||||
were not registered. Its first run also exposed the worktree-local missing
|
||||
Node dependency (`typebox`); `npm ci` installed the lockfile dependency.
|
||||
- After the principal/legacy policy was clarified, the resolver tests initially
|
||||
failed because `resolve_principal` did not exist.
|
||||
|
||||
## Implementation
|
||||
## GREEN evidence
|
||||
|
||||
- `useSessionStream` accepts an optional `generation` argument (default `0`) and includes it
|
||||
in the stream effect dependencies. A generation change therefore runs the existing cleanup,
|
||||
closes the old source, and opens the same URL again.
|
||||
- `AppShell` captures whether the requested Resume ID is already active before its existing
|
||||
optimistic state updates. It increments the stream generation only after `resumeSession(id)`
|
||||
succeeds and only for that same-ID case.
|
||||
- The existing optimistic session switch, phase refresh, and failed-Resume rollback remain
|
||||
unchanged. A failed POST cannot increment the generation.
|
||||
- Focused Python regression set: `66 passed`:
|
||||
`test_session_repository_workflow`, `test_session_repository`, session mutation/list/
|
||||
documents/schema-linking, CTE plan/next, decision phase gate, and phase requirement tests.
|
||||
- Gate suite: `127 passed`, including
|
||||
`session-repository-writes.test.js`.
|
||||
- Changed-source Ruff checks pass. `git diff --check` passes.
|
||||
|
||||
## TDD evidence
|
||||
## Implemented boundary
|
||||
|
||||
- RED command:
|
||||
`cd frontend && npx vitest run src/stream/useSessionStream.test.tsx src/shell/AppShell.session-mgmt.test.tsx`
|
||||
- RED result: 2 expected failures and 15 passes. The hook test observed
|
||||
`first.closed === false`; the AppShell test observed one `FakeEventSource` instead of two
|
||||
after the second same-ID Resume.
|
||||
- GREEN focused result: the same command passed 2/2 files and 17/17 tests after the minimal
|
||||
production wiring.
|
||||
- Added `resolve_principal`: PostgreSQL session storage requires trusted
|
||||
`THT_PRINCIPAL_ISSUER` and `THT_PRINCIPAL_SUBJECT`, optional display name, and
|
||||
strict admin parsing (`1`/`true`). It fails closed and never substitutes a local
|
||||
identity. Filesystem storage uses `local_principal()`.
|
||||
- Filesystem repository creates UUIDv4 sessions only and permits safe historical
|
||||
timestamp IDs (`YYYY-MM-DD-HHMMSS`) for read/mutate compatibility. PostgreSQL
|
||||
remains UUIDv4 only.
|
||||
- Phase helpers fold `SessionSnapshot` ledger/artifacts; decision, phase, CTE,
|
||||
session mutation/list/document paths, retrieval-pack persistence, SQL promotion
|
||||
lookup, and task-doc/CTE test helpers gained repository/snapshot paths.
|
||||
- Finalization now publishes report, evidence, and finalized manifest through
|
||||
`repository.finalize`: one PostgreSQL transaction; filesystem writes artifacts
|
||||
before the finalized manifest commit marker. Solved-question indexing stays
|
||||
best-effort after this durable write.
|
||||
- Added `tht cte save --session --name --file -` and
|
||||
`tht sql set-final --session --file -`; Pi tools and SKILL.md now use them.
|
||||
|
||||
## Full verification
|
||||
## Outstanding in-scope migration work
|
||||
|
||||
- Baseline before edits: `cd frontend && npx vitest run` — 42/42 files and 251/251 tests passed.
|
||||
- Focused tests: 2/2 files and 17/17 tests passed.
|
||||
- Full frontend suite: `cd frontend && npx vitest run` — 42/42 files and 253/253 tests passed.
|
||||
- Typecheck: `cd frontend && npx tsc -b` — exit 0.
|
||||
- Production build: `cd frontend && npm run build` — exit 0; Vite transformed 4,835 modules
|
||||
and completed the production bundle.
|
||||
- `git diff --check` — passed.
|
||||
Do not treat this task as complete yet. Remaining direct session path consumers are:
|
||||
|
||||
The suite and build retained the pre-existing MSW unhandled-request, React ref/`act`, Node type
|
||||
stripping, and Vite chunk-size warnings. This task introduced no new warning category.
|
||||
- `harness/tht/cli/memory_cmd.py`: lines 60, 93, 165, 400, 458.
|
||||
- `harness/tht/cli/sql_cmd.py`: `_session_sql_file` at line 254 remains a legacy
|
||||
Path-returning bridge for preview/save/export.
|
||||
- `harness/tht/cli/session_cmd.py:session_dir` remains only as a compatibility
|
||||
bridge for the out-of-scope datamart command and the still-unmigrated memory/
|
||||
SQL consumers; workflow mutations in session_cmd do not call it.
|
||||
|
||||
## Files
|
||||
|
||||
- `frontend/src/stream/useSessionStream.ts`
|
||||
- `frontend/src/stream/useSessionStream.test.tsx`
|
||||
- `frontend/src/shell/AppShell.tsx`
|
||||
- `frontend/src/shell/AppShell.session-mgmt.test.tsx`
|
||||
- `.superpowers/sdd/task-3-report.md`
|
||||
|
||||
## Self-review
|
||||
|
||||
- Confirmed the old EventSource is closed before the replacement is retained by React's effect
|
||||
lifecycle, and the replacement uses the identical session URL.
|
||||
- Confirmed same-ID detection happens before the optimistic `setActiveSessionId(id)` call.
|
||||
- Confirmed the generation increments only after a successful Resume POST; the catch/rollback
|
||||
branch is unchanged.
|
||||
- Confirmed a different ID leaves the generation unchanged, so the existing session-ID effect
|
||||
change creates exactly one replacement connection.
|
||||
- Confirmed the diff is frontend-only apart from this report and contains no backend, Docker,
|
||||
configuration, or session changes.
|
||||
|
||||
## Concerns
|
||||
|
||||
None.
|
||||
The full Python suite has not been conclusively re-run to completion after the
|
||||
latest changes. An earlier root-directory invocation failed only because a
|
||||
pre-existing test expects `workflow.yaml` relative to `harness/`. Full gate tests
|
||||
are green. Full-repo Ruff currently fails on pre-existing test-file lint findings;
|
||||
changed-source Ruff passes.
|
||||
|
||||
Reference in New Issue
Block a user