Skip to content

feat: PLTF-3553 Add org condenser max_tokens rollout configuration - #337

Merged
aivong-openhands merged 8 commits into
mainfrom
openhands-org-condenser-defaults-enterprise-20260909
Sep 11, 2026
Merged

aivong-openhands merged 8 commits into
mainfrom
openhands-org-condenser-defaults-enterprise-20260909

Conversation

@aivong-openhands

@aivong-openhands aivong-openhands commented Sep 9, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Add env-driven org-default condenser max_tokens configuration.
  • Seed new applicable OpenHands/default and legacy agent_kind: "llm" org settings while skipping ACP and no-op variants.
  • Add startup rollout reconciliation for existing applicable org rows with PostgreSQL advisory locking and retry semantics.
  • Preserve unrelated condenser fields, clean JSON-null member overrides, and deep-merge org/member settings.
  • Add PostgreSQL CI coverage for reconciliation behavior.

Validation

  • uv run pre-commit run --config ./dev_config/python/.pre-commit-config.yaml --files ...
  • uv run pytest -q tests/unit/test_org_defaults_config.py tests/unit/storage/test_org_default_settings.py tests/unit/test_org_member_store.py tests/unit/test_user_store.py tests/unit/test_org_store.py::test_create_org_applies_configured_condenser_max_tokens tests/unit/test_org_store.py::test_create_org_skips_configured_condenser_max_tokens_for_acp
  • uv run pytest -q tests/integration/test_org_condenser_reconciliation_postgres.py (locally skipped without POSTGRES_TEST_DATABASE_URL; new workflow runs it with PostgreSQL 16)

This PR was created by an AI agent (OpenHands) on behalf of the user.

@aivong-openhands can click here to continue refining the PR


Enterprise server image for this PR:

ghcr.io/openhands/enterprise-server:sha-cb8a658

Co-authored-by: openhands <openhands@all-hands.dev>
@aivong-openhands aivong-openhands changed the title Add org condenser max_tokens rollout configuration feat: PLTF-3553 Add org condenser max_tokens rollout configuration Sep 9, 2026
@github-actions github-actions Bot added the type: feat A new feature label Sep 9, 2026
@github-actions

github-actions Bot commented Sep 9, 2026 •

Copy link
Copy Markdown

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  server
  org_defaults_config.py
  server/app_lifespan
  saas_app_lifespan_service.py 48, 106-116, 166-177
  storage
  org_default_settings.py 60-67
  org_member_store.py
  org_service.py
  org_store.py
  user_store.py
Project Total  

This report was generated by python-coverage-comment-action

@aivong-openhands

Copy link
Copy Markdown
Contributor Author

Mutation review of the tests in this PR

I ran the new/changed tests against 17 hand-written mutants of the diff — 4 controls that revert what the PR claims to do, 13 candidates in the machinery around it. Baseline for the exercise: tests/unit/test_org_defaults_config.py, tests/unit/storage/test_org_default_settings.py, tests/unit/test_org_member_store.py, tests/unit/test_saas_lifespan.py, tests/integration/test_org_condenser_reconciliation_postgres.py (against real PostgreSQL 16) plus the four new test_user_store.py/test_org_store.py cases — 59 passed in ~4s.

Controls — all caught ✅

Mutant Result
Revert user_store agent-settings deep_merge back to {**org, **member} ❌ caught
Drop strip_unset_condenser_max_tokens from OrgMemberStore.get_kwargs_from_user_settings ❌ caught
Drop apply_configured_org_condenser_default from OrgStore.create_org ❌ caught
SQL: drop the NULLIF(..., 'null'::jsonb) so JSON-null max_tokens counts as already set ❌ caught

The central claims are pinned, and the reason they are pinned is worth naming: the PostgreSQL integration test drives the real jsonb statement against a real server with nine deliberately awkward rows (condenser: null, max_tokens: null, condenser: ['bad'], agent_kind: 'acp', no_op) and asserts the whole resulting condenser dict with ==, not just the one key. That is what kills the NULLIF control — an assertion on max_tokens alone would not have. Same for the member-diff tests: asserting kwargs['agent_settings_diff'] == {'condenser': {'enabled': False}} on the whole dict is what catches the strip helper going away.

Survivors — 13 mutants, suite stayed green ✅

