fix(login): preserve runpodctl credentials in ~/.runpod/config.toml by vaskoz · Pull Request #333 · runpod/flash

vaskoz · 2026-05-06T11:25:00Z

Summary

flash login was clobbering the top-level apikey/apiurl values that runpodctl writes to ~/.runpod/config.toml. The cause: save_api_key() delegated to runpod-python's set_credentials(overwrite=True), which opens the file in "w" mode and rewrites it with only a [default] section.
save_api_key() now does a line-based merge that updates only the [default].api_key field and preserves everything else — runpodctl's top-level keys, other profile sections, and any unrelated fields within [default].

Test plan

New unit tests cover: preserving runpodctl top-level keys, adding [default] when missing, preserving other profile sections, fresh-file creation
Existing test_credentials.py and test_login.py still pass (26/26)
Smoke-tested merger against real-world TOML shapes (file with both formats coexisting, only top-level keys, empty file, [default] without api_key, blank lines between sections)

🤖 Generated with Claude Code

flash login was clobbering the top-level apikey/apiurl values that runpodctl writes to ~/.runpod/config.toml, because it delegated to runpod-python's set_credentials() which opens the file in "w" mode. save_api_key() now does a line-based merge that updates only the [default].api_key field and preserves all other content. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR updates flash login credential persistence to avoid clobbering runpodctl’s top-level apikey/apiurl keys in ~/.runpod/config.toml by performing a targeted text merge that updates only [default].api_key.

Changes:

Replace runpod-python’s set_credentials(overwrite=True) usage with a line-based upsert that preserves unrelated TOML content.
Add unit tests covering mixed runpodctl + [default] formats, missing [default], preserving other profiles, and fresh-file creation.

Reviewed changes

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

File	Description
`src/runpod_flash/core/credentials.py`	Implements `_upsert_default_api_key()` and updates `save_api_key()` to preserve non-flash TOML content.
`tests/unit/test_credentials.py`	Adds unit tests to validate config preservation behavior when saving API keys.

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

+_DEFAULT_HEADER_RE = re.compile(r"^\s*\[default\]\s*$")
+_SECTION_HEADER_RE = re.compile(r"^\s*\[[^\]]+\]\s*$")


+    def test_preserves_runpodctl_top_level_keys(self, isolate_credentials_file):
+        isolate_credentials_file.parent.mkdir(parents=True, exist_ok=True)
+        isolate_credentials_file.write_text(
+            "apikey = 'rpa_runpodctl_key'\n"
+            "apiurl = 'https://api.runpod.io/graphql'\n"
+            "\n"
+            "[default]\n"
+            'api_key = "old-flash-key"\n'
+        )
+        save_api_key("new-flash-key")
+        text = isolate_credentials_file.read_text()
+        assert "apikey = 'rpa_runpodctl_key'" in text
+        assert "apiurl = 'https://api.runpod.io/graphql'" in text
+        parsed = tomllib.loads(text)
+        assert parsed["apikey"] == "rpa_runpodctl_key"
+        assert parsed["apiurl"] == "https://api.runpod.io/graphql"
+        assert parsed["default"]["api_key"] == "new-flash-key"
+
+    def test_adds_default_section_when_missing(self, isolate_credentials_file):
+        isolate_credentials_file.parent.mkdir(parents=True, exist_ok=True)
+        isolate_credentials_file.write_text(
+            "apikey = 'rpa_runpodctl_key'\napiurl = 'https://api.runpod.io/graphql'\n"
+        )
+        save_api_key("flash-key")
+        text = isolate_credentials_file.read_text()
+        parsed = tomllib.loads(text)
+        assert parsed["apikey"] == "rpa_runpodctl_key"
+        assert parsed["apiurl"] == "https://api.runpod.io/graphql"
+        assert parsed["default"]["api_key"] == "flash-key"


+    insert_idx = default_end
+    while insert_idx > default_start + 1 and lines[insert_idx - 1].strip() == "":
+        insert_idx -= 1
+    lines.insert(insert_idx, new_line + "\n")
+    return "".join(lines)


deanq

Inline review — see comments. Bottom line: the manual TOML merge has several edge cases (inline comments on [default], CRLF, untested escaping branches) that a tomlkit round-trip would eliminate. Falls back to specific edge-case requests if the manual approach stays.

deanq · 2026-05-12T16:09:43Z

+    return f'"{escaped}"'
+
+
+def _upsert_default_api_key(content: str, api_key: str) -> str:


Consider replacing the manual line-based merge with tomlkit. The goal here — preserve foreign top-level keys, other sections, and original formatting — is exactly what tomlkit provides via round-trip. It would collapse _upsert_default_api_key, _toml_quote, and the three regexes into ~5 lines and eliminate the inline-comment, CRLF, escaping, and section-ordering edge cases flagged below. If there's a deliberate reason to avoid the dependency (it's already pulled in transitively by runpod-python), worth calling out in the docstring.

