This skill is the project-specific implementation guide for shipping safe changes
in AgenticOrg. Use it to keep diffs small, preserve enterprise controls, and
verify the actual risk boundary you touched.
It is also the mandatory audit lens for enterprise-grade reviews in this repo.
If the task is a whole-codebase scan, enterprise hardening review, architecture
safety review, or release sign-off, this skill is not optional. If the task is
bug-sheet triage or reopen analysis, use this skill together with
agenticorg-bug-fix-fail-closed.
When the user asks for a deep review, release sign-off, or enterprise
hardening audit, do all of the following before issuing a verdict:
Code that "looks reasonable" is not evidence for enterprise sign-off.
These are recurring CI failure modes from the April 2026 enterprise program. Check for each before pushing.
-
TypeScript strict mode: noUnusedLocals is enabled. If you declare a const like const isDowngrade = ... but only use isUpgrade, the build fails with TS6133. Remove it or prefix with _.
-
Regression tests that inspect source code: Some tests in test_bugs_april06_2026.py grep function definition lines for Depends. If your function signature spans multiple lines, Depends must appear on the def line itself or the test fails. Keep Depends on the same line as the function name for short signatures.
-
Integration tests with hardcoded versions: test_api_integration.py asserts the version string from /health. When you bump the version, update the test too.
-
CORS startup crash: The CORS check reads settings.cors_allowed_origins which maps to AGENTICORG_CORS_ALLOWED_ORIGINS (not CORS_ALLOWED_ORIGINS). A hard raise blocks deploys if the var is unset. Use a warning plus safe fallback.
-
Route collisions: FastAPI silently registers both handlers for the same path and the first registered one wins. The old duplicate /billing/invoices bug is fixed, but you should still check for collisions before adding or moving handlers. Search with rg -n "/invoices|prefix=.*billing" api/v1.
-
Production release gates: Tagged releases now require approval-gate, and production health now passes only on "status":"healthy". Do not weaken either rule by reintroducing "degraded" acceptance or bypassing approval for release tags.
-
Regression tests that assert old permissive behavior: When tightening a security or operational gate, always search for tests that still encode the old behavior. Search with rg -n "degraded|subscription_id|invite|Depends|permissive" tests.
-
Legacy security fallbacks: Be suspicious of compatibility branches that keep insecure behavior alive for "old tenants" or "old data". The current Stripe cancellation fallback to caller-supplied subscription_id is the model example of what must be burned down, not normalized.
-
E2E auth helpers that only set a token: Role-gated routes now fail closed when user hydration is missing. If Playwright stores only localStorage.token, protected pages can redirect or render false negatives even though the backend session is valid.
-
Frontend/API contract drift on dashboards: Recent live regressions came from pages expecting items or deadlines while APIs returned packs or upcoming_deadlines. When touching dashboards, catalogs, or summary pages, verify the exact JSON keys against the backend handler before shipping.
-
Concurrent deploy pipelines race on the same Helm release: If multiple PRs merge to main within minutes, every CI/CD run enters the deploy-production stage, each helm upgrade updates the same Kubernetes Deployment, and earlier pipelines' kubectl rollout status sees a newer image and times out. The deploy job reports "failure" even though production is healthy. Add concurrency: production-deploy at the job level to serialize, or use gh pr merge --merge-queue. Before declaring a real deploy failure, curl /api/v1/health first — rollout-status timing out and prod being HTTP 200 is the usual pattern, not a code regression.
-
Liveness probe initialDelaySeconds must accommodate image size and startup work: The API image is 3.2 GB and loads 54 connectors + LangGraph at boot. initialDelaySeconds=10s plus period=30s/failure=3 gives only ~100s before kubelet kills the container, which is not enough on slow Autopilot nodes. Prefer a startup probe, or bump liveness initial delay to 60–90s. Symptom: Readiness probe failed: connect: connection refused events even though the pod eventually logs "Application startup complete".
-
continue-on-error: true on e2e jobs silently accumulates drift: The 438-test Playwright regression suite had been green-marked by CI for many deploys while mostly failing, because continue-on-error: true masks the real conclusion. Any e2e/integration job with this flag deserves a periodic human audit (run locally against prod, inspect the pass/fail count). When rewriting the suite to green, flip to continue-on-error: false the moment it is stable — leaving the flag on is how drift comes back.
-
Regression tests that pin exact serialized key sets: Tests like assert set(result.keys()) == expected_keys in test_agents_and_sales.py::test_all_expected_keys_present break immediately when a new field is added to _agent_to_dict (BUG-013 added connector_ids and CI went red on the next push). Before merging a serializer change, grep tests/ for set(result.keys()), expected_keys, and literal dict-key assertions — update them in the same PR.
-
Regression tests that grep source code for specific literals: Some tests in test_ca_api_functional.py inspect function source with inspect.getsource() and assert substrings like '"approved"' in source or "gst_auto_file" in source. These check implementation details, not behavior. When you tighten a gate (e.g., remove an auto-approval branch), these tests fail for the right reason but block the PR. Grep for inspect.getsource in tests before refactoring control-flow and update the assertions to match the new intent, not the old.
Do not hide behind soft language such as "mostly fine", "looks good overall",
or "probably shippable". If a stop-ship gate is still open, say so directly.
-
Null safety in aggregation queries: Dashboard endpoints that float() or index dict results from SQL aggregates MUST handle None. Always use float(x) if x is not None else 0.0.
-
Never return secrets in API responses: Return has_credentials: bool not actual values. Secrets live in connector_configs.credentials_encrypted.
-
0-step workflow guard: Validate at BOTH creation AND run time. Creation validation exists but data can be modified directly.
-
Chat output formatting: raw_output may contain a nested JSON string. Always try json.loads() before displaying.
-
Dead code that is actually a model field: When ruff flags a line like phone: str = "" as dead code after a return statement, check whether it was supposed to be a Pydantic model field that got displaced. Moving it to the correct class fixes both the lint warning and the AttributeError: object has no attribute crash.
-
Dockerfile installs base deps only: pip install . does NOT install optional dependency groups. If a feature requires composio-core, presidio, or other packages listed under [project.optional-dependencies], the Dockerfile must use pip install ".[v4]" or the SDK will be missing at runtime.
-
RLS-protected tables require get_tenant_session(tid) not async_session_factory(): Any endpoint that queries an RLS-protected table MUST use get_tenant_session() which sets the agenticorg.tenant_id GUC. Using async_session_factory() bypasses the GUC and returns 0 rows because FORCE ROW LEVEL SECURITY blocks the query. This caused approval_policies, invoices, and workflow_variants to return empty despite having data.
-
KPI endpoints must return structured fields, never raw task_output: CxO dashboards bind to agent_count, total_tasks_30d, success_rate, hitl_interventions, total_cost_usd, domain_breakdown[]. If the KPI builder returns raw task_output dicts instead (like {"items": 8, "result": "Demo bank_reconciliation"}), the UI shows NaN. Always use _compute_basic_metrics() with SQL aggregation.
-
Domain names in ROLE_DOMAIN_MAP must match agent_task_results.domain exactly: COO maps to ["operations", "it", "support"] not "ops". CBO maps to ["legal", "risk", "corporate", "comms"] not "strategy". The SQL uses domain = ANY(:domains) so partial matches fail silently with 0 results.
-
Seed data must match the ORM model schema exactly: When inserting via raw SQL, check ALL NOT NULL columns in the ORM model. Missing tool_functions (JSONB NOT NULL) in connector INSERT crashes. Missing mfa_enabled (BOOLEAN NOT NULL) in user INSERT crashes. Always read the model first.
-
VAPID keys and API keys must be set BEFORE testing: Features that depend on env vars (COMPOSIO_API_KEY, VAPID_PUBLIC_KEY, VAPID_PRIVATE_KEY) will silently return empty/0 if the var is missing. Always verify the env var is set on the running pod before concluding the feature is broken.
-
Always verify you are on main before committing: After PR merges or branch switches, git branch may show a stale codex branch. Run git checkout main && git pull before committing. If you committed on the wrong branch, git cherry-pick <hash> onto main.
-
async_session_factory() vs get_tenant_session() audit: When a new RLS-protected table is created, grep ALL API files for async_session_factory and verify none of them query the new table. The following files still use async_session_factory and may break if they touch RLS tables: invoices.py, workflow_variants.py, sso.py, branding.py.
-
Keep documentation and SEO assets in sync with code: When adding agents, connectors, or solution pages, update ALL of these:
README.md — agent count, connector count, version badgeui/index.html — meta tags (description, og:description, twitter:description), JSON-LD softwareVersion, pricing JSON-LDui/public/sitemap.xml — add new public routesui/scripts/generate-sitemap.mjs — add new routes to staticPages[]ui/public/llms.txt and ui/public/llms-full.txt — auto-generated on build, but verify agent/connector countsui/package.json — keep version in sync with pyproject.toml- Do NOT claim more agents/connectors than actually exist in code. Count from
_AGENT_TYPE_DEFAULT_TOOLS in api/v1/agents.py and connector directories under connectors/.
-
Version must be synchronized across all locations: When bumping version, update ALL of:
pyproject.toml → versionapi/main.py → FastAPI version=api/v1/health.py → APP_VERSIONui/package.json → versionui/index.html → JSON-LD softwareVersionREADME.md → version badgetests/integration/test_api_integration.py → version assertion
-
E2E tests must grant admin scopes: The test fixture must set agenticorg:scopes: ["agenticorg:admin"] — empty scopes will cause 403 on all admin-gated routes (report-schedules, connectors, companies, workflows, etc.).
-
Connector test endpoint must read encrypted credentials: The /connectors/{id}/test endpoint must read from connector_configs.credentials_encrypted first (decrypting at runtime), falling back to legacy Connector.auth_config only if no encrypted config exists. Never read only from auth_config.
-
Login throttling must use Redis: Auth rate limiting uses Redis with atomic TTL keys for cross-pod consistency. In-memory dict is fallback only. Never rely on process-local state for security controls in a multi-pod deployment.
-
/auth/me must read onboarding_complete from tenant settings: Never hardcode onboarding_complete: True. Read from tenant.settings.get("onboarding_complete", False) — same as /login and /signup. Session state must be consistent across all auth hydration paths.
-
E2E and Playwright tests must block deploy: Post-deploy E2E tests in deploy.yml must NOT use continue-on-error: true. Only synthetic/LLM-quality tests (non-deterministic) may be non-blocking. Deterministic product-path tests must fail the pipeline.
-
Do not dump kubectl describe or pod logs in CI: Rollout diagnostics should show only pod name, status, ready, and restart count — never full describe or logs output which may expose secrets, env vars, or internal state.
-
Pre-flight validation must not hard-reject on auto-populated defaults: When adding request-time validation like run_agent's _validate_authorized_tools check, remember that create_agent intentionally skips validation for tools auto-populated from _AGENT_TYPE_DEFAULT_TOOLS / _DOMAIN_DEFAULT_TOOLS. Many of those names (check_order_status, schedule_social_post, search_content_fulltext) are not in the connector registry. A hard 400 on any missing name regresses every newly-created default agent. The correct shape is: filter unresolvable tools, log a warning, only fail when the resolvable set is empty. Codex flagged this as P1 on PR #150.
-
CSV export must neutralize formula injection: Audit / report exports that only escape quotes still emit executable formulas when a cell starts with =, +, -, @, CR, LF, or TAB. Action and Actor fields carry user-controlled text (filing types, emails, rejection reasons), so Excel/Sheets will execute the formula on the reviewer's machine. Prefix such cells with a single quote before quoting. Apply to every CSV export path, not just the audit log. Pattern:
const csvEscape = (v: string) => {
const guarded = /^[=+\-@\r\n\t]/.test(v) ? `'${v}` : v;
return `"${guarded.replace(/"/g, '""')}"`;
};
-
E2E helpers must never cache fallback values: When a setup API call (/auth/me, /companies, /tenants/current) fails transiently, returning a hardcoded fallback is fine, but caching that fallback locks the entire Playwright run to the wrong identity/tenant. Only populate the cache on a 2xx response. On failure, return the fallback without storing it so the next call retries the live API. Codex P2 on PR #151 for ui/e2e/helpers/auth.ts::getProfile.
-
E2E helpers must not return cross-tenant hardcoded IDs: A fixed UUID fallback in getCompanyId() (or any get<Entity>Id helper) is correct for exactly one tenant. In any other BASE_URL / env it silently routes every CompanyDetail assertion against an entity that does not exist, producing persistent false negatives. Throw a clear error instead — the test fails cleanly and the retry path re-hits the real API. Codex P2 on PR #151.
-
E2E auth seeding must mirror what AuthContext would hydrate: Layout.tsx filters sidebar nav by localStorage.user.role. Seeding a fake role like "ceo" hides every CxO nav link because the real demo account is role: "admin". Result: dozens of tests redirect to onboarding or fail to find nav links, even though auth is technically valid. Fetch the profile live from /api/v1/auth/me and seed that verbatim, or keep one hardcoded profile per known account and verify it matches the live response. Never invent the user shape.
-
E2E selectors must scope to <main> for in-page elements that share labels with sidebar nav: page.getByText("Approvals", { exact: true }).first() resolves to the sidebar's /dashboard/approvals link before any CompanyDetail tab button named "Approvals". Clicking navigates away from the company entirely. For in-page tabs, buttons, and any element whose name could collide with a nav link, scope with page.locator("main button").filter({ hasText: /^Approvals$/ }).first() (or use the tabButton() helper in ui/e2e/helpers/auth.ts).
-
Dependabot bumps for proprietary SaaS must be closed, not merged: The project rule is open-source only (no LangSmith, no proprietary APM). A silent merge of a langsmith or langfuse version bump violates that rule and pulls a SaaS dependency into the runtime. When triaging dependabot PRs, check the package's license and hosting model first. Close with a comment pointing to the policy and the OSS equivalent (e.g., OpenTelemetry, Phoenix Arize OSS).
-
Dependabot dep-conflict failures expose pre-existing latent pins: The testcontainers >=4.14 bump in PR #144 triggered a uv pip install resolution failure because testcontainers[redis]>=4.14 transitively requires redis>=7, but celery[redis]>=5.4.0 requires redis<6.5 via kombu 5.6. The ceiling pin in pyproject.toml (testcontainers>=4.8.0,<4.14.0) is intentional and documented. Before merging any dep bump, grep pyproject.toml and requirements*.txt for <-pinned comments — those pins encode real conflicts and are not just conservative. Close conflicting bumps with a reference to the comment line.
-
Singleton thread-safety: flip the initialized flag LAST, not first. Session 5 BUG-S5-005 (core/pii/redactor.py) crashed under concurrent agent startup because __init__ set self._initialized = True before binding self._analyzer. A second caller's PIIRedactor() saw _initialized=True, skipped the init block, and hit AttributeError on the next redact(). Two invariants must hold for any lazy-init singleton in this codebase: (a) wrap the whole __init__ body in with self._lock: — not just the assignment step; (b) declare the attributes it intends to bind as class-level defaults (_analyzer: Any = None) so reads never hit AttributeError even mid-init; (c) flip _initialized = True only as the very last statement, after every other attribute is bound and recognizers are registered. A regression test must spin up >1 threads calling the constructor simultaneously — single-threaded tests won't catch the bug.
-
Frontend/backend validators must share a regex. Session 5 TC-007/TC-009/TC-012: SIP URI and E.164 phone validators existed in neither layer. When you add input validation, add the SAME regex to both api/v1/<domain>.py and ui/src/pages/<Page>.tsx, and cross-reference them in a comment on both sides. One-sided validation on the frontend is a UX hint but not a security control; one-sided validation on the backend gives users a generic "422" with no context. The regression test suite should cover both layers: pytest parametrizes bad inputs against the backend validator, Playwright parametrizes the same inputs against the form's inline-error path.
-
Response-shape drift between frontend and backend is silent. Session 5 TC-002: the backend returned imported: <int> but the UI read data.imported?.length on the number, collapsing to undefined → 0. The import banner reported 0 even for successful multi-lead imports. When changing an endpoint's response shape, grep the repo for every consumer and update the read path. If a field's semantics change (list → count), rename it so stale reads fail loudly instead of quietly.
-
DB-level UniqueConstraint must match the soft-delete filter. Session 5 TC-003: the Python query filtered is_active=True, but the UniqueConstraint("tenant_id", "name", "agent_type") in the model did NOT — so even after the user soft-deleted a template, the DB still rejected a new insert with the same name. For any table with is_active (or deleted_at) soft-delete semantics, use a partial unique index scoped to live rows: Index(..., unique=True, postgresql_where="is_active = true"). The Alembic migration must drop the old full constraint before creating the partial index.
-
"Fallback-only" DB mirrors hide the primary source's flakiness. Session 5 TC-013: upload_document() in api/v1/knowledge.py only wrote to Postgres when RAGFlow failed, treating the DB as a disaster-recovery backup. When RAGFlow succeeded but its search index lagged, the document disappeared from the UI after a refresh (the list call fell back to DB, which had no record). When a feature has two data stores (primary + fallback), always mirror to both on every write and merge both on every read with a dedupe key. Otherwise, any eventual-consistency gap in the primary silently drops data from the UI.
-
CSV imports need emptiness + encoding + header validation BEFORE the row loop. Session 5 TC-005: the old endpoint accepted any file, parsed what it could, and returned {"imported": 0}. Users were misled into thinking an invalid file imported "0 leads" successfully. At the import boundary, validate in order: (a) file extension (.csv), (b) non-empty body, (c) UTF-8/BOM-tolerant decoding with a 422 on UnicodeDecodeError, (d) required headers (accept known aliases like full_name → name). Return 422 with a structured detail.message that the UI can surface verbatim. Never return 200 with imported=0 for a clearly-invalid input.
-
Missing backend endpoint shows up as 404/405 on the frontend, not a crash. Session 5 TC-006 (/voice/test-connection) and BUG-S5-001 (/companies/test-tally) both had UI code calling routes that didn't exist — the frontend rendered "Connection test unavailable (API offline)" and kept moving, which users read as "probably fine". When adding a new feature, do a parity audit: grep the UI for api.post( / api.get( / fetch() URLs that don't exist in any @router.post/@router.get decorator and add the missing endpoints (or remove the dead UI). Treat UI-to-API URL drift as a shipping blocker, not a warning.
-
CI services: containers must use credentials that match the app's default Settings, OR the app's env var names must match what you set. Session 5 e2e-tests regression: I created the Postgres service container with POSTGRES_USER: e2e and set AGENTICORG_DATABASE_URL, but the Settings field is named db_url (→ AGENTICORG_DB_URL) and its default URL uses user=agenticorg. Result: the app ignored my DB env var, fell back to the default URL, and authentication failed. Before shipping a service-container addition: (a) grep core/config.py for the exact env var name — remember env_prefix = "AGENTICORG_" maps db_url to AGENTICORG_DB_URL; (b) if the goal is zero env-var plumbing, create the service with the Settings default credentials so the app works without any overrides.
-
init_db() adds columns and policies; it does NOT create tables from scratch. In the e2e-tests CI job (session 5), a fresh Postgres service container + asyncio.run(init_db()) left the agents/companies/... tables missing, because init_db() only issues ALTER TABLE ... ADD COLUMN IF NOT EXISTS and ENABLE ROW LEVEL SECURITY, not CREATE TABLE. Tests that spin up an empty DB must call conn.run_sync(BaseModel.metadata.create_all) first. Production is immune because Alembic or a prior init_db run on an existing DB already created the tables — but any new hermetic-test scenario needs create_all explicitly.
-
Alembic revision = "..." must be ≤32 characters. The alembic_version.version_num column is VARCHAR(32). A 37-char revision (e.g. v483_prompt_template_partial_unique) passes ruff, passes local unit tests, and then crashes integration tests on subprocess.CalledProcessError from scripts/alembic_migrate.py with value too long for type character varying(32) buried in the Postgres logs. The repo preflight (scripts/preflight.sh) now greps every migrations/versions/*.py for the literal and fails the push if any revision exceeds the limit.
-
Seed test fixtures with real UUIDs and full NOT-NULL sets. The test_cxo_flows.py fixture used f"e2e-tenant-{hex8}" as tenant_id and omitted slug. Both problems are silent in unit tests — they only fire when the fixture touches Postgres. Before writing a fixture INSERT: open the model file, list every column with nullable=False AND no default/server_default, and include all of them. Fake string IDs never cross an FK to a UUID column. For any hermetic-DB fixture, seed inside the module-scoped fixture (single asyncio.run()), not in a per-test fixture — pytest-asyncio gives per-test fixtures a fresh event loop, and any Future produced against the module-scoped engine will raise "attached to a different loop".
-
verify=False in production paths is Bandit B501 High and will fail CI. Any time you need to skip TLS verification inside api/, auth/, core/, or connectors/, stop and redesign. The one we shipped in api/v1/voice.py for a SIP reachability probe was replaced with a raw TCP socket connect because SIP isn't HTTP anyway — the "bypass TLS to probe HTTPS" shape was doubly wrong. For genuine dev-only TLS skips, keep them in scripts/, tests/, or behind a settings.environment == "test" guard that the preflight still inspects.
-
Run the full local preflight before git push, not the partial one. Three separate sessions landed ruff I001 (import-sort) violations on CI because each session ran ruff check <touched-file> locally. I001 cascades — touching one import in a file can require reordering imports elsewhere that didn't change, and the per-file invocation doesn't see those siblings. Always run ruff check . (the whole tree). The same logic applies to alembic revisions, bandit scans, and tsc — run the full check, not the scoped one, because CI will.