# Mutant Result
1 apply_org_condenser_max_tokens_default never sets max_tokens on the OpenHandsAgentSettings (pydantic) branch ✅ 59 passed
2 create_base_org_agent_settings returns settings without applying the configured default ✅ 59 passed
3 OrgService.apply_litellm_settings_to_org stops applying the default ✅ 59 passed
4 UserStore.create_user stops applying the default to the new personal org ✅ 59 passed
5 UserStore.migrate_user stops applying the default to the migrated org ✅ 59 passed
6 SaasAppLifespanService.__aenter__ never calls _reconcile_org_condenser_defaults ✅ 59 passed
7 Reconciliation ignores apply_to_existing and always runs ✅ 59 passed
8 Transient failure is never retried ✅ 59 passed
9 _is_transient_reconciliation_error always returns False ✅ 59 passed
10 Advisory lock + lock_timeout removed from the reconciliation transaction ✅ 59 passed
11 Reconciliation UPDATE no longer bumps updated_at ✅ 59 passed
12 strip_unset_condenser_max_tokens leaves an empty condenser: {} behind ✅ 59 passed
13 Conversation settings revert to shallow merge ✅ 59 passed (see "equivalent mutant")

Every fix below was verified twice: it passes on this branch unmodified, and it fails with its mutant applied.


1. The pydantic branch of apply_org_condenser_max_tokens_default is entirely unasserted (survivors 1, 2)

tests/unit/storage/test_org_default_settings.py only ever passes dicts, so the whole isinstance(agent_settings, OpenHandsAgentSettings) branch — and create_base_org_agent_settings, which is the only caller of it — is unpinned. This is not a rarely-taken path: OrgService.create_org_entity is the main org-creation route, and it builds its settings through create_base_org_agent_settings. Deleting the setattr there means every org created through the org API silently never receives the configured max_tokens, and no test notices.

def test_base_org_agent_settings_carry_configured_condenser_max_tokens(monkeypatch):
    from storage.org_default_settings import create_base_org_agent_settings

    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS', '200000')

    settings = create_base_org_agent_settings()

    assert settings.condenser.max_tokens == 200000


def test_applies_to_model_settings_and_preserves_existing_without_overwrite():
    from openhands.sdk.settings import default_agent_settings
    from storage.org_default_settings import apply_org_condenser_max_tokens_default

    settings = default_agent_settings()
    result = apply_org_condenser_max_tokens_default(settings, max_tokens=200000)
    assert result.condenser.max_tokens == 200000

    result = apply_org_condenser_max_tokens_default(result, max_tokens=999)
    assert result.condenser.max_tokens == 200000

    result = apply_org_condenser_max_tokens_default(
        result, max_tokens=999, overwrite=True
    )
    assert result.condenser.max_tokens == 999

The second test also pins the overwrite/no-overwrite semantics on the model branch, which the dict tests cover but this branch does not.

2. Three of the four call sites that apply the default have no test (survivors 3, 4, 5)

OrgStore.create_org is covered by the two new test_org_store.py cases. The other three — OrgService.apply_litellm_settings_to_org, UserStore.create_user, UserStore.migrate_user — can each be deleted with the suite green. apply_litellm_settings_to_org is the most consequential of the three, because it runs after create_org_entity and re-setattrs agent_settings from OrgStore.get_kwargs_from_settings(settings); if that ordering ever changes, the re-application is the only thing keeping the default alive on the org-API path, and nothing checks it is still there.

def test_apply_litellm_settings_applies_configured_condenser_default(monkeypatch):
    from openhands.app_server.settings.settings_models import Settings
    from storage.org import Org
    from storage.org_service import OrgService

    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS', '200000')

    org = Org(
        id=uuid.uuid4(),
        name='org',
        agent_settings={'agent_kind': 'openhands', 'condenser': {'max_size': 240}},
    )
    OrgService.apply_litellm_settings_to_org(org, Settings(language='en'))

    assert org.agent_settings['condenser']['max_tokens'] == 200000
    assert org.agent_settings['condenser']['max_size'] == 240

Asserting max_size survives alongside is what makes this test also cover the "preserve unrelated condenser fields" claim on this path.

For create_user, the existing _seed_admin_role / _mock_create_default_settings_returning_default scaffolding in tests/unit/test_user_store.py already gets you there:

@pytest.mark.asyncio
async def test_create_user_applies_configured_condenser_default(
    async_session_maker, monkeypatch
):
    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS', '200000')
    await _seed_admin_role(async_session_maker)

    with (
        patch('storage.user_store.a_session_maker', async_session_maker),
        patch('storage.role_store.a_session_maker', async_session_maker),
        _mock_create_default_settings_returning_default(),
    ):
        user = await UserStore.create_user(
            str(uuid.uuid4()), {'email': 'a@example.com', 'preferred_username': 'a'}
        )

    assert user is not None
    async with async_session_maker() as session:
        org = (await session.execute(select(Org))).scalars().one()
    assert org.agent_settings['condenser']['max_tokens'] == 200000

migrate_user can reuse the mock-session setup from test_migrate_user_preserves_normalized_default_tools verbatim, adding the env var and asserting org.agent_settings['condenser']['max_tokens'] == 200000 on the Org pulled out of session.add.call_args_list.

