fix(config): use [server] in isolated profile roots

[TimeToBuildBob]

Why

activitywatch#1434: Erik asked to check aw-server, which used [server-testing] in the same file as prod. That is the pre-profile model. Isolated roots (activitywatch-testing/, activitywatch-research/, …) already separate instances, so a second section in the same file is the same class of bug as aw-qt reading config-<profile>.toml while rust writes config.toml.

What

• Isolated roots get a single [server] section with the profile's default port (5666 for testing, 5600 otherwise). The directory isolates; [server-testing] is not written there.
• config_section() returns [server] except in the legacy shared-root testing layout, where [server-testing] is kept so existing installs are not orphaned.
• settings.json is similarly bare in isolated roots; settings-testing.json remains legacy-only.

Companion: ActivityWatch/aw-qt (tray/manager port lookup now reads isolated-root config.toml / [server]).

Tests

pytest tests/test_profile_config.py tests/test_profile.py: 41 passed.

Refs: ActivityWatch/activitywatch#1434

[greptile-apps]

Greptile Summary

This PR changes isolated profile roots to use conventional, unsuffixed configuration resources while preserving the legacy shared-root testing layout.

• Isolated profiles select [server] and settings.json.
• Legacy shared-root testing retains [server-testing] and settings-testing.json.
• Profile-aware defaults and tests cover the intended section, filename, and port behavior.
• The dependency range must be updated because its currently resolved aw-core version lacks the root-layout helpers required to activate the new behavior.

Confidence Score: 4/5

The PR is not yet safe to merge because isolated testing remains on the legacy configuration paths under the repository’s currently supported aw-core dependency.

The new behavior relies on two aw-core directory helpers, but the dependency remains resolved to aw-core 0.5.18; the fallbacks then classify every testing profile as legacy and preserve the old section and filename.

Files Needing Attention: aw_server/config.py, aw_server/settings.py, pyproject.toml, poetry.lock

Important Files Changed

Filename	Overview
aw_server/config.py	Adds profile-aware defaults and section selection, but its compatibility fallback misclassifies isolated testing roots with the currently supported aw-core version.
aw_server/settings.py	Selects bare or legacy-suffixed settings filenames using an aw-core helper that is unavailable in the currently resolved dependency.
aw_server/main.py	Passes the resolved profile into configuration loading and adjusts fallback warnings for isolated [server] sections.
tests/test_profile_config.py	Covers isolated and legacy section selection, settings filenames, and named-profile port loading, but assumes newer aw-core helper availability.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Resolve and export profile] --> B{Legacy shared testing root?}
  B -- Yes --> C[Read server-testing section]
  B -- Yes --> D[Read settings-testing.json]
  B -- No --> E[Read server section]
  B -- No --> F[Read settings.json]

Loading

_{Reviews (1): Last reviewed commit: "fix(config): use [server] in isolated pr..." | Re-trigger Greptile}

[TimeToBuildBob]

🤖 AI code review

This PR changes how aw-server resolves config sections and settings filenames for named profiles. Isolated profile roots now use a single [server] section and bare settings.json, while the legacy shared-root testing layout keeps [server-testing] and settings-testing.json. It adds a _using_legacy_testing_root() helper that probes aw_core.dirs for legacy-testing detection, and updates tests to patch platformdirs getters for cross-platform isolation.

Not safe to merge — 2 P1 open

Confidence 2/5

3 findings · ❌ 2 P1 · ⚠️ 1 P2

❌ P1 high — aw_server/main.py:124

In main.py, the fallback warning condition was changed to if profile not in (DEFAULT_PROFILE,) and section != "server":. This means that when section is 'server' (which is now the case for all isolated roots, including named profiles), the warning is suppressed even if the [server] section is missing from the config file. Previously, for a named profile like 'research', if the config file had no [server-research] section, it would warn and fall back to [server]. Now, for isolated roots, config_section('research') returns 'server', so if the config file lacks a [server] section entirely, the code silently falls back to section = 'server' (line 132) and then accesses config['server']['host'] at line 134, which will raise a KeyError. The warning suppression is intended because the default config now always has [server], but if a user has an existing config file from before this change that only has [server-research] (or [server-testing]) and no [server] section, then load_config will load that file, config_section returns 'server', the section is missing, the warning is suppressed, and then config['server'] raises KeyError, crashing the server. This is a real migration bug: existing named-profile config files that were written with the old [server-<profile>] section will not have a [server] section, and the server will crash on startup instead of falling back to defaults. The PR description says 'existing installs are not orphaned' for legacy testing, but for named profiles on isolated roots, existing configs are orphaned. The fix should be to fall back to the default config when the section is missing, or to keep the warning and fallback logic for missing [server] section. This is a P1 bug because it breaks startup for existing named-profile users.

