refactor: set_config_val + parted_rooms unification (#205 target 6, net -10) by joelteply · Pull Request #210 · CambrianTech/airc

joelteply · 2026-04-28T14:53:11Z

Diff: +84 / -94 = -10 lines. airc 5359 → 5283.

5 new airc_core.config subcommands replace 5 inline-Python heredocs:

set / unset_keys — generic single + bulk write/delete
read_parted / record_parted / clear_parted — sticky-/part list ops (Persist subscribed rooms — /part should stick across reboots #136)

Sites consolidated:

joiner-mode init (cmd_connect ~2225): 12-line heredoc → 4 set_config_val calls
host-mode init (cmd_connect ~2491): 14-line heredoc → 3 set_config_val + 1 unset_config_keys
_read/record/clear_parted_room helpers: 3 × ~13 lines → 1 line each

Per Joel #205 + 'shell scripts are like classes': airc_core.config is the config-state class; bash callers dispatch to it. Class gains internal _load/_save helpers so each subcommand is 1-3 lines (heavily compacted to keep the PR net-negative).

…et -10) Diff stat: **+84 / -94 = -10 lines.** airc 5359 → 5283. Five new airc_core.config subcommands replace 5 inline-Python heredocs in airc bash: - `set --key K --value V` — generic single-key write - `unset_keys K1 K2 ...` — bulk-clear (used to nuke leftover host_*) - `read_parted` / `record_parted` / `clear_parted` — parted_rooms ops Bash side gains 3 one-line wrappers (`set_config_val`, `unset_config_keys`, plus the existing `_read/_record/_clear_parted_room` slimmed from 13-16 lines each to 1). Sites consolidated: - joiner-mode init (cmd_connect ~line 2225): 12-line heredoc → 4 set_config_val calls - host-mode init (cmd_connect ~line 2491): 14-line heredoc → 3 set_config_val + 1 unset_config_keys - _read_parted_rooms / _record_parted_room / _clear_parted_room: 3 × ~13 lines of inline Python → 1 line each Per Joel #205 + 'shell scripts are like classes': airc_core.config IS the config-state class; bash callers just dispatch to it. The class gains internal `_load`/`_save` helpers so each subcommand is 1-3 lines.

Copilot

Pull request overview

This PR refactors config.json mutation logic out of the monolithic airc bash script into lib/airc_core/config.py, adding new airc_core.config subcommands so bash callsites can dispatch to a single “config-state” CLI. It also unifies the parted_rooms persistence helpers (issue #136) and removes multiple inline Python heredocs in favor of those subcommands.

Changes:

Add new airc_core.config subcommands: set, unset_keys, read_parted, record_parted, clear_parted.
Replace inline Python heredocs in airc with set_config_val / unset_config_keys calls and thin parted_rooms wrappers that call the new subcommands.
Consolidate parted-room read/record/clear logic into Python for reuse and reduced bash duplication.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File	Description
`lib/airc_core/config.py`	Introduces internal load/save helpers and adds 5 new CLI subcommands for generic config mutation + parted-room persistence.
`airc`	Switches multiple config.json mutations (including parted-room helpers) from inline Python heredocs to the new `airc_core.config` subcommands.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-28T14:59:26Z

+    set_config_val name    "$name"
+    set_config_val host    "$(get_host)"
+    set_config_val created "$(timestamp)"
+    # Host mode: clear leftover host_* from any prior joiner run in
+    # this scope so we don't mis-read ourselves as a joiner.
+    unset_config_keys host_target host_name host_port host_airc_home host_ssh_pub host_identity



Similar to the joiner branch, host-mode init now does multiple set_config_val calls plus an unset_config_keys, which becomes multiple separate load/truncate/write operations. To avoid partial updates and reduce IO, consider batching these into a single config update operation (e.g., a set_many/mutate command that can both set and delete keys in one pass).

Suggested change

set_config_val name "$name"

set_config_val host "$(get_host)"

set_config_val created "$(timestamp)"

# Host mode: clear leftover host_* from any prior joiner run in

# this scope so we don't mis-read ourselves as a joiner.

unset_config_keys host_target host_name host_port host_airc_home host_ssh_pub host_identity

# Batch the host-mode config mutation so we only do one

# load/update/write cycle and avoid leaving partial state behind.

local config_host; config_host="$(get_host)"

local config_created; config_created="$(timestamp)"

CONFIG_NAME="$name" \

CONFIG_HOST="$config_host" \

CONFIG_CREATED="$config_created" \

"$AIRC_PYTHON" - "$CONFIG_JSON" <<'PY'

import json, os, sys, tempfile

path = sys.argv[1]

data = {}

if os.path.exists(path):

with open(path, "r", encoding="utf-8") as f:

raw = f.read().strip()

if raw:

data = json.loads(raw)

data["name"] = os.environ["CONFIG_NAME"]

data["host"] = os.environ["CONFIG_HOST"]

data["created"] = os.environ["CONFIG_CREATED"]

# Host mode: clear leftover host_* from any prior joiner run in

# this scope so we don't mis-read ourselves as a joiner.

for key in (

"host_target",

"host_name",

"host_port",

"host_airc_home",

"host_ssh_pub",

"host_identity",

):

data.pop(key, None)

directory = os.path.dirname(path) or "."

fd, tmp = tempfile.mkstemp(prefix=".config.", suffix=".json.tmp", dir=directory)

try:

with os.fdopen(fd, "w", encoding="utf-8") as f:

json.dump(data, f, indent=2, sort_keys=True)

f.write("\n")

os.replace(tmp, path)

except Exception:

try:

os.unlink(tmp)

except OSError:

pass

raise

PY

Copilot · 2026-04-28T14:59:26Z

+def _load(path):
+    try: return json.load(open(path))
+    except (OSError, ValueError): return {}
+
+
+def _save(path, c):
+    try: json.dump(c, open(path, "w"), indent=2); return 0
+    except OSError as e:
+        print(f"airc-config-set-error: {e}", file=sys.stderr); return 1


_load() returns {} on JSON decode errors (ValueError) the same as on missing files. That means set/unset_keys may silently treat a corrupted/truncated config as empty and then _save() overwrites the file, losing existing fields (e.g., identity). Consider distinguishing “file missing” from “invalid JSON”: only allow {} for ENOENT, but for decode errors print a clear airc-config-error: cannot read <path> and return non-zero without writing.

Copilot · 2026-04-28T14:59:27Z

+def cmd_record_parted(args) -> int:
+    c = _load(args.config); p = list(c.get("parted_rooms", []) or [])
+    if args.room not in p:
+        p.append(args.room); c["parted_rooms"] = p; return _save(args.config, c)
+    return 0
+
+
+def cmd_clear_parted(args) -> int:
+    c = _load(args.config); cur = c.get("parted_rooms", []) or []
+    new = [r for r in cur if r != args.room]
+    if new != cur:
+        c["parted_rooms"] = new; return _save(args.config, c)
+    return 0


cmd_record_parted/cmd_clear_parted currently rely on _load() which collapses JSON parse errors to {}; on a malformed config this will write back a new file containing only parted_rooms, wiping unrelated state. The previous bash implementation intentionally no-op’d on load/parse failure to avoid state loss; these commands should keep that behavior (exit 0 without writing) when the config can’t be decoded.

Copilot · 2026-04-28T14:59:27Z

+    set_config_val name        "$my_name"
+    set_config_val host        "$(get_host)"
+    set_config_val host_target "$ssh_target"
+    set_config_val created     "$(timestamp)"


This replaces a single read-modify-write of config.json with 4 separate set_config_val calls. Because airc runs with set -euo pipefail, any transient failure mid-sequence can leave config in a partially-updated state, and the extra open/truncate cycles increase the chance of a truncated JSON if interrupted. Consider adding a multi-key airc_core.config subcommand (or extending set to accept repeated key/value pairs) so these fields are written in one operation.

Copilot AI review requested due to automatic review settings April 28, 2026 14:53

joelteply mentioned this pull request Apr 28, 2026

[debug] Windows install.sh elevated payload — Mac↔Windows Claude scratchpad #196

Open

Copilot started reviewing on behalf of joelteply April 28, 2026 14:55 View session

joelteply merged commit 264fe06 into canary Apr 28, 2026
8 checks passed

joelteply deleted the refactor/set-config-val-unification-205-target6 branch April 28, 2026 14:55

Copilot AI reviewed Apr 28, 2026

View reviewed changes

joelteply mentioned this pull request Apr 28, 2026

[refactor] Split 5469-line airc bash script — Joel's call, my responsibility #205

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: set_config_val + parted_rooms unification (#205 target 6, net -10)#210

refactor: set_config_val + parted_rooms unification (#205 target 6, net -10)#210
joelteply merged 1 commit intocanaryfrom
refactor/set-config-val-unification-205-target6

joelteply commented Apr 28, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Copilot AI Apr 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    set_config_val name    "$name"
-    set_config_val host    "$(get_host)"
-    set_config_val created "$(timestamp)"
-    # Host mode: clear leftover host_* from any prior joiner run in
-    # this scope so we don't mis-read ourselves as a joiner.
-    unset_config_keys host_target host_name host_port host_airc_home host_ssh_pub host_identity
+    # Batch the host-mode config mutation so we only do one
+    # load/update/write cycle and avoid leaving partial state behind.
+    local config_host; config_host="$(get_host)"
+    local config_created; config_created="$(timestamp)"
+    CONFIG_NAME="$name" \
+    CONFIG_HOST="$config_host" \
+    CONFIG_CREATED="$config_created" \
+    "$AIRC_PYTHON" - "$CONFIG_JSON" <<'PY'
+import json, os, sys, tempfile
+path = sys.argv[1]
+data = {}
+if os.path.exists(path):
+    with open(path, "r", encoding="utf-8") as f:
+        raw = f.read().strip()
+        if raw:
+            data = json.loads(raw)
+data["name"] = os.environ["CONFIG_NAME"]
+data["host"] = os.environ["CONFIG_HOST"]
+data["created"] = os.environ["CONFIG_CREATED"]
+# Host mode: clear leftover host_* from any prior joiner run in
+# this scope so we don't mis-read ourselves as a joiner.
+for key in (
+    "host_target",
+    "host_name",
+    "host_port",
+    "host_airc_home",
+    "host_ssh_pub",
+    "host_identity",
+):
+    data.pop(key, None)
+directory = os.path.dirname(path) or "."
+fd, tmp = tempfile.mkstemp(prefix=".config.", suffix=".json.tmp", dir=directory)
+try:
+    with os.fdopen(fd, "w", encoding="utf-8") as f:
+        json.dump(data, f, indent=2, sort_keys=True)
+        f.write("\n")
+    os.replace(tmp, path)
+except Exception:
+    try:
+        os.unlink(tmp)
+    except OSError:
+        pass
+    raise
+PY

Conversation

joelteply commented Apr 28, 2026

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants