Skip to content

Commit ab70ad6

Browse files
tdsteinclaudenealrichardson
authored
fix: preserve title_is_default in deploy manifest/bundle redeploys (#836)
* fix: preserve title_is_default in deploy manifest/bundle redeploys `deploy manifest` and `deploy bundle` always pre-resolve `title` to a manifest/bundle-derived default before constructing RSConnectExecutor, which computes `title_is_default = not title`. As a result these two commands always passed `title_is_default=False`, even when the user never supplied `--title`, causing every redeploy to an existing --app-id to issue PATCH /v1/content/{guid} whenever the derived default title differed from the content's current title. Under Connect trusted publishing, that endpoint is forbidden outright, failing the whole deploy with a 403. Under an API key, the content was silently renamed to a junk derived title instead. RSConnectExecutor now accepts an explicit title_is_default override so callers that pre-resolve a default title can still report whether the user actually passed --title, while new-content creation keeps defaulting the title from the manifest/bundle as before. Fixes #835 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix: isolate the app store per test on Linux and Windows `TestMain` pointed `HOME` at `test-home` to keep the app store off the host, but `metadata.config_dirname()` only consults `HOME` on macOS: on Linux it prefers `XDG_CONFIG_HOME` and on Windows `APPDATA`. Both are set on GitHub runners, so the store escaped `test-home` and persisted across tests there. `test_deploy_manifest_new_content_uses_manifest_derived_title` deploys the same manifest as an earlier `test_deploy_draft` case, so it picked up that run's saved app ID, took the redeploy path instead of the new-content path, and skipped the title PATCH it asserts on -- failing on every Linux and Windows job while macOS passed. Redirect all three config-dir variables into `test-home`, and collapse the new-content test's inline endpoint registrations into the shared `_register_deploy_endpoints` helper. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Talk less --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: Neal Richardson <neal.p.richardson@gmail.com>
1 parent 97cd790 commit ab70ad6

4 files changed

Lines changed: 251 additions & 8 deletions

File tree

docs/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## Unreleased
99

10+
- `rsconnect deploy manifest` and `rsconnect deploy bundle`,
11+
when redeploying to an existing `--app-id`, only update the title
12+
when the user explicitly passes `--title`, matching the other deploy
13+
subcommands.
1014
- Added support for Python 3.14. The test suite now runs on Python 3.14 in CI.
1115
- `rsconnect deploy` subcommands now accept `--quiet`, which suppresses the
1216
step-by-step progress lines and the streamed server build log, printing only

rsconnect/api.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1267,6 +1267,7 @@ def __init__(
12671267
new: Optional[bool] = None,
12681268
app_id: Optional[str] = None,
12691269
title: Optional[str] = None,
1270+
title_is_default: Optional[bool] = None,
12701271
visibility: Optional[str] = None,
12711272
disable_env_management: Optional[bool] = None,
12721273
env_vars: Optional[dict[str, str]] = None,
@@ -1293,7 +1294,12 @@ def __init__(
12931294
self.app_store: AppStore = AppStore(fake_module_file_from_directory(self.path))
12941295
self.app_store_version: int | None = None
12951296
self.api_key_is_required: bool | None = None
1296-
self.title_is_default: bool = not title
1297+
# `deploy manifest` / `deploy bundle` pre-resolve `title` to a
1298+
# manifest/bundle-derived default rather than the generic
1299+
# `_default_title(self.path)` fallback above, so they pass
1300+
# `title_is_default` explicitly to report whether the user really
1301+
# supplied `--title`.
1302+
self.title_is_default: bool = not title if title_is_default is None else title_is_default
12971303
self.deployment_name: str | None = None
12981304

12991305
# Git deployment parameters

rsconnect/main.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1898,6 +1898,7 @@ def deploy_manifest(
18981898

18991899
file_name = validate_manifest_file(file)
19001900
app_mode = read_manifest_app_mode(file_name)
1901+
title_is_default = not title
19011902
title = title or default_title_from_manifest(file)
19021903

19031904
ce = RSConnectExecutor(
@@ -1915,6 +1916,7 @@ def deploy_manifest(
19151916
new=new,
19161917
app_id=app_id,
19171918
title=title,
1919+
title_is_default=title_is_default,
19181920
visibility=visibility,
19191921
env_vars=env_vars,
19201922
)
@@ -1994,6 +1996,7 @@ def deploy_bundle(
19941996
output_params(ctx, locals().items())
19951997

19961998
app_mode = read_bundle_app_mode(file)
1999+
title_is_default = not title
19972000
title = title or default_title_from_bundle(file)
19982001

19992002
ce = RSConnectExecutor(
@@ -2011,6 +2014,7 @@ def deploy_bundle(
20112014
new=new,
20122015
app_id=app_id,
20132016
title=title,
2017+
title_is_default=title_is_default,
20142018
visibility=visibility,
20152019
env_vars=env_vars,
20162020
)

tests/test_main.py

Lines changed: 236 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,20 +48,28 @@ def _load_json(data):
4848

4949

5050
class TestMain:
51+
# Every env var ``metadata.config_dirname()`` consults. ``HOME`` alone only
52+
# isolates macOS: Linux prefers ``XDG_CONFIG_HOME`` and Windows prefers
53+
# ``APPDATA``, so on those platforms the app store would escape ``test-home``
54+
# and leak saved deployment metadata (app IDs) between tests.
55+
_config_home_vars = ("HOME", "XDG_CONFIG_HOME", "APPDATA")
56+
5157
def setup_method(self):
5258
# Isolate from any real ``~/.rsconnect-python/`` on the host.
53-
# ``teardown_method`` restores ``HOME`` so the relative path does not
59+
# ``teardown_method`` restores these so the relative path does not
5460
# leak into later tests that invoke ``uv`` and would otherwise create
5561
# ``<cwd>/test-home/.cache/uv/`` inside their working directory.
56-
self._saved_home = os.environ.get("HOME")
62+
self._saved_config_home = {var: os.environ.get(var) for var in self._config_home_vars}
5763
shutil.rmtree("test-home", ignore_errors=True)
58-
os.environ["HOME"] = "test-home"
64+
for var in self._config_home_vars:
65+
os.environ[var] = "test-home"
5966

6067
def teardown_method(self):
61-
if self._saved_home is None:
62-
os.environ.pop("HOME", None)
63-
else:
64-
os.environ["HOME"] = self._saved_home
68+
for var, value in self._saved_config_home.items():
69+
if value is None:
70+
os.environ.pop(var, None)
71+
else:
72+
os.environ[var] = value
6573
shutil.rmtree("test-home", ignore_errors=True)
6674

6775
@pytest.fixture(autouse=True)
@@ -972,6 +980,227 @@ def post_application_deploy_callback(request, uri, response_headers):
972980
if original_server_value:
973981
os.environ["CONNECT_SERVER"] = original_server_value
974982

983+
def _register_deploy_endpoints(self, guid, existing_title, patch_calls, app_mode_ordinal=15, new_name=None):
984+
# Common Connect endpoints for a deploy targeting content ``guid``, whose
985+
# title Connect currently reports as ``existing_title``.
986+
# app_mode_ordinal defaults to AppModes.PYTHON_SHINY (15), matching
987+
# pyshiny_with_manifest; the bundle test overrides it to
988+
# AppModes.PYTHON_API (8) to match bundle.tar.gz's manifest.
989+
# Pass ``new_name`` to also register the two endpoints only brand new
990+
# content hits: the uniqueness check (GET /v1/content?name=...) and
991+
# creation (POST /v1/content).
992+
httpretty.register_uri(
993+
httpretty.GET,
994+
"http://fake_server/__api__/server_settings",
995+
body=json.dumps({"version": "9999.99.99"}),
996+
adding_headers={"Content-Type": "application/json"},
997+
status=200,
998+
)
999+
httpretty.register_uri(
1000+
httpretty.GET,
1001+
"http://fake_server/__api__/v1/user",
1002+
body=open("tests/testdata/connect-responses/me.json", "r").read(),
1003+
adding_headers={"Content-Type": "application/json"},
1004+
status=200,
1005+
)
1006+
content_body = json.dumps(
1007+
{
1008+
"id": "1234",
1009+
"guid": guid,
1010+
"title": existing_title,
1011+
"app_mode": app_mode_ordinal,
1012+
"content_url": f"http://fake_server/content/{guid}",
1013+
"dashboard_url": f"http://fake_server/connect/#/apps/{guid}",
1014+
}
1015+
)
1016+
httpretty.register_uri(
1017+
httpretty.GET,
1018+
f"http://fake_server/__api__/v1/content/{guid}",
1019+
body=content_body,
1020+
adding_headers={"Content-Type": "application/json"},
1021+
status=200,
1022+
)
1023+
if new_name:
1024+
httpretty.register_uri(
1025+
httpretty.GET,
1026+
f"http://fake_server/__api__/v1/content?name={new_name}",
1027+
body=json.dumps([]),
1028+
adding_headers={"Content-Type": "application/json"},
1029+
status=200,
1030+
)
1031+
# content_create only ever sends {"name": ...}, so the created
1032+
# content comes back carrying the server-assigned ``existing_title``
1033+
# rather than the caller's derived default.
1034+
httpretty.register_uri(
1035+
httpretty.POST,
1036+
"http://fake_server/__api__/v1/content",
1037+
body=content_body,
1038+
adding_headers={"Content-Type": "application/json"},
1039+
status=200,
1040+
)
1041+
1042+
def patch_callback(request, uri, response_headers):
1043+
patch_calls.append(_load_json(request.body))
1044+
return [200, {"Content-Type": "application/json"}, content_body]
1045+
1046+
httpretty.register_uri(
1047+
httpretty.PATCH,
1048+
f"http://fake_server/__api__/v1/content/{guid}",
1049+
body=patch_callback,
1050+
)
1051+
httpretty.register_uri(
1052+
httpretty.POST,
1053+
f"http://fake_server/__api__/v1/content/{guid}/bundles",
1054+
body=json.dumps({"id": "FAKE_BUNDLE_ID"}),
1055+
adding_headers={"Content-Type": "application/json"},
1056+
status=200,
1057+
)
1058+
1059+
deploy_api_invoked = []
1060+
1061+
def post_application_deploy_callback(request, uri, response_headers):
1062+
deploy_api_invoked.append(True)
1063+
return [201, {"Content-Type": "application/json"}, json.dumps({"task_id": "FAKE_TASK_ID"})]
1064+
1065+
httpretty.register_uri(
1066+
httpretty.POST,
1067+
f"http://fake_server/__api__/v1/content/{guid}/deploy",
1068+
body=post_application_deploy_callback,
1069+
)
1070+
httpretty.register_uri(
1071+
httpretty.GET,
1072+
"http://fake_server/__api__/v1/tasks/FAKE_TASK_ID?wait=1",
1073+
body=json.dumps({"output": ["FAKE_OUTPUT"], "last": "FAKE_LAST", "finished": True, "code": 0}),
1074+
adding_headers={"Content-Type": "application/json"},
1075+
status=200,
1076+
)
1077+
return deploy_api_invoked
1078+
1079+
@httpretty.activate(verbose=True, allow_net_connect=False)
1080+
def test_deploy_manifest_redeploy_preserves_title(self, caplog):
1081+
# Regression test for #835: redeploying existing content via
1082+
# `deploy manifest --app-id` without `--title` must not PATCH the
1083+
# content's title, even though `title` is pre-resolved to the
1084+
# manifest-derived default ("app5") before RSConnectExecutor is
1085+
# constructed. Under trusted publishing, PATCH /v1/content/{guid} is
1086+
# forbidden outright, so a spurious PATCH here would 403 the whole
1087+
# deploy; under an API key it would silently rename the content.
1088+
original_api_key_value = os.environ.pop("CONNECT_API_KEY", None)
1089+
original_server_value = os.environ.pop("CONNECT_SERVER", None)
1090+
guid = "1234-5678-9012-3456"
1091+
patch_calls = []
1092+
1093+
try:
1094+
deploy_api_invoked = self._register_deploy_endpoints(guid, "My Curated Title", patch_calls)
1095+
1096+
runner = CliRunner()
1097+
args = apply_common_args(
1098+
["deploy", "manifest", get_manifest_path("pyshiny_with_manifest", "")],
1099+
server="http://fake_server",
1100+
key="FAKE_API_KEY",
1101+
)
1102+
args += ["--app-id", guid, "--no-verify"]
1103+
with caplog.at_level("INFO"):
1104+
result = runner.invoke(cli, args)
1105+
assert result.exit_code == 0, result.output
1106+
assert deploy_api_invoked == [True]
1107+
assert patch_calls == []
1108+
finally:
1109+
if original_api_key_value:
1110+
os.environ["CONNECT_API_KEY"] = original_api_key_value
1111+
if original_server_value:
1112+
os.environ["CONNECT_SERVER"] = original_server_value
1113+
1114+
@httpretty.activate(verbose=True, allow_net_connect=False)
1115+
def test_deploy_manifest_redeploy_with_explicit_title_still_updates(self, caplog):
1116+
# When the user does pass --title, an existing app with a different
1117+
# title should still be updated (the pre-#835-fix behavior, preserved).
1118+
original_api_key_value = os.environ.pop("CONNECT_API_KEY", None)
1119+
original_server_value = os.environ.pop("CONNECT_SERVER", None)
1120+
guid = "1234-5678-9012-3456"
1121+
patch_calls = []
1122+
1123+
try:
1124+
deploy_api_invoked = self._register_deploy_endpoints(guid, "My Curated Title", patch_calls)
1125+
1126+
runner = CliRunner()
1127+
args = apply_common_args(
1128+
["deploy", "manifest", get_manifest_path("pyshiny_with_manifest", "")],
1129+
server="http://fake_server",
1130+
key="FAKE_API_KEY",
1131+
)
1132+
args += ["--app-id", guid, "--title", "Explicit New Title", "--no-verify"]
1133+
with caplog.at_level("INFO"):
1134+
result = runner.invoke(cli, args)
1135+
assert result.exit_code == 0, result.output
1136+
assert deploy_api_invoked == [True]
1137+
assert patch_calls == [{"title": "Explicit New Title"}]
1138+
finally:
1139+
if original_api_key_value:
1140+
os.environ["CONNECT_API_KEY"] = original_api_key_value
1141+
if original_server_value:
1142+
os.environ["CONNECT_SERVER"] = original_server_value
1143+
1144+
@httpretty.activate(verbose=True, allow_net_connect=False)
1145+
def test_deploy_bundle_redeploy_preserves_title(self, caplog):
1146+
# Same regression as test_deploy_manifest_redeploy_preserves_title, but
1147+
# for `deploy bundle --app-id`.
1148+
original_api_key_value = os.environ.pop("CONNECT_API_KEY", None)
1149+
original_server_value = os.environ.pop("CONNECT_SERVER", None)
1150+
guid = "1234-5678-9012-3456"
1151+
patch_calls = []
1152+
bundle_path = join("tests", "testdata", "bundle.tar.gz")
1153+
1154+
try:
1155+
deploy_api_invoked = self._register_deploy_endpoints(
1156+
guid, "My Curated Title", patch_calls, app_mode_ordinal=8
1157+
)
1158+
1159+
runner = CliRunner()
1160+
args = apply_common_args(["deploy", "bundle", bundle_path], server="http://fake_server", key="FAKE_API_KEY")
1161+
args += ["--app-id", guid, "--no-verify"]
1162+
with caplog.at_level("INFO"):
1163+
result = runner.invoke(cli, args)
1164+
assert result.exit_code == 0, result.output
1165+
assert deploy_api_invoked == [True]
1166+
assert patch_calls == []
1167+
finally:
1168+
if original_api_key_value:
1169+
os.environ["CONNECT_API_KEY"] = original_api_key_value
1170+
if original_server_value:
1171+
os.environ["CONNECT_SERVER"] = original_server_value
1172+
1173+
@httpretty.activate(verbose=True, allow_net_connect=False)
1174+
def test_deploy_manifest_new_content_uses_manifest_derived_title(self, caplog):
1175+
# New content (no --app-id) must still default its title from the
1176+
# manifest ("app5", derived from the entrypoint), not the executor's
1177+
# generic path-based fallback ("manifest").
1178+
original_api_key_value = os.environ.pop("CONNECT_API_KEY", None)
1179+
original_server_value = os.environ.pop("CONNECT_SERVER", None)
1180+
guid = "1234-5678-9012-3456"
1181+
patch_calls = []
1182+
1183+
try:
1184+
deploy_api_invoked = self._register_deploy_endpoints(guid, "Untitled", patch_calls, new_name="app5")
1185+
1186+
runner = CliRunner()
1187+
args = apply_common_args(
1188+
["deploy", "manifest", get_manifest_path("pyshiny_with_manifest", "")],
1189+
server="http://fake_server",
1190+
key="FAKE_API_KEY",
1191+
)
1192+
args.append("--no-verify")
1193+
with caplog.at_level("INFO"):
1194+
result = runner.invoke(cli, args)
1195+
assert result.exit_code == 0, result.output
1196+
assert deploy_api_invoked == [True]
1197+
assert patch_calls == [{"title": "app5"}]
1198+
finally:
1199+
if original_api_key_value:
1200+
os.environ["CONNECT_API_KEY"] = original_api_key_value
1201+
if original_server_value:
1202+
os.environ["CONNECT_SERVER"] = original_server_value
1203+
9751204
# noinspection SpellCheckingInspection
9761205
@pytest.mark.skip(reason="Skipping R manifest test (requires R 3.5, docker containers have moved on).")
9771206
def test_deploy_manifest(self):

0 commit comments

Comments
 (0)