Skip to content

fix(addie): surface chat auth errors and correlate failed requests - #7669

Merged
bokelley merged 7 commits into
mainfrom
conductor/fix-addie-audit-findings
Sep 23, 2026
Merged

bokelley merged 7 commits into
mainfrom
conductor/fix-addie-audit-findings

Conversation

@bokelley

@bokelley bokelley commented Sep 23, 2026 •

Copy link
Copy Markdown
Contributor

Chat auth and organization failures currently collapse into a transient “Failed to send message” toast. This change renders a persistent, actionable error with a validated support request ID, omits absent organization selectors, and correlates HTTP/SSE failures across response headers, bodies, and content-free structured logs. Explicitly nonrecoverable stream errors remain nonrecoverable; logical turn IDs still govern safe continuation.

Also repairs the established deployment path: the latest Deploy run 35908554464 failed because its shallow checkout could not resolve the prior release commit for ancestry validation. Fetch full history in the deploy job and guard that requirement in the existing workflow test. No release checks are bypassed, and no application secret/configuration values are changed.

Validation:

  • Original incident implementation: 374 focused tests passed; full root suite passed 1,081 tests; full server suite completed 672 files with 11,218 passed and 31 skipped. The two failures (an obsolete organization-selector source assertion and missing Python in the test image) were corrected and their suites passed on rerun. No application code changed after that full run.
  • Updated branch: precommit passed root tests, repository guards, and server/fixed-trace typechecks. Docker reruns passed all 290 focused chat/auth tests across 5 files and all 79 release/deployment-fence tests. Addie generated tool reference/inventory/portability checks passed. The first release-test attempt lacked jq in the local image; adding jq to that image made the unchanged suites pass.
  • Independent security, chat-flow, and deployment expert reviews found no actionable issues, including auth/CSRF boundaries, output escaping, request-ID privacy, and continuation safety.

The branch was merged with current main to exclude the already merged #7654 changes from this PR. Addie code version is 2026.09.68. This is an application/deployment-only fix: no protocol changeset or database migration is needed.

All 29 GitHub checks passed; 2 optional checks skipped. Automated review found no blocking/medium findings and requires a human/CODEOWNERS approval solely because the deployment workflow is a gated path. No review threads are unresolved.

Production verification will follow the merged-main Build Check → Deploy path, checking every Fly web/worker machine and health check, served version/UI, and a safe rejected request's correlation. No destructive authenticated scenario will be fabricated. Upstream failures that never reach Express can only carry the client-generated reference.

@aao-secretariat aao-secretariat Bot added the ladon/needs-human-review Ladon has escalated this PR for human review. label Sep 23, 2026

@aao-secretariat aao-secretariat Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ladon verdict: Escalate to human review

Escalate — gated path requires human/CODEOWNERS approval.

This PR modifies .github/workflows/deploy.yml, which is under a hard, deterministic approval gate (gated_paths: true). The required review status is REVIEW_REQUIRED, not APPROVED, so per decision-table row 2 the gate is not satisfied and the outcome must be escalate.

The reviewer found no blocking or medium findings — the diff itself (Addie chat error-surfacing/correlation plus a deploy fetch-depth fix) is clean: correlation-ID privacy/injection guards verified, safe client output rendering, recoverable:false preservation checked against all stream_error emissions, org_id-omission tenant boundary intact, no protocol surface touched, no changeset needed. The high-risk flag fires only on (modified) files with no accompanying findings, so it does not independently escalate. This escalation is driven solely by the unsatisfied gated-paths approval gate.

Escalation reasons

  • .github/workflows/deploy.yml (modified) is under the hard approval gate; review_decision is REVIEW_REQUIRED — human/CODEOWNERS approval required before merge.

Why human review

  • Modifies gated path .github/workflows/deploy.yml while review_decision is REVIEW_REQUIRED (not APPROVED) — human/CODEOWNERS approval required.
  • This PR touches a path under a hard, non-overridable approval gate (.github/workflows/deploy.yml (modified) matches .github/workflows/**) and the current GitHub review decision is 'REVIEW_REQUIRED', not APPROVED. This is a hard gate enforced in code — Ladon cannot auto-approve until a human/CODEOWNERS approval is recorded, regardless of how clean the diff is.

@bokelley
bokelley merged commit 877587c into main Sep 23, 2026
32 checks passed
@bokelley
bokelley deleted the conductor/fix-addie-audit-findings branch September 23, 2026 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ladon/needs-human-review Ladon has escalated this PR for human review.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant