Repository navigation
Serve governed console workflows and verify protocol traces - #399
Conversation
🚀 Deploying Preview to Cloudflare 🚀Preview URL: https://surface-console-combined-389.spec.contextful.work (commit dcc7979)This URL reflects your latest Preview deploymentPreview Deployments by commit
|
debuggingfuture
changed the title
debuggingfuture
changed the title
![flaredispatch-fractalboxdev[bot]](https://avatars.githubusercontent.com/in/4273665?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI code review — 💬 Comment
Risk tier: full · 0 critical · 3 warnings · 0 suggestions
Reviewers: security 0 · performance 3 · code-quality
1. ⚠️ Warning — Memory recall performs serial remote calls per subject
📍 apps/console/src/answer_live.ts:111-124
Each turn first fetches up to 100 subjects, then performs one 'memory.recall' request per selected subject in a sequential loop. Even with the current cap of three subjects, this adds multiple round trips to every answer and makes latency scale with subject discovery. Batch the recall query if supported, or issue the bounded recalls concurrently with 'Promise.all' and an explicit concurrency limit.
2. ⚠️ Warning — Browse discovery fans out up to 64 describe requests
📍 apps/console/src/browse.ts:68-76
'discover' calls 'listed', which already performs two remote calls, then starts one additional 'context.describe' call per admitted table via 'Promise.all'. A listing with 64 tables therefore produces up to 66 requests in a single user operation and an unbounded burst against the backend. Reuse descriptions from the listing or batch/cache them, and cap concurrency if per-table calls are unavoidable.
3. ⚠️ Warning — Sensitive question and answer are sent unredacted to the model endpoint
📍 apps/console/src/answer_live.ts:65-70
The distillation request forwards the raw question and answer to 'CONTEXTFUL_MODEL_ENDPOINT' before applying the configured denylist. Answers can contain operator/store data, so this creates an external disclosure path that bypasses the console's redaction controls. Redact or otherwise policy-filter both fields before transmission, or ensure this endpoint is an explicitly approved data processor with the required controls.
There was a problem hiding this comment.
AI code review — 🛑 Request changes
Risk tier: full · 1 critical · 2 warnings · 1 suggestions
Reviewers: security
1. ⚠️ Warning — Learning writes block the answer response
📍 apps/console/src/answer_live.ts:126-151
After the live answer is generated, 'learnAfterAnswer' awaits a model distillation request and up to three sequential memory writes before 'turn' returns. This adds multiple network round trips directly to every evidence-bearing query's latency; schedule learning asynchronously or otherwise decouple it from the response path.
2. 💡 Suggestion — Memory recall performs multiple serial remote calls per turn
📍 apps/console/src/answer_live.ts:83-110
Each turn with a session performs 'context.describe', a subject discovery query, and then up to three sequential 'memory.recall' calls. These calls are repeated for every question and the recall loop is not parallelized; batch the recall or issue independent subject recalls concurrently to avoid avoidable latency.
3. 🛑 Critical — Dynamic test-first package checks are not required by the proposed ruleset
📍 .github/rulesets/contextful-gate.proposed.json:21
The ruleset requires only 'flare-dispatch/check:test-first.validate', while the adjacent release-gate documentation says test-first also emits one 'flare-dispatch/check:test-first.package' check per changed package. Those package-specific checks are absent from the required-status list, so a pull request can satisfy the ruleset without running or passing the package test-first checks. Add a mechanism to require the dynamic contexts or change the gate so the required static check represents their result.
4. ⚠️ Warning — Distillation sends unredacted conversation data to the model endpoint
📍 apps/console/src/answer_live.ts:65-72
The distillation request forwards the raw question and full answer to CONTEXTFUL_MODEL_ENDPOINT before applying the configured denylist. This can transmit sensitive identifiers or policy-protected content to an external model and creates an additional copy outside the console's redaction boundary. Redact both fields before this request, or use an explicitly approved privacy-preserving distillation path.
Admin edit saves a validated, version-bound store draft; apply rechecks the owner and claims that draft under an operator attestation whose nonce persists across restarts. Control routes admit only an untenanted Admin grant over `*` and answer `ControlAdminGrantMissing`, `ControlRequestMalformed` and `ControlUnavailable` under their own clauses. Read cancellation reasserts the interrupt past the deadline, and evaluate and formal feature runs provision pinned Lean and Wasm.
6d8a772 to
dcc7979
Compare
There was a problem hiding this comment.
AI code review — 💬 Comment
Risk tier: full · 0 critical · 2 warnings · 1 suggestions
Reviewers: security 1 · performance 2 · code-quality
1. ⚠️ Warning — Admin store listing may disclose backend configuration
📍 apps/console/src/index.ts:348
The '/admin/api/stores' endpoint serializes 'adapters.stores' directly. In the hosted setup this is populated from 'registry.entries', which also supplies backend control endpoints and may contain connection/authuration fields. Return only an explicit public '{id,label}' projection; otherwise any authenticated admin can receive internal store URLs and any sensitive configuration retained on the registry entries.
2. ⚠️ Warning — Install and build the console during every integration test run
📍 crates/acceptance/tests/integration/m12.rs:85-89
The test now runs 'pnpm install --frozen-lockfile' and 'pnpm build' for each invocation of 'm12_console'. This adds substantial dependency installation and compilation overhead (and network/filesystem contention) to the acceptance suite. Build the console once in test setup/CI and reuse the artifact, or make this an explicit packaging test.
3. 💡 Suggestion — Control requests load revocation state twice
📍 crates/contextful-agent/src/http.rs:404-418
'control_request' calls 'admitted', which already loads the revocation state and constructs the admission, then loads revocation again inside its callback before invoking the handler. This doubles revocation-store work for every control request. Reuse the 'Admission' passed to the callback for the admin-grant boundary check, or otherwise pass the already-loaded state through.
Query operators ask cited questions, inspect bounded snapshot previews and retain learning within their credential's scope. Admin operators inspect workflows and audit records, save versioned drafts and apply them with signed attribution. Separate page grants govern reads and changes.
Behavior and assurance
contextful-edgeretains HTTP health and MCP credential admission.Boundary questions
Validation
Local evidence on the head rebased onto
main:contextful-spec lint: 0 findings afterstateandextract.cargo test --locked -p contextful-agent -p contextful-cli -p contextful-context -p contextful-core -p contextful-engine -p contextful-ci --all-features: agent 37, CLI 343, context 322 (4 ignored), core 464, engine 118, CI 183 passing.cargo test --locked -p contextful-acceptance: 22 passing, 2 ignored.apps/console:pnpm typecheckclean;pnpm test96/96.contextful-ci gate --stage test-first --base origin/main: passed in 180.4 s.Control routes admit only an untenanted Admin grant over
*(surface.apply.served-admin-grant); a tenant-scoped Admin grant answersControlAdminGrantMissing.ControlRequestMalformedandControlUnavailableownsurface.apply.served-malformedandsurface.apply.served-unavailable.Closes #31
Closes #387
Closes #383
Closes #386