feat(content): harden 006 rollout gates and start 008 profile isolation#3
feat(content): harden 006 rollout gates and start 008 profile isolation#3
Conversation
AlexU-A
left a comment
There was a problem hiding this comment.
Code Review: PR #3 — Feature 006 Cutover + 008 Profile Isolation + 009 Pipelines
Reviewed file list (78 files) and key security/architecture sections in detail.
Security — Profile Access (profiles/access.ts) — Good
ProfileAccessDeniedErrorwith audit logging tocontentOperationLogsassertProfileAccessOrAudit()is fail-closed: denies access and logs before throwing- Entitlement-based profile ID check is correct
isSessionAccessible()in router.ts checks userId + tenantId (consistent with PR #2's approach)
Architecture — Pipeline Engine (pipelines/) — Well Structured
PipelineEngine: Sequential stage execution with timeout (withTimeout), abort signal support, and retry logicPipelineRunner: Queue with backpressure (PipelineQueueBackpressureError), configurable concurrency (default 2), max queue depth (200)StagePolicyGate: Policy gates on privileged stages — good for the propose-then-execute pattern- Bug triage pilot pipeline with heuristic severity estimation
- Default mode is
dry-run— safe default for pipeline execution
Mediation Router (PR #3 version)
- Uses
isSessionAccessible(session, userId, tenantId)— slightly different from PR #2's three-way check (PR #2 also checks apiKeyId). These PRs will need reconciliation on merge. - Catches
ProfileAccessDeniedErrorwith 403 — correct HTTP status for authorization failure
Testing
- New tests for: module wiring, production provider, profile ingestion queue, profile isolation, tenant isolation, pipeline engine, pipeline runner
- Coverage looks adequate for the new modules
Content Infrastructure
- Feature 006 staging rehearsal scripts with rollback support
- Drizzle migration 0001_fast_shadowcat.sql (213 lines)
- Status consistency CI integration
Concern: PR Scope
This is a large PR touching 78 files across 3 features (006, 008, 009) plus specs 007/010/011. Consider whether future PRs can be scoped more narrowly to ease review.
Merge Note
PR #2 and #3 touch overlapping files (mediation/router.ts, scheduler/index.ts). PR #2 should merge first, then #3 will need a rebase to resolve the session-matching approach (three-way vs two-way check).
No blocking issues. Ready for CODEOWNER merge after PR #2.
|
Closing as superseded. Feature 006 was fully implemented via the spec-kitty workflow directly on main (all 12 WPs merged). The pipelines code in this PR (engine.ts, runner.ts, etc.) is the old custom executor that Feature 011 (PR #33) is actively replacing with Inngest. Spec artifacts and runbooks were incorporated through the proper workflow. |
Summary
cron-parserimport compatibility, generation/search bridge mismatch, PostgreSQL source filter query)status/feature-readiness.json, generated status table, README/ROADMAP/contributing + PR template)Validation
python3 scripts/verify-status-consistency.pypython3 scripts/generate-status-snippets.py --checkcd joyus-ai-mcp-server && npm run typecheckcd joyus-ai-mcp-server && npm test -- --run tests/content/integration tests/pipelinesDATABASE_URL=... ./deploy/scripts/feature-006-staging-rehearsal.shDATABASE_URL=... DO_ROLLBACK=true ./deploy/scripts/feature-006-staging-rehearsal.shdeploy/scripts/feature-006-smoke.shpassing health/auth/happy-path/citationsNotes
not_readypending named staging migration+smoke records and staging soak evidence.