Skip to content

Claude step: Fix bug with SRT where the claude settings flag was passed to SRT#2063

Merged
flin-8 merged 2 commits into
mainfrom
frank/bugbash
Jul 7, 2026
Merged

Claude step: Fix bug with SRT where the claude settings flag was passed to SRT#2063
flin-8 merged 2 commits into
mainfrom
frank/bugbash

Conversation

@flin-8

@flin-8 flin-8 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

When running Claude inside SRT, the deployment fails because SRT is trying to read Claude's settings file. This PR calls Claude using the -c flag

CleanShot 2026-07-07 at 11 22 15@2x

Related to MD-2265: SRT sandbox errors without helpful message

@flin-8 flin-8 self-assigned this Jul 7, 2026
@flin-8 flin-8 changed the title Fix bug with SRT where the claude settings flag was passed to SRT Claude step: Fix bug with SRT where the claude settings flag was passed to SRT Jul 7, 2026
}

static string EscapeArg(string arg)
public static string EscapeArg(string arg)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extract to another class?

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.

Alternatively, we could escape internally and call something like var claudeArgs = argsBuilder.BuildEscaped();.

Doesn't over the binary path - but that is probably safer and/or can be escaped elsewhere?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the args builder should know how to escape the arguments (that's up to the caller).

I'd lean towards reusing this guy (in a later PR):

string Escape(string argValue, bool escapeArg)

@eddymoulton eddymoulton 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.

You could change the way you've done it (see the comment in the args builder) but this is good enough to go in.

arguments.Should().StartWith($"--settings {TestSandboxRuntimeSettingsPath} claude --model");
// The claude invocation is passed as a single escaped argument to srt's -c flag.
arguments.Should().StartWith($"--settings {TestSandboxRuntimeSettingsPath} -c \"claude --model");
arguments.Should().Contain("-p \\\"look! a \\\\\\\"quote\\\\\\\"\\\"");

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.

This is a hilarious amount of escapes 😂

It might be clearer if we write it as a raw string literal ("""), but it's not a big deal.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah raw looks better 😂

}

static string EscapeArg(string arg)
public static string EscapeArg(string arg)

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.

Alternatively, we could escape internally and call something like var claudeArgs = argsBuilder.BuildEscaped();.

Doesn't over the binary path - but that is probably safer and/or can be escaped elsewhere?

@flin-8 flin-8 merged commit 1cf5084 into main Jul 7, 2026
35 checks passed
@flin-8 flin-8 deleted the frank/bugbash branch July 7, 2026 02:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants