Fix Admin Settings 500 from cross-pane Jinja variable scope - #1325
Merged
Paul Lizer (paullizer) merged 1 commit intoAug 20, 2026
Merged
Conversation
The Document Action Capabilities card moved into admin/_panes/actions.html
but the two {% set %} statements feeding it stayed in admin/_panes/agents.html.
admin_settings.html includes those panes as siblings, and Jinja gives each
include its own context copy, so analyze_capability and comparison_capability
were always Undefined. Attribute access on Undefined raised
jinja2.exceptions.UndefinedError and every GET /admin/settings returned 500.
Both statements now live in the pane that renders the card.
The traceback never reached the App Service console because
configure_azure_monitor attaches a handler to the root logger, which makes
logging.basicConfig a no-op and makes Flask skip its own stderr handler, so
unhandled exceptions went to Application Insights and nowhere else.
ensure_console_error_logging now keeps a stderr handler on app.logger.
An AST scan of all 44 panes surfaced two more names that always resolved to
Undefined: enable_dai_debug in cosmos.html, which kept the Document Access
Index diagnostics hidden even when enabled, and the dead
window.enableDocumentClassification and window.enableExternalLinks globals,
whose only consumers were two never-read declarations in admin_settings.js.
Those globals are removed rather than repaired, because "False" is truthy in
JavaScript and would have flipped the || false fallback.
The composed-template test helpers flatten every include into one string, which
is why the existing suite could not see this. The new test reads each pane in
isolation and derives its allowlist by parsing the route's render_template call
and the inject_settings context processor, so it cannot go stale.
Version 0.260.018 -> 0.260.019.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
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.
The 500
Every
GET /admin/settingsreturned 500 on the Azure deployment after the Admin Settings IA merge. The App Service container log showed nothing but the access-log line, so the exception was recovered from Application Insights:Root cause
The pane split moved the Document Action Capabilities card into
admin/_panes/actions.htmlbut left the two{% set %}statements feeding it behind inadmin/_panes/agents.html.admin_settings.htmlincludes those panes as siblings, and Jinja gives each include its own context copy — a{% set %}in one pane is never visible to the next. Both names were therefore alwaysUndefined, and attribute access onUndefinedraises.Neither name was used anywhere in
agents.html, so they were pure leftovers there. Both now live in the pane that renders the card.This was never Azure-specific — the failure is deterministic and data-independent. Any run of the merged code fails; a local instance that looked healthy was serving pre-merge templates.
Why the traceback was invisible
configure_azure_monitor()attaches a handler to the root logger. That makeslogging.basicConfig()a no-op, and makes Flask'screate_logger()find a level handler viahas_level_handler()and skip attaching its own stderr handler. Unhandled tracebacks went to Application Insights and nowhere else.ensure_console_error_logging()now keeps a stderr handler pinned atERRORonapp.logger— scoped to the app logger and never the root, so libraryDEBUGoutput can't flood the container log.Also fixed
An AST scan of all 44 panes surfaced two more names that always resolved to
Undefined:enable_dai_debug(cosmos.html, 6 references) — the bare name was never passed, so the Document Access Index backfill controls, shadow validation metrics and reset modal stayed hidden even when an admin enabled the setting. Nowsettings.enable_dai_debug.enable_document_classification/enable_external_links— emitted""intowindow.*globals whose only consumers were two never-readletdeclarations inadmin_settings.js. Removed rather than repaired: emitting"{{ settings.enable_external_links }}"yields the string"False", which is truthy, and would have silently flippedwindow.enableExternalLinks || falsetotrue.Four other names the scan reported (
option_value,release_card_id,release_collapse_id,preview_card_id,preview_collapse_id) are{% set %}inside their own{% for %}bodies and resolve correctly. The new test deliberately does not flag them.Why the suite missed this
test_support/templates.py::resolve_template_includesinlines everyadmin/include into one flat string before assertions run. Flattened,agents.html's{% set %}appears ahead ofactions.html's markup, so every composed-template test saw a valid document.Tests
functional_tests/test_admin_settings_pane_variable_scope.py(new, 5 tests) reads each pane in isolation and derives its allowlist byast-parsing therender_template('admin_settings.html', ...)call and theinject_settingscontext processor, so it can't go stale as template variables change. Loop-scoped{% set %}names are excluded. The final test rendersagents.html+actions.htmltogether through a realFileSystemLoader, exactly as the parent composes them.Verified to fail on the pre-fix templates:
functional_tests/test_flask_exception_console_logging.py(new, 4 tests) pins the logging guarantee, including that the handler is attached exactly once across worker reloads and that the root logger is untouched.ui_tests/test_admin_document_action_capabilities_card.pywas stale — it still clicked#agents-tabfor a card that now lives in the Actions tab. Retargeted and extended to assert each capability limit input renders a value; itsassert response.okis a direct browser-level guard against this 500.functional_tests/test_admin_settings_template_composition.pygained an explicit exemption for tests that read pane partials directly, since composing the parent hides exactly the dependency this fix is about.Validation
15 suites pass, including all admin-settings and route-policy tests. Four suites (
test_admin_settings_tab_preservation.py,test_single_app_template_json_bootstrap_safety.py,test_stored_xss_admin_rendering_fix.py,test_admin_settings_safe_int_fallback_fix.py) fail identically before and after this change and are unrelated — the last of those asserts an exactVERSION = "0.240.002"literal, which the repo's own instructions warn against.Version
0.260.018→0.260.019.deployers/version.txtuntouched — nothing underdeployers/changed.Docs:
docs/explanation/fixes/ADMIN_SETTINGS_PANE_VARIABLE_SCOPE_FIX.mdplus release notes.