deanq · 2026-05-12T16:09:44Z

+# that runpod-python uses for its `[default]` profile. We must preserve those
+# (and any other unrelated content) when updating flash's api_key, so flash
+# login does not clobber runpodctl's credentials.
+_DEFAULT_HEADER_RE = re.compile(r"^\s*\[default\]\s*$")


^\s*\[default\]\s*$ rejects valid TOML like [default] # flash profile. When that's present the function falls through to the "append new section" branch and writes a second [default] block, producing a file tomllib will refuse to load on the next flash login. Either tighten the regex to allow trailing comments or switch to a TOML parser.

deanq · 2026-05-12T16:09:44Z

    return path


+def _toml_quote(value: str) -> str:


_toml_quote handles \ and " escaping but no test exercises either branch. A regression that drops the backslash replacement would only surface on a pathological key. One round-trip test through tomllib with a key containing both characters would lock the contract.

deanq · 2026-05-12T16:09:44Z

+
+    for i in range(default_start + 1, default_end):
+        if _API_KEY_LINE_RE.match(lines[i]):
+            ending = "\n" if lines[i].endswith("\n") else ""


CRLF handling: splitlines(keepends=True) preserves \r\n, but this replacement uses ending = "\n" if lines[i].endswith("\n") else "", which strips the \r. A file edited on Windows ends up with mixed line endings after one flash login. Either preserve the original line ending or normalize on read.

deanq · 2026-05-12T16:09:44Z

+    new_line = f"api_key = {_toml_quote(api_key)}"
+
+    if not content:
+        return f"[default]\n{new_line}\n"


Nit: the literal "default" / "[default]" appears in the regex (line 27), this empty-file template, and the append template below. Per the project's "magic values as named constants" guideline, consider extracting _DEFAULT_SECTION = "default" and deriving the rest.

deanq · 2026-05-12T16:09:44Z

+    ):
+        save_api_key("first-key")
+        parsed = tomllib.loads(isolate_credentials_file.read_text())
+        assert parsed == {"default": {"api_key": "first-key"}}


Missing unit test: the PR description mentions smoke-testing [default] without an api_key field, but there's no unit test for it. This is the only path that exercises the insert_idx walk-back over trailing blank lines in _upsert_default_api_key. Suggested:

def test_inserts_api_key_into_default_without_existing_key(self, isolate_credentials_file): isolate_credentials_file.parent.mkdir(parents=True, exist_ok=True) isolate_credentials_file.write_text('[default]\nfoo = "bar"\n\n[other]\nx = 1\n') save_api_key("new-key") parsed = tomllib.loads(isolate_credentials_file.read_text()) assert parsed["default"]["api_key"] == "new-key" assert parsed["default"]["foo"] == "bar" assert parsed["other"]["x"] == 1

deanq · 2026-05-12T16:09:44Z

+        )
+        save_api_key("new-flash-key")
+        text = isolate_credentials_file.read_text()
+        assert "apikey = 'rpa_runpodctl_key'" in text


Nit: these substring assertions are brittle to whitespace changes and duplicate the contract already verified by the tomllib.loads block below. Dropping them keeps the test behavior-focused.

deanq requested a review from Copilot May 6, 2026 21:48

Copilot started reviewing on behalf of deanq May 6, 2026 21:48 View session

Copilot AI reviewed May 6, 2026

View reviewed changes

deanq reviewed May 12, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(login): preserve runpodctl credentials in ~/.runpod/config.toml#333

fix(login): preserve runpodctl credentials in ~/.runpod/config.toml#333
vaskoz wants to merge 1 commit into
mainfrom
fix/login-preserve-runpodctl-credentials

vaskoz commented May 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

deanq left a comment

Uh oh!

deanq May 12, 2026

Uh oh!

deanq May 12, 2026

Uh oh!

deanq May 12, 2026

Uh oh!

deanq May 12, 2026

Uh oh!

deanq May 12, 2026

Uh oh!

deanq May 12, 2026

Uh oh!

deanq May 12, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		_DEFAULT_HEADER_RE = re.compile(r"^\s\[default\]\s$")
		_SECTION_HEADER_RE = re.compile(r"^\s\[[^\]]+\]\s$")

		return f'"{escaped}"'


		def _upsert_default_api_key(content: str, api_key: str) -> str:

Conversation

vaskoz commented May 6, 2026

Summary

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

deanq left a comment

Choose a reason for hiding this comment

Uh oh!

deanq May 12, 2026

Choose a reason for hiding this comment

Uh oh!

deanq May 12, 2026

Choose a reason for hiding this comment

Uh oh!

deanq May 12, 2026

Choose a reason for hiding this comment

Uh oh!

deanq May 12, 2026

Choose a reason for hiding this comment

Uh oh!

deanq May 12, 2026

Choose a reason for hiding this comment

Uh oh!

deanq May 12, 2026

Choose a reason for hiding this comment

Uh oh!

deanq May 12, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants