mikebridge commented on PR #41550: URL: https://github.com/apache/superset/pull/41550#issuecomment-5143181591
**Codex reconciliation, part 2 — final disposition.** Following the [part-1 addendum](https://github.com/apache/superset/pull/41550#issuecomment-5143045971) (four live findings fixed in `45eed1366e`), the remaining actionable Codex items are fixed in `b439dcf553`. That completes the Codex list: **7 fixed, 3 ticketed, 5 declined with reasons.** ### Fixed in `b439dcf553` 1. **Page admission widened to any archived type** (Codex M2; relayed by five capstone lenses as a "tracked" limitation — now fixed instead). `@has_access` binds a single `class_permission_name`, so the shell that fronts three independently-gated types admitted only Chart readers. The gate is now explicit: unauthenticated → the standard redirect-to-login, a reader of any of Chart/Dashboard/Dataset → admitted, a reader of none → 403, flag-off → 404 first as before. Five new admission tests; the dashboard-only-reader and none-of-the-three tests both fail against the old decorator. UPDATING.md and the PR description no longer document a limitation that no longer exists. 2. **The client agrees with the gate** (Codex LOW1). The no-readable-types fallback used to offer all three types, so such a viewer's page immediately fetched an API that answers 403 and read as broken. It now renders a one-sentence empty state and issues no fetch. Control: restoring the fallback fails the new test. 3. **Selected type is derived, not stored** (Codex M5). The once-only `useState` initializer could not follow `availableTypes` if roles resolved after mount; the effective selection is now computed against the available list on every render. ### Ticketed (pre-existing follow-ups) - Purge-policy registration + CI tripwire for future `SoftDeleteMixin` adopters (Codex M8) — tracked as sc-115410. - Reconcile lease/heartbeat for >1h in-flight purges (Codex M6) — the false-*confirm* half was the part with compliance teeth and is fixed (`reconciled_absent`, part 1); the lease is design work. - Purge-route dedup across the three APIs (Codex LOW3) — tracked with the existing route-triplication follow-up. ### Declined, with reasons - **M3** (SQL-discoverable `EXTRA_EDITORS_RESOLVER` audience): the resolver is arbitrary per-deployment Python — there is no SQL form to build. The enumeration contract is documented as *narrower than the true restore audience, never wider*; affected principals can still restore directly. - **M9** (dashboard-composition mutation during chart purge): deliberate; `suppress_purge_association_versions` documents why purge intentionally applies legacy hard-delete semantics to association rows. - **M10** (typed ORM columns): the codebase's house style on SQLAlchemy 1.4 is legacy `Column` declarations; diverging in one new model would be the inconsistency. - **M11** (test typing in `version_restore_tests.py`): belongs to the versioning stream, not this PR. - **M12** (1,323-assignment annotation audit): Codex's own qualification applies — not a defect count; handled by the normal typing rules on touched code. _Generated by Claude (AI) on behalf of @mikebridge._ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