3. The startup rollout is untested end to end (survivors 6, 7)

tests/unit/test_saas_lifespan.py exercises __aenter__, so removing the await self._reconcile_org_condenser_defaults() line from it is a one-character-ish change that no test sees — the rollout simply never runs in production and CI is green. Worse, deleting or not config.apply_to_existing also survives: an operator who sets only ..._MAX_TOKENS (intending new orgs only) would get every existing org rewritten on the next pod restart, which is the exact blast-radius the flag exists to prevent.

@pytest.mark.asyncio
async def test_aenter_runs_org_condenser_reconciliation():
    from server.app_lifespan.saas_app_lifespan_service import SaasAppLifespanService

    svc = SaasAppLifespanService()
    with (
        patch('server.app_lifespan.saas_app_lifespan_service.init_analytics_service'),
        patch.object(
            SaasAppLifespanService,
            '_reconcile_org_condenser_defaults',
            new_callable=AsyncMock,
        ) as mock_reconcile,
    ):
        await svc.__aenter__()

    mock_reconcile.assert_awaited_once()


@pytest.mark.asyncio
async def test_reconciliation_is_skipped_unless_apply_to_existing(monkeypatch):
    from server.app_lifespan.saas_app_lifespan_service import SaasAppLifespanService

    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS', '200000')
    monkeypatch.delenv(
        'OPENHANDS_ORG_DEFAULTS_CONDENSER_APPLY_TO_EXISTING', raising=False
    )

    with patch.object(
        SaasAppLifespanService,
        '_reconcile_org_condenser_defaults_once',
        new_callable=AsyncMock,
    ) as mock_once:
        await SaasAppLifespanService()._reconcile_org_condenser_defaults()

    mock_once.assert_not_awaited()


@pytest.mark.asyncio
async def test_reconciliation_runs_when_apply_to_existing(monkeypatch):
    from server.app_lifespan.saas_app_lifespan_service import SaasAppLifespanService

    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS', '200000')
    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_APPLY_TO_EXISTING', 'true')

    with patch.object(
        SaasAppLifespanService,
        '_reconcile_org_condenser_defaults_once',
        new_callable=AsyncMock,
    ) as mock_once:
        await SaasAppLifespanService()._reconcile_org_condenser_defaults()

    mock_once.assert_awaited_once_with(max_tokens=200000, overwrite_existing=False)

4. Retry and transient-error classification (survivors 8, 9)

Both halves of the retry are unpinned. Replacing the retry block with a bare raise survives, and so does making _is_transient_reconciliation_error return False unconditionally — which would turn every deadlock or lock-timeout at startup into a permanently-skipped rollout with a single logged exception. The whole _TRANSIENT_SQLSTATES / _TRANSIENT_SQLSTATE_PREFIXES table has no test at all.

@pytest.mark.asyncio
async def test_transient_failure_is_retried_once_then_succeeds(monkeypatch):
    from server.app_lifespan.saas_app_lifespan_service import (
        SaasAppLifespanService,
        TransientReconciliationError,
    )
    from storage.org_store import OrgCondenserReconciliationResult

    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS', '200000')
    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_APPLY_TO_EXISTING', 'true')

    success = OrgCondenserReconciliationResult(1, 0, 0, 0)
    with (
        patch(
            'server.app_lifespan.saas_app_lifespan_service.asyncio.sleep',
            new_callable=AsyncMock,
        ) as mock_sleep,
        patch.object(
            SaasAppLifespanService,
            '_reconcile_org_condenser_defaults_once',
            new_callable=AsyncMock,
            side_effect=[TransientReconciliationError(), success],
        ) as mock_once,
    ):
        await SaasAppLifespanService()._reconcile_org_condenser_defaults()

    assert mock_once.await_count == 2
    mock_sleep.assert_awaited_once()


@pytest.mark.asyncio
async def test_non_transient_failure_is_not_retried(monkeypatch):
    from server.app_lifespan.saas_app_lifespan_service import SaasAppLifespanService

    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS', '200000')
    monkeypatch.setenv('OPENHANDS_ORG_DEFAULTS_CONDENSER_APPLY_TO_EXISTING', 'true')

    with patch.object(
        SaasAppLifespanService,
        '_reconcile_org_condenser_defaults_once',
        new_callable=AsyncMock,
        side_effect=RuntimeError('boom'),
    ) as mock_once:
        await SaasAppLifespanService()._reconcile_org_condenser_defaults()

    assert mock_once.await_count == 1


