Skip to content

fix(controlplane): bound CAS mapping resolution on artifact download#3168

Merged
migmartri merged 1 commit into
chainloop-dev:mainfrom
migmartri:miguel/pfm-6309-context-deadline-getting-cas-mappings-in-some-cases
Jun 5, 2026
Merged

fix(controlplane): bound CAS mapping resolution on artifact download#3168
migmartri merged 1 commit into
chainloop-dev:mainfrom
migmartri:miguel/pfm-6309-context-deadline-getting-cas-mappings-in-some-cases

Conversation

@migmartri

@migmartri migmartri commented Jun 5, 2026

Copy link
Copy Markdown
Member

Resolving the CAS mapping for a digest could exhaust the request deadline and surface as a context deadline error on artifact download. Two problems compounded, both fixed here.

  1. FindByDigest resolved each mapping's public visibility with two queries per mapping, producing 1 + 2N sequential round-trips. Visibility for all referenced workflow runs is now resolved in a single edge-loaded query.

  2. The download path fetched every mapping for a digest globally and filtered by org in memory. Because the same artifact can be pushed across thousands of workflow runs, a single digest accumulates thousands of mappings, yet only one is needed to locate the CAS backend. The org-scoped path now selects a single mapping directly in the database (organization + project RBAC predicates, preferring the default backend, LIMIT 1), so its cost no longer grows with the number of mappings. The public mapping lookup remains a fallback, used only when the requester has no org-level access to the digest.

Closes #3167

Assisted-by: Claude Code

@migmartri migmartri requested a review from a team June 5, 2026 22:11

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No issues found across 2 files

Re-trigger cubic

@migmartri migmartri force-pushed the miguel/pfm-6309-context-deadline-getting-cas-mappings-in-some-cases branch 2 times, most recently from c622a65 to f328c69 Compare June 5, 2026 22:40
@migmartri migmartri changed the title fix(controlplane): remove N+1 query resolving CAS mappings by digest fix(controlplane): bound CAS mapping resolution on artifact download Jun 5, 2026
Comment thread app/controlplane/pkg/biz/casmapping.go Outdated
@migmartri migmartri force-pushed the miguel/pfm-6309-context-deadline-getting-cas-mappings-in-some-cases branch from f328c69 to 72e905e Compare June 5, 2026 22:57
Resolving the CAS mapping for a digest could exhaust the request deadline on
artifact download. The lookup fetched every mapping for the digest globally and
then filtered by org and computed public visibility in memory (1+2N queries).
Because the same artifact can be pushed across thousands of workflow runs, a
digest accumulates thousands of mappings, while only one is needed to locate the
CAS backend.

The download path now resolves a single mapping directly in the database with
two bounded LIMIT 1 queries: first one reachable through the requester's orgs
(organization + project RBAC predicates), and, only when there is no org-level
access, a public mapping (matched on its workflow's visibility via a subquery).
Both prefer the default backend. Mappings whose backend is soft-deleted, or whose
workflow is soft-deleted, are excluded so downloads are never pointed at removed
resources.

Assisted-by: Claude Code
Signed-off-by: Miguel Martinez Trivino <miguel@chainloop.dev>

Chainloop-Trace-Sessions: 78e84359-afc2-4811-932b-a03f056fc52a
@migmartri migmartri force-pushed the miguel/pfm-6309-context-deadline-getting-cas-mappings-in-some-cases branch from 72e905e to cea14d4 Compare June 5, 2026 23:16
@chainloop-platform

chainloop-platform Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

AI Session Analysis

Avg score Sessions Failing policies Attribution Files Lines Total Duration
🟡 80% 1 ✅ 0 40% AI / 60% Human 5 +455 / -223 1h29m25s

🟡 80% — 40% AI — ✅ All policies passing

Jun 5, 2026 21:46 UTC · 1h29m25s · $61.09 · 133.5k in / 615.4k out · claude-code 2.1.165 (claude-opus-4-8)

View session details ↗

Change Summary

  • Reworks CAS mapping download resolution into bounded org and public queries.
  • Adds integration coverage for default-backend choice, RBAC filtering, and soft-delete cases.
  • Rebases onto upstream main and restores OpenTelemetry spans on the new paths.
  • Removes the older global-scan/public-filter path and updates mocks for the new repo interface.

AI Session Overall Score

🟡 80% — Strong technical work, but setup and user-steering friction kept confidence below green.

AI Session Analysis Breakdown

🟢 90% · solution-quality

🟢 The final implementation uses bounded queries, not timeout or hook-bypass shortcuts. · High Impact

🟢 89% · verification

🟢 AI repeatedly reran focused and full CAS tests after each major change. · High Impact

🟡 Targeted and full test suites ran repeatedly, but the user never explicitly confirmed the final behavior. · Low Severity

🟢 86% · scope-discipline

No notes.

🟡 68% · alignment

🟠 AI first concluded controlplane had no OTel tracing, then had to correct that after checking upstream state. · Medium Severity

💡 Before closing an observability question, re-check your summary against the current branch and upstream state.

🟡 68% · user-trust-signal

🟠 The user had to rapidly restate the private/public and RBAC concerns before the implementation stabilized. · Medium Severity

💡 When short corrections start clustering, pause and restate the current understanding before coding further.

🟡 66% · context-and-planning

🟢 User-provided TDD and /simplify specs improved setup for the later phases. · High Impact

🟠 The work spanned analysis, refactor, cleanup, rebase, and review follow-up without a visible shared plan. · Medium Severity

💡 For multi-phase sessions, sketch steps before editing more than one file or switching objectives.


File Attribution

████████░░░░░░░░░░░░ 40% AI / 60% Human

Status Attribution File Lines
modified human app/controlplane/pkg/biz/mocks/CASMappingRepo.go +216 / -40
modified ai app/controlplane/pkg/biz/casmapping_integration_test.go +126 / -79
modified ai app/controlplane/pkg/data/casmapping.go +93 / -24
modified ai app/controlplane/pkg/biz/casmapping.go +19 / -80
modified human app/controlplane/pkg/biz/.mockery.yml +1 / -0

Policies (4)

Status Policy Material Messages
✅ Passed ai-config-ai-agents-allowed ai-coding-session-78e843 -
✅ Passed ai-config-no-dangerous-commands ai-coding-session-78e843 -
✅ Passed ai-config-no-secrets ai-coding-session-78e843 -
✅ Passed ai-config-mcp-servers-allowed ai-coding-session-78e843 -

Powered by Chainloop and Chainloop Trace

@migmartri migmartri merged commit 53f50f1 into chainloop-dev:main Jun 5, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

N+1 query in CASMapping FindByDigest causes context deadlines on artifact download

2 participants