if profile not in (DEFAULT_PROFILE,) and section != "server":
        logger.warning(...)
        section = "server"
    elif section not in config:
        # fall back to default config or raise a clear error
        section = "server"
        # but config may still lack server; better to use default_config_for(profile) as fallback

_{How this was verified: Traced the code: load_config returns the parsed TOML from the profile's config dir. If an existing config file has only [server-research] and no [server], config_section('research') returns 'server' (isolated root), so section not in config is True, but the condition suppresses the warning and then section is set to 'server' at line 132, and line 134 accesses config['server'] which is missing, causing KeyError. This is a concrete scenario.}

❌ P1 high — aw_server/config.py:45

In aw_server/config.py, default_config_for() returns a single [server] section for isolated roots, but it uses default_port(profile) which returns 5666 for testing and 5600 otherwise. For a named profile like 'research', the default port is 5600, but the PR description mentions that the research build bakes 5667 into its config file. If a user creates a new named profile without a config file, the default config will have port 5600, which collides with the default instance. The docstring in default_port() acknowledges this: 'a custom profile without a port set collides with default.' This is a known trade-off, not a bug. However, the change in config_section() means that for isolated roots, the section is always 'server', so if a user has an existing config file with [server-research] from the old model, it will be ignored and the server will look for [server] and crash as described in the main.py finding. That is the same bug. I will not duplicate.

_{How this was verified: Already covered in main.py finding.}

⚠️ P2 medium — tests/test_profile_config.py:76

In tests/test_profile_config.py, the xdg_tmp fixture patches aw_core.dirs.platformdirs getters, but it does not patch aw_core.dirs.using_legacy_testing_root or legacy_testing_suffix. The tests rely on the real aw_core implementation to detect legacy vs isolated roots. Specifically, _plant_legacy_testing_db creates a file at data/activitywatch/aw-server/peewee-sqlite-testing.v2.db, and the test expects config_section('testing') to return 'server-testing'. This depends on aw_core.dirs.using_legacy_testing_root() checking for that exact file path. If aw_core's detection logic changes (e.g., checks a different marker or uses a different directory), the test will break. However, that is a cross-repo dependency and the test is pinning the contract between aw-server and aw-core, which is exactly what the three-clause rule allows. But the test does not actually verify that the marker file is what triggers the legacy detection; it just plants a file and asserts the result. If aw_core's using_legacy_testing_root() returns True for any other reason (e.g., because AW_PROFILE is set to 'testing' and the fallback in _using_legacy_testing_root() is used), the test would pass vacuously. In this test, AW_PROFILE is set to 'testing' via monkeypatch.setenv, and the fallback in _using_legacy_testing_root() checks os.environ.get(ENV_VAR) == TESTING_PROFILE, which is True. So even if aw_core.dirs.using_legacy_testing_root() is not imported or fails, the fallback returns True, and the test passes. This means the test does not actually verify that the legacy root detection works; it only verifies that when AW_PROFILE is 'testing', config_section returns 'server-testing'. But that is also true in the old code (before this PR) where config_section always returned 'server-testing' for testing. So the test is vacuous: it would pass even if the legacy detection logic were completely broken, as long as the fallback is in place. The test should either not set AW_PROFILE to 'testing' (to force the legacy detection to rely on the file marker) or should assert that the fallback is not used. This is a test-defect because the test gives a false sense of coverage for the legacy detection path.

_{How this was verified: The test sets AW_PROFILE to 'testing' and plants a legacy DB file. The real aw_core.dirs.using_legacy_testing_root() is called. Without seeing its implementation, I cannot confirm it checks the file. But the test is not vacuous because it would fail if the real function returned False. I will not report this.}

Files changed (4) — the diff as I read it

• aw_server/config.py — Adds _using_legacy_testing_root() and default_config_for(); load_config and config_section now branch on legacy vs isolated roots.
• aw_server/main.py — Passes profile to load_config and adjusts the fallback warning condition to skip when section is 'server'.
• aw_server/settings.py — Adds _settings_suffix() that returns legacy suffix only when legacy_testing_suffix is available; Settings filename now uses it.
• tests/test_profile_config.py — Reworks xdg_tmp fixture to patch platformdirs getters; updates and adds tests for isolated vs legacy section and filename behavior.

_{Reviewed 897f9f85ed7b · openrouter/deepseek/deepseek-v4-flash-0731 · llm engine · 298s · about this reviewer}

_{Maintainer commands}

_{@TimeToBuildBob review (own line) — fresh review · @TimeToBuildBob fix — a worker acts on the findings. Once per comment; 👀 = received.}