def test_operational_error_is_classified_transient():
    from sqlalchemy.exc import DBAPIError, OperationalError

    from server.app_lifespan.saas_app_lifespan_service import (
        _is_transient_reconciliation_error,
    )

    assert _is_transient_reconciliation_error(
        OperationalError('SELECT 1', {}, Exception('conn reset'))
    )

    class _PgError(Exception):
        sqlstate = '40P01'

    assert _is_transient_reconciliation_error(
        DBAPIError('SELECT 1', {}, _PgError('deadlock'))
    )
    assert not _is_transient_reconciliation_error(
        DBAPIError('SELECT 1', {}, Exception('syntax error'))
    )

The _PgError shim is deliberately minimal, but it does exercise the real _sqlstate attribute lookup rather than mocking the classifier out — worth extending with one case per entry in _TRANSIENT_SQLSTATES plus an '08006' for the prefix rule.

5. Nothing checks the advisory lock is taken (survivor 10)

The lock is the only thing preventing N replicas from running the same rewrite concurrently at rollout, and both the SET LOCAL lock_timeout = '5s' and the pg_advisory_xact_lock can be deleted with the suite green. The integration test calls OrgStore.reconcile_... directly and never goes through _reconcile_org_condenser_defaults_once, so that method has zero coverage. Asserting on the executed SQL is enough here:

@pytest.mark.asyncio
async def test_reconciliation_takes_advisory_lock_with_timeout():
    from server.app_lifespan.saas_app_lifespan_service import SaasAppLifespanService

    executed = []

    class _Ctx:
        async def __aenter__(self):
            return None

        async def __aexit__(self, *args):
            return False

    class _Session:
        def begin(self):
            return _Ctx()

        async def execute(self, statement, params=None):
            executed.append(str(statement))

    session = _Session()

    class _SessionCtx:
        async def __aenter__(self):
            return session

        async def __aexit__(self, *args):
            return False

    with (
        patch('storage.database.a_session_maker', lambda: _SessionCtx()),
        patch(
            'storage.org_store.OrgStore.reconcile_applicable_org_condenser_max_tokens',
            new_callable=AsyncMock,
        ),
    ):
        await SaasAppLifespanService()._reconcile_org_condenser_defaults_once(
            max_tokens=200000, overwrite_existing=False
        )

    assert any('lock_timeout' in sql for sql in executed)
    assert any('pg_advisory_xact_lock' in sql for sql in executed)

6. updated_at is not asserted (survivor 11)

Org.updated_at is documented in the model as "Timestamp for optimistic locking - tracks last modification", and the raw UPDATE sets it by hand because SQLAlchemy's onupdate does not fire for Core statements. Drop updated_at = now() and every test still passes — meaning a rollout could silently rewrite agent_settings under a concurrent reader holding a stale timestamp. This also pins the negative case, that untouched rows are not bumped:

@pytest.mark.asyncio
async def test_reconcile_bumps_updated_at_only_for_changed_rows(
    postgres_session_maker,
):
    stale = datetime.now(UTC) - timedelta(days=30)
    async with postgres_session_maker() as session:
        async with session.begin():
            session.add_all([
                Org(id=uuid.uuid4(), name='changed',
                    agent_settings={'agent_kind': 'openhands'}, updated_at=stale),
                Org(id=uuid.uuid4(), name='untouched',
                    agent_settings={'agent_kind': 'acp'}, updated_at=stale),
            ])

    async with postgres_session_maker() as session:
        async with session.begin():
            await OrgStore.reconcile_applicable_org_condenser_max_tokens(
                session, max_tokens=200000, overwrite_existing=False
            )

    async with postgres_session_maker() as session:
        rows = dict((await session.execute(select(Org.name, Org.updated_at))).all())

    assert rows['changed'] > stale
    assert rows['untouched'] == stale

7. The strip helper's cleanup of an emptied condenser (survivor 12)

test_get_kwargs_from_user_settings_strips_null_condenser_max_tokens uses {'enabled': False, 'max_tokens': None}, so condenser is never left empty and the final if not condenser: pop('condenser') is dead as far as the suite is concerned. That branch matters: a member whose only override was max_tokens: null should end up with no condenser key at all, otherwise an empty {} rides into the member diff and shows up in deep_merge. One extra case covers it:

def test_org_member_diff_drops_condenser_when_only_null_max_tokens():
    user_settings = UserSettings(
        agent_settings={'condenser': {'max_tokens': None}},
        conversation_settings={},
    )

    kwargs = OrgMemberStore.get_kwargs_from_user_settings(user_settings)

    assert kwargs['agent_settings_diff'] == {}

Equivalent mutant: conversation-settings deep_merge (survivor 13)

