Conversation
|
Merging to
After your PR is submitted to the merge queue, this comment will be automatically updated with its status. If the PR fails, failure details will also be posted here |
Contributor
🤖 CI report
|
| First copy | Second copy | Lines | Tokens |
|---|---|---|---|
packages/agent/packages/agent/src/server/agent-server.ts:3226 |
packages/agent/packages/agent/src/server/agent-server.ts:3642 |
23 | 128 |
✅ Bundle size — no change
Uncompressed size of every built .js bundle, compared against the base branch.
Total: 69.76 MiB · no change
No file changed by more than 1000 B.
Posted automatically by build-bundle-size-report · uncompressed bytes from dist-report
✅ Eager graph — within budget
How much code each root ships on the eager path — downloaded and parsed before the surface is interactive. Measured from the esbuild output chunks (post-tree-shake, static imports only); lazy import() / React.lazy chunks are not counted.
| Root | Eager (shipped) | Δ vs base | Budget |
|---|---|---|---|
entry (logged-out pages, app bootstrap)src/index.tsx |
1.63 MiB · 22 files | no change | █████████░ 88.7% of 1.84 MiB |
logged-out boot: index + App + bootApp (preloaded by every page, including /login)src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts |
3.72 MiB · 660 files | no change | █████████░ 92.3% of 4.03 MiB |
authenticated shell (every logged-in page)src/scenes/AuthenticatedShell.tsx |
7.59 MiB · 2,407 files | no change | █████████░ 91.0% of 8.34 MiB |
dashboard scenesrc/scenes/dashboard/Dashboard.tsx |
9.66 MiB · 3,389 files | no change | ███████░░░ 71.7% of 13.48 MiB |
today home pathsrc/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx |
7.60 MiB · 2,415 files | no change | █████████░ 88.5% of 8.58 MiB |
events scenesrc/scenes/activity/explore/EventsScene.tsx |
9.28 MiB · 3,241 files | no change | ███████░░░ 73.5% of 12.64 MiB |
replay detail scenesrc/scenes/session-recordings/detail/SessionRecordingDetail.tsx |
12.12 MiB · 4,129 files | no change | ████████░░ 77.1% of 15.72 MiB |
🟢 node_modules/monaco-editor/ stays out of src/index.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 [object Object] stays out of src/index.tsx
🟢 node_modules/monaco-editor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/layout/navigation-3000/navigationLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/scenes/dashboard/dashboardLogic.tsx stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/lemon-ui/LemonMarkdown/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/RichContentEditor/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/lib/components/CodeSnippet/ stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 src/taxonomy/core-filter-definitions-by-group.json stays out of src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
🟢 node_modules/monaco-editor/ stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/lib/components/ActivityLog/describers stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/session-recordings/player/sessionRecordingPlayerLogic.ts stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 [object Object] stays out of src/scenes/AuthenticatedShell.tsx
🟢 src/scenes/project-homepage/ai-first/AiFirstHomepage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
🟢 src/scenes/project-homepage/today/TodayReportPage.tsx stays out of src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
Largest files eagerly shipped from src/index.tsx
| Size | File |
|---|---|
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 24.6 KiB | ../node_modules/.pnpm/buffer@6.0.3/node_modules/buffer/index.js |
| 6.3 KiB | ../node_modules/.pnpm/react@18.3.1/node_modules/react/cjs/react.production.min.js |
| 4.5 KiB | ../node_modules/.pnpm/@jspm+core@2.1.0/node_modules/@jspm/core/nodelibs/browser/process.js |
| 3.9 KiB | ../node_modules/.pnpm/scheduler@0.23.2/node_modules/scheduler/cjs/scheduler.production.min.js |
| 1.4 KiB | ../node_modules/.pnpm/base64-js@1.5.1/node_modules/base64-js/index.js |
| 1.3 KiB | src/index.tsx |
| 1.3 KiB | src/RootErrorBoundary.tsx |
| 912 B | ../node_modules/.pnpm/ieee754@1.2.1/node_modules/ieee754/index.js |
| 854 B | src/scenes/ChunkLoadErrorBoundary.tsx |
Largest files eagerly shipped from src/index.tsx + src/scenes/App.tsx + src/scenes/bootApp.ts
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 100.5 KiB | src/lib/api.ts |
| 92.5 KiB | src/products.tsx |
| 69.4 KiB | src/lib/lemon-ui/icons/icons.tsx |
| 40.1 KiB | src/lib/utils/eventUsageLogic.ts |
| 38.7 KiB | ../node_modules/.pnpm/@dnd-kit+core@6.0.8_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@dnd-kit/core/dist/core.esm.js |
| 33.9 KiB | ../node_modules/.pnpm/kea@4.0.0-pre.6_patch_hash=139b8d1f1304f9d9da452a9a1244c94ea679dbcb85687d8999563146879fb6f5_react@18.3.1/node_modules/kea/lib/index.cjs.js |
| 29.0 KiB | ../node_modules/.pnpm/zod@4.3.6/node_modules/zod/v4/core/schemas.js |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 279.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 110.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 100.5 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 92.5 KiB | src/products.tsx |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Largest files eagerly shipped from src/scenes/dashboard/Dashboard.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 279.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 181.8 KiB | src/queries/validators.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 110.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 100.5 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 92.5 KiB | src/products.tsx |
Largest files eagerly shipped from src/scenes/AuthenticatedShell.tsx + src/scenes/project-homepage/ProjectHomepage.tsx + src/scenes/project-homepage/today/TodayHome.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 279.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 110.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 100.5 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 92.5 KiB | src/products.tsx |
| 90.6 KiB | ../node_modules/.pnpm/@tiptap+core@3.20.6_@tiptap+pm@3.20.6/node_modules/@tiptap/core/dist/index.js |
Largest files eagerly shipped from src/scenes/activity/explore/EventsScene.tsx
| Size | File |
|---|---|
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 279.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 181.8 KiB | src/queries/validators.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 110.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 100.5 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
| 92.5 KiB | src/products.tsx |
Largest files eagerly shipped from src/scenes/session-recordings/detail/SessionRecordingDetail.tsx
| Size | File |
|---|---|
| 315.5 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/rrweb.js |
| 306.2 KiB | ../node_modules/.pnpm/posthog-js@1.435.5_@types+react@18.3.27_react@18.3.1/node_modules/posthog-js/dist/module.mjs |
| 279.9 KiB | src/taxonomy/core-filter-definitions-by-group.json |
| 220.3 KiB | ../node_modules/.pnpm/@posthog+icons@0.38.0_react-dom@18.3.1_react@18.3.1__react@18.3.1/node_modules/@posthog/icons/dist/posthog-icons.es.js |
| 181.8 KiB | src/queries/validators.js |
| 153.7 KiB | ../node_modules/.pnpm/re2js@0.4.1/node_modules/re2js/build/index.esm.js |
| 126.8 KiB | ../node_modules/.pnpm/react-dom@18.3.1_react@18.3.1/node_modules/react-dom/cjs/react-dom.production.min.js |
| 110.0 KiB | ../packages/quill/packages/quill/dist/index.js |
| 100.5 KiB | src/lib/api.ts |
| 93.3 KiB | ../node_modules/.pnpm/prosemirror-view@1.40.1/node_modules/prosemirror-view/dist/index.js |
Posted automatically by check-eager-graph · sizes are eager output bytes (shipped, post-tree-shake) from the esbuild metafile · part of #32479
✅ Toolbar bundle — eager 2.20 MiB within budget
What the toolbar ships to customer pages, measured from the esbuild output (minified, post-tree-shake). The eager set is the entry plus everything statically imported from it — fetched before any feature runs; deferred chunks load lazily. The eager guardrail is 5.72 MiB. Each output file must also stay below 10 MB, where CloudFront stops compressing it. The module boundary is enforced separately by check-toolbar-graph.
| Metric | Size | Δ vs base | Budget |
|---|---|---|---|
| Eager (shipped) entry + static imports |
2.20 MiB · 19 files | no change | ████░░░░░░ 38.4% of 5.72 MiB |
| Deferred (lazy) | 2.11 MiB · 44 files | no change | n/a — loads on demand |
Loader dist/toolbar.js |
1.2 KiB | no change | █░░░░░░░░░ 6.0% of 19.5 KiB |
Largest eagerly-shipped chunks
| Size | File |
|---|---|
| 834.2 KiB | dist/toolbar/toolbar-app-LXAUZCP3.css |
| 657.4 KiB | dist/toolbar/chunk-chunk-PDXEE7YG.js |
| 259.4 KiB | dist/toolbar/chunk-chunk-EXS5VGKQ.js |
| 138.2 KiB | dist/toolbar/chunk-chunk-YTISI4G3.js |
| 131.8 KiB | dist/toolbar/chunk-chunk-FDH2IBXT.js |
| 75.2 KiB | dist/toolbar/toolbar-app-6G7F6XL7.js |
| 69.0 KiB | dist/toolbar/chunk-chunk-TSAL54PB.js |
| 35.6 KiB | dist/toolbar/chunk-chunk-U65TWWSR.js |
| 21.0 KiB | dist/toolbar/chunk-chunk-JYRTCG7P.js |
| 6.8 KiB | dist/toolbar/chunk-chunk-DV7IWQNF.js |
Posted automatically by check-toolbar-size · sizes are toolbar output bytes (shipped, post-tree-shake) from the esbuild metafile
✅ Dist folder size — no change
Total size of the built frontend/dist folder (all assets), compared against the base branch.
Total: 959.82 MiB · no change
⚠️ Django migration SQL — 4 new migrations to review
We've detected new migrations on this PR. Review the SQL output for each migration:
products/context_layer/backend/migrations/0005_contextselectionattempt.py
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/anyio/from_thread.py:119: SyntaxWarning: 'return' in a 'finally' block
return result
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
ed = p(logger, meth_name, ed) # type: ignore[arg-type]
2026-10-01T16:25:57.600785Z [error ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=7911 tid=139955385383808
Traceback (most recent call last):
File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
geoip: Optional[GeoIP2] = GeoIP2(cache=8)
~~~~~~^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
raise GeoIP2Exception(
"Path must be a valid database or directory containing databases."
)
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/sshtunnel.py:1040: SyntaxWarning: 'return' in a 'finally' block
return (ssh_host,
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:
WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model ContextSelectionAttempt
--
CREATE TABLE "context_layer_contextselectionattempt" ("id" uuid NOT NULL PRIMARY KEY, "message_id" varchar(128) NOT NULL, "input_hash" varchar(64) NOT NULL, "mode" varchar(16) NOT NULL, "status" varchar(32) NOT NULL, "context" text NOT NULL, "evidence" jsonb NOT NULL, "receipt" jsonb NOT NULL, "created_at" timestamp with time zone NOT NULL, "expires_at" timestamp with time zone NOT NULL, "actor_id" integer NOT NULL, "run_id" uuid NOT NULL, "team_id" integer NOT NULL, CONSTRAINT "context_selection_run_message" UNIQUE ("run_id", "message_id"));
ALTER TABLE "context_layer_contextselectionattempt" ADD CONSTRAINT "context_layer_contex_run_id_3c90dd97_fk_posthog_t" FOREIGN KEY ("run_id") REFERENCES "posthog_task_run" ("id") DEFERRABLE INITIALLY DEFERRED;
CREATE INDEX "context_layer_contextselectionattempt_expires_at_61039b51" ON "context_layer_contextselectionattempt" ("expires_at");
CREATE INDEX "context_layer_contextselectionattempt_actor_id_9ad21048" ON "context_layer_contextselectionattempt" ("actor_id");
CREATE INDEX "context_layer_contextselectionattempt_run_id_3c90dd97" ON "context_layer_contextselectionattempt" ("run_id");
CREATE INDEX "context_layer_contextselectionattempt_team_id_d1eda022" ON "context_layer_contextselectionattempt" ("team_id");
COMMIT;products/context_layer/backend/migrations/0006_contextselectionassignment.py
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
ed = p(logger, meth_name, ed) # type: ignore[arg-type]
2026-10-01T16:26:24.610862Z [error ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=8796 tid=140222099340160
Traceback (most recent call last):
File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
geoip: Optional[GeoIP2] = GeoIP2(cache=8)
~~~~~~^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
raise GeoIP2Exception(
"Path must be a valid database or directory containing databases."
)
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:
WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model ContextSelectionAssignment
--
CREATE TABLE "context_layer_contextselectionassignment" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "mode" varchar(16) NOT NULL, "created_at" timestamp with time zone NOT NULL, "task_id" uuid NOT NULL UNIQUE, "team_id" integer NOT NULL);
CREATE INDEX "context_layer_contextselectionassignment_team_id_aa3cc058" ON "context_layer_contextselectionassignment" ("team_id");
COMMIT;products/context_layer/backend/migrations/0007_contextselectionprojection.py
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
ed = p(logger, meth_name, ed) # type: ignore[arg-type]
2026-10-01T16:26:41.982654Z [error ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=9288 tid=139884292623232
Traceback (most recent call last):
File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
geoip: Optional[GeoIP2] = GeoIP2(cache=8)
~~~~~~^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
raise GeoIP2Exception(
"Path must be a valid database or directory containing databases."
)
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:
WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model ContextSelectionProjection
--
CREATE TABLE "context_layer_contextselectionprojection" ("id" uuid NOT NULL PRIMARY KEY, "version" varchar(64) NOT NULL, "payload" jsonb NOT NULL, "expires_at" timestamp with time zone NOT NULL, "team_id" integer NOT NULL, CONSTRAINT "context_projection_team_version" UNIQUE ("team_id", "version"));
CREATE INDEX "context_layer_contextselectionprojection_expires_at_5a1cdb2f" ON "context_layer_contextselectionprojection" ("expires_at");
CREATE INDEX "context_layer_contextselectionprojection_team_id_4f5b3feb" ON "context_layer_contextselectionprojection" ("team_id");
COMMIT;products/context_layer/backend/migrations/0008_context_selection_search.py
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/structlog/stdlib.py:1166: UserWarning: Remove `format_exc_info` from your processor chain if you want pretty exceptions.
ed = p(logger, meth_name, ed) # type: ignore[arg-type]
2026-10-01T16:26:59.174497Z [error ] Path must be a valid database or directory containing databases. [posthog.exceptions_capture] pid=9792 tid=140225723358080
Traceback (most recent call last):
File "/home/runner/work/posthog/posthog/posthog/geoip.py", line 15, in <module>
geoip: Optional[GeoIP2] = GeoIP2(cache=8)
~~~~~~^^^^^^^^^
File "/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/django/contrib/gis/geoip2.py", line 116, in __init__
raise GeoIP2Exception(
"Path must be a valid database or directory containing databases."
)
django.contrib.gis.geoip2.GeoIP2Exception: Path must be a valid database or directory containing databases.
/opt/hostedtoolcache/Python/3.14.7/x64/lib/python3.14/site-packages/langchain_core/_api/deprecation.py:27: UserWarning: Core Pydantic V1 functionality isn't compatible with Python 3.14 or greater.
from pydantic.v1.fields import FieldInfo as FieldInfoV1
System check identified some issues:
WARNINGS:
?: (axes.W001) You are using the django-axes cache handler for login attempt tracking. Your cache configuration is however invalid and will not work correctly with django-axes. This can leave security holes in your login systems as attempts are not tracked correctly. Reconfigure settings.AXES_CACHE and settings.CACHES per django-axes configuration documentation.
?: (staticfiles.W004) The directory '/home/runner/work/posthog/posthog/frontend/dist' in the STATICFILES_DIRS setting does not exist.
BEGIN;
--
-- Create model ContextSelectionSearchState
--
CREATE TABLE "context_layer_contextselectionsearchstate" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "version" varchar(64) NOT NULL, "archive_id" uuid NOT NULL, "built_at" timestamp with time zone NOT NULL, "refresh_seconds" double precision NOT NULL, "team_id" integer NOT NULL UNIQUE);
--
-- Create model ContextSelectionSearchDocument
--
CREATE TABLE "context_layer_contextselectionsearchdocument" ("id" bigint NOT NULL PRIMARY KEY GENERATED BY DEFAULT AS IDENTITY, "source_kind" varchar(32) NOT NULL, "source_id" varchar(64) NOT NULL, "title" text NOT NULL, "text" text NOT NULL, "revision" varchar(128) NOT NULL, "status" varchar(64) NOT NULL, "reference" text NOT NULL, "tables" jsonb NOT NULL, "search_vector" tsvector NULL, "team_id" integer NOT NULL, CONSTRAINT "context_search_source" UNIQUE ("team_id", "source_kind", "source_id"));
CREATE INDEX "context_layer_contextselectionsearchdocument_team_id_0e737516" ON "context_layer_contextselectionsearchdocument" ("team_id");
CREATE INDEX "context_search_vector_gin" ON "context_layer_contextselectionsearchdocument" USING gin ("search_vector");
COMMIT;Last updated: 2026-10-01 16:27 UTC (d04acca)
✅ Django migration risk — migration analysis complete
We've analyzed your migrations for potential risks.
Summary: 4 Safe | 0 Needs Review | 0 Blocked
✅ Safe
Brief or no lock, backwards compatible
context_layer.0005_contextselectionattempt
└─ #1 ✅ CreateModel
Creating new table is safe
model: ContextSelectionAttempt
│
└──> ℹ️ INFO:
ℹ️ Skipped operations on newly created tables (empty tables
don't cause lock contention).
context_layer.0006_contextselectionassignment
└─ #1 ✅ CreateModel
Creating new table is safe
model: ContextSelectionAssignment
│
└──> ℹ️ INFO:
ℹ️ Skipped operations on newly created tables (empty tables
don't cause lock contention).
context_layer.0007_contextselectionprojection
└─ #1 ✅ CreateModel
Creating new table is safe
model: ContextSelectionProjection
│
└──> ℹ️ INFO:
ℹ️ Skipped operations on newly created tables (empty tables
don't cause lock contention).
context_layer.0008_context_selection_search
└─ #1 ✅ CreateModel
Creating new table is safe
model: ContextSelectionSearchState
└─ #2 ✅ CreateModel
Creating new table is safe
model: ContextSelectionSearchDocument
│
└──> ℹ️ INFO:
ℹ️ Skipped operations on newly created tables (empty tables
don't cause lock contention).
Last updated: 2026-10-01 16:27 UTC (d04acca)
Contributor
|
[Critical risk] Adds database schema and API endpoints for AI context selection. The PR should not merge until cross-run history isolation, retry evidence, and selection deadline enforcement are fixed. Reviews (1) · Last reviewed commit: "feat(ai): add gated context selection fo..." |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: PostHog/posthog/.coderabbit.yaml Review profile: QUIET Plan: Enterprise Run ID: 📒 Files selected for processing (4)
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds context selection for eligible Cloud Claude, Codex, and Pi runs. It adds eligibility state, task-persistent experiment assignments, cached source projections, scoped candidate selection, and preparation and receipt APIs. Cloud and Pi agents can inject selected context and record delivery outcomes. The changes also add selection evidence and task export, expiry cleanup, tests, and documentation of eligibility, limits, retention, and receipt interpretation. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to For allowlisted projects, an eligibility lookup failure can delay or prevent task startup. Make this optional experiment fail closed before merging. Pi receipt completeness also needs confirmation against native event ordering; retries now have an explicit baseline fallback contract. Security Architecture ReviewSecurity architecture risk: 🟡 Moderate · up to Automatic use of project knowledge and additional retention of submitted requests create a meaningful security review surface. Access checks limit exposure, but interrupted requests can undermine the association between requests and delivery records, and some recovery behavior remains unverified. Retained concerns
Security review detailsSecurity Blast Radius
Security Findings and Attack Paths
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
🚥 Pre-merge checks | ❌ 1❌ Failed checks (1 warning)
Full details: Description checkExplanation The description includes the required sections, testing rationale, release status, documentation update, agent context, and flow diagrams. However, it materially conflicts with the provided diff and objectives: it states that Pi context selection and projection, archive, refresh, and receipt machinery were removed, while the changes add those components and their tests. This makes the current scope and shipped behavior inaccurate. Resolution Update the description to match the actual diff. Describe the added Pi integration and the projection, archive, refresh, receipt, export, migration, and retention components, or remove those changes from the PR. Reconcile the testing and rollout statements with the final implementation. ✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: b4b84de5-e11a-4eed-8029-ccc1d5ff48b4
📒 Files selected for processing (34)
docs/published/handbook/engineering/ai/sandboxed-agents.mdpackages/agent/packages/agent-contracts/src/domain-types.tspackages/agent/packages/agent/src/context-selection/schemas.tspackages/agent/packages/agent/src/posthog-api.tspackages/agent/packages/agent/src/server/agent-server.tspackages/agent/packages/agent/src/server/cloud-prompt.tspackages/agent/packages/agent/src/server/context-selection.test.tspackages/agent/packages/agent/src/server/context-selection.tsposthog/egress/typesafe/README.mdposthog/settings/web.pyposthog/tasks/scheduled.pyproducts/business_knowledge/backend/facade/__init__.pyproducts/context_layer/backend/facade/api.pyproducts/context_layer/backend/management/__init__.pyproducts/context_layer/backend/management/commands/__init__.pyproducts/context_layer/backend/management/commands/export_context_selections.pyproducts/context_layer/backend/migrations/0005_contextselectionattempt.pyproducts/context_layer/backend/migrations/0006_contextselectionassignment.pyproducts/context_layer/backend/migrations/0007_contextselectionprojection.pyproducts/context_layer/backend/migrations/max_migration.txtproducts/context_layer/backend/models.pyproducts/context_layer/backend/routes.pyproducts/context_layer/backend/selection_export.pyproducts/context_layer/backend/selection_model.pyproducts/context_layer/backend/selection_search.pyproducts/context_layer/backend/selection_service.pyproducts/context_layer/backend/selection_sources.pyproducts/context_layer/backend/selection_types.pyproducts/context_layer/backend/selection_views.pyproducts/context_layer/backend/tasks.pyproducts/context_layer/backend/test/test_selection.pyproducts/tasks/backend/facade/api.pyproducts/tasks/backend/temporal/process_task/activities/get_task_processing_context.pytach.toml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
adboio
added
the
desktop-skip-backend-check
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Note
Quiet mode is enabled, so only the most important comments were posted inline. Other review comments are grouped below.
🟡 Other comments (3)
packages/agent/packages/agent/src/server/agent-server.ts-2823-2838 (1)
2823-2838: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRetries call
prepareagain with the samemessage_id.Each retry with a non-null
contextMessageIdcallsprepareContextSelectionwith the same message ID. The server enforces uniqueness on(run, message_id), so the second call returns the duplicate result with no context. The test confirms this behavior: the "duplicate" mock producescontext_included=false. A connection-error retry re-sends the original prompt without the treatment context. The treatment arm then loses context on retried turns, and the experiment evidence shows those turns as not exposed. If this behavior is intended, document it. Otherwise, reuse the first prepared selection across the retry attempts.packages/agent/packages/agent/src/pi/context-selection.ts-102-155 (1)
102-155: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winA failed
preparePromptthrow leaves the pending entry consumed with no exposure record.
this.pending.splice(index, 1)andpersistPending()run beforepreparePrompt.preparePromptcatches its own API errors, butctx.getSystemPrompt(),hash(prompt)insidepreparePrompt, orpersistPending()(appendCustomEntry) can still throw. In that case the outercatchreturns the original messages.this.exposednever receiveskey, and the input ID is already removed. On the model retry for the same turn, the handler finds no pending entry. If a second identical queued prompt exists, the retry consumes that prompt's ID instead. The comment on line 149 says this case must be prevented.Record the exposure as soon as the pending entry is spliced. Set it before any call that can throw.
Proposed fix
if (index >= 0 && !this.exposed.has(key)) { const [input] = this.pending.splice(index, 1); + this.exposed.set(key, null); this.persistPending();packages/agent/packages/agent/src/pi/context-selection.ts-143-143 (1)
143-143: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winA second model-context event overwrites
this.activebefore the earlier delivery is finished.Line 143 replaces
this.activewith no check for an existing value. If a new pending input is matched beforeturn_endoragent_settledruns, the earlier delivery never receives itscompletedorfailedreceipt. Its evidence then stays in thedispatchingstate. Before you assign the new delivery, finish the existing one as failed.Proposed fix
+ const previous = this.active; + if (previous) + await previous.finish({ stopReason: "superseded" }, true); this.active = delivery;
🧹 Nitpick comments (2)
products/context_layer/backend/selection_views.py (1)
137-145: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
data.pop("run_id")changes the validated data inside the worker thread.
executeruns on the executor and changesvalidated_datain place. Copy the data beforebounded_requestand pass the copy.Fix
- data = serializer.validated_data + data = dict(serializer.validated_data) + run_id = data.pop("run_id") def execute(deadline: float) -> Response: - run = self._run(request, data.pop("run_id")) + run = self._run(request, run_id)posthog/tasks/scheduled.py (1)
271-278: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMove the import to module level.
The guideline requires module-level imports. Other tasks in this file are imported at module level.
As per coding guidelines: "Always place imports at the top of the file (module level), never inside functions or methods (local imports)".Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: 0bd12fb4-7fde-4efb-a9ab-b9c6009c7ba0
📒 Files selected for processing (42)
docs/published/handbook/engineering/ai/sandboxed-agents.mdpackages/agent/packages/agent-contracts/src/domain-types.tspackages/agent/packages/agent/src/context-selection/schemas.tspackages/agent/packages/agent/src/pi/context-selection.test.tspackages/agent/packages/agent/src/pi/context-selection.tspackages/agent/packages/agent/src/pi/rpc-client.tspackages/agent/packages/agent/src/pi/rpc-host.tspackages/agent/packages/agent/src/posthog-api.tspackages/agent/packages/agent/src/server/agent-server.test.tspackages/agent/packages/agent/src/server/agent-server.tspackages/agent/packages/agent/src/server/cloud-prompt.tspackages/agent/packages/agent/src/server/context-selection.test.tspackages/agent/packages/agent/src/server/context-selection.tspackages/agent/packages/agent/src/server/pi-agent-server.test.tspackages/agent/packages/agent/src/server/pi-agent-server.tsposthog/egress/typesafe/README.mdposthog/settings/web.pyposthog/tasks/scheduled.pyproducts/context_layer/backend/facade/api.pyproducts/context_layer/backend/management/__init__.pyproducts/context_layer/backend/management/commands/__init__.pyproducts/context_layer/backend/management/commands/export_context_selections.pyproducts/context_layer/backend/migrations/0005_contextselectionattempt.pyproducts/context_layer/backend/migrations/0006_contextselectionassignment.pyproducts/context_layer/backend/migrations/0007_contextselectionprojection.pyproducts/context_layer/backend/migrations/max_migration.txtproducts/context_layer/backend/models.pyproducts/context_layer/backend/routes.pyproducts/context_layer/backend/selection_execution.pyproducts/context_layer/backend/selection_export.pyproducts/context_layer/backend/selection_model.pyproducts/context_layer/backend/selection_receipts.pyproducts/context_layer/backend/selection_search.pyproducts/context_layer/backend/selection_service.pyproducts/context_layer/backend/selection_sources.pyproducts/context_layer/backend/selection_types.pyproducts/context_layer/backend/selection_views.pyproducts/context_layer/backend/tasks.pyproducts/context_layer/backend/test/test_selection.pyproducts/tasks/backend/facade/api.pyproducts/tasks/backend/temporal/process_task/activities/get_task_processing_context.pytach.toml
💤 Files with no reviewable changes (2)
- products/context_layer/backend/management/commands/init.py
- products/context_layer/backend/management/init.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 9 remain after this review.
Comment on lines
+1356
to
+1360
| state_updates["context_selection_eligible"] = context_layer_facade.context_selection_enabled_for_run( | ||
| task_run.team_id, | ||
| task_run.id, | ||
| actor_user or task.created_by, | ||
| ) |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make the eligibility check fail closed.
context_selection_enabled_for_run does not catch errors. It runs a DB lookup and calls get_feature_flag_or_none. If either call raises, get_task_processing_context raises before the run starts. A staff-only experiment can then block every allowlisted run. Nearby flag checks in this file catch exceptions, log them, and return False. Handle this check the same way.
Proposed fix
- state_updates["context_selection_eligible"] = context_layer_facade.context_selection_enabled_for_run(
- task_run.team_id,
- task_run.id,
- actor_user or task.created_by,
- )
+ try:
+ context_selection_eligible = context_layer_facade.context_selection_enabled_for_run(
+ task_run.team_id,
+ task_run.id,
+ actor_user or task.created_by,
+ )
+ except Exception as e:
+ log_with_activity_context("context_selection_eligibility_failed", run_id=run_id, error=str(e))
+ context_selection_eligible = False
+ state_updates["context_selection_eligible"] = context_selection_eligible📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| state_updates["context_selection_eligible"] = context_layer_facade.context_selection_enabled_for_run( | |
| task_run.team_id, | |
| task_run.id, | |
| actor_user or task.created_by, | |
| ) | |
| try: | |
| context_selection_eligible = context_layer_facade.context_selection_enabled_for_run( | |
| task_run.team_id, | |
| task_run.id, | |
| actor_user or task.created_by, | |
| ) | |
| except Exception as e: | |
| log_with_activity_context("context_selection_eligibility_failed", run_id=run_id, error=str(e)) | |
| context_selection_eligible = False | |
| state_updates["context_selection_eligible"] = context_selection_eligible |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
products/context_layer/backend/selection_service.py (1)
129-130: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
evidence["model"]can differ from the model that actually runs.
preparereadssettings.HOGQL_PROMPT_JEV_MODELonce.model_requestandSelectionJudge.judgeread the same setting again later. The setting is static per process, so the values match in practice. The model is still resolved in three places. Resolve it once per attempt and pass it down, so the evidence always matches the request.The docs say evidence retains the requested model and the actual model returned by System One.
evidence["model"]records only the requested model. The actual model appears only in the per-callresponseevidence.products/context_layer/backend/selection_model.py (1)
56-85: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
judgeduplicates request construction already inmodel_request.
judgerebuildsstate,question, and the candidate branch.model_requestdoes the same, andrequest_descriptorcallsmodel_requestagain to hash it. Each judgment therefore builds the request twice. The send path (client.decide) and the hashed path can drift apart.Extract a helper that returns
(state, question). Use it in bothmodel_requestandjudge. This also removes the duplicatedGATE if candidate is None else RELEVANCEchoice.products/context_layer/backend/test/test_selection.py (1)
280-289: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTest depends on the gateway-wrapped error type.
The test asserts
error_type == "SystemOneRequestFailed"after patchinghttpx.Client.post. This ties the test to the System One client's exception wrapping. The test still checks the stated behavior: a failed call keeps request evidence and leavesprobabilityunset. Keep the assertion if that wrapping is a stable contract ofbuild_system_one_client. Otherwise assert only thaterror_typeis present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: PostHog/posthog/.coderabbit.yaml
Review profile: QUIET
Plan: Enterprise
Run ID: dcb7f86a-fe65-4647-a95c-8521b2c928e0
📒 Files selected for processing (6)
docs/published/handbook/engineering/ai/sandboxed-agents.mdposthog/egress/typesafe/README.mdposthog/settings/web.pyproducts/context_layer/backend/selection_model.pyproducts/context_layer/backend/selection_service.pyproducts/context_layer/backend/test/test_selection.py
💤 Files with no reviewable changes (2)
- posthog/egress/typesafe/README.md
- posthog/settings/web.py
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 8 remain after this review.
adboio
changed the title
Contributor
🦔 PostHog Review reviewed this pull requestFound 2 must fix, 4 should fix, 4 consider. Published 10 findings (view the review). |
Contributor
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
![posthog[bot]](https://avatars.githubusercontent.com/in/1203049?s=60&v=4){kind=small}
Comment on lines
+202
to
+205
| rendered = render(scored, selection_id) | ||
| observation["decisions"] = rendered.decisions | ||
| observation["selected_ids"] = rendered.selected_ids | ||
| return rendered.context, "selected" if rendered.selected_ids else "empty" |
Contributor
There was a problem hiding this comment.
Use the same context length limit on the server and agent
Issue description
render limits context with Python len, which counts Unicode code points. The agent's contextSelectionResponseSchema uses z.string().max(8_000), which counts UTF-16 units. These limits differ for emoji and other non-BMP characters. Two valid source descriptions containing 2,500 emoji each produced 5,668 server characters but 10,668 JavaScript units. The agent rejects that response and delivers the prompt without context, although selection reports success.
Why we think it's a valid issue
- Checked:
products/context_layer/backend/selection_search.py(render),products/context_layer/backend/selection_types.py:14-15(MAX_CONTEXT_CHARS,MAX_ITEMS),products/context_layer/backend/selection_sources.py:27,59,199(source text truncation),products/context_layer/backend/selection_views.py:51-57(PreparedContextSerializer),packages/agent/packages/agent/src/context-selection/schemas.ts,packages/agent/packages/agent/src/posthog-api.ts:107-125(prepareContextSelection), andpackages/agent/packages/agent/src/server/context-selection.ts:76-104(preparePrompt). - Found:
renderadds a block only whilelen(header + body + block + footer) <= MAX_CONTEXT_CHARS, which is 8,000. Pythonlencounts code points.json.dumps(..., ensure_ascii=False)keeps non-BMP characters such as most pictographic emoji as single code points.PreparedContextSerializer.contexthas nomax_length, so the server sends the string as it is. - Found: The agent parses the response with
z.string().max(8_000)(schemas.ts:6). Zod checks JavaScript.length, which counts UTF-16 code units, and each non-BMP character counts as 2. A server context of N code points that contains k non-BMP characters fails the parse when N + k > 8,000. - Found: The trigger is plausible, not just theoretical. Source text can be up to 2,800 code points (
selection_sources.py:27). The greedy renderer then fills the space left with shorter candidates, up to 5 items, so rendered contexts often end close to 8,000. Then a modest number of non-BMP characters in a skill or knowledge document can push the JavaScript length over the limit. - Found: When the parse fails,
contextSelectionResponseSchema.parsethrows insideprepareContextSelection.preparePromptcatches the error, reportsprepare_failedwithout aselection_id, and returns the original prompt (context-selection.ts:95-103). The server already captured the$ai_spanwithreason: "selected"and the full context (selection_service.py:109-129). - Impact: In the treatment arm, the affected turns silently lose the selected context. The selection span still says the context was selected, so the experiment's evidence counts these turns as treated when the model never received the references. The failure is fail-safe: the prompt still arrives, and nothing crashes. The repository already counts UTF-16 units this way at
posthog/api/services/query.py:286. - Priority: Lowered to
consider. The mismatch is real, but it needs content close to the budget with many non-BMP characters. It fails safe, and it affects only a staff-only experiment for allowlisted teams.
Suggested fix
Make the renderer and agent schema use the same length unit. The server can count UTF-16 units before adding each complete reference, following the existing pattern in posthog/api/services/query.py. Alternatively, make the client schema count Unicode code points. Verify the response contract with non-BMP characters.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_service.py#L202-205
<issue_description>
`render` limits context with Python `len`, which counts Unicode code points. The agent's `contextSelectionResponseSchema` uses `z.string().max(8_000)`, which counts UTF-16 units. These limits differ for emoji and other non-BMP characters. Two valid source descriptions containing 2,500 emoji each produced 5,668 server characters but 10,668 JavaScript units. The agent rejects that response and delivers the prompt without context, although selection reports success.
</issue_description>
<issue_validation>
- **Checked:** `products/context_layer/backend/selection_search.py` (`render`), `products/context_layer/backend/selection_types.py:14-15` (`MAX_CONTEXT_CHARS`, `MAX_ITEMS`), `products/context_layer/backend/selection_sources.py:27,59,199` (source text truncation), `products/context_layer/backend/selection_views.py:51-57` (`PreparedContextSerializer`), `packages/agent/packages/agent/src/context-selection/schemas.ts`, `packages/agent/packages/agent/src/posthog-api.ts:107-125` (`prepareContextSelection`), and `packages/agent/packages/agent/src/server/context-selection.ts:76-104` (`preparePrompt`).
- **Found:** `render` adds a block only while `len(header + body + block + footer) <= MAX_CONTEXT_CHARS`, which is 8,000. Python `len` counts code points. `json.dumps(..., ensure_ascii=False)` keeps non-BMP characters such as most pictographic emoji as single code points. `PreparedContextSerializer.context` has no `max_length`, so the server sends the string as it is.
- **Found:** The agent parses the response with `z.string().max(8_000)` (`schemas.ts:6`). Zod checks JavaScript `.length`, which counts UTF-16 code units, and each non-BMP character counts as 2. A server context of N code points that contains k non-BMP characters fails the parse when N + k > 8,000.
- **Found:** The trigger is plausible, not just theoretical. Source text can be up to 2,800 code points (`selection_sources.py:27`). The greedy renderer then fills the space left with shorter candidates, up to 5 items, so rendered contexts often end close to 8,000. Then a modest number of non-BMP characters in a skill or knowledge document can push the JavaScript length over the limit.
- **Found:** When the parse fails, `contextSelectionResponseSchema.parse` throws inside `prepareContextSelection`. `preparePrompt` catches the error, reports `prepare_failed` without a `selection_id`, and returns the original prompt (`context-selection.ts:95-103`). The server already captured the `$ai_span` with `reason: "selected"` and the full context (`selection_service.py:109-129`).
- **Impact:** In the treatment arm, the affected turns silently lose the selected context. The selection span still says the context was selected, so the experiment's evidence counts these turns as treated when the model never received the references. The failure is fail-safe: the prompt still arrives, and nothing crashes. The repository already counts UTF-16 units this way at `posthog/api/services/query.py:286`.
- **Priority:** Lowered to `consider`. The mismatch is real, but it needs content close to the budget with many non-BMP characters. It fails safe, and it affects only a staff-only experiment for allowlisted teams.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Make the renderer and agent schema use the same length unit. The server can count UTF-16 units before adding each complete reference, following the existing pattern in `posthog/api/services/query.py`. Alternatively, make the client schema count Unicode code points. Verify the response contract with non-BMP characters.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Changed the agent response schema to count Unicode code points, matching the backend renderer. Added acceptance/rejection coverage at the 8,000-code-point boundary using non-BMP characters. This also addresses the duplicate comment on the client schema.
| export const contextSelectionResponseSchema = z | ||
| .object({ | ||
| selection_id: z.string().max(128), | ||
| context: z.string().max(8_000), |
Contributor
There was a problem hiding this comment.
Use the same character-count limit as the backend
Issue description
The backend renderer limits context with Python len(), which counts Unicode code points. Zod's max() uses JavaScript string.length, which counts UTF-16 code units. Emoji and other supplementary characters therefore count twice here. A backend-rendered response with 8,000 code points and three emoji has 8,003 code units and fails validation. preparePrompt catches that failure and silently drops valid treatment context.
Why we think it's a valid issue
- Checked: The backend renderer
products/context_layer/backend/selection_search.py, the limits inselection_types.py, the source truncation inselection_sources.py, the response serializer inselection_views.py, the client parse inpackages/agent/packages/agent/src/posthog-api.ts, and the catch path inpackages/agent/packages/agent/src/server/context-selection.ts. - Found:
selection_search.py:47does the budget check with Pythonlen(header + body + block + footer) > MAX_CONTEXT_CHARS, andMAX_CONTEXT_CHARS = 8_000(selection_types.py:14). Pythonlen()counts code points. - Found:
selection_search.py:44serializes each record withjson.dumps(..., ensure_ascii=False). Supplementary characters such as emoji therefore stay as literal characters in the context string. They are not\uXXXXescapes. In a JS string, each one has a.lengthof 2. - Found:
PreparedContextSerializer.contextinselection_views.py:53has nomax_length. Nothing on the server side caps the context in UTF-16 units. The only check of that kind isz.string().max(8_000)atschemas.ts:6. - Found: The packer is greedy and does not stop early. When a block does not fit, it marks it
character_budgetand continues with smaller blocks. Each source text can have up to 2,800 characters (selection_sources.py:27). For these reasons, a treatment context can end near 8,000 code points. - Found:
posthog-api.ts:124callscontextSelectionResponseSchema.parse(response). On a parse error, the code atcontext-selection.ts:95-101catches it, sends a debug-levelprepare_failedevent that has no reason, and sends the prompt without context. - Impact: The trigger is a treatment context that is within N code points of 8,000 and contains N or more supplementary characters, for example emoji in team skills or Business Knowledge documents. Then a valid response fails validation, and the agent drops the hidden context for that turn. Ordinary delivery continues, so users see no error. But the backend span records a delivered bundle that the model never receives. The debug log does not show that the cause is a length-unit mismatch.
- Impact: The requests in the other direction are safe.
userText.slice(-20_000)and the historyslice(-12_000)measure UTF-16 units, so they never exceed the code-point limits on the backend. - Priority: Lower to
consider. The failure needs a context near the cap that also contains supplementary characters. The fallback keeps normal delivery. The feature is behind a flag with an allowlist. The fix is still cheap: count code points, or loosen the client cap, because the server already applies the limit.
Suggested fix
Validate the limit in Unicode code points, for example with z.string().refine(value => Array.from(value).length <= 8_000). Keep the backend limit unchanged. Verify that a response containing exactly 8,000 code points, including supplementary characters, passes validation.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/context-selection/schemas.ts#L6
<issue_description>
The backend renderer limits context with Python len(), which counts Unicode code points. Zod's max() uses JavaScript string.length, which counts UTF-16 code units. Emoji and other supplementary characters therefore count twice here. A backend-rendered response with 8,000 code points and three emoji has 8,003 code units and fails validation. preparePrompt catches that failure and silently drops valid treatment context.
</issue_description>
<issue_validation>
- **Checked:** The backend renderer `products/context_layer/backend/selection_search.py`, the limits in `selection_types.py`, the source truncation in `selection_sources.py`, the response serializer in `selection_views.py`, the client parse in `packages/agent/packages/agent/src/posthog-api.ts`, and the catch path in `packages/agent/packages/agent/src/server/context-selection.ts`.
- **Found:** `selection_search.py:47` does the budget check with Python `len(header + body + block + footer) > MAX_CONTEXT_CHARS`, and `MAX_CONTEXT_CHARS = 8_000` (`selection_types.py:14`). Python `len()` counts code points.
- **Found:** `selection_search.py:44` serializes each record with `json.dumps(..., ensure_ascii=False)`. Supplementary characters such as emoji therefore stay as literal characters in the context string. They are not `\uXXXX` escapes. In a JS string, each one has a `.length` of 2.
- **Found:** `PreparedContextSerializer.context` in `selection_views.py:53` has no `max_length`. Nothing on the server side caps the context in UTF-16 units. The only check of that kind is `z.string().max(8_000)` at `schemas.ts:6`.
- **Found:** The packer is greedy and does not stop early. When a block does not fit, it marks it `character_budget` and continues with smaller blocks. Each source text can have up to 2,800 characters (`selection_sources.py:27`). For these reasons, a treatment context can end near 8,000 code points.
- **Found:** `posthog-api.ts:124` calls `contextSelectionResponseSchema.parse(response)`. On a parse error, the code at `context-selection.ts:95-101` catches it, sends a debug-level `prepare_failed` event that has no reason, and sends the prompt without context.
- **Impact:** The trigger is a treatment context that is within N code points of 8,000 and contains N or more supplementary characters, for example emoji in team skills or Business Knowledge documents. Then a valid response fails validation, and the agent drops the hidden context for that turn. Ordinary delivery continues, so users see no error. But the backend span records a delivered bundle that the model never receives. The debug log does not show that the cause is a length-unit mismatch.
- **Impact:** The requests in the other direction are safe. `userText.slice(-20_000)` and the history `slice(-12_000)` measure UTF-16 units, so they never exceed the code-point limits on the backend.
- **Priority:** Lower to `consider`. The failure needs a context near the cap that also contains supplementary characters. The fallback keeps normal delivery. The feature is behind a flag with an allowlist. The fix is still cheap: count code points, or loosen the client cap, because the server already applies the limit.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Validate the limit in Unicode code points, for example with z.string().refine(value => Array.from(value).length <= 8_000). Keep the backend limit unchanged. Verify that a response containing exactly 8,000 code points, including supplementary characters, passes validation.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Changed the schema to validate Unicode code points rather than UTF-16 units. Exactly 8,000 emoji now pass and 8,001 fail. This is the same fix as the server-side length-limit finding.
Comment on lines
+3566
to
+3574
| const result = await this.promptWithUpstreamRetry( | ||
| { | ||
| sessionId: acpSessionId, | ||
| prompt: builtPrompt.prompt, | ||
| ...(builtPrompt.meta ? { _meta: builtPrompt.meta } : {}), | ||
| }, | ||
| true, | ||
| builtPrompt.messageId, | ||
| ); |
Contributor
There was a problem hiding this comment.
Supply prior conversation to selection during native resume
Issue description
For a native resume in a fresh process, sendResumeContinuation supplies only the pending human prompt. Deferred prewarmed native resumes also use the human prompt without a history block. ContextSelection starts empty and extracts restored history only from hidden prompt blocks, so both paths submit empty history. The prior conversation exists in resumeState, but selection cannot use it to interpret requests such as "Does that definition still apply?".
Why we think it's a valid issue
- Checked:
prepareNativeResume,sendResumeContinuation,runResumeTurn,promptWithUpstreamRetry,preparePrewarmedResumePrompt,sendResumeMessageandwrapPromptWithSummaryResumeinagent-server.ts. AlsoContextSelection.dispatchandpreparePromptincontext-selection.ts, and the use ofselection.historyinproducts/context_layer/backend/selection_service.py. - Found: The prior conversation is in memory during a native resume.
prepareNativeResumecallsloadResumeStatebefore it picks the native path (agent-server.ts:1123-1124). For Claude, the native path is the default when a prior session ID exists and the session JSONL hydrates (agent-server.ts:1150-1175). For Codex, it is the default on a warm sandbox (agent-server.ts:1135-1148). - Found:
sendResumeContinuationbuilds its prompt frompendingUserPrompt.promptonly and adds no hidden history block (agent-server.ts:3416-3424).runResumeTurnsends it throughpromptWithUpstreamRetry, and that function callscontextSelection.dispatch(..., request.prompt)(agent-server.ts:2834-2848). - Found: For a deferred prewarmed native resume,
preparePrewarmedResumePromptreturns the prompt unchanged (agent-server.ts:3301-3308). The user-message handler then passes that prompt todispatchashumanPrompt(agent-server.ts:1616-1635). - Found:
dispatchgets restored history only from hidden blocks (context-selection.ts:restoredHistory: text(humanPrompt.filter(isHidden))).preparePromptclearsthis.historywhenhistoryRunIdchanges. As a result, both native paths sendhistory: ""withhistory_source: "resume_prompt". - Found: The summary paths put
formatConversationForResume(resumeState.conversation)in a hidden block (agent-server.ts:3238-3243,3336-3342), so selection gets the prior conversation there. The test atcontext-selection.test.ts:123-124covers only that path. - Found: The backend uses
selection.historyin the gate (selection_service.py:149), the search terms (selection_service.py:165) and each rerank call (selection_service.py:182). - Impact: On the first human turn after a native resume, which is the normal resume for Claude, all three backend stages see only the new prompt. A follow-up that refers to earlier turns cannot be resolved. The gate can skip that turn, or search can retrieve unrelated references. The spans also record an empty
resume_prompthistory, so offline evaluation of resumed turns is biased toward the summary path. The fix only needs the boundedresumeState.conversationas selection history. The model prompt does not change.
Suggested fix
Initialize ACP selection history from the bounded resumeState.conversation during native resume setup, or pass that history separately to dispatch. Cover deferred prewarmed resumes as well. Keep the native model prompt unchanged so the model does not receive duplicate conversation history.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/server/agent-server.ts#L3566-3574
@packages/agent/packages/agent/src/server/agent-server.ts#L1616-1619
<issue_description>
For a native resume in a fresh process, sendResumeContinuation supplies only the pending human prompt. Deferred prewarmed native resumes also use the human prompt without a history block. ContextSelection starts empty and extracts restored history only from hidden prompt blocks, so both paths submit empty history. The prior conversation exists in resumeState, but selection cannot use it to interpret requests such as "Does that definition still apply?".
</issue_description>
<issue_validation>
- **Checked:** `prepareNativeResume`, `sendResumeContinuation`, `runResumeTurn`, `promptWithUpstreamRetry`, `preparePrewarmedResumePrompt`, `sendResumeMessage` and `wrapPromptWithSummaryResume` in `agent-server.ts`. Also `ContextSelection.dispatch` and `preparePrompt` in `context-selection.ts`, and the use of `selection.history` in `products/context_layer/backend/selection_service.py`.
- **Found:** The prior conversation is in memory during a native resume. `prepareNativeResume` calls `loadResumeState` before it picks the native path (`agent-server.ts:1123-1124`). For Claude, the native path is the default when a prior session ID exists and the session JSONL hydrates (`agent-server.ts:1150-1175`). For Codex, it is the default on a warm sandbox (`agent-server.ts:1135-1148`).
- **Found:** `sendResumeContinuation` builds its prompt from `pendingUserPrompt.prompt` only and adds no hidden history block (`agent-server.ts:3416-3424`). `runResumeTurn` sends it through `promptWithUpstreamRetry`, and that function calls `contextSelection.dispatch(..., request.prompt)` (`agent-server.ts:2834-2848`).
- **Found:** For a deferred prewarmed native resume, `preparePrewarmedResumePrompt` returns the prompt unchanged (`agent-server.ts:3301-3308`). The user-message handler then passes that prompt to `dispatch` as `humanPrompt` (`agent-server.ts:1616-1635`).
- **Found:** `dispatch` gets restored history only from hidden blocks (`context-selection.ts`: `restoredHistory: text(humanPrompt.filter(isHidden))`). `preparePrompt` clears `this.history` when `historyRunId` changes. As a result, both native paths send `history: ""` with `history_source: "resume_prompt"`.
- **Found:** The summary paths put `formatConversationForResume(resumeState.conversation)` in a hidden block (`agent-server.ts:3238-3243`, `3336-3342`), so selection gets the prior conversation there. The test at `context-selection.test.ts:123-124` covers only that path.
- **Found:** The backend uses `selection.history` in the gate (`selection_service.py:149`), the search terms (`selection_service.py:165`) and each rerank call (`selection_service.py:182`).
- **Impact:** On the first human turn after a native resume, which is the normal resume for Claude, all three backend stages see only the new prompt. A follow-up that refers to earlier turns cannot be resolved. The gate can skip that turn, or search can retrieve unrelated references. The spans also record an empty `resume_prompt` history, so offline evaluation of resumed turns is biased toward the summary path. The fix only needs the bounded `resumeState.conversation` as selection history. The model prompt does not change.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Initialize ACP selection history from the bounded resumeState.conversation during native resume setup, or pass that history separately to dispatch. Cover deferred prewarmed resumes as well. Keep the native model prompt unchanged so the model does not receive duplicate conversation history.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Seeded selection history from resumeState for both native resume continuation and deferred prewarmed native resume. Extended the native/summary resume delivery cases to verify that prior user and assistant text reaches selection, and covered retention on later turns.
Comment on lines
+89
to
+90
| for (const text of readPersistedPiQueue(sessions.getEntries()).followUp) | ||
| this.blocked.add(hash(text)); |
Contributor
There was a problem hiding this comment.
Exclude restored steering messages from context selection
Issue description
The constructor blocks restored follow-ups but leaves restored steering texts eligible. rpc-host.ts restores both queues. Pi drains steering before its first model call. If a new registered prompt matches a restored steer, the handler consumes the new prompt's ID for the restored steer. This injects context for an excluded input and records the wrong message association.
Why we think it's a valid issue
- Checked:
context-selection.ts(the constructor, thecontexthandler,registerandblockText),queue-persistence.ts,rpc-host.ts,pi-agent-server.ts(dispatchUserMessageandpi/rpchandling), the existing tests incontext-selection.test.ts, and the installed Pi runtime (pi-coding-agent/dist/core/agent-session.jsandpi-agent-core/dist/agent-loop.js). - Found:
context-selection.ts:89-90adds onlyreadPersistedPiQueue(...).followUptexts toblocked. Restoredsteeringtexts stay eligible. - Found:
rpc-host.ts:140-143callsruntime.session.steer(message)for each restored steer while the session is idle.agent-session.js_queueSteerputs the message in the agent steering queue. No code callsblockTextfor these restored texts:pi-agent-server.ts:1057-1058and:923block only live steer commands. - Found:
agent-loop.js:82-83drains steering at the start ofrunLoop, and:96-103appends those messages after the prompt messages and before the first model call. So the firstcontextevent after a restore has[..., newPrompt, restoredSteer], andfindLastatcontext-selection.ts:98-100picks the restored steer. - Found:
dispatchUserMessage(pi-agent-server.ts:1060) registers the new prompt ID before it sends thepromptcommand. When the restored steer has the same text,blocked.has(hash(userText))is false andpending.findIndexmatches. The handler then splices the new prompt's ID (:116), callspreparePromptwithmessageId: input.idfor the restored steer, and attaches the injected context after the restored steer. The new prompt never gets its own selection. - Found: The test at
context-selection.test.ts:161-174covers a restoredfollowUponly. The comment atcontext-selection.ts:65and the PR description both say that restored queues skip selection, so the code breaks its own stated invariant for steering. - Impact: The selection span records the new prompt's message ID for a different message (the restored steer). Context is injected for an input that the design excludes. This corrupts the message association that offline evaluation depends on.
- Priority: Lowered to
should_fix. The trigger needs two things: a non-empty persisted steering queue at restore, and a first post-restore human prompt with identical text (for example, a user who resends a steer that seemed lost). Because the text is identical, the injected context is still relevant to what the model sees. The feature is behindphai-context-selectionand an allowlist that is empty by default. The fix is one line: also blocksteeringtexts in the constructor.
Suggested fix
Read the persisted queue once and block texts from both steering and followUp. Add a regression test with a restored steer and a new registered prompt that contain identical text.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/pi/context-selection.ts#L89-90
<issue_description>
The constructor blocks restored follow-ups but leaves restored steering texts eligible. rpc-host.ts restores both queues. Pi drains steering before its first model call. If a new registered prompt matches a restored steer, the handler consumes the new prompt's ID for the restored steer. This injects context for an excluded input and records the wrong message association.
</issue_description>
<issue_validation>
- **Checked:** `context-selection.ts` (the constructor, the `context` handler, `register` and `blockText`), `queue-persistence.ts`, `rpc-host.ts`, `pi-agent-server.ts` (`dispatchUserMessage` and `pi/rpc` handling), the existing tests in `context-selection.test.ts`, and the installed Pi runtime (`pi-coding-agent/dist/core/agent-session.js` and `pi-agent-core/dist/agent-loop.js`).
- **Found:** `context-selection.ts:89-90` adds only `readPersistedPiQueue(...).followUp` texts to `blocked`. Restored `steering` texts stay eligible.
- **Found:** `rpc-host.ts:140-143` calls `runtime.session.steer(message)` for each restored steer while the session is idle. `agent-session.js` `_queueSteer` puts the message in the agent steering queue. No code calls `blockText` for these restored texts: `pi-agent-server.ts:1057-1058` and `:923` block only live steer commands.
- **Found:** `agent-loop.js:82-83` drains steering at the start of `runLoop`, and `:96-103` appends those messages after the prompt messages and before the first model call. So the first `context` event after a restore has `[..., newPrompt, restoredSteer]`, and `findLast` at `context-selection.ts:98-100` picks the restored steer.
- **Found:** `dispatchUserMessage` (`pi-agent-server.ts:1060`) registers the new prompt ID before it sends the `prompt` command. When the restored steer has the same text, `blocked.has(hash(userText))` is false and `pending.findIndex` matches. The handler then splices the new prompt's ID (`:116`), calls `preparePrompt` with `messageId: input.id` for the restored steer, and attaches the injected context after the restored steer. The new prompt never gets its own selection.
- **Found:** The test at `context-selection.test.ts:161-174` covers a restored `followUp` only. The comment at `context-selection.ts:65` and the PR description both say that restored queues skip selection, so the code breaks its own stated invariant for steering.
- **Impact:** The selection span records the new prompt's message ID for a different message (the restored steer). Context is injected for an input that the design excludes. This corrupts the message association that offline evaluation depends on.
- **Priority:** Lowered to `should_fix`. The trigger needs two things: a non-empty persisted steering queue at restore, and a first post-restore human prompt with identical text (for example, a user who resends a steer that seemed lost). Because the text is identical, the injected context is still relevant to what the model sees. The feature is behind `phai-context-selection` and an allowlist that is empty by default. The fix is one line: also block `steering` texts in the constructor.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Read the persisted queue once and block texts from both steering and followUp. Add a regression test with a restored steer and a new registered prompt that contain identical text.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
No longer applicable after the refactor: feaa5db2a62 removed Pi context selection and explicitly excluded Pi from eligibility. The current implementation only selects context for Claude and Codex; the restored-steering path reviewed here no longer exists.
Comment on lines
+57
to
+58
| except Exception: | ||
| return None |
Contributor
There was a problem hiding this comment.
Preserve diagnostics when the scorer fails
Issue description
Every configuration, network, HTTP, and response error becomes None without a diagnostic record. The caller records only gate_error or unscored_count, so its outer exception handler cannot retain the error type. Failures before the gateway receives the request also have no gateway trace. These outcomes do not distinguish missing configuration, provider failures, and invalid responses during rollout.
Why we think it's a valid issue
- Checked:
SelectionJudge.judgeinproducts/context_layer/backend/selection_model.py:29-58, its only caller_selectinproducts/context_layer/backend/selection_service.py, the outer handler and$ai_spancapture in the same file,posthog/llm/system_one_client.py,posthog/llm/system_one.py:35-42, and the logging precedent inposthog/hogql/transforms/prompt_jev.py:169-181. - Found: The
tryblock atselection_model.py:37coversbuild_system_one_client. That function raisesSystemOneNotConfiguredwhen no gateway is usable. It can also reject a non-HTTPS gateway URL. These configuration failures never reach the gateway, so the gateway records no trace for them. - Found:
client.decideraisesSystemOneRequestFailedwithstatus_codefor HTTP errors (system_one_client.py:96-100). It raises the same class without a status for transport errors (system_one_client.py:79) and for malformed payloads (system_one.py:152-218). The bareexcept Exception: return Noneatselection_model.py:57-58discards all of these, and nothing logs them. - Found: The outer
except Exception as errorinselection_service.pywritesobservation["error_type"]. It never runs for scorer failures, becausejudgereturnsNone. A gate failure records onlyreason="gate_error". A rerank failure only increasesobservation["unscored_count"], and that count also includes candidates skipped for capacity and candidates that did not finish before the deadline. - Found: The repository already logs this failure type safely.
prompt_jev.py:171-177logsstatus_code,model, and the exception type, and excludes inputs and response bodies. - Impact: The span records that a failure happened, but not why. Suppose the gateway is missing or misconfigured in one region. Then every treatment selection returns
gate_error, and treatment delivers the same thing as control. Operators then cannot tell a configuration gap from provider 4xx/5xx errors or malformed responses without reproducing the failure. This matches the "swallowed errors that hide failures" keep criterion. The fix is small: add one structuredlogger.warningor oneobservationfield. - Priority: Lowered to
consider. TheNonefallback is intentional, the failure itself shows in the span (gate_error,unscored_count), and the feature is restricted to staff and allowlisted teams behindphai-context-selection. Only the cause is missing. This makes rollout harder to debug, but it does not cause wrong behavior for users.
Suggested fix
Keep the None fallback, but record the exception type, HTTP status when available, selection_id, and selection step. Include candidate_id for rerank failures. Follow the safe structured warning pattern in posthog/hogql/transforms/prompt_jev.py. Exclude prompt text, candidate text, and response bodies from diagnostics.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_model.py#L57-58
<issue_description>
Every configuration, network, HTTP, and response error becomes None without a diagnostic record. The caller records only gate_error or unscored_count, so its outer exception handler cannot retain the error type. Failures before the gateway receives the request also have no gateway trace. These outcomes do not distinguish missing configuration, provider failures, and invalid responses during rollout.
</issue_description>
<issue_validation>
- **Checked:** `SelectionJudge.judge` in `products/context_layer/backend/selection_model.py:29-58`, its only caller `_select` in `products/context_layer/backend/selection_service.py`, the outer handler and `$ai_span` capture in the same file, `posthog/llm/system_one_client.py`, `posthog/llm/system_one.py:35-42`, and the logging precedent in `posthog/hogql/transforms/prompt_jev.py:169-181`.
- **Found:** The `try` block at `selection_model.py:37` covers `build_system_one_client`. That function raises `SystemOneNotConfigured` when no gateway is usable. It can also reject a non-HTTPS gateway URL. These configuration failures never reach the gateway, so the gateway records no trace for them.
- **Found:** `client.decide` raises `SystemOneRequestFailed` with `status_code` for HTTP errors (`system_one_client.py:96-100`). It raises the same class without a status for transport errors (`system_one_client.py:79`) and for malformed payloads (`system_one.py:152-218`). The bare `except Exception: return None` at `selection_model.py:57-58` discards all of these, and nothing logs them.
- **Found:** The outer `except Exception as error` in `selection_service.py` writes `observation["error_type"]`. It never runs for scorer failures, because `judge` returns `None`. A gate failure records only `reason="gate_error"`. A rerank failure only increases `observation["unscored_count"]`, and that count also includes candidates skipped for capacity and candidates that did not finish before the deadline.
- **Found:** The repository already logs this failure type safely. `prompt_jev.py:171-177` logs `status_code`, `model`, and the exception type, and excludes inputs and response bodies.
- **Impact:** The span records that a failure happened, but not why. Suppose the gateway is missing or misconfigured in one region. Then every treatment selection returns `gate_error`, and treatment delivers the same thing as control. Operators then cannot tell a configuration gap from provider 4xx/5xx errors or malformed responses without reproducing the failure. This matches the "swallowed errors that hide failures" keep criterion. The fix is small: add one structured `logger.warning` or one `observation` field.
- **Priority:** Lowered to `consider`. The `None` fallback is intentional, the failure itself shows in the span (`gate_error`, `unscored_count`), and the feature is restricted to staff and allowlisted teams behind `phai-context-selection`. Only the cause is missing. This makes rollout harder to debug, but it does not cause wrong behavior for users.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Keep the None fallback, but record the exception type, HTTP status when available, selection_id, and selection step. Include candidate_id for rerank failures. Follow the safe structured warning pattern in posthog/hogql/transforms/prompt_jev.py. Exclude prompt text, candidate text, and response bodies from diagnostics.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Selection spans now retain scorer error types with candidate identifiers for gate and rerank failures. The concurrent error snapshots do not mutate an already captured list. Failure still leaves the normal prompt flow available; the regression verifies that gate failures retain diagnostics.
Comment on lines
+84
to
+92
| const history = this.history || restoredHistory.slice(-12_000); | ||
| try { | ||
| prepared = await this.api.prepareContextSelection({ | ||
| run_id: runId, | ||
| message_id: messageId, | ||
| prompt: userText.slice(-20_000), | ||
| prompt_char_count: userText.length, | ||
| history, | ||
| history_source: this.history ? "runtime" : historySource, |
Contributor
There was a problem hiding this comment.
Preserve restored history after the first resumed turn
Issue description
A summary resume uses restoredHistory for its first selection request, but never stores that history. After successful delivery, recordUser initializes runtime history with only the new user text. Subsequent requests select this nonempty history and discard the earlier conversation, even when it fits within the character limit. Short answers such as "Yes" then leave selection without the topic needed to interpret later follow-ups.
Why we think it's a valid issue
- Checked:
preparePrompt,recordUserandrecordAssistantinpackages/agent/packages/agent/src/server/context-selection.ts. The summary resume prompt builders inagent-server.ts(sendResumeMessage,wrapPromptWithSummaryResume). All callers of the history methods. Pi has its own path insrc/pi/context-selection.ts:133, which passeshistorySource: "runtime"and supplies its own history. - Found:
preparePromptsetsconst history = this.history || restoredHistory.slice(-12_000). Restored history is therefore only a fallback for an emptythis.history, and the code never writes it intothis.history. - Found: After the first resumed turn succeeds,
recordUsersetsthis.history = "\nUser: <new text>"(context-selection.ts:105-108), andrecordAssistantappends that turn's answer (context-selection.ts:110-113). On later turnsthis.historyis not empty, so the fallback never runs again. Also, follow-up prompts carry no hidden history block, sorestoredHistoryis empty on those turns. - Found: The 12,000-character cap applies to the combined history. Keeping the restored conversation therefore needs no extra bound. A one-line seed of
this.historyfrom the bounded restored text before the firstrecordUseris enough. - Impact: On ACP runs that resume from a summary (a fresh Codex sandbox, or Claude without a session JSONL), selection on turn 2 and later sees only turns from this process. If the first resumed exchange does not restate the topic, the gate and the rerank lose the earlier subject. Then the gate can skip, or the rerank can rank unrelated references higher.
- Impact: The effect is smaller than it first appears. The full answer to the first resumed turn goes into history (
agent-server.ts:3590-3593), and that answer usually names the topic. The model's own prompt does not change. The fix for 2-3-4 (seed history fromresumeState.conversation) would also fix this path. - Priority: Lower to
consider. The problem is real and cheap to fix. But it occurs only on summary resumes, only when the first resumed exchange does not carry the topic, and it only lowers the quality of optional hidden references.
Suggested fix
When ACP dispatch records its first completed user turn, initialize history with the bounded restored history before appending that turn. Preserve the run ID check and character limit. Keep Pi's preparePrompt path using its caller-supplied current runtime history.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/server/context-selection.ts#L84-92
@packages/agent/packages/agent/src/server/context-selection.ts#L105-107
<issue_description>
A summary resume uses restoredHistory for its first selection request, but never stores that history. After successful delivery, recordUser initializes runtime history with only the new user text. Subsequent requests select this nonempty history and discard the earlier conversation, even when it fits within the character limit. Short answers such as "Yes" then leave selection without the topic needed to interpret later follow-ups.
</issue_description>
<issue_validation>
- **Checked:** `preparePrompt`, `recordUser` and `recordAssistant` in `packages/agent/packages/agent/src/server/context-selection.ts`. The summary resume prompt builders in `agent-server.ts` (`sendResumeMessage`, `wrapPromptWithSummaryResume`). All callers of the history methods. Pi has its own path in `src/pi/context-selection.ts:133`, which passes `historySource: "runtime"` and supplies its own history.
- **Found:** `preparePrompt` sets `const history = this.history || restoredHistory.slice(-12_000)`. Restored history is therefore only a fallback for an empty `this.history`, and the code never writes it into `this.history`.
- **Found:** After the first resumed turn succeeds, `recordUser` sets `this.history = "\nUser: <new text>"` (`context-selection.ts:105-108`), and `recordAssistant` appends that turn's answer (`context-selection.ts:110-113`). On later turns `this.history` is not empty, so the fallback never runs again. Also, follow-up prompts carry no hidden history block, so `restoredHistory` is empty on those turns.
- **Found:** The 12,000-character cap applies to the combined history. Keeping the restored conversation therefore needs no extra bound. A one-line seed of `this.history` from the bounded restored text before the first `recordUser` is enough.
- **Impact:** On ACP runs that resume from a summary (a fresh Codex sandbox, or Claude without a session JSONL), selection on turn 2 and later sees only turns from this process. If the first resumed exchange does not restate the topic, the gate and the rerank lose the earlier subject. Then the gate can skip, or the rerank can rank unrelated references higher.
- **Impact:** The effect is smaller than it first appears. The full answer to the first resumed turn goes into history (`agent-server.ts:3590-3593`), and that answer usually names the topic. The model's own prompt does not change. The fix for 2-3-4 (seed history from `resumeState.conversation`) would also fix this path.
- **Priority:** Lower to `consider`. The problem is real and cheap to fix. But it occurs only on summary resumes, only when the first resumed exchange does not carry the topic, and it only lowers the quality of optional hidden references.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
When ACP dispatch records its first completed user turn, initialize history with the bounded restored history before appending that turn. Preserve the run ID check and character limit. Keep Pi's preparePrompt path using its caller-supplied current runtime history.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Restored history is now stored in the selector before the first resumed selection request, so subsequent human and assistant turns extend that history rather than replacing it. Extended the restored-history regression to verify the next turn still includes the earlier conversation.
Comment on lines
+80
to
+84
| if (this.historyRunId !== runId) { | ||
| this.historyRunId = runId; | ||
| this.history = ""; | ||
| } | ||
| const history = this.history || restoredHistory.slice(-12_000); |
Contributor
There was a problem hiding this comment.
Reset selection history after a successful /clear
Issue description
The selector resets history only when runId changes. Claude's successful /clear creates a fresh model conversation while keeping the cloud run ID unchanged. Selection therefore continues using pre-clear user and assistant text. Later treatment prompts can inject context chosen from the conversation the user cleared. Dispatch also appends /clear to that retained history.
Why we think it's a valid issue
- Checked: The history fields in
ContextSelection(context-selection.ts:24-25,80-84,105-113). The user-message path inagent-server.ts:1576-1635. A search ofagent-server.tsforCONVERSATION_CLEARED. The Claude adapter's/clearflow (claude-agent.ts:226-241,789-794,2228-2460). The web callers inproducts/posthog_ai/frontend/logics/runInteractionLogic.ts:1654andproducts/tasks/backend/presentation/views/api.py:2177-2204. - Found: Users can reach
/clearin a cloud run. The PostHog AI composer sends/clearas an ordinary message (runInteractionLogic.ts:1654). The tasks API tells callers to send/clearto an active run as a message (api.py:2177-2204). - Found:
/cleargoes through selection. The handler excludes only/compact(manualCompactPrompt ? undefined : messageId,agent-server.ts:1616-1618). For/clear,dispatchtherefore runspreparePromptand callssend. After that it runsrecordUser, which appendsUser: /clearto the existing history (context-selection.ts:51-55,105-108). A hidden context block does not prevent clear detection, becausepromptSlashCommandreads only visible blocks (claude-agent.ts:231). - Found: Nothing resets selection history.
ContextSelectionclears history only whenhistoryRunId !== runId(context-selection.ts:80-83), and/clearkeeps the same run ID. The adapter sendsPOSTHOG_NOTIFICATIONS.CONVERSATION_CLEARED(claude-agent.ts:2448). Onlyresume-saga.tsandjsonl-hydration.tshandle it, andagent-server.tshas no handler that resets selection. - Found: The adapter's contract for
/clearis that nothing from before the boundary reaches the fresh session (claude-agent.ts:2414, "Nothing from before the boundary should reach the fresh session"). - Impact: After a successful
/clear, every laterpreparecall sends up to 12,000 characters of pre-clear text ashistory. The backend uses that history in the gate (selection_service.py:149), the search terms (selection_service.py:165) and each rerank call (selection_service.py:182). On treatment runs, references chosen from the cleared topic can therefore be injected into the fresh conversation. The selection span also records the cleared text. The fix is small: reset history for the active run whenCONVERSATION_CLEAREDarrives, and do not record the/clearturn.
Suggested fix
Add an explicit history reset for the active run. Call it when the adapter confirms POSTHOG_NOTIFICATIONS.CONVERSATION_CLEARED, consistent with the existing resume boundary. Prevent dispatch from recording the successful /clear command after that reset.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/server/context-selection.ts#L80-84
@packages/agent/packages/agent/src/server/context-selection.ts#L105-112
<issue_description>
The selector resets history only when runId changes. Claude's successful /clear creates a fresh model conversation while keeping the cloud run ID unchanged. Selection therefore continues using pre-clear user and assistant text. Later treatment prompts can inject context chosen from the conversation the user cleared. Dispatch also appends /clear to that retained history.
</issue_description>
<issue_validation>
- **Checked:** The history fields in `ContextSelection` (`context-selection.ts:24-25`, `80-84`, `105-113`). The user-message path in `agent-server.ts:1576-1635`. A search of `agent-server.ts` for `CONVERSATION_CLEARED`. The Claude adapter's `/clear` flow (`claude-agent.ts:226-241`, `789-794`, `2228-2460`). The web callers in `products/posthog_ai/frontend/logics/runInteractionLogic.ts:1654` and `products/tasks/backend/presentation/views/api.py:2177-2204`.
- **Found:** Users can reach `/clear` in a cloud run. The PostHog AI composer sends `/clear` as an ordinary message (`runInteractionLogic.ts:1654`). The tasks API tells callers to send `/clear` to an active run as a message (`api.py:2177-2204`).
- **Found:** `/clear` goes through selection. The handler excludes only `/compact` (`manualCompactPrompt ? undefined : messageId`, `agent-server.ts:1616-1618`). For `/clear`, `dispatch` therefore runs `preparePrompt` and calls `send`. After that it runs `recordUser`, which appends `User: /clear` to the existing history (`context-selection.ts:51-55`, `105-108`). A hidden context block does not prevent clear detection, because `promptSlashCommand` reads only visible blocks (`claude-agent.ts:231`).
- **Found:** Nothing resets selection history. `ContextSelection` clears history only when `historyRunId !== runId` (`context-selection.ts:80-83`), and `/clear` keeps the same run ID. The adapter sends `POSTHOG_NOTIFICATIONS.CONVERSATION_CLEARED` (`claude-agent.ts:2448`). Only `resume-saga.ts` and `jsonl-hydration.ts` handle it, and `agent-server.ts` has no handler that resets selection.
- **Found:** The adapter's contract for `/clear` is that nothing from before the boundary reaches the fresh session (`claude-agent.ts:2414`, "Nothing from before the boundary should reach the fresh session").
- **Impact:** After a successful `/clear`, every later `prepare` call sends up to 12,000 characters of pre-clear text as `history`. The backend uses that history in the gate (`selection_service.py:149`), the search terms (`selection_service.py:165`) and each rerank call (`selection_service.py:182`). On treatment runs, references chosen from the cleared topic can therefore be injected into the fresh conversation. The selection span also records the cleared text. The fix is small: reset history for the active run when `CONVERSATION_CLEARED` arrives, and do not record the `/clear` turn.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Add an explicit history reset for the active run. Call it when the adapter confirms POSTHOG_NOTIFICATIONS.CONVERSATION_CLEARED, consistent with the existing resume boundary. Prevent dispatch from recording the successful /clear command after that reset.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Reset selection history on the adapter conversation-cleared notification and excluded slash commands from selection/history collection. Regressions cover the notification boundary and verify that the next selection request has empty history.
Comment on lines
+154
to
+155
| if shared_catalog and access.check_access_level_for_resource("data_catalog", "viewer"): | ||
| denied = Database.create_for(team=team, user=user, user_access_control=access)._denied_tables |
Contributor
There was a problem hiding this comment.
Avoid building the entire catalog to check candidate permissions
Issue description
Database.create_for builds the complete project catalog just to obtain _denied_tables. This includes unrelated warehouse schemas, saved queries, and joins. Successful selection can do this twice: during retrieval and after reranking. These calls fetch fresh sources rather than using the catalog cache. Their cost grows with the entire project instead of the bounded candidates. This can consume the three-second selection budget even for one matching metric.
Why we think it's a valid issue
- Checked:
validate_candidatesinproducts/context_layer/backend/selection_sources.py. Its two callers.Database.create_forand_sources_cache_keyinposthog/hogql/database/database.py. The selection budgets inselection_service.py,selection_views.pyandposthog/settings/web.py. The narrow helpers thatproducts/data_quality/backend/logic/subject_access.pyuses. - Found:
selection_sources.py:155callsDatabase.create_for(team=team, user=user, user_access_control=access)only to read_denied_tables._sources_cache_keyreturnsNonewhen auser_access_controlis passed (database.py,if team is None or user_access_control is not None ...).use_cached_sourcesalso defaults toFalse. So every call runs_fetch_sourcesand_build_from_sourcesfrom scratch. That work loads the team, feature flags, external data sources, all saved queries, endpoint saved queries, joins, expressions and all warehouse tables with credentials, then builds the whole table tree. - Found: The build runs twice when catalog candidates match and survive scoring. The first call is
search_sources→validate_candidates(selection_sources.py:104). The second is the post-rerank revalidation (selection_service.py:199). - Found: The selection deadline is
CONTEXT_SELECTION_TIMEOUT_SECONDS = 3.0(posthog/settings/web.py:1563), and the HTTP handler wraps it inbounded_request(self.team_id, 3.5, execute)(selection_views.py).check_deadlineruns only between stages, so it cannot stop a slow build. On a large project the build blocks the prepare request, which runs before the human turn reaches the agent, and then the selection fails withTimeoutErrorand returns no context. - Found: Much of the build gives no extra information here. The
shared_cataloggate atselection_sources.py:148-153already skips teams that have anyAccessControlrow forwarehouse_tableorexternal_data_source, so for the teams that reach line 155 the denied set is mostly the system-table denials plus anywarehouse_viewdenials.system_table_denials(database.py:767) exists for this purpose. Its docstring says it lets a caller that only needs the denial set avoid a whole database build.reference_gate(subject_access.py:220) combines it withwarehouse_facade.allowed_table_ids(..., ids=...)anddata_modeling_facade.allowed_saved_query_ids(..., ids=...), which keeps the cost tied to what is referenced rather than to the whole team. - Found:
metrics_visible_to_user(products/data_catalog/backend/logic/metrics.py:557) uses the same full-build pattern. That code is a REST list path with no 3-second budget, so it does not justify this use in a deadline-bound path that runs on every prompt. - Impact: For allowlisted teams with large warehouse catalogs, every eligible prompt waits for one or two full catalog builds before it is delivered. The selection can run out of its budget even when only one metric matches. The feature then silently returns no context, and the shadow and treatment data under-represent large projects. The fix is concrete and an existing pattern shows how to do it.
Suggested fix
Use system_table_denials for system references and narrow permission checks for the warehouse tables and views referenced by candidates. The reference_gate implementation in products/data_quality/backend/logic/subject_access.py demonstrates this approach. Preserve fresh permission checks after reranking, but avoid constructing unrelated catalog objects.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_sources.py#L154-155
<issue_description>
Database.create_for builds the complete project catalog just to obtain _denied_tables. This includes unrelated warehouse schemas, saved queries, and joins. Successful selection can do this twice: during retrieval and after reranking. These calls fetch fresh sources rather than using the catalog cache. Their cost grows with the entire project instead of the bounded candidates. This can consume the three-second selection budget even for one matching metric.
</issue_description>
<issue_validation>
- **Checked:** `validate_candidates` in `products/context_layer/backend/selection_sources.py`. Its two callers. `Database.create_for` and `_sources_cache_key` in `posthog/hogql/database/database.py`. The selection budgets in `selection_service.py`, `selection_views.py` and `posthog/settings/web.py`. The narrow helpers that `products/data_quality/backend/logic/subject_access.py` uses.
- **Found:** `selection_sources.py:155` calls `Database.create_for(team=team, user=user, user_access_control=access)` only to read `_denied_tables`. `_sources_cache_key` returns `None` when a `user_access_control` is passed (`database.py`, `if team is None or user_access_control is not None ...`). `use_cached_sources` also defaults to `False`. So every call runs `_fetch_sources` and `_build_from_sources` from scratch. That work loads the team, feature flags, external data sources, all saved queries, endpoint saved queries, joins, expressions and all warehouse tables with credentials, then builds the whole table tree.
- **Found:** The build runs twice when catalog candidates match and survive scoring. The first call is `search_sources` → `validate_candidates` (`selection_sources.py:104`). The second is the post-rerank revalidation (`selection_service.py:199`).
- **Found:** The selection deadline is `CONTEXT_SELECTION_TIMEOUT_SECONDS = 3.0` (`posthog/settings/web.py:1563`), and the HTTP handler wraps it in `bounded_request(self.team_id, 3.5, execute)` (`selection_views.py`). `check_deadline` runs only between stages, so it cannot stop a slow build. On a large project the build blocks the prepare request, which runs before the human turn reaches the agent, and then the selection fails with `TimeoutError` and returns no context.
- **Found:** Much of the build gives no extra information here. The `shared_catalog` gate at `selection_sources.py:148-153` already skips teams that have any `AccessControl` row for `warehouse_table` or `external_data_source`, so for the teams that reach line 155 the denied set is mostly the system-table denials plus any `warehouse_view` denials. `system_table_denials` (`database.py:767`) exists for this purpose. Its docstring says it lets a caller that only needs the denial set avoid a whole database build. `reference_gate` (`subject_access.py:220`) combines it with `warehouse_facade.allowed_table_ids(..., ids=...)` and `data_modeling_facade.allowed_saved_query_ids(..., ids=...)`, which keeps the cost tied to what is referenced rather than to the whole team.
- **Found:** `metrics_visible_to_user` (`products/data_catalog/backend/logic/metrics.py:557`) uses the same full-build pattern. That code is a REST list path with no 3-second budget, so it does not justify this use in a deadline-bound path that runs on every prompt.
- **Impact:** For allowlisted teams with large warehouse catalogs, every eligible prompt waits for one or two full catalog builds before it is delivered. The selection can run out of its budget even when only one metric matches. The feature then silently returns no context, and the shadow and treatment data under-represent large projects. The fix is concrete and an existing pattern shows how to do it.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Use system_table_denials for system references and narrow permission checks for the warehouse tables and views referenced by candidates. The reference_gate implementation in products/data_quality/backend/logic/subject_access.py demonstrates this approach. Preserve fresh permission checks after reranking, but avoid constructing unrelated catalog objects.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Replaced the full Database.create_for catalog build with system_table_denials. Customized warehouse resources are excluded by the expanded shared-catalog guard, so the remaining system-table permission check no longer builds unrelated schemas, saved queries, or joins. The live catalog and warehouse-access regressions pass.
Comment on lines
+80
to
+83
| skills = LLMSkill.objects.filter(team=team, deleted=False, is_latest=True, category="").only( | ||
| "id", "team_id", "name", "description", "version" | ||
| ) | ||
| candidates.extend(search_rows("skill", skills, query, ("name", "description", "body"))) |
Contributor
There was a problem hiding this comment.
Filter eligible skills before applying the source limit
Issue description
search_rows selects the top 18 skill matches before validate_candidates excludes skills with object-specific access controls. If those 18 matches have such controls, selection returns no skills. An eligible shared skill ranked nineteenth never reaches reranking. Restricted matches therefore prevent the selector from supplying available, relevant context.
Why we think it's a valid issue
- Checked: The skill retrieval path at
selection_sources.py:79-83. The cap and slice insearch_rowsatselection_sources.py:114.SOURCE_LIMITSinselection_types.py:18-24. The skill revalidation atselection_sources.py:128-145.LLMSkillViewSet._get_search_querysetatproducts/skills/backend/api/skills.py:814-853.UserAccessControl.filter_queryset_by_access_levelatproducts/access_control/backend/facade/user_access_control.py:1170-1211. - Found: The first-pass skill queryset filters only on
team,deleted=False,is_latest=Trueandcategory=""(selection_sources.py:80).search_rowsthen ranks and slices toSOURCE_LIMITS["skill"] = 18(selection_types.py:19,selection_sources.py:114). The exclusion of skills with any object-levelAccessControlrow (private_skill_ids,selection_sources.py:128-132) andfilter_queryset_by_access_level(selection_sources.py:139-144) run only after that slice, invalidate_candidates. A restricted skill therefore takes one of the 18 slots and is removed later, and nothing refills the slot. - Found: The skills REST search applies
filter_queryset_by_access_levelbefore it matches and orders (skills.py:847), so the codebase already applies access filters before ranking. - Found: The text query ORs up to 60 prompt tokens (
selection_sources.py:74-77). In a team with many skills, more than 18 skills can match, so the cap can fill. A restricted skill in the top 18 then removes one eligible candidate from reranking. - Impact: The effect is on recall only. No restricted skill leaks, because revalidation still removes it. The loss occurs only in teams that put object-level controls on skills that rank in the top 18. If only some of the top matches are restricted, the result is fewer candidates, not zero. Zero skills needs 18 restricted top matches. The feature is also optional, staff-only, and limited to allowlisted teams, and a missed candidate means less hidden context, not wrong output.
- Priority: Lowered to
consider. The mechanism is confirmed and the fix is a cheap.excludeand access filter before the slice, but the affected group is small and the consequence is lower context quality, not a correctness, security or data problem.
Suggested fix
Exclude skills with object-specific llm_skill AccessControl rows before calling search_rows. Apply UserAccessControl filtering before ordering and slicing, as LLMSkillViewSet._get_search_queryset does. Keep final revalidation to detect access changes during scoring.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_sources.py#L80-83
@products/context_layer/backend/selection_sources.py#L114
<issue_description>
search_rows selects the top 18 skill matches before validate_candidates excludes skills with object-specific access controls. If those 18 matches have such controls, selection returns no skills. An eligible shared skill ranked nineteenth never reaches reranking. Restricted matches therefore prevent the selector from supplying available, relevant context.
</issue_description>
<issue_validation>
- **Checked:** The skill retrieval path at `selection_sources.py:79-83`. The cap and slice in `search_rows` at `selection_sources.py:114`. `SOURCE_LIMITS` in `selection_types.py:18-24`. The skill revalidation at `selection_sources.py:128-145`. `LLMSkillViewSet._get_search_queryset` at `products/skills/backend/api/skills.py:814-853`. `UserAccessControl.filter_queryset_by_access_level` at `products/access_control/backend/facade/user_access_control.py:1170-1211`.
- **Found:** The first-pass skill queryset filters only on `team`, `deleted=False`, `is_latest=True` and `category=""` (`selection_sources.py:80`). `search_rows` then ranks and slices to `SOURCE_LIMITS["skill"] = 18` (`selection_types.py:19`, `selection_sources.py:114`). The exclusion of skills with any object-level `AccessControl` row (`private_skill_ids`, `selection_sources.py:128-132`) and `filter_queryset_by_access_level` (`selection_sources.py:139-144`) run only after that slice, in `validate_candidates`. A restricted skill therefore takes one of the 18 slots and is removed later, and nothing refills the slot.
- **Found:** The skills REST search applies `filter_queryset_by_access_level` before it matches and orders (`skills.py:847`), so the codebase already applies access filters before ranking.
- **Found:** The text query ORs up to 60 prompt tokens (`selection_sources.py:74-77`). In a team with many skills, more than 18 skills can match, so the cap can fill. A restricted skill in the top 18 then removes one eligible candidate from reranking.
- **Impact:** The effect is on recall only. No restricted skill leaks, because revalidation still removes it. The loss occurs only in teams that put object-level controls on skills that rank in the top 18. If only some of the top matches are restricted, the result is fewer candidates, not zero. Zero skills needs 18 restricted top matches. The feature is also optional, staff-only, and limited to allowlisted teams, and a missed candidate means less hidden context, not wrong output.
- **Priority:** Lowered to `consider`. The mechanism is confirmed and the fix is a cheap `.exclude` and access filter before the slice, but the affected group is small and the consequence is lower context quality, not a correctness, security or data problem.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Exclude skills with object-specific llm_skill AccessControl rows before calling search_rows. Apply UserAccessControl filtering before ordering and slicing, as LLMSkillViewSet._get_search_queryset does. Keep final revalidation to detect access changes during scoring.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Moved object-specific skill access filtering into the SQL query before ranking and applying the 18-result limit, using a typed EXISTS check. Final permission revalidation remains in place. Added a regression with 18 restricted matches plus an eligible shared skill.
| bool(knowledge_ids) and not AccessControl.objects.filter(team=team, resource="business_knowledge").exists() | ||
| ) | ||
| if knowledge_ids and shared_knowledge and access.check_access_level_for_resource("business_knowledge", "viewer"): | ||
| result.extend(knowledge_record(item, team.id) for item in get_chunks_by_ids(team.id, knowledge_ids)) |
Contributor
There was a problem hiding this comment.
Resolve the canonical project for Business Knowledge
Issue description
An eligible cloud run can belong to a child environment, but Business Knowledge chunks live on its canonical parent. This call passes the child Team to search_knowledge_for_team. That helper calls search_knowledge(team.id), whose canonical=True decorator skips parent resolution. Retrieval therefore returns no parent knowledge. Revalidation also passes the child ID to get_chunks_by_ids, so it cannot retain parent chunks.
Why we think it's a valid issue
- Checked:
search_business_knowledgeand the revalidation branch inselection_sources.py:172-191. The_searchwrapper inselection_service.py.search_knowledge_for_team,search_knowledge,get_chunks_by_idsand_safe_chunks_qsinproducts/business_knowledge/backend/logic.py.with_team_scopeandteam_scopeinposthog/models/scoping/__init__.py. The Business Knowledge route and model definitions. How the tasks product handles child-environment teams. - Found:
search_knowledge_for_teampassesteam.idunchanged tosearch_knowledge(logic.py:2603).search_knowledge(logic.py:2476) andget_chunks_by_ids(logic.py:2786) use@with_team_scope(canonical=True). That option skips the parent lookup, and its docstring says a non-canonical id "will return zero rows from the wrong scope". Both functions also read through_safe_chunks_qs(team_id), which filtersteam_id=team_idexplicitly (logic.py:2435). The outerteam_scope(team.id)inselection_service._searchresolves to the parent, but the innercanonical=Truescope and the explicit filter both use the child id. - Found: Business Knowledge rows live on the canonical team.
KnowledgeChunkis a plainmodels.Modelwith no save-time remapping. The REST routes are project-nested (business_knowledge/backend/routes.py). Theis_available_for_teamdocstring (logic.py:2283-2290) says that rows are "project-scoped under the canonical parent" and that callers must resolveteam.parent_team_id or team.id.api/sandbox.py:49buildsUserAccessControlonself.team.parent_team or self.team. - Found: Task runs can belong to child environments.
workflow_dispatch.py:198usestask_run.team.parent_team_id or task_run.team_id, andtasks/backend/admin.py:257rejects environment teams for preference rows. The selection path passesrun.teamstraight through (selection_service.py:44, 165), so a child team reachessearch_knowledge_for_team(team, ...)atselection_sources.py:190andget_chunks_by_ids(team.id, ...)atselection_sources.py:177. - Found: The access gates use the child team too.
AccessControl.objects.filter(team=team, resource="business_knowledge")atselection_sources.py:174and:188, andUserAccessControl(user=user, team=team)at:186, so restrictions defined on the parent are not checked. This causes no leak now, because retrieval returns nothing. A fix that resolves only the search team would open one, so the fix must use the canonical team for every step. - Impact: For any eligible run in a child environment, the Business Knowledge source silently returns zero candidates every time. The selection span records this as normal, and nothing reports an error. The reference URL at
selection_sources.py:202also uses the child id. The shadow and treatment data therefore under-count Business Knowledge for these projects.
Suggested fix
Resolve the Business Knowledge source team using team.parent_team_id or team.id. Use that team consistently for access checks, retrieval, revalidation, and reference URLs. Follow the parent resolution already used by is_available_for_team and is_maintained_for_team. Keep skill queries scoped to the original environment.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_sources.py#L177
@products/context_layer/backend/selection_sources.py#L190-191
<issue_description>
An eligible cloud run can belong to a child environment, but Business Knowledge chunks live on its canonical parent. This call passes the child Team to search_knowledge_for_team. That helper calls search_knowledge(team.id), whose canonical=True decorator skips parent resolution. Retrieval therefore returns no parent knowledge. Revalidation also passes the child ID to get_chunks_by_ids, so it cannot retain parent chunks.
</issue_description>
<issue_validation>
- **Checked:** `search_business_knowledge` and the revalidation branch in `selection_sources.py:172-191`. The `_search` wrapper in `selection_service.py`. `search_knowledge_for_team`, `search_knowledge`, `get_chunks_by_ids` and `_safe_chunks_qs` in `products/business_knowledge/backend/logic.py`. `with_team_scope` and `team_scope` in `posthog/models/scoping/__init__.py`. The Business Knowledge route and model definitions. How the tasks product handles child-environment teams.
- **Found:** `search_knowledge_for_team` passes `team.id` unchanged to `search_knowledge` (`logic.py:2603`). `search_knowledge` (`logic.py:2476`) and `get_chunks_by_ids` (`logic.py:2786`) use `@with_team_scope(canonical=True)`. That option skips the parent lookup, and its docstring says a non-canonical id "will return zero rows from the wrong scope". Both functions also read through `_safe_chunks_qs(team_id)`, which filters `team_id=team_id` explicitly (`logic.py:2435`). The outer `team_scope(team.id)` in `selection_service._search` resolves to the parent, but the inner `canonical=True` scope and the explicit filter both use the child id.
- **Found:** Business Knowledge rows live on the canonical team. `KnowledgeChunk` is a plain `models.Model` with no save-time remapping. The REST routes are project-nested (`business_knowledge/backend/routes.py`). The `is_available_for_team` docstring (`logic.py:2283-2290`) says that rows are "project-scoped under the canonical parent" and that callers must resolve `team.parent_team_id or team.id`. `api/sandbox.py:49` builds `UserAccessControl` on `self.team.parent_team or self.team`.
- **Found:** Task runs can belong to child environments. `workflow_dispatch.py:198` uses `task_run.team.parent_team_id or task_run.team_id`, and `tasks/backend/admin.py:257` rejects environment teams for preference rows. The selection path passes `run.team` straight through (`selection_service.py:44, 165`), so a child team reaches `search_knowledge_for_team(team, ...)` at `selection_sources.py:190` and `get_chunks_by_ids(team.id, ...)` at `selection_sources.py:177`.
- **Found:** The access gates use the child team too. `AccessControl.objects.filter(team=team, resource="business_knowledge")` at `selection_sources.py:174` and `:188`, and `UserAccessControl(user=user, team=team)` at `:186`, so restrictions defined on the parent are not checked. This causes no leak now, because retrieval returns nothing. A fix that resolves only the search team would open one, so the fix must use the canonical team for every step.
- **Impact:** For any eligible run in a child environment, the Business Knowledge source silently returns zero candidates every time. The selection span records this as normal, and nothing reports an error. The reference URL at `selection_sources.py:202` also uses the child id. The shadow and treatment data therefore under-count Business Knowledge for these projects.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Resolve the Business Knowledge source team using team.parent_team_id or team.id. Use that team consistently for access checks, retrieval, revalidation, and reference URLs. Follow the parent resolution already used by is_available_for_team and is_maintained_for_team. Keep skill queries scoped to the original environment.
</potential_solution>
Contributor
Author
There was a problem hiding this comment.
Valid; addressed in 191d1fc7e71f.
Resolved Business Knowledge to the canonical parent team consistently for retrieval, access checks, revalidation, and reference URLs. Skill/catalog lookup remains on the original environment. Added a child-environment regression verifying parent knowledge retrieval/revalidation and suppression by parent access controls.
Comment on lines
+2546
to
+2551
| chunks_by_id = { | ||
| chunk.id: chunk | ||
| for chunk in _safe_chunks_qs(team_id) | ||
| .filter(id__in=[anchor.id for anchor in anchor_chunks]) | ||
| .select_related("source", "document") | ||
| } |
Contributor
There was a problem hiding this comment.
Exclude full documents from anchor hydration
Issue description
The new anchor-only query joins source and document without restricting selected fields. Django therefore loads KnowledgeDocument.content and metadata for every matching chunk. Document text can reach 1,000,000 bytes, and multiple anchors from one document repeat that text in separate result rows. Context selection can transfer about 8 MB of unused document text for eight small passages. This adds database work and memory use within the short retrieval budget. The existing neighbor-expansion and get_chunks_by_ids queries already exclude these fields.
Why we think it's a valid issue
- Checked: The new
expand_neighbors=Falsebranch insearch_knowledgeatproducts/business_knowledge/backend/logic.py:2545-2552. I compared it with the neighbor-expansion query in the same function and withget_chunks_by_idsatlogic.py:2805-2838. I also checked the field sizes inKnowledgeDocumentand the single caller inproducts/context_layer/backend/selection_sources.py:209-211. - Found: The new query uses
.select_related("source", "document")with no.only(...). Django therefore selects every column ofKnowledgeChunk,KnowledgeSourceandKnowledgeDocument. That includesKnowledgeDocument.content(aTextField) andKnowledgeDocument.metadata(aJSONField) atmodels/knowledge_document.py:27-28. - Found:
KnowledgeDocument.contentholds the full parsed document.MAX_TEXT_SIZE_BYTES = 1_000_000(constants.py:16) bounds text sources, andCLASSIFY_MAX_TOTAL_CHARS = 1_000_000lets SAFE documents up to 1M chars become searchable._result_from_chunk(logic.py:2440-2453) reads onlydocument.titleanddocument.urlfrom the document, so the content and metadata are never used. - Found: The other two hydration queries already apply the same
.only(...)projection. One is the neighbor-expansion query in the same function. The other isget_chunks_by_ids(logic.py:2823-2835). The new branch is the only one that does not. - Found:
search_business_knowledgecalls this path on every eligible prompt withlimit=SOURCE_LIMITS["business_knowledge"], which is 8 (selection_types.py:23). Each chunk row repeats the joined document columns, so several anchors from one large document fetch and detoast the same 1 MB text several times. - Impact: A team with large crawled or uploaded documents can transfer megabytes of unused text from Postgres for each prompt. That text then sits in Python memory. The work happens inside the retrieval deadline, and the search gets only half of the remaining deadline. This adds avoidable latency and can push the search past its budget, which drops the Business Knowledge context. The fix is small: replace the branch with
get_chunks_by_ids(team_id, [a.id for a in anchor_chunks]). That helper already keeps the input order, applies_safe_chunks_qs, and drops IDs that no longer resolve.
Suggested fix
Apply the same .only(...) projection used by get_chunks_by_ids, including chunk content and the required source and document labels. Alternatively, reuse that helper with the ordered anchor IDs. Preserve relevance order and the current safety filters.
Prompt to fix with AI (copy-paste)
## Context
@products/business_knowledge/backend/logic.py#L2546-2551
<issue_description>
The new anchor-only query joins source and document without restricting selected fields. Django therefore loads KnowledgeDocument.content and metadata for every matching chunk. Document text can reach 1,000,000 bytes, and multiple anchors from one document repeat that text in separate result rows. Context selection can transfer about 8 MB of unused document text for eight small passages. This adds database work and memory use within the short retrieval budget. The existing neighbor-expansion and get_chunks_by_ids queries already exclude these fields.
</issue_description>
<issue_validation>
- **Checked:** The new `expand_neighbors=False` branch in `search_knowledge` at `products/business_knowledge/backend/logic.py:2545-2552`. I compared it with the neighbor-expansion query in the same function and with `get_chunks_by_ids` at `logic.py:2805-2838`. I also checked the field sizes in `KnowledgeDocument` and the single caller in `products/context_layer/backend/selection_sources.py:209-211`.
- **Found:** The new query uses `.select_related("source", "document")` with no `.only(...)`. Django therefore selects every column of `KnowledgeChunk`, `KnowledgeSource` and `KnowledgeDocument`. That includes `KnowledgeDocument.content` (a `TextField`) and `KnowledgeDocument.metadata` (a `JSONField`) at `models/knowledge_document.py:27-28`.
- **Found:** `KnowledgeDocument.content` holds the full parsed document. `MAX_TEXT_SIZE_BYTES = 1_000_000` (`constants.py:16`) bounds text sources, and `CLASSIFY_MAX_TOTAL_CHARS = 1_000_000` lets SAFE documents up to 1M chars become searchable. `_result_from_chunk` (`logic.py:2440-2453`) reads only `document.title` and `document.url` from the document, so the content and metadata are never used.
- **Found:** The other two hydration queries already apply the same `.only(...)` projection. One is the neighbor-expansion query in the same function. The other is `get_chunks_by_ids` (`logic.py:2823-2835`). The new branch is the only one that does not.
- **Found:** `search_business_knowledge` calls this path on every eligible prompt with `limit=SOURCE_LIMITS["business_knowledge"]`, which is 8 (`selection_types.py:23`). Each chunk row repeats the joined document columns, so several anchors from one large document fetch and detoast the same 1 MB text several times.
- **Impact:** A team with large crawled or uploaded documents can transfer megabytes of unused text from Postgres for each prompt. That text then sits in Python memory. The work happens inside the retrieval deadline, and the search gets only half of the remaining deadline. This adds avoidable latency and can push the search past its budget, which drops the Business Knowledge context. The fix is small: replace the branch with `get_chunks_by_ids(team_id, [a.id for a in anchor_chunks])`. That helper already keeps the input order, applies `_safe_chunks_qs`, and drops IDs that no longer resolve.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Apply the same .only(...) projection used by get_chunks_by_ids, including chunk content and the required source and document labels. Alternatively, reuse that helper with the ordered anchor IDs. Preserve relevance order and the current safety filters.
</potential_solution>
| def validate_candidates(team: Team, user: User, candidates: list[Candidate]) -> list[Candidate]: | ||
| if not candidates: | ||
| return [] | ||
| access = UserAccessControl(user=user, team=team) |
Contributor
There was a problem hiding this comment.
Check parent permissions for project-scoped catalog sources
Issue description
metrics_for_team and relationships_for_team use TeamScopedManager.for_team, which resolves a child environment to its parent project. The access checker and shared_catalog query still use the child team. A parent data_catalog denial therefore blocks a parent metric through the parent path but permits it through the child path. Parent warehouse restrictions also escape the shared-source guard. Final revalidation repeats this mismatch, so an eligible child run can receive restricted parent definitions.
Why we think it's a valid issue
- Checked:
search_sourcesandvalidate_candidatesinproducts/context_layer/backend/selection_sources.py. I also checked the catalog helpers inproducts/data_catalog/backend/logic/{metrics,relationships,certifications}.py, the manager scoping inposthog/models/scoping/manager.py, the team lookup inUserAccessControl(products/access_control/backend/facade/user_access_control.py), how the catalog API is routed, and the PR's child-environment test. - Found:
metrics_for_team(metrics.py:552-554) andrelationships_for_team(relationships.py:306-307) callModel.objects.for_team(team.id).MetricandRelationshipProposaluseTeamScopedRootMixin.TeamScopedManager.for_team(manager.py:118-131) callsresolve_effective_team_id, which maps a child environment to its parent. A child-team run therefore gets the parent project's metrics and relationships atselection_sources.py:93,:106,:163and:176. - Found: The permission gates still use the literal child team.
UserAccessControl(user=user, team=team)atselection_sources.py:129preloads onlyAccessControlrows whereteam_id = self._team.id(user_access_control.py:543), and the UAC code has noparent_teamresolution. Theshared_catalogguard atselection_sources.py:155-160filtersAccessControl.objects.filter(team=team, ...), andsystem_table_denials(team, user, access)at:162also gets the child team. Adata_catalog, warehouse,external_data_sourceorinsightrestriction stored on the parent team does not match either gate. - Found: The catalog API sits on the projects router (
products/data_catalog/backend/routes.py:11-21).TeamAndOrgViewSetMixin.teamloads the team byproject_id(posthog/api/routing.py:467-476), so the existing endpoints check these metrics against the parent team's ACLs. The selection path does not. - Found: The PR already resolves the parent for Business Knowledge (
selection_sources.py:180,:199). Its testtest_child_environment_uses_parent_knowledge_and_permissions(test/test_selection.py:282-305) puts theAccessControlrow onself.team(the parent) and expects the child run to exclude the source. No test covers this for catalog sources. The parent-ACL test attest_selection.py:271-280runs only withself.team, not a child. - Impact: Take a project with a child environment and a parent-level
data_catalog: none(or warehouse) restriction. An eligible run in the child environment gets the parent's metric definitions and relationship reasoning in its hidden model context, which the project API would deny. Revalidation in_select(selection_service.py:204) callsvalidate_candidates(run.team, ...)with the same child team, so it does not remove them either. This is an authorization bypass in the gate the PR describes as constraining injection by actor permissions.
Suggested fix
Check permissions against the team that owns each source. Use the canonical parent for metrics and relationship proposals, including AccessControl guards and system-table denials. Keep certifications and skills scoped to their environment. Add child-environment regressions with parent catalog and warehouse restrictions. Verify that retrieval and final revalidation both exclude restricted sources.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_sources.py#L129
@products/context_layer/backend/selection_sources.py#L155-163
<issue_description>
metrics_for_team and relationships_for_team use TeamScopedManager.for_team, which resolves a child environment to its parent project. The access checker and shared_catalog query still use the child team. A parent data_catalog denial therefore blocks a parent metric through the parent path but permits it through the child path. Parent warehouse restrictions also escape the shared-source guard. Final revalidation repeats this mismatch, so an eligible child run can receive restricted parent definitions.
</issue_description>
<issue_validation>
- **Checked:** `search_sources` and `validate_candidates` in `products/context_layer/backend/selection_sources.py`. I also checked the catalog helpers in `products/data_catalog/backend/logic/{metrics,relationships,certifications}.py`, the manager scoping in `posthog/models/scoping/manager.py`, the team lookup in `UserAccessControl` (`products/access_control/backend/facade/user_access_control.py`), how the catalog API is routed, and the PR's child-environment test.
- **Found:** `metrics_for_team` (`metrics.py:552-554`) and `relationships_for_team` (`relationships.py:306-307`) call `Model.objects.for_team(team.id)`. `Metric` and `RelationshipProposal` use `TeamScopedRootMixin`. `TeamScopedManager.for_team` (`manager.py:118-131`) calls `resolve_effective_team_id`, which maps a child environment to its parent. A child-team run therefore gets the parent project's metrics and relationships at `selection_sources.py:93`, `:106`, `:163` and `:176`.
- **Found:** The permission gates still use the literal child team. `UserAccessControl(user=user, team=team)` at `selection_sources.py:129` preloads only `AccessControl` rows where `team_id = self._team.id` (`user_access_control.py:543`), and the UAC code has no `parent_team` resolution. The `shared_catalog` guard at `selection_sources.py:155-160` filters `AccessControl.objects.filter(team=team, ...)`, and `system_table_denials(team, user, access)` at `:162` also gets the child team. A `data_catalog`, warehouse, `external_data_source` or `insight` restriction stored on the parent team does not match either gate.
- **Found:** The catalog API sits on the projects router (`products/data_catalog/backend/routes.py:11-21`). `TeamAndOrgViewSetMixin.team` loads the team by `project_id` (`posthog/api/routing.py:467-476`), so the existing endpoints check these metrics against the parent team's ACLs. The selection path does not.
- **Found:** The PR already resolves the parent for Business Knowledge (`selection_sources.py:180`, `:199`). Its test `test_child_environment_uses_parent_knowledge_and_permissions` (`test/test_selection.py:282-305`) puts the `AccessControl` row on `self.team` (the parent) and expects the child run to exclude the source. No test covers this for catalog sources. The parent-ACL test at `test_selection.py:271-280` runs only with `self.team`, not a child.
- **Impact:** Take a project with a child environment and a parent-level `data_catalog: none` (or warehouse) restriction. An eligible run in the child environment gets the parent's metric definitions and relationship reasoning in its hidden model context, which the project API would deny. Revalidation in `_select` (`selection_service.py:204`) calls `validate_candidates(run.team, ...)` with the same child team, so it does not remove them either. This is an authorization bypass in the gate the PR describes as constraining injection by actor permissions.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Check permissions against the team that owns each source. Use the canonical parent for metrics and relationship proposals, including AccessControl guards and system-table denials. Keep certifications and skills scoped to their environment. Add child-environment regressions with parent catalog and warehouse restrictions. Verify that retrieval and final revalidation both exclude restricted sources.
</potential_solution>
Comment on lines
+157
to
+162
| and not AccessControl.objects.filter( | ||
| team=team, resource__in=["data_catalog", *WAREHOUSE_ACCESS_SCOPES, "external_data_source", "insight"] | ||
| ).exists() | ||
| ) | ||
| if shared_catalog and access.check_access_level_for_resource("data_catalog", "viewer"): | ||
| denied = {f"system.{name}" for name in system_table_denials(team, user, access)} |
Contributor
There was a problem hiding this comment.
Apply shared-source restrictions to referenced system tables
Issue description
The shared-source guard omits permission scopes used by many system tables. For example, system.logs_views uses the logs scope, which this query does not include. An administrator passes system_table_denials even when other conversation readers have no logs access. Selection can then inject a metric definition that references this restricted table. metrics_visible_to_user hides that metric from the denied reader, but the agent can disclose its definition in the shared conversation. Final revalidation uses the same incomplete guard.
Why we think it's a valid issue
- Checked: The
shared_catalogguard and thesystem_table_denialsfilter invalidate_candidates(products/context_layer/backend/selection_sources.py:155-178). I also checked how metric definitions record table references (products/data_catalog/backend/logic/validation.py:51-93,:212-236), the scopes onsystem.*tables (posthog/hogql/database/schema/system.py),metrics_visible_to_user(products/data_catalog/backend/logic/metrics.py:557-572), and the guard test (products/context_layer/backend/test/test_selection.py:270-280). - Found: The guard skips catalog sources only when the project customizes
data_catalog,WAREHOUSE_ACCESS_SCOPES,external_data_sourceorinsight(selection_sources.py:157-159). The code comment at:153-154says the purpose is to skip semantic sources when the project customizes access to the underlying resources, because shared chats can outlive the actor. - Found: A HogQL metric definition can read
system.*tables._validate_hogqlresolves the query throughHogQLContext, and_TableReferenceCollector.visit_join_expr(validation.py:87-93) stores dotted names such assystem.logs_viewsinreferenced_table_names. System tables carry about 30 other access scopes, for examplelogs(system.py:2200-2203),notebook,feature_flaganderror_tracking. The guard covers none of them. - Found: The only other filter is
system_table_denials(team, user, access)atselection_sources.py:162. It checks only the actor. When the actor can read logs, a metric that readssystem.logs_viewspasses it. For a member who cannot read logs,metrics_visible_to_user(metrics.py:557-572) hides the same metric, so the product does treat that definition as restricted. The guard test parameterizes only the warehouse scopes (test_selection.py:270). - Impact: The gap is real, and it breaks the PR's own rule for shared conversations. But the trigger is narrow. It needs a metric over a
system.*table, customized access for that table's resource, a privileged actor, and a less-privileged reader of the same Slack thread or chat history. The leaked data is the metric's query text, not the rows of the restricted table. The agent already runs with the actor's scopes, so it can fetch and post the same definition through its normal tools. - Priority: I lowered this to
consider. The leak happens only through a defense-in-depth audience guard, it needs several unusual conditions at once, and the feature is limited to staff and allowlisted teams. It does not let the actor bypass their own permissions.
Suggested fix
Use access_controlled_system_tables() to identify the scopes of referenced system tables. Include inherited resource scopes when checking customized access. Exclude affected candidates from shared context even when the actor can read them. Add a regression with a privileged actor and a logs restriction.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_sources.py#L157-162
<issue_description>
The shared-source guard omits permission scopes used by many system tables. For example, system.logs_views uses the logs scope, which this query does not include. An administrator passes system_table_denials even when other conversation readers have no logs access. Selection can then inject a metric definition that references this restricted table. metrics_visible_to_user hides that metric from the denied reader, but the agent can disclose its definition in the shared conversation. Final revalidation uses the same incomplete guard.
</issue_description>
<issue_validation>
- **Checked:** The `shared_catalog` guard and the `system_table_denials` filter in `validate_candidates` (`products/context_layer/backend/selection_sources.py:155-178`). I also checked how metric definitions record table references (`products/data_catalog/backend/logic/validation.py:51-93`, `:212-236`), the scopes on `system.*` tables (`posthog/hogql/database/schema/system.py`), `metrics_visible_to_user` (`products/data_catalog/backend/logic/metrics.py:557-572`), and the guard test (`products/context_layer/backend/test/test_selection.py:270-280`).
- **Found:** The guard skips catalog sources only when the project customizes `data_catalog`, `WAREHOUSE_ACCESS_SCOPES`, `external_data_source` or `insight` (`selection_sources.py:157-159`). The code comment at `:153-154` says the purpose is to skip semantic sources when the project customizes access to the underlying resources, because shared chats can outlive the actor.
- **Found:** A HogQL metric definition can read `system.*` tables. `_validate_hogql` resolves the query through `HogQLContext`, and `_TableReferenceCollector.visit_join_expr` (`validation.py:87-93`) stores dotted names such as `system.logs_views` in `referenced_table_names`. System tables carry about 30 other access scopes, for example `logs` (`system.py:2200-2203`), `notebook`, `feature_flag` and `error_tracking`. The guard covers none of them.
- **Found:** The only other filter is `system_table_denials(team, user, access)` at `selection_sources.py:162`. It checks only the actor. When the actor can read logs, a metric that reads `system.logs_views` passes it. For a member who cannot read logs, `metrics_visible_to_user` (`metrics.py:557-572`) hides the same metric, so the product does treat that definition as restricted. The guard test parameterizes only the warehouse scopes (`test_selection.py:270`).
- **Impact:** The gap is real, and it breaks the PR's own rule for shared conversations. But the trigger is narrow. It needs a metric over a `system.*` table, customized access for that table's resource, a privileged actor, and a less-privileged reader of the same Slack thread or chat history. The leaked data is the metric's query text, not the rows of the restricted table. The agent already runs with the actor's scopes, so it can fetch and post the same definition through its normal tools.
- **Priority:** I lowered this to `consider`. The leak happens only through a defense-in-depth audience guard, it needs several unusual conditions at once, and the feature is limited to staff and allowlisted teams. It does not let the actor bypass their own permissions.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Use access_controlled_system_tables() to identify the scopes of referenced system tables. Include inherited resource scopes when checking customized access. Exclude affected candidates from shared context even when the actor can read them. Add a regression with a privileged actor and a logs restriction.
</potential_solution>
|
|
||
| resetHistory(runId: string, history = ""): void { | ||
| this.historyRunId = runId; | ||
| this.history = history.slice(-12_000); |
Contributor
There was a problem hiding this comment.
Preserve Unicode characters when truncating requests
Issue description
String.slice() can split a supplementary Unicode character at the truncation boundary. For example, ("😀" + "x".repeat(19_999)).slice(-20_000) starts with a lone low surrogate. JSON.stringify preserves it as an escape, and the backend JSON parser retains it. HTTPX then raises UnicodeEncodeError when GatewaySystemOneClient encodes the request as UTF-8. SelectionJudge catches that error and returns no gate result, so the turn receives no context. The history truncation has the same problem at its 12,000-unit boundary. I reproduced both encoding failures; ordinary prompt delivery still succeeds.
Why we think it's a valid issue
- Checked:
context-selection.ts:38,:77,:83,:103,:109. These lines truncate withString.slice(), which counts UTF-16 code units.posthog-api.ts:107-123sends the result throughJSON.stringify. On the backend I readselection_views.py(PrepareSerializer),selection_service.py(prepare,_select),selection_model.py(SelectionJudge.judge) andposthog/llm/system_one_client.py(GatewaySystemOneClient.decide). - Found: In Node,
("😀" + "x".repeat(19_999)).slice(-20_000)gives a string that starts with a lone\ude00.JSON.stringifysends it as the escape"\ude00xxx...". - Found: The failure happens earlier than the issue says. The
promptandhistoryfields inPrepareSerializer(selection_views.py) are DRF 3.17.2CharFields. Their defaultProhibitSurrogateCharactersValidatorrejects the lone surrogate withsurrogate_characters_not_allowed(I reproduced this for both fields). The endpoint returns HTTP 400 beforeprepare()runs, so the code never reachesSelectionJudgeor httpx. - Found: If the serializer let the string through, httpx 0.28.1
encode_json(ensure_ascii=Falsethen.encode("utf-8")) would raiseUnicodeEncodeError: surrogates not allowed. I reproduced this too. The issue's second failure path is real, but the serializer hides it. - Found: The
catchatcontext-selection.ts:89-95catches the failed request and reportsprepare_failed. The prompt then goes out without context. Ordinary delivery still works. - Impact: The trigger is real input, not a guess. History reaches the 12,000-unit cap after a few long assistant replies (
recordAssistantappends the full agent response each turn). After that, every turn cuts at an arbitrary point. A turn fails whenever that point falls inside an emoji or another supplementary-plane character. For that turn, the agent gets no context and the backend records no$ai_spanselection span. This puts silent gaps in the experiment data, and the gaps lean toward emoji-heavy conversations. The next append moves the cut point, so the failure lasts only one turn. The prompt path fails only for prompts over 20,000 units. - Impact: The fix is small and matches the PR's own approach:
schemas.ts:6already counts code points withArray.from(value), and the backend limits (MAX_PROMPT_CHARS,MAX_HISTORY_CHARS) count code points. Each failure is rare and the fallback is graceful, soconsideris the right priority.
Suggested fix
Use a shared helper that truncates by Unicode code points, such as Array.from(value).slice(-limit).join(""). Apply it to prompt truncation and every history truncation. Verify that an emoji at each boundary remains intact and that the resulting backend model request encodes successfully.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/server/context-selection.ts#L38
@packages/agent/packages/agent/src/server/context-selection.ts#L77-84
@packages/agent/packages/agent/src/server/context-selection.ts#L103-109
<issue_description>
String.slice() can split a supplementary Unicode character at the truncation boundary. For example, ("😀" + "x".repeat(19_999)).slice(-20_000) starts with a lone low surrogate. JSON.stringify preserves it as an escape, and the backend JSON parser retains it. HTTPX then raises UnicodeEncodeError when GatewaySystemOneClient encodes the request as UTF-8. SelectionJudge catches that error and returns no gate result, so the turn receives no context. The history truncation has the same problem at its 12,000-unit boundary. I reproduced both encoding failures; ordinary prompt delivery still succeeds.
</issue_description>
<issue_validation>
- **Checked:** `context-selection.ts:38`, `:77`, `:83`, `:103`, `:109`. These lines truncate with `String.slice()`, which counts UTF-16 code units. `posthog-api.ts:107-123` sends the result through `JSON.stringify`. On the backend I read `selection_views.py` (`PrepareSerializer`), `selection_service.py` (`prepare`, `_select`), `selection_model.py` (`SelectionJudge.judge`) and `posthog/llm/system_one_client.py` (`GatewaySystemOneClient.decide`).
- **Found:** In Node, `("😀" + "x".repeat(19_999)).slice(-20_000)` gives a string that starts with a lone `\ude00`. `JSON.stringify` sends it as the escape `"\ude00xxx..."`.
- **Found:** The failure happens earlier than the issue says. The `prompt` and `history` fields in `PrepareSerializer` (`selection_views.py`) are DRF 3.17.2 `CharField`s. Their default `ProhibitSurrogateCharactersValidator` rejects the lone surrogate with `surrogate_characters_not_allowed` (I reproduced this for both fields). The endpoint returns HTTP 400 before `prepare()` runs, so the code never reaches `SelectionJudge` or httpx.
- **Found:** If the serializer let the string through, httpx 0.28.1 `encode_json` (`ensure_ascii=False` then `.encode("utf-8")`) would raise `UnicodeEncodeError: surrogates not allowed`. I reproduced this too. The issue's second failure path is real, but the serializer hides it.
- **Found:** The `catch` at `context-selection.ts:89-95` catches the failed request and reports `prepare_failed`. The prompt then goes out without context. Ordinary delivery still works.
- **Impact:** The trigger is real input, not a guess. History reaches the 12,000-unit cap after a few long assistant replies (`recordAssistant` appends the full agent response each turn). After that, every turn cuts at an arbitrary point. A turn fails whenever that point falls inside an emoji or another supplementary-plane character. For that turn, the agent gets no context and the backend records no `$ai_span` selection span. This puts silent gaps in the experiment data, and the gaps lean toward emoji-heavy conversations. The next append moves the cut point, so the failure lasts only one turn. The prompt path fails only for prompts over 20,000 units.
- **Impact:** The fix is small and matches the PR's own approach: `schemas.ts:6` already counts code points with `Array.from(value)`, and the backend limits (`MAX_PROMPT_CHARS`, `MAX_HISTORY_CHARS`) count code points. Each failure is rare and the fallback is graceful, so `consider` is the right priority.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Use a shared helper that truncates by Unicode code points, such as Array.from(value).slice(-limit).join(""). Apply it to prompt truncation and every history truncation. Verify that an emoji at each boundary remains intact and that the resulting backend model request encodes successfully.
</potential_solution>
Comment on lines
+193
to
+202
| done, unfinished = wait(pending, timeout=max(0, deadline - time.monotonic())) | ||
| scored: list[tuple[Candidate, float]] = [] | ||
| for future in done: | ||
| probability = future.result() | ||
| if probability is not None: | ||
| scored.append((pending[future], probability)) | ||
| observation["unscored_count"] = len(candidates) - len(scored) | ||
| for future in unfinished: | ||
| future.cancel() | ||
| check_deadline(deadline) |
Contributor
There was a problem hiding this comment.
Preserve completed scores when one rerank times out
Issue description
The rerank wait consumes the entire remaining selection budget. If one scorer remains unfinished, the wait returns at the deadline with successful candidates in done. check_deadline then raises before revalidation or rendering. prepare returns empty context with reason="error", even when other candidates exceed the relevance threshold. An isolated reproduction with one immediate score of 0.9 and one delayed scorer confirmed this behavior. A single slow candidate therefore suppresses all completed results.
Why we think it's a valid issue
- Checked: The rerank and deadline flow in
_selectandprepare(products/context_layer/backend/selection_service.py:76-210),SelectionJudge.judge(selection_model.py), the outer wrapperbounded_request(selection_execution.py:23-47, called with 3.5s atselection_views.py:101),CONTEXT_SELECTION_TIMEOUT_SECONDS(posthog/settings/web.py:1567),SOURCE_LIMITS(selection_types.py:18-24), and the deadline tests intest/test_selection.py. - Found:
wait(pending, timeout=max(0, deadline - time.monotonic()))atselection_service.py:193uses the whole remaining budget. If one future is still running,waitreturns only at the deadline.check_deadline(deadline)at:202then raisesTimeoutError("selector_deadline").preparecatches it at:100-102and setscontext, reason = "", "error". The scores already indonenever reachvalidate_candidatesorrender. - Found: A slow scorer cannot finish early.
SelectionJudge.judgesets the HTTP timeout toremaining = self.deadline - time.monotonic(), so a slow call ends at about the same deadline. A queued judge that starts after the deadline raisesTimeoutError(selection_model.py, theremaining <= 0check). If that future is indone,future.result()atselection_service.py:196raises again and also discards all scores. - Found: The trigger is realistic. The whole budget is 3.0s (
web.py:1567), and the gate call and the retrieval step (up to half of the remaining time,selection_service.py:173) run first. Rerank can then have up to about 40 candidates (18 skills, 8 metrics, 3 certifications, 3 relationships, 8 Business Knowledge chunks). All of them share one 8-worker_EXECUTORfor the whole process (selection_service.py:35), so later candidates wait in a queue behind other calls and other requests. - Found: No test covers a partial rerank.
test_candidates_are_reranked_in_parallellets every scorer finish.test_slow_knowledge_does_not_suppress_ready_skillsshows the intended design for the retrieval step: slow work must not suppress ready results. The rerank step has no such protection. - Impact: When one scorer is slow or queued, every run drops all relevant context and records
reason="error". That happens even when other candidates score above the threshold. In treatment, the feature then delivers nothing for any turn with a large candidate set. In shadow, the spans report errors where a selection was possible, so the experiment data is wrong. Normal delivery still works, so the priority staysshould_fix.
Suggested fix
Set an earlier rerank cutoff that reserves time for permission revalidation and rendering. At that cutoff, process completed scores and omit unfinished candidates. Keep the final deadline and access checks. Add a regression that confirms a fast candidate reaches the context while another scorer exceeds the cutoff.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_service.py#L193-202
<issue_description>
The rerank wait consumes the entire remaining selection budget. If one scorer remains unfinished, the wait returns at the deadline with successful candidates in done. check_deadline then raises before revalidation or rendering. prepare returns empty context with reason="error", even when other candidates exceed the relevance threshold. An isolated reproduction with one immediate score of 0.9 and one delayed scorer confirmed this behavior. A single slow candidate therefore suppresses all completed results.
</issue_description>
<issue_validation>
- **Checked:** The rerank and deadline flow in `_select` and `prepare` (`products/context_layer/backend/selection_service.py:76-210`), `SelectionJudge.judge` (`selection_model.py`), the outer wrapper `bounded_request` (`selection_execution.py:23-47`, called with 3.5s at `selection_views.py:101`), `CONTEXT_SELECTION_TIMEOUT_SECONDS` (`posthog/settings/web.py:1567`), `SOURCE_LIMITS` (`selection_types.py:18-24`), and the deadline tests in `test/test_selection.py`.
- **Found:** `wait(pending, timeout=max(0, deadline - time.monotonic()))` at `selection_service.py:193` uses the whole remaining budget. If one future is still running, `wait` returns only at the deadline. `check_deadline(deadline)` at `:202` then raises `TimeoutError("selector_deadline")`. `prepare` catches it at `:100-102` and sets `context, reason = "", "error"`. The scores already in `done` never reach `validate_candidates` or `render`.
- **Found:** A slow scorer cannot finish early. `SelectionJudge.judge` sets the HTTP timeout to `remaining = self.deadline - time.monotonic()`, so a slow call ends at about the same deadline. A queued judge that starts after the deadline raises `TimeoutError` (`selection_model.py`, the `remaining <= 0` check). If that future is in `done`, `future.result()` at `selection_service.py:196` raises again and also discards all scores.
- **Found:** The trigger is realistic. The whole budget is 3.0s (`web.py:1567`), and the gate call and the retrieval step (up to half of the remaining time, `selection_service.py:173`) run first. Rerank can then have up to about 40 candidates (18 skills, 8 metrics, 3 certifications, 3 relationships, 8 Business Knowledge chunks). All of them share one 8-worker `_EXECUTOR` for the whole process (`selection_service.py:35`), so later candidates wait in a queue behind other calls and other requests.
- **Found:** No test covers a partial rerank. `test_candidates_are_reranked_in_parallel` lets every scorer finish. `test_slow_knowledge_does_not_suppress_ready_skills` shows the intended design for the retrieval step: slow work must not suppress ready results. The rerank step has no such protection.
- **Impact:** When one scorer is slow or queued, every run drops all relevant context and records `reason="error"`. That happens even when other candidates score above the threshold. In treatment, the feature then delivers nothing for any turn with a large candidate set. In shadow, the spans report errors where a selection was possible, so the experiment data is wrong. Normal delivery still works, so the priority stays `should_fix`.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Set an earlier rerank cutoff that reserves time for permission revalidation and rendering. At that cutoff, process completed scores and omit unfinished candidates. Keep the final deadline and access checks. Add a regression that confirms a fast candidate reaches the context while another scorer exceeds the cutoff.
</potential_solution>
Comment on lines
+1357
to
+1361
| state_updates["context_selection_eligible"] = context_layer_facade.context_selection_enabled_for_run( | ||
| task_run.team_id, | ||
| task_run.id, | ||
| actor_user or task.created_by, | ||
| ) |
Contributor
There was a problem hiding this comment.
Refresh selection eligibility when the Slack actor changes
Issue description
A Slack run that starts with a non-staff actor stores false here. When a staff actor takes over, send_followup_to_sandbox updates slack_actor_user_id and refreshes credentials, but it does not update context_selection_eligible. The agent also reads this field only during startup. ContextSelection.dispatch therefore skips preparation for the staff actor's prompts, even when selection_mode returns treatment. Selection remains disabled until the agent restarts.
Why we think it's a valid issue
- Checked:
get_task_processing_context.py:1357-1361,send_followup_to_sandbox.py:315-378,context_layer/backend/facade/api.py:185-196,selection_service.py:50-68,selection_views.py:78-98,tasks/backend/facade/api.py:11685-11693, and in the agent:agent-server.ts:1844-1880,agent-server.ts:3075-3076,context-selection.ts:49. - Found: The activity computes
context_selection_eligibleone time, from the actor at workflow start.selection_modereturnsdisabledwhenactor.is_staffis false (selection_service.py:51-52). So a non-staff Slack starter writesfalseto run state. - Found: On an actor change,
send_followup_to_sandbox.py:363-378writes the newslack_actor_user_idand callsrefresh_store_skills_state(..., reason="slack_actor_change"). It does not recomputecontext_selection_eligible. The codebase therefore treats an actor change as a real case and refreshes other per-actor state, but not this flag. - Found: The agent sets
this.contextSelection.enabledonly inprepareInitialTaskMessage(agent-server.ts:3075-3076). Therefresh_sessionhandler (agent-server.ts:1844-1880) updatescontextSelectionApiKeyand store skills, but it does not touchenabled.ContextSelection.dispatchreturnssend(prompt)early whenenabledis false (context-selection.ts:49). So the staff actor's prompts never reach the prepare endpoint. - Found: The opposite direction (staff starter, then non-staff actor) is safe. The prepare endpoint checks
is_current_task_run_actor(selection_views.py:97), andpreparere-runsselection_modeagainst the current actor (selection_service.py:90). - Impact: In an allowlisted team, a Slack thread that a non-staff user starts gets no selection for any later staff turn in the same run, even when the flag returns
treatmentorshadow. No selection span is recorded for those turns, so the experiment silently loses coverage for mixed-actor Slack threads. The failure is fail-closed: nothing leaks and no data is corrupted. - Priority: Lowered to
consider. The defect is real and has a concrete trigger. But it affects only a staff-only experiment in an explicit allowlist, it fails closed, and it only reduces sample coverage. It causes no user-visible harm.
Suggested fix
Recompute context_selection_eligible when the Slack actor changes, before refreshing the sandbox session. Update ContextSelection.enabled from the refreshed run state during refresh_session. Keep the prepare endpoint's current actor checks.
Prompt to fix with AI (copy-paste)
## Context
@products/tasks/backend/temporal/process_task/activities/get_task_processing_context.py#L1357-1361
<issue_description>
A Slack run that starts with a non-staff actor stores false here. When a staff actor takes over, send_followup_to_sandbox updates slack_actor_user_id and refreshes credentials, but it does not update context_selection_eligible. The agent also reads this field only during startup. ContextSelection.dispatch therefore skips preparation for the staff actor's prompts, even when selection_mode returns treatment. Selection remains disabled until the agent restarts.
</issue_description>
<issue_validation>
- **Checked:** `get_task_processing_context.py:1357-1361`, `send_followup_to_sandbox.py:315-378`, `context_layer/backend/facade/api.py:185-196`, `selection_service.py:50-68`, `selection_views.py:78-98`, `tasks/backend/facade/api.py:11685-11693`, and in the agent: `agent-server.ts:1844-1880`, `agent-server.ts:3075-3076`, `context-selection.ts:49`.
- **Found:** The activity computes `context_selection_eligible` one time, from the actor at workflow start. `selection_mode` returns `disabled` when `actor.is_staff` is false (`selection_service.py:51-52`). So a non-staff Slack starter writes `false` to run state.
- **Found:** On an actor change, `send_followup_to_sandbox.py:363-378` writes the new `slack_actor_user_id` and calls `refresh_store_skills_state(..., reason="slack_actor_change")`. It does not recompute `context_selection_eligible`. The codebase therefore treats an actor change as a real case and refreshes other per-actor state, but not this flag.
- **Found:** The agent sets `this.contextSelection.enabled` only in `prepareInitialTaskMessage` (`agent-server.ts:3075-3076`). The `refresh_session` handler (`agent-server.ts:1844-1880`) updates `contextSelectionApiKey` and store skills, but it does not touch `enabled`. `ContextSelection.dispatch` returns `send(prompt)` early when `enabled` is false (`context-selection.ts:49`). So the staff actor's prompts never reach the prepare endpoint.
- **Found:** The opposite direction (staff starter, then non-staff actor) is safe. The prepare endpoint checks `is_current_task_run_actor` (`selection_views.py:97`), and `prepare` re-runs `selection_mode` against the current actor (`selection_service.py:90`).
- **Impact:** In an allowlisted team, a Slack thread that a non-staff user starts gets no selection for any later staff turn in the same run, even when the flag returns `treatment` or `shadow`. No selection span is recorded for those turns, so the experiment silently loses coverage for mixed-actor Slack threads. The failure is fail-closed: nothing leaks and no data is corrupted.
- **Priority:** Lowered to `consider`. The defect is real and has a concrete trigger. But it affects only a staff-only experiment in an explicit allowlist, it fails closed, and it only reduces sample coverage. It causes no user-visible harm.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Recompute context_selection_eligible when the Slack actor changes, before refreshing the sandbox session. Update ContextSelection.enabled from the refreshed run state during refresh_session. Keep the prepare endpoint's current actor checks.
</potential_solution>
Comment on lines
+3632
to
+3634
| true, | ||
| builtPrompt.messageId, | ||
| ); |
Contributor
There was a problem hiding this comment.
Seed selection history for autonomous summary resumes
Issue description
In a fresh process, a summary resume without a pending human message leaves builtPrompt.messageId undefined. promptWithUpstreamRetry therefore bypasses ContextSelection.dispatch, which normally initializes selection history. Unlike sendResumeContinuation, sendResumeMessage does not call resetHistory. The added recordAssistant call also discards the continuation answer because historyRunId is unset. runResumeTurn then clears resumeState. An isolated reproduction confirmed that the next human message sends empty selection history despite the restored conversation. The gate and reranker cannot use that conversation to interpret follow-ups such as "Does that definition still apply?".
Why we think it's a valid issue
- Checked: I traced history seeding through
ContextSelection(context-selection.ts:36-40,:74-77,:107-110) and through each resume entry point inagent-server.ts:sendResumeContinuation,preparePrewarmedResumePrompt,sendResumeMessage,runResumeTurn,promptWithUpstreamRetry, and the follow-up command path at:1590-1657. I also checked howprovision_sandbox.py:598-604setsPOSTHOG_RESUME_IDLE. - Found: The native-resume paths seed history from
resumeState.conversation, atagent-server.ts:3467-3472and:3350-3355. Two other paths seed it indirectly. Their prompts carry the conversation in hidden blocks, anddispatchreads those blocks asrestoredHistory(context-selection.ts:57,:77). These paths are the deferred summary resume (wrapPromptWithSummaryResume, passed ashumanPromptat:1656) and the summary resume that has a pending message (request.promptat:2895). - Found: When
sendResumeMessage(:3261) has no pending prompt, it returnsmessageId: undefined.runResumeTurnpasses that value topromptWithUpstreamRetry(:3626-3634). BecausecontextMessageIdis falsy, the call goes straight toclientConnection.prompt(:2897).dispatchnever runs, sohistoryRunIdstays unset in a fresh process.recordAssistantat:3650-3653then returns early onthis.historyRunId !== runId(context-selection.ts:108), so the history loses the continuation reply.resumeStatebecomesnullat:3641. - Found: On the next human message,
preparePrewarmedResumePromptsees noresumeStateand does not wrap the prompt.preparePromptcallsresetHistory(runId)with an empty string, and the plain user prompt has no hidden blocks to restore. The backend getshistory: ""withhistory_source: "resume_prompt". - Found: This path is reachable in production. It needs a summary fallback (native resume unavailable), no pending message, and
POSTHOG_RESUME_IDLEunset. The variable is unset forresume_from_run_idruns and for non-idlesame_run_resumeruns (provision_sandbox.py:599-604).prepareInitialTaskMessagethen returnsaction: "resume", andsendInitialTaskMessageroutes tosendResumeMessage(:3142-3147). - Impact: For the rest of that run, the history sent to the gate, to the reranker, and to
search_sources(which searchesprompt + "\n" + historyatselection_service.py:169) never contains the restored conversation. Retrieval for follow-ups such as "does that still apply?" gets weaker, and the selection span recordsresume_promptas the history source with empty history. The main agent still has the full summary, and nothing fails. Other resume paths show that the author intended to seed history. The fix follows the existingresetHistory(...)pattern. - Priority: I lowered this to
consider. The gap is on a fallback recovery path (summary resume, no pending message) of a feature that is behind a flag and an allowlist. It only lowers selection quality. It does not break delivery, corrupt data, or show an error to users.
Suggested fix
Call resetHistory with the bounded resumeState.conversation when setting up a summary resume, before dispatching its continuation. Follow the existing native-resume initialization pattern. Keep selection disabled for the autonomous turn. Verify that the next human selection receives the restored conversation and the continuation answer.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/server/agent-server.ts#L3632-3634
@packages/agent/packages/agent/src/server/agent-server.ts#L3650-3653
<issue_description>
In a fresh process, a summary resume without a pending human message leaves builtPrompt.messageId undefined. promptWithUpstreamRetry therefore bypasses ContextSelection.dispatch, which normally initializes selection history. Unlike sendResumeContinuation, sendResumeMessage does not call resetHistory. The added recordAssistant call also discards the continuation answer because historyRunId is unset. runResumeTurn then clears resumeState. An isolated reproduction confirmed that the next human message sends empty selection history despite the restored conversation. The gate and reranker cannot use that conversation to interpret follow-ups such as "Does that definition still apply?".
</issue_description>
<issue_validation>
- **Checked:** I traced history seeding through `ContextSelection` (`context-selection.ts:36-40`, `:74-77`, `:107-110`) and through each resume entry point in `agent-server.ts`: `sendResumeContinuation`, `preparePrewarmedResumePrompt`, `sendResumeMessage`, `runResumeTurn`, `promptWithUpstreamRetry`, and the follow-up command path at `:1590-1657`. I also checked how `provision_sandbox.py:598-604` sets `POSTHOG_RESUME_IDLE`.
- **Found:** The native-resume paths seed history from `resumeState.conversation`, at `agent-server.ts:3467-3472` and `:3350-3355`. Two other paths seed it indirectly. Their prompts carry the conversation in hidden blocks, and `dispatch` reads those blocks as `restoredHistory` (`context-selection.ts:57`, `:77`). These paths are the deferred summary resume (`wrapPromptWithSummaryResume`, passed as `humanPrompt` at `:1656`) and the summary resume that has a pending message (`request.prompt` at `:2895`).
- **Found:** When `sendResumeMessage` (`:3261`) has no pending prompt, it returns `messageId: undefined`. `runResumeTurn` passes that value to `promptWithUpstreamRetry` (`:3626-3634`). Because `contextMessageId` is falsy, the call goes straight to `clientConnection.prompt` (`:2897`). `dispatch` never runs, so `historyRunId` stays unset in a fresh process. `recordAssistant` at `:3650-3653` then returns early on `this.historyRunId !== runId` (`context-selection.ts:108`), so the history loses the continuation reply. `resumeState` becomes `null` at `:3641`.
- **Found:** On the next human message, `preparePrewarmedResumePrompt` sees no `resumeState` and does not wrap the prompt. `preparePrompt` calls `resetHistory(runId)` with an empty string, and the plain user prompt has no hidden blocks to restore. The backend gets `history: ""` with `history_source: "resume_prompt"`.
- **Found:** This path is reachable in production. It needs a summary fallback (native resume unavailable), no pending message, and `POSTHOG_RESUME_IDLE` unset. The variable is unset for `resume_from_run_id` runs and for non-idle `same_run_resume` runs (`provision_sandbox.py:599-604`). `prepareInitialTaskMessage` then returns `action: "resume"`, and `sendInitialTaskMessage` routes to `sendResumeMessage` (`:3142-3147`).
- **Impact:** For the rest of that run, the history sent to the gate, to the reranker, and to `search_sources` (which searches `prompt + "\n" + history` at `selection_service.py:169`) never contains the restored conversation. Retrieval for follow-ups such as "does that still apply?" gets weaker, and the selection span records `resume_prompt` as the history source with empty history. The main agent still has the full summary, and nothing fails. Other resume paths show that the author intended to seed history. The fix follows the existing `resetHistory(...)` pattern.
- **Priority:** I lowered this to `consider`. The gap is on a fallback recovery path (summary resume, no pending message) of a feature that is behind a flag and an allowlist. It only lowers selection quality. It does not break delivery, corrupt data, or show an error to users.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Call resetHistory with the bounded resumeState.conversation when setting up a summary resume, before dispatching its continuation. Follow the existing native-resume initialization pattern. Keep selection disabled for the autonomous turn. Verify that the next human selection receives the restored conversation and the continuation answer.
</potential_solution>
Comment on lines
+146
to
+152
| skills = access.filter_queryset_by_access_level( | ||
| LLMSkill.objects.filter(team=team, id__in=ids["skill"], deleted=False, is_latest=True, category="").exclude( | ||
| id__in=private_skill_ids | ||
| ), | ||
| resource="llm_skill", | ||
| ) | ||
| result.extend(make_record("skill", row) for row in skills) |
Contributor
There was a problem hiding this comment.
Keep unused skill bodies out of validation queries
Issue description
Skill validation reloads complete LLMSkill rows, including body, although make_record reads only the ID, team ID, name, description, and version. The initial retrieval query restricts these fields, but validation removes that benefit. A skill body can reach 1,000,000 bytes. Eighteen candidates can therefore load about 18 MB of unused bodies per validation pass. Validation runs during retrieval and again after scoring, which increases database transfer, memory use, and prompt delay.
Why we think it's a valid issue
- Checked: The skill branch of
validate_candidates(products/context_layer/backend/selection_sources.py:141-152), the retrieval query insearch_sources(:83-90),make_record(:37-41), theLLMSkillmodel (products/skills/backend/models/skills.py:46-104), the skill body limit (products/skills/backend/api/skill_services.py:27),UserAccessControl.filter_queryset_by_access_level(products/access_control/backend/facade/user_access_control.py:1170-1210), and the two places that call validation (selection_sources.py:111andselection_service.py:204). - Found: The validation queryset at
selection_sources.py:147-151has no.only(...). It therefore loads everyLLMSkillcolumn:body(aTextField,skills.py:74),metadata,allowed_tools,license,compatibilityand the timestamps.make_recordreads onlyid,team_id,name,descriptionandversion. The retrieval query at:83-85already limits the rows to those five fields. - Found:
MAX_SKILL_BODY_BYTES = 1_000_000(skill_services.py:27), andSOURCE_LIMITS["skill"] = 18.search_sourcesvalidates up to 18 skills, and_selectvalidates the scored subset again.filter_queryset_by_access_levelonly addsfilter/excludeclauses onidandcreated_by(user_access_control.py:1196-1209), so a.only(...)on the base queryset still works with it. - Impact: Each validation pass sends every matched skill body from Postgres into Python memory, and the code never uses it. The 3.0s selection deadline covers this work. The fix is to copy the
.only(...)projection from the retrieval query. - Priority: I lowered this to
consider. Skill bodies are markdown instructions and are usually a few KB to tens of KB, so a normal pass moves well under 1 MB and costs a few milliseconds. The 18 MB figure needs every candidate near the 1 MB limit. That differs from the Business Knowledge case (3-2-2), where full source documents are often large and repeat once per chunk.
Suggested fix
Apply .only("id", "team_id", "name", "description", "version") to the validated skill queryset, matching the initial retrieval projection. Keep the access filters and final source revalidation.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_sources.py#L146-152
<issue_description>
Skill validation reloads complete LLMSkill rows, including body, although make_record reads only the ID, team ID, name, description, and version. The initial retrieval query restricts these fields, but validation removes that benefit. A skill body can reach 1,000,000 bytes. Eighteen candidates can therefore load about 18 MB of unused bodies per validation pass. Validation runs during retrieval and again after scoring, which increases database transfer, memory use, and prompt delay.
</issue_description>
<issue_validation>
- **Checked:** The skill branch of `validate_candidates` (`products/context_layer/backend/selection_sources.py:141-152`), the retrieval query in `search_sources` (`:83-90`), `make_record` (`:37-41`), the `LLMSkill` model (`products/skills/backend/models/skills.py:46-104`), the skill body limit (`products/skills/backend/api/skill_services.py:27`), `UserAccessControl.filter_queryset_by_access_level` (`products/access_control/backend/facade/user_access_control.py:1170-1210`), and the two places that call validation (`selection_sources.py:111` and `selection_service.py:204`).
- **Found:** The validation queryset at `selection_sources.py:147-151` has no `.only(...)`. It therefore loads every `LLMSkill` column: `body` (a `TextField`, `skills.py:74`), `metadata`, `allowed_tools`, `license`, `compatibility` and the timestamps. `make_record` reads only `id`, `team_id`, `name`, `description` and `version`. The retrieval query at `:83-85` already limits the rows to those five fields.
- **Found:** `MAX_SKILL_BODY_BYTES = 1_000_000` (`skill_services.py:27`), and `SOURCE_LIMITS["skill"] = 18`. `search_sources` validates up to 18 skills, and `_select` validates the scored subset again. `filter_queryset_by_access_level` only adds `filter`/`exclude` clauses on `id` and `created_by` (`user_access_control.py:1196-1209`), so a `.only(...)` on the base queryset still works with it.
- **Impact:** Each validation pass sends every matched skill body from Postgres into Python memory, and the code never uses it. The 3.0s selection deadline covers this work. The fix is to copy the `.only(...)` projection from the retrieval query.
- **Priority:** I lowered this to `consider`. Skill bodies are markdown instructions and are usually a few KB to tens of KB, so a normal pass moves well under 1 MB and costs a few milliseconds. The 18 MB figure needs every candidate near the 1 MB limit. That differs from the Business Knowledge case (3-2-2), where full source documents are often large and repeat once per chunk.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Apply .only("id", "team_id", "name", "description", "version") to the validated skill queryset, matching the initial retrieval projection. Keep the access filters and final source revalidation.
</potential_solution>
Comment on lines
+198
to
+210
| def search_business_knowledge(team: Team, user: User, query: str) -> list[Candidate]: | ||
| team = team.parent_team or team | ||
| if not posthog_feature_flag_enabled( | ||
| "product-business-knowledge", str(user.distinct_id), team_id=team.id, organization_id=team.organization_id | ||
| ): | ||
| return [] | ||
| if not UserAccessControl(user=user, team=team).check_access_level_for_resource("business_knowledge", "viewer"): | ||
| return [] | ||
| if AccessControl.objects.filter(team=team, resource="business_knowledge").exists(): | ||
| return [] | ||
| with team_scope(team.id, canonical=True): | ||
| results = search_knowledge_for_team( | ||
| team, query, limit=SOURCE_LIMITS["business_knowledge"], expand_neighbors=False |
Contributor
There was a problem hiding this comment.
Enforce the token's project scope before reading parent knowledge
Issue description
A child-environment sandbox receives an OAuth token with scoped_teams=[child.id]. Selection passes only named scopes, so this function reads parent knowledge without checking the token's project restriction. If the actor can read the parent, treatment delivers parent passages to the child-scoped sandbox. Final revalidation checks actor access again and permits the same disclosure. APIScopePermission rejects this credential on the parent document endpoint. CanonicalTeamTokenPermission also explicitly rejects child-only credentials for parent-owned Business Knowledge operations. A source-level reproduction confirmed that selection returns parent content while that permission denies access.
Why we think it's a valid issue
- Checked: The selection endpoint (
products/context_layer/backend/selection_views.py:59-101), sandbox token minting (posthog/temporal/oauth.py:584-600,products/tasks/backend/temporal/oauth.py:235), team-scope enforcement inAPIScopePermission(posthog/permissions.py:905-948),get_authenticator_scoped_team_ids(permissions.py:796-809),CanonicalTeamTokenPermissionand where it is used (products/business_knowledge/backend/api/settings.py:23-54,api/sandbox.py:40,api/playground.py:49), the Business Knowledge read viewsets (api/views.py:477-592,routes.py:14-62), and the Business Knowledge paths inselection_sources.py:179-212. - Found: Sandbox tokens get
scoped_teams=[team_id], withtask.team_idas the team (posthog/temporal/oauth.py:596). A task in a child environment therefore gets a token for that child only.preparepasses only the token's namedscopestring into selection (selection_views.py:96) and drops the team leg. - Found:
search_business_knowledgeswitches toteam.parent_team or team(selection_sources.py:199), and revalidation does the same at:180. The only checks there are the parent-teamUserAccessControlandAccessControlchecks. Nothing compares the parent id with the credential's scoped teams. The PR's testtest_child_environment_uses_parent_knowledge_and_permissions(test/test_selection.py:282-305) checks that a child run receives parent passages. - Found: Every API path denies this same credential.
check_team_and_org_permissionsraisesPermissionDeniedwhenview.team.id not in scoped_teams(permissions.py:943-945). The Business Knowledge viewsets are on the projects router, so/api/projects/{parent}/business_knowledge/...resolves to the parent team and is rejected. The Business Knowledge team also addedCanonicalTeamTokenPermission("A token scoped only to a child environment cannot touch the parent-owned setting",settings.py:24) to the settings, sandbox and playground viewsets. ThereferenceURL in the candidate (selection_sources.py:223) points to/api/projects/{parent}/..., and the sandbox cannot open it with its own token. - Impact: A treatment run in a child environment injects parent-owned passages into the sandbox's model context, past the credential's project restriction. This contradicts the PR's statement that OAuth scopes constrain injection. The same gap applies to the parent-resolved catalog rows in 1-2-1.
- Priority: I lowered this to
should_fix. Selection still checks that the actor has Business Knowledge viewer access on the parent team (selection_sources.py:188-190,:204-207). So the data reaches an agent that acts for a user who may read it, and no unauthorized person gets it. The broken boundary is credential least privilege. The trigger also needs a child environment, an allowlisted team and a staff actor.
Suggested fix
Pass the authenticated credential's scoped team IDs alongside its named scopes into selection. Require credential access to the owning team before retrieval and during final revalidation. Follow get_authenticator_scoped_team_ids and CanonicalTeamTokenPermission semantics. Skip unauthorized parent sources without widening the token. Add an OAuth regression where the actor can read parent knowledge but the child-only token receives no parent content.
Prompt to fix with AI (copy-paste)
## Context
@products/context_layer/backend/selection_sources.py#L198-210
@products/context_layer/backend/selection_sources.py#L179-193
<issue_description>
A child-environment sandbox receives an OAuth token with scoped_teams=[child.id]. Selection passes only named scopes, so this function reads parent knowledge without checking the token's project restriction. If the actor can read the parent, treatment delivers parent passages to the child-scoped sandbox. Final revalidation checks actor access again and permits the same disclosure. APIScopePermission rejects this credential on the parent document endpoint. CanonicalTeamTokenPermission also explicitly rejects child-only credentials for parent-owned Business Knowledge operations. A source-level reproduction confirmed that selection returns parent content while that permission denies access.
</issue_description>
<issue_validation>
- **Checked:** The selection endpoint (`products/context_layer/backend/selection_views.py:59-101`), sandbox token minting (`posthog/temporal/oauth.py:584-600`, `products/tasks/backend/temporal/oauth.py:235`), team-scope enforcement in `APIScopePermission` (`posthog/permissions.py:905-948`), `get_authenticator_scoped_team_ids` (`permissions.py:796-809`), `CanonicalTeamTokenPermission` and where it is used (`products/business_knowledge/backend/api/settings.py:23-54`, `api/sandbox.py:40`, `api/playground.py:49`), the Business Knowledge read viewsets (`api/views.py:477-592`, `routes.py:14-62`), and the Business Knowledge paths in `selection_sources.py:179-212`.
- **Found:** Sandbox tokens get `scoped_teams=[team_id]`, with `task.team_id` as the team (`posthog/temporal/oauth.py:596`). A task in a child environment therefore gets a token for that child only. `prepare` passes only the token's named `scope` string into selection (`selection_views.py:96`) and drops the team leg.
- **Found:** `search_business_knowledge` switches to `team.parent_team or team` (`selection_sources.py:199`), and revalidation does the same at `:180`. The only checks there are the parent-team `UserAccessControl` and `AccessControl` checks. Nothing compares the parent id with the credential's scoped teams. The PR's test `test_child_environment_uses_parent_knowledge_and_permissions` (`test/test_selection.py:282-305`) checks that a child run receives parent passages.
- **Found:** Every API path denies this same credential. `check_team_and_org_permissions` raises `PermissionDenied` when `view.team.id not in scoped_teams` (`permissions.py:943-945`). The Business Knowledge viewsets are on the projects router, so `/api/projects/{parent}/business_knowledge/...` resolves to the parent team and is rejected. The Business Knowledge team also added `CanonicalTeamTokenPermission` ("A token scoped only to a child environment cannot touch the parent-owned setting", `settings.py:24`) to the settings, sandbox and playground viewsets. The `reference` URL in the candidate (`selection_sources.py:223`) points to `/api/projects/{parent}/...`, and the sandbox cannot open it with its own token.
- **Impact:** A treatment run in a child environment injects parent-owned passages into the sandbox's model context, past the credential's project restriction. This contradicts the PR's statement that OAuth scopes constrain injection. The same gap applies to the parent-resolved catalog rows in 1-2-1.
- **Priority:** I lowered this to `should_fix`. Selection still checks that the actor has Business Knowledge viewer access on the parent team (`selection_sources.py:188-190`, `:204-207`). So the data reaches an agent that acts for a user who may read it, and no unauthorized person gets it. The broken boundary is credential least privilege. The trigger also needs a child environment, an allowlisted team and a staff actor.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Pass the authenticated credential's scoped team IDs alongside its named scopes into selection. Require credential access to the owning team before retrieval and during final revalidation. Follow get_authenticator_scoped_team_ids and CanonicalTeamTokenPermission semantics. Skip unauthorized parent sources without widening the token. Add an OAuth regression where the actor can read parent knowledge but the child-only token receives no parent content.
</potential_solution>
Comment on lines
+2877
to
+2881
| ? await this.contextSelection.dispatch( | ||
| session.payload.run_id, | ||
| contextMessageId, | ||
| attempt.prompt, | ||
| (prompt) => { |
Contributor
There was a problem hiding this comment.
Preserve prompt order during startup context preparation
Issue description
The new preparation await lets a non-steer follow-up overtake a boot-owned initial or resume prompt. Startup delivery does not reserve nonSteerDeliveryTail, and beginSelfDelivery only prevents duplicate message IDs. While startup preparation waits, executeCommand can prepare and deliver another message. If that preparation finishes first, the adapter receives the follow-up before the request it refers to. The workflow can dispatch this follow-up while startup runs. A reproduction using the actual delivery methods confirmed reversed adapter delivery and reversed user history.
Why we think it's a valid issue
- Checked:
acquireNonSteerDeliveryTurn(agent-server.ts:2807-2815) is the only ordering lock between deliveries. Only theuser_messagehandler takes it (:1497-1498) and releases it after the turn (:1789).runStartupTurn(:2769-2776) only incrementsactiveStartupTurnCount.beginSelfDelivery(:3900-3912) only deduplicates by message ID. I comparedpromptWithUpstreamRetrywith the master version of the file from the GitHub API. - Found: On master,
promptWithUpstreamRetrycallssession.clientConnection.prompt(attempt)with no await before it. The PR insertsawait this.contextSelection.dispatch(...)(:2876-2896) ahead of that call. That await is a backend round trip with a 5 s client timeout (posthog-api.ts:120) and a 3.5 s server bound (selection_views.py,bounded_request(..., 3.5, ...)). The startup prompt now waits seconds before it reaches the adapter. - Found: The eligible runs reach this path. For Claude and Codex runs,
initial_messageisNone(facade/api.py:6067-6070,:9163-9167). Interactive runs skip_forward_pending_user_message(workflow.py:2730-2736). So in web and Slack runs, the agent sends its first prompt itself as a startup turn, throughsendInitialTaskMessageandrunStartupTurn. In that case the workflow has no_active_followup_task, and_has_dispatchable_followup(workflow.py:674-679) dispatches any queued non-steer follow-up at once. One example is a second Slack message sent during sandbox provisioning. A steer that the agent declines withstartup_turn(:1554-1555) also returns to the queue as non-steer (workflow.py:724-734). - Found: That follow-up takes the free non-steer lock and runs its own
dispatchcall. A short follow-up can finish selection faster than the startup prompt, because a gate skip is one System One call, while the startup prompt goes through the gate, retrieval, and concurrent rerank. When the follow-up finishes first, itsclientConnection.prompt(:1643) reaches the adapter before the startup prompt (:2893). - Impact: The agent can handle a follow-up such as "also break it down by country" before the request it refers to. This is a visible ordering error in exactly the runs this feature targets. On master the window was the startup's short API preparation. The PR widens it to several seconds, and every eligible run pays the round trip, including control runs, because the backend decides the variant. The fix uses the existing lock and does not need a new mechanism. The flag and allowlist limit how many users see it, but the bug still meets the correctness bar.
Suggested fix
Reserve the existing nonSteerDeliveryTail queue slot for boot-owned initial and resume turns before context preparation. Release it after turn processing completes. Reuse the existing reservation for forwarded prewarmed messages to avoid acquiring it twice. Verify ordering with a deferred startup preparation response and a concurrent non-steer follow-up.
Prompt to fix with AI (copy-paste)
## Context
@packages/agent/packages/agent/src/server/agent-server.ts#L2877-2881
<issue_description>
The new preparation await lets a non-steer follow-up overtake a boot-owned initial or resume prompt. Startup delivery does not reserve nonSteerDeliveryTail, and beginSelfDelivery only prevents duplicate message IDs. While startup preparation waits, executeCommand can prepare and deliver another message. If that preparation finishes first, the adapter receives the follow-up before the request it refers to. The workflow can dispatch this follow-up while startup runs. A reproduction using the actual delivery methods confirmed reversed adapter delivery and reversed user history.
</issue_description>
<issue_validation>
- **Checked:** `acquireNonSteerDeliveryTurn` (`agent-server.ts:2807-2815`) is the only ordering lock between deliveries. Only the `user_message` handler takes it (`:1497-1498`) and releases it after the turn (`:1789`). `runStartupTurn` (`:2769-2776`) only increments `activeStartupTurnCount`. `beginSelfDelivery` (`:3900-3912`) only deduplicates by message ID. I compared `promptWithUpstreamRetry` with the master version of the file from the GitHub API.
- **Found:** On master, `promptWithUpstreamRetry` calls `session.clientConnection.prompt(attempt)` with no await before it. The PR inserts `await this.contextSelection.dispatch(...)` (`:2876-2896`) ahead of that call. That await is a backend round trip with a 5 s client timeout (`posthog-api.ts:120`) and a 3.5 s server bound (`selection_views.py`, `bounded_request(..., 3.5, ...)`). The startup prompt now waits seconds before it reaches the adapter.
- **Found:** The eligible runs reach this path. For Claude and Codex runs, `initial_message` is `None` (`facade/api.py:6067-6070`, `:9163-9167`). Interactive runs skip `_forward_pending_user_message` (`workflow.py:2730-2736`). So in web and Slack runs, the agent sends its first prompt itself as a startup turn, through `sendInitialTaskMessage` and `runStartupTurn`. In that case the workflow has no `_active_followup_task`, and `_has_dispatchable_followup` (`workflow.py:674-679`) dispatches any queued non-steer follow-up at once. One example is a second Slack message sent during sandbox provisioning. A steer that the agent declines with `startup_turn` (`:1554-1555`) also returns to the queue as non-steer (`workflow.py:724-734`).
- **Found:** That follow-up takes the free non-steer lock and runs its own `dispatch` call. A short follow-up can finish selection faster than the startup prompt, because a gate skip is one System One call, while the startup prompt goes through the gate, retrieval, and concurrent rerank. When the follow-up finishes first, its `clientConnection.prompt` (`:1643`) reaches the adapter before the startup prompt (`:2893`).
- **Impact:** The agent can handle a follow-up such as "also break it down by country" before the request it refers to. This is a visible ordering error in exactly the runs this feature targets. On master the window was the startup's short API preparation. The PR widens it to several seconds, and every eligible run pays the round trip, including control runs, because the backend decides the variant. The fix uses the existing lock and does not need a new mechanism. The flag and allowlist limit how many users see it, but the bug still meets the correctness bar.
</issue_validation>
## Task
Investigate the issue and solve it
<potential_solution>
Reserve the existing nonSteerDeliveryTail queue slot for boot-owned initial and resume turns before context preparation. Release it after turn processing completes. Reuse the existing reservation for forwarded prewarmed messages to avoid acquiring it twice. Verify ordering with a deferred startup preparation response and a concurrent non-steer follow-up.
</potential_solution>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
PostHog AI users on web and Slack need relevant project knowledge supplied before their prompts reach the model.
The experiment must support evaluation of context use without a separate prompt archive or retrieval projection.
Changes
skill-getwith the referenced name and version before using a skill. General knowledge searches cannot replace that fetch.Pi runs skip context selection. Autonomous continuations, steering, and slash commands also skip it.
Note
Selection spans describe prepared context. Downstream traces establish model exposure and support evaluation of use; trace retention and truncation still limit completeness.
The highest-risk changes are direct retrieval and permission revalidation. The feature remains restricted to staff and allowlisted teams. No visible UI changes.
Before:
flowchart LR U[Web or Slack prompt] --> A{{Cloud agent}} --> R[Answer and traces] classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; class U phYellow; class A phBlue; class R phGray;After:
flowchart LR U[Web or Slack prompt] --> G{{System One gate}} G -->|Context may help| S[Search current sources] S --> K{{Parallel System One rerank}} K --> H[Bounded hidden references] --> A{{Cloud agent}} G -->|Skip or failure| A S -->|Failure| A K -->|Empty or failure| A K -.-> T[Selection span] A --> R[Answer and traces] classDef phBlue fill:#1d4aff,stroke:#1d4aff,color:#fff; classDef phYellow fill:#f9bd2b,stroke:#f9bd2b,color:#000; classDef phGray fill:#e5e7eb,stroke:#c7ccd1,color:#000; class U phYellow; class G,S,K,A phBlue; class H,T,R phGray;How did you test this code?
Test rationale: Existing selection tests now exercise live edits and deletion, tenant and source access, catalog definitions and drift, concurrent reranking, and telemetry failure. ACP runtime tests cover hidden delivery, retries, and history. The existing eligibility matrix now verifies that Pi skips selection. These boundaries require database and runtime tests rather than archive assertions. The existing delimiter-escape case covers the renamed wrapper; live validation checks model wording.
Local validation ran the backend selection suite, focused ACP tests, repository-wide mypy, agent typecheck and build, API generation, and repository preflight.
The user reported local Claude and Codex replies containing the fictional activation events and reference code. The fresh Claude reply hid retrieval mechanics.
The earlier Codex run used the supplied description after a failed knowledge search, without fetching the skill. It demonstrated context use, without source verification.
The revised local fixture keeps a new reference code only in the skill body. Fresh Claude and Codex tool logs confirm successful versioned skill fetches.
Both fetch responses contained the body-only code. Saved task summaries used that code and the skill's local-test qualifications without revealing retrieval mechanics.
Codex attempted the unavailable Business Knowledge search first, then fetched the skill successfully. The user confirmed both replies returned the new code without mentioning suggestions or injection.
The agent did not independently inspect rendered replies or raw model input. Existing selection tests and preflight passed after the prompting update. No unit test mirrors the prompt wording.
Slack, Business Knowledge end-to-end retrieval, and deployed selection-span capture remain unverified rollout checks.
Release status
phai-context-selectionalso requires an explicit project allowlist, which defaults to empty.Automatic notifications
Docs update
Updated the sandboxed-agents handbook. Removed the obsolete context-selection entry from TypeSafe caller documentation.
🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Agent: Codex, GPT-6.
Removed the draft-only projection, archive, refresh, and receipt machinery. Existing master migrations remain intact. Validation describes the current implementation.
Repository skills used across this PR: integrating-with-posthog-ai, improving-drf-endpoints, implementing-mcp-tools, django-migrations, writing-dataclasses, outbound-egress, migrating-llm-gateway-callers, writing-tests, writing-code-comments, writing-user-facing-copy, writing-pr-descriptions, running-ci-preflight, reviewing-with-coderabbit, hogli, run-posthog, and querying-local-postgres.
CodeRabbit remains unverified because its CLI is unavailable. The duplicate search found no conflicting feature-flag PR. Invented fixtures exclude private trace content. The private session link is omitted.
Prior review context
The shared agent package triggers desktop/backend coupling checks, but backend eligibility keeps desktop disabled and permits either deployment order. The documented exception label applies.