Reverting conversation_settings to a shallow {**org, **member} also survives, but I think this one is genuinely equivalent today: every persisted field on ConversationSettings is a scalar (schema_version, max_iterations, confirmation_mode, security_analyzer) — the nested ones like observability_metadata are all exclude=True and never reach model_dump. So there is nothing for a deep merge to do. Either there is a case I am missing, or the change is a consistency-with-agent_settings move rather than a behavioural one, in which case it needs no test.


Not a test gap

  • apply_org_condenser_max_tokens_default is typed _SettingsT = TypeVar('_SettingsT', dict[str, Any], OpenHandsAgentSettings), and a real ACPAgentSettings model instance would fall through to the dict branch and raise AttributeError: 'ACPAgentSettings' object has no attribute 'get'. It is unreachable today because every caller passes either a dict (JSON column) or the OpenHands variant, and Org.__init__ dumps models to dicts — so the constraint is doing its job. Worth a comment or an explicit isinstance guard if that ever loosens.
  • The PostgreSQL fixture builds the table with Org.__table__.create rather than running Alembic, so the integration test validates the SQL against the model's schema, not the migrated one. Fine for jsonb-shape assertions; it would not catch a column-type drift between the model and the deployed database.
  • The new workflow runs only tests/integration/test_org_condenser_reconciliation_postgres.py by name, while its paths: filter is the whole of storage/**, server/**, tests/**. Any future file under tests/integration/ marked postgresql will silently not run. -m postgresql tests/integration would track the marker you just added to pytest.ini.

This comment was generated by an AI agent (OpenHands) on behalf of the user.

Copy link
Copy Markdown
Contributor Author

Addressed the mutation-test feedback in commit 5564ccf:

  • Added create_user and migrate_user coverage for applying configured org condenser max_tokens defaults.
  • Added startup coverage for running the org condenser reconciliation hook, including apply_to_existing gating.
  • Added retry/transient-error classification coverage and advisory-lock SQL assertions.
  • Added an integration assertion that updated_at is bumped only for rows changed by reconciliation.
  • Added coverage for dropping an emptied condenser diff after stripping null max_tokens.
  • Updated the PostgreSQL workflow to run all postgresql-marked integration tests under tests/integration.

Validation run locally:

  • uv run pytest tests/unit/test_org_defaults_config.py tests/unit/storage/test_org_default_settings.py tests/unit/test_org_member_store.py tests/unit/test_saas_lifespan.py tests/unit/test_org_store.py tests/unit/test_user_store.py — 181 passed, 3 skipped
  • uv run pytest -s -m postgresql tests/integration — 3 skipped locally because POSTGRES_TEST_DATABASE_URL is not configured
  • uv run pre-commit run --config ./dev_config/python/.pre-commit-config.yaml — passed

There were no inline review threads to resolve.

This PR comment was created by an AI agent (OpenHands) on behalf of the user.

Copy link
Copy Markdown
Contributor Author

Impact assessment: self-hosted / customer installs vs SaaS

Verdict: Additive-inert by default: no customer self-hosted/OHE, SaaS prod, SaaS staging, SaaS dev, or feature-preview behavior changes unless that surface explicitly sets the new OPENHANDS_ORG_DEFAULTS_CONDENSER_* env vars; opt-out exists by leaving MAX_TOKENS unset, and existing-org rollout is separately gated by APPLY_TO_EXISTING.

What changed

  1. server/org_defaults_config.py

    • New symbols:
      • OrgDefaultsCondenserConfig at server/org_defaults_config.py:16
      • _parse_bool_env at server/org_defaults_config.py:25
      • _parse_positive_int_env at server/org_defaults_config.py:35
      • get_org_defaults_condenser_config at server/org_defaults_config.py:48
    • Reads:
      • OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS at server/org_defaults_config.py:52
      • OPENHANDS_ORG_DEFAULTS_CONDENSER_APPLY_TO_EXISTING at server/org_defaults_config.py:55
      • OPENHANDS_ORG_DEFAULTS_CONDENSER_OVERWRITE_EXISTING at server/org_defaults_config.py:58
    • Consumers:
      • storage/org_default_settings.py:10, storage/org_default_settings.py:17
      • server/app_lifespan/saas_app_lifespan_service.py:87, server/app_lifespan/saas_app_lifespan_service.py:89
      • tests in tests/unit/test_org_defaults_config.py, tests/unit/storage/test_org_default_settings.py, and tests/unit/test_saas_lifespan.py
  2. storage/org_default_settings.py

    • New symbols:
      • _configured_max_tokens at storage/org_default_settings.py:17
      • create_base_org_agent_settings at storage/org_default_settings.py:21
      • _dict_agent_kind at storage/org_default_settings.py:33
      • _dict_condenser_kind at storage/org_default_settings.py:38
      • apply_org_condenser_max_tokens_default at storage/org_default_settings.py:43
      • apply_configured_org_condenser_default at storage/org_default_settings.py:92
      • strip_unset_condenser_max_tokens at storage/org_default_settings.py:106
    • Consumers:
      • storage/org_service.py:27, storage/org_service.py:28, storage/org_service.py:131, storage/org_service.py:148
      • storage/org_store.py:44, storage/org_store.py:133
      • storage/user_store.py:33, storage/user_store.py:159, storage/user_store.py:359
      • storage/org_member_store.py:20, storage/org_member_store.py:182
  3. storage/org_store.py

    • New symbols:
      • OrgCondenserReconciliationResult at storage/org_store.py:54
      • OrgStore.reconcile_applicable_org_condenser_max_tokens at storage/org_store.py:144
    • Changed consumer path:
      • OrgStore.create_org applies configured condenser defaults at storage/org_store.py:133
    • Consumers:
      • server/app_lifespan/saas_app_lifespan_service.py:161
      • tests/integration/test_org_condenser_reconciliation_postgres.py
      • tests/unit/test_saas_lifespan.py
  4. server/app_lifespan/saas_app_lifespan_service.py

    • New symbols:
      • _ORG_CONDENSER_RECONCILIATION_LOCK_ID at server/app_lifespan/saas_app_lifespan_service.py:21
      • _TRANSIENT_SQLSTATES at server/app_lifespan/saas_app_lifespan_service.py:22
      • _TRANSIENT_SQLSTATE_PREFIXES at server/app_lifespan/saas_app_lifespan_service.py:29
      • TransientReconciliationError at server/app_lifespan/saas_app_lifespan_service.py:31
      • _sqlstate at server/app_lifespan/saas_app_lifespan_service.py:35
      • _is_transient_reconciliation_error at server/app_lifespan/saas_app_lifespan_service.py:44
      • SaasAppLifespanService._reconcile_org_condenser_defaults at server/app_lifespan/saas_app_lifespan_service.py:86
      • SaasAppLifespanService._reconcile_org_condenser_defaults_once at server/app_lifespan/saas_app_lifespan_service.py:142
    • Changed consumer path:
      • SaasAppLifespanService.__aenter__ now calls reconciliation at server/app_lifespan/saas_app_lifespan_service.py:83
    • Startup consumers:
      • selected when OPENHANDS_CONFIG_CLS contains saas at openhands/app_server/config.py:174
      • mounted into FastAPI lifespan at openhands/app_server/app.py:50
  5. storage/org_service.py

    • Changed symbols:
      • OrgService.create_org_entity now uses create_base_org_agent_settings at storage/org_service.py:131
      • OrgService.apply_litellm_settings_to_org now reapplies configured condenser default after LiteLLM-derived org settings at storage/org_service.py:148
    • Consumers:
      • storage/org_service.py:251
      • storage/org_service.py:259
  6. storage/user_store.py

    • Changed symbols:
      • UserStore.create_user applies configured condenser defaults to newly created personal orgs at storage/user_store.py:159
      • UserStore.migrate_user applies configured condenser defaults to migrated orgs at storage/user_store.py:359
      • UserStore._create_user_settings_from_entities now uses deep_merge for nested org/member agent and conversation settings at storage/user_store.py:1274, storage/user_store.py:1278
    • Consumers:
      • user creation path starts at storage/user_store.py:70
      • migration path starts at storage/user_store.py:275
      • entity-to-legacy settings conversion starts at storage/user_store.py:1248
  7. storage/org_member_store.py

    • Changed symbol:
      • OrgMemberStore.get_kwargs_from_user_settings now strips JSON-null condenser.max_tokens member overrides at storage/org_member_store.py:178, storage/org_member_store.py:182
    • Consumers:
      • storage/user_store.py:393
      • tests in tests/unit/test_org_member_store.py
  8. CI-only changes:

    • Added .github/workflows/postgres-integration-tests.yml, which runs PostgreSQL 16 integration tests for storage/**, server/**, tests/**, pyproject.toml, uv.lock, and that workflow.
    • Added postgresql pytest marker at pytest.ini:7.

Environment × flag matrix (before → after)

Deployment Real input/config source MAX_TOKENS APPLY_TO_EXISTING OVERWRITE_EXISTING Result
Customer self-hosted / OHE / Replicated OpenHands chart 0.63.0/0.64.0 defaults and Replicated templates: no OPENHANDS_ORG_DEFAULTS_CONDENSER_*; chart pins OH_DEPLOYMENT_MODE=self_hosted; .Values.env can pass arbitrary overrides unset → unset unset/default false → unset/default false unset/default false → unset/default false unchanged
SaaS production OpenHands/saas-deploy app/openhands/environments/production/values.yaml + chart 0.63.0; no new env vars rendered unset → unset unset/default false → unset/default false unset/default false → unset/default false unchanged
SaaS staging OpenHands/saas-deploy app/openhands/environments/staging/values.yaml + chart 0.63.0; no new env vars rendered unset → unset unset/default false → unset/default false unset/default false → unset/default false unchanged
SaaS dev OpenHands/saas-deploy app/openhands/environments/development/values.yaml + chart 0.64.0; current enterprise-server pin is sha-0a11356; no new env vars rendered unset → unset unset/default false → unset/default false unset/default false → unset/default false unchanged
SaaS feature previews OpenHands/saas-deploy app/openhands/environments/feature/values.yaml + chart 0.49.0; no new env vars rendered; PR image can be deployed per preview unset → unset unset/default false → unset/default false unset/default false → unset/default false unchanged

Validation level: PR state and checks verified with gh (OPEN, draft, all listed checks passing including PostgreSQL 16 integration tests); SaaS/chart inputs verified from checked-out GitOps values, pulled Helm chart templates, and local helm template renders; env parser behavior standalone-validated; targeted unit tests passed locally (65 passed). No live kubectl/KOTS customer install access was available, so live pod env values are not verified.

Behaviors that change, and for whom

  • No listed surface changes on image rollout alone, because the new env vars are absent everywhere I found in SaaS GitOps values, Helm chart defaults, and Replicated chart templates.
  • Any surface that sets OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS=<positive int> changes new applicable org defaults:
    • Applies to org settings with agent_kind absent, openhands, or legacy llm.
    • Applies when condenser is absent, malformed, or condenser_kind absent/llm_summarizing.
    • Skips ACP and explicit non-llm_summarizing condenser variants.
    • Preserves an explicit existing condenser.max_tokens unless the reconciliation path is run with overwrite enabled.
  • Existing org rows change only when both:
    • OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS is set, and
    • OPENHANDS_ORG_DEFAULTS_CONDENSER_APPLY_TO_EXISTING=true or 1.
  • If APPLY_TO_EXISTING=true and OVERWRITE_EXISTING=false, existing applicable org rows with missing or JSON-null condenser.max_tokens are filled, while explicit values are preserved.
  • If APPLY_TO_EXISTING=true and OVERWRITE_EXISTING=true, existing applicable org rows with explicit different condenser.max_tokens are overwritten. That is the only fleet-wide posture/behavior-change mode.
  • Legacy member settings migration changes slightly: condenser.max_tokens: null in member diffs no longer masks org-level defaults, and deep merge now preserves unrelated nested org/member settings instead of shallow-overwriting nested dictionaries.
  • On configured existing-org rollout, SaaS app startup exercises a PostgreSQL reconciliation query under an advisory lock. Transient failures get one retry; remaining failures are logged and startup continues.

Why customer / self-hosted installs are safe

  1. Customer/OHE installs are unchanged by default: neither the pulled chart defaults nor Replicated templates set OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS, so get_org_defaults_condenser_config() returns max_tokens=None and all application helpers no-op.
  2. Existing-org mutation is double gated. Even if an operator sets MAX_TOKENS, existing org rows are not reconciled unless APPLY_TO_EXISTING is also explicitly true/1.
  3. There is an opt-out and staging mechanism: unset MAX_TOKENS disables the feature; set only MAX_TOKENS for new-org-only behavior; add APPLY_TO_EXISTING later for controlled existing-row rollout; leave OVERWRITE_EXISTING=false to preserve explicit customer/org choices.
  4. Variant guards prevent the default from applying to ACP agents and non-LLM-summarizing/no-op condenser settings.
  5. The existing-row reconciler is operationally defensive: it uses a PostgreSQL advisory transaction lock, a short lock timeout, transient retry handling, and logs/swallow failures rather than blocking app startup.
  6. No productized KOTS UI option for these env vars was found, so customer installs do not receive a surprise installer-driven value. Operators using raw Helm .Values.env can still opt in deliberately.

Rollout note

PR #337 is open but draft. Its checks are passing, including the new PostgreSQL 16 integration workflow, and the PR body advertises enterprise-server image ghcr.io/openhands/enterprise-server:sha-5564ccf.

The change reaches a surface only after that enterprise-server image, or a later image containing the merge commit, is deployed there. With current SaaS GitOps values, deploying the image alone remains inert because the OPENHANDS_ORG_DEFAULTS_CONDENSER_* env vars are not set in dev, staging, production, or feature values. To stage rollout, add OPENHANDS_ORG_DEFAULTS_CONDENSER_MAX_TOKENS first in dev or a feature preview for new-org-only behavior; then optionally set OPENHANDS_ORG_DEFAULTS_CONDENSER_APPLY_TO_EXISTING=true for existing orgs; set OPENHANDS_ORG_DEFAULTS_CONDENSER_OVERWRITE_EXISTING=true only for a deliberate overwrite rollout. For customer/OHE releases, the chart/image update is still inert unless the operator supplies these env vars through Helm values or another environment-injection mechanism.


This analysis was produced by an AI agent (OpenHands) on behalf of @aivong-openhands.

@aivong-openhands
aivong-openhands marked this pull request as ready for review September 10, 2026 15:42
aivong-openhands and others added 3 commits September 11, 2026 10:56
Port the org condenser reconcile tests off the bespoke Postgres harness
(a GitHub service container + POSTGRES_TEST_DATABASE_URL, with the schema
hand-built via Org table metadata) and onto the existing testcontainers
harness fixtures in tests/unit/conftest.py.

The async_session_maker fixture already provides a per-test database
cloned from the alembic-migrated template, so the tests now exercise the
real schema (including migration 108 server_default on org.agent_settings)
instead of a model-derived one. They run under the existing py-tests
workflow with no new CI, service container, or POSTGRES_TEST_DATABASE_URL
wiring.

Verified locally against testcontainers postgres:16: all 3 tests pass.

Follow-ups that need the workflow OAuth scope (this automation cannot push
workflow files): delete .github/workflows/postgres-integration-tests.yml,
and close PR #369 which only worked around the old harness collision.

Co-authored-by: openhands <openhands@all-hands.dev>
@aivong-openhands

Copy link
Copy Markdown
Contributor Author

✅ Live validation on a local KinD cluster

We stood up a full OpenHands Enterprise Helm install on a local KinD cluster and validated this PR end‑to‑end together with the matching chart change (OpenHands‑Cloud #1214).

Setup

  • Chart: OpenHands‑Cloud #1214 (orgDefaults.condenser.* values), local subcharts vendored + helm dependency build.
  • App image: ghcr.io/openhands/enterprise-server:sha-686d477 (this PR), multi‑arch, running natively.
  • Base domain oh.aivong.xin with a real wildcard TLS cert (Traefik default cert), GitHub App login, LiteLLM → Anthropic.

What we verified

  • Env contract matches — the chart renders OPENHANDS_ORG_DEFAULTS_CONDENSER_{MAX_TOKENS,APPLY_TO_EXISTING,OVERWRITE_EXISTING} exactly as this PR's get_org_defaults_condenser_config() reads them.
  • App tier starts cleanly with the condenser env set (the app pod reaches 1/1 Ready, no crashloop). Also confirmed the chart's int64 coercion renders maxTokens: 1e6 as "1000000" (not "1e+06"), which this PR's isdecimal() parse requires.
  • New‑org path — a real org created via GitHub login received condenser.condenser_kind=llm_summarizing, max_tokens=200000 (create_base_org_agent_settings() applied the deployment default).
  • Startup reconciliation of existing orgs — seeded pre‑feature org rows and confirmed all three flag combinations against live PostgreSQL:
    • applyToExisting=true, overwriteExisting=false → filled the org with no max_tokens, preserved an org already set to 100000, skipped a noop condenser (updated_count=1, skipped_condenser_variant_count=1).
    • overwriteExisting=true → overwrote the already‑set org too (updated_count=2).
    • applyToExisting=false → reconciliation early‑returned; existing orgs untouched, new‑org default still applied.
  • Config change on a running install — helm upgrade from maxTokens=200000 → 100000 with overwriteExisting=true moved the existing org's value 200000 → 100000, logged by org_condenser_defaults_reconciliation_succeeded.
  • End‑to‑end conversation ran successfully (agent + LLM + sandbox) against the deployed image.

The reconciliation advisory‑lock/retry path and the app‑server startup lifespan trigger both behaved as designed; no issues attributable to this PR were found. (Two environment‑only hiccups — a stale LiteLLM virtual‑key cache and KinD sandbox memory pressure — were unrelated to this change and resolved locally.)


This comment was created by an AI agent (OpenHands) on behalf of the user.

@dylan-openhands dylan-openhands 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.

Thanks for the live validation!

@aivong-openhands
aivong-openhands merged commit 4158ee3 into main Sep 11, 2026
16 checks passed
@aivong-openhands
aivong-openhands deleted the openhands-org-condenser-defaults-enterprise-20260909 branch September 11, 2026 19:46
@openhands-release-bot openhands-release-bot Bot added the released: 1.61.0 Shipped in 1.61.0 label Sep 11, 2026
@openhands-release-bot

Copy link
Copy Markdown
Contributor

🚀 Released in 1.61.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

released: 1.61.0 Shipped in 1.61.0 type: feat A new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants