Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 6 additions & 25 deletions apps/mobile/src/features/threads/ComposerCommandPopover.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { isLiquidGlassSupported, LiquidGlassView } from "@callstack/liquid-glass";
import type { ComposerTriggerKind } from "@t3tools/shared/composerTrigger";
import type { ServerProviderSkill, ServerProviderSlashCommand } from "@t3tools/contracts";
import { SymbolView } from "../../components/AppSymbol";
import { memo } from "react";
import { Pressable, ScrollView, useColorScheme, View, type ViewStyle } from "react-native";

import { AppText as Text } from "../../components/AppText";
import { GlassSurface } from "../../components/GlassSurface";
import { PierreEntryIcon } from "../../components/PierreEntryIcon";
export type ComposerCommandItem =
| {
Expand Down Expand Up @@ -56,33 +56,14 @@ function PopoverSurface(props: {
...props.style,
};

if (isLiquidGlassSupported) {
return (
<LiquidGlassView
effect="clear"
interactive={false}
tintColor={props.isDarkMode ? "rgba(30,30,32,0.95)" : "rgba(255,255,255,0.92)"}
colorScheme={props.isDarkMode ? "dark" : "light"}
style={baseStyle}
>
{props.children}
</LiquidGlassView>
);
}

return (
<View
style={[
baseStyle,
{
backgroundColor: props.isDarkMode ? "rgba(44,44,46,0.96)" : "rgba(255,255,255,0.96)",
borderWidth: 1,
borderColor: props.isDarkMode ? "rgba(255,255,255,0.08)" : "rgba(0,0,0,0.06)",
},
]}
<GlassSurface
glassEffectStyle="clear"
tintColor={props.isDarkMode ? "rgba(30,30,32,0.95)" : "rgba(255,255,255,0.92)"}
style={baseStyle}
>
{props.children}
</View>
</GlassSurface>
);
}

Expand Down
3 changes: 3 additions & 0 deletions apps/server/src/auth/RpcAuthorization.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,13 @@ export const RPC_REQUIRED_SCOPES = {
[WS_METHODS.pullRequestsActivity]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsDiffFileContents]: AuthOrchestrationReadScope,
[WS_METHODS.pullRequestsRunAction]: AuthOrchestrationOperateScope,
[WS_METHODS.pullRequestsUpdate]: AuthOrchestrationOperateScope,
[WS_METHODS.pullRequestsComment]: AuthOrchestrationOperateScope,
[WS_METHODS.pullRequestsUpdateComment]: AuthOrchestrationOperateScope,
[WS_METHODS.pullRequestsSubmitReview]: AuthOrchestrationOperateScope,
[WS_METHODS.pullRequestsReplyToThread]: AuthOrchestrationOperateScope,
[WS_METHODS.pullRequestsSetThreadResolution]: AuthOrchestrationOperateScope,
[WS_METHODS.pullRequestsSetReaction]: AuthOrchestrationOperateScope,
// Read scope like the reads it un-caches: refreshing is part of reading, and a read-only
// client pressing refresh must not be told it may not look again.
[WS_METHODS.pullRequestsInvalidate]: AuthOrchestrationReadScope,
Expand Down
54 changes: 54 additions & 0 deletions apps/server/src/git/GitManager.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ import * as Duration from "effect/Duration";
import * as Effect from "effect/Effect";
import * as FileSystem from "effect/FileSystem";
import * as Layer from "effect/Layer";
import * as Logger from "effect/Logger";
import * as Option from "effect/Option";
import * as PlatformError from "effect/PlatformError";
import * as References from "effect/References";
import * as Scope from "effect/Scope";
import { ChildProcessSpawner } from "effect/unstable/process";
import { expect } from "vite-plus/test";
Expand Down Expand Up @@ -1658,6 +1660,58 @@ it.layer(GitManagerTestLayer)("GitManager", (it) => {
}),
);

it.effect("status logs actionable provider detail without exposing the upstream cause", () =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
yield* initRepo(repoDir);
yield* runGit(repoDir, ["checkout", "-b", "feature/status-rate-limited"]);
const remoteDir = yield* createBareRemote();
yield* runGit(repoDir, ["remote", "add", "origin", remoteDir]);
yield* runGit(repoDir, ["push", "-u", "origin", "feature/status-rate-limited"]);

const upstreamCause = "GraphQL rate limit for user ID 51714798 and token secret-value";
const { manager } = yield* makeManager({
ghScenario: {
failWith: new GitHubCli.GitHubCliRateLimitError({
command: "gh",
cwd: repoDir,
cause: new Error(upstreamCause),
}),
},
});
const logs: Array<{ message: string; annotations: Record<string, unknown> }> = [];
const logger = Logger.make<unknown, void>(({ fiber, message }) => {
logs.push({
message: String(message),
annotations: { ...fiber.getRef(References.CurrentLogAnnotations) },
});
});

const status = yield* manager
.status({ cwd: repoDir })
.pipe(Effect.provide(Logger.layer([logger], { mergeWithExisting: false })));

expect(status.pr).toBeNull();
const warning = logs.find((entry) => entry.message.includes("PR lookup failed"));
expect(warning?.annotations).toMatchObject({
operation: "lookupStatusPr",
branch: "feature/status-rate-limited",
errorTag: "SourceControlProviderError",
provider: "github",
providerOperation: "listChangeRequests",
providerCommand: "gh",
errorDetail:
"GitHub API rate limit exceeded. Run `gh api rate_limit` to inspect the quota and reset time.",
});
const loggedText = [
warning?.message ?? "",
...Object.values(warning?.annotations ?? {}).map(String),
].join("\n");
expect(loggedText).not.toContain(upstreamCause);
expect(loggedText).not.toContain("secret-value");
}),
);

it.effect("status keeps the last known PR when a later lookup fails", () =>
Effect.gen(function* () {
const repoDir = yield* makeTempDir("t3code-git-manager-");
Expand Down
11 changes: 11 additions & 0 deletions apps/server/src/git/GitManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import * as Option from "effect/Option";
import * as Order from "effect/Order";
import * as Path from "effect/Path";
import * as Ref from "effect/Ref";
import * as Schema from "effect/Schema";
import {
GitActionProgressEvent,
GitActionProgressPhase,
Expand All @@ -30,6 +31,7 @@ import {
VcsResolveBranchChangeRequestInput,
VcsResolveBranchChangeRequestResult,
ModelSelection,
SourceControlProviderError,
type SourceControlWritingStyleSettings,
} from "@t3tools/contracts";
import {
Expand Down Expand Up @@ -129,6 +131,7 @@ const PR_LOOKUP_CACHE_TTL = Duration.minutes(2);
const PR_LOOKUP_FAILURE_BASE_TTL = Duration.seconds(20);
const PR_LOOKUP_FAILURE_MAX_TTL = Duration.minutes(15);
const PR_LOOKUP_CACHE_CAPACITY = 2_048;
const isSourceControlProviderError = Schema.is(SourceControlProviderError);

/**
* How long a failed PR lookup is cached, given the number of consecutive
Expand Down Expand Up @@ -1121,6 +1124,14 @@ export const make = Effect.gen(function* () {
typeof error === "object" && error !== null && "_tag" in error
? String(error._tag)
: typeof error,
...(isSourceControlProviderError(error)
? {
provider: error.provider,
providerOperation: error.operation,
providerCommand: error.command ?? "unknown",
errorDetail: error.detail,
}
: {}),
}),
Effect.andThen(resolveBranchHeadContext(cwd, details)),
Effect.map((headContext) =>
Expand Down
106 changes: 106 additions & 0 deletions apps/server/src/pullRequest/AzureDevOpsPullRequestCli.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,40 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
}),
);

it.effect("stores the squash choice with an auto-completion, as a merge now does", () =>
Effect.gen(function* () {
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;

yield* cli.runPullRequestAction({
cwd: "/w",
number: 42,
action: "enable-auto-merge",
mergeMethod: "squash",
});

expect(argsOfCall(0)).toEqual([
"repos",
"pr",
"update",
"--detect",
"true",
"--id",
"42",
"--auto-complete",
"true",
"--squash",
"true",
"--only-show-errors",
"--output",
"json",
]);
}),
);

it.effect.each([
{ action: "enable-auto-merge", expected: ["--auto-complete", "true", "--squash", "false"] },
{ action: "disable-auto-merge", expected: ["--auto-complete", "false"] },
{ action: "draft", expected: ["--draft", "true"] },
{ action: "ready", expected: ["--draft", "false"] },
{ action: "close", expected: ["--status", "abandoned"] },
Expand Down Expand Up @@ -370,6 +403,79 @@ layer("AzureDevOpsPullRequestCli.layer", (it) => {
}),
);

it.effect.each([
{ name: "a title", rewrite: { title: "Add the page" }, expected: ["--title=Add the page"] },
{
name: "a description",
rewrite: { body: "Why the page changed" },
expected: ["--description=Why the page changed"],
},
{
name: "both",
rewrite: { title: "Add the page", body: "Why the page changed" },
expected: ["--title=Add the page", "--description=Why the page changed"],
},
] as const)("rewrites $name, sending nothing it was not given", ({ rewrite, expected }) =>
Effect.gen(function* () {
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;

yield* cli.updatePullRequest({ cwd: "/w", number: 42, ...rewrite });

expect(argsOfCall(0)).toEqual([
"repos",
"pr",
"update",
"--detect",
"true",
"--id",
"42",
...expected,
"--only-show-errors",
"--output",
"json",
]);
}),
);

it.effect("sends a description that starts with a dash as one value, not as a flag", () =>
Effect.gen(function* () {
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
const cli = yield* AzureDevOpsPullRequestCli.AzureDevOpsPullRequestCli;

yield* cli.updatePullRequest({
cwd: "/w",
number: 42,
body: "- rewrote the page\n- kept the rest",
});

// One argument, so the leading dash of an ordinary bullet list never reaches az as a flag,
// and the whole text stays together where `--description` would otherwise take several.
expect(argsOfCall(0)).toContain("--description=- rewrote the page\n- kept the rest");
}),
);

it.effect("rewrites through the provider, which says it takes one", () =>
Effect.gen(function* () {
mockedExecute.mockReturnValue(Effect.succeed(output("{}")));
const provider = yield* AzureDevOpsPullRequestProvider.make;

// False for a remark because nothing here can post one, so there is none to rewrite.
expect(provider.capabilities.edit).toEqual({ changeRequest: true, comment: false });
assert.isDefined(provider.updateChangeRequest);
yield* provider.updateChangeRequest({
cwd: "/w",
repository: "web",
host: "dev.azure.com",
number: 42,
title: "Add the page",
});

expect(argsOfCall(0)).toContain("--title=Add the page");
expect(argsOfCall(0)).not.toContain("--description");
}),
);

it.effect("reads the conversation through the REST API, pinned to a version", () =>
Effect.gen(function* () {
mockedExecute.mockReturnValueOnce(
Expand Down
40 changes: 40 additions & 0 deletions apps/server/src/pullRequest/AzureDevOpsPullRequestCli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,14 @@ export class AzureDevOpsPullRequestCli extends Context.Service<
readonly mergeMethod?: PullRequestMergeMethod;
}) => Effect.Effect<void, AzureDevOpsPullRequestCliError>;

/** Rewrites the pull request's own words, through the same command that moves it. */
readonly updatePullRequest: (input: {
readonly cwd: string;
readonly number: number;
readonly title?: string | undefined;
readonly body?: string | undefined;
}) => Effect.Effect<void, AzureDevOpsPullRequestCliError>;

/**
* Adds reviewers to a pull request, or takes them off it. `az repos pr reviewer` is the whole
* of what Azure offers here: it adds and removes named identities, and has no counterpart that
Expand Down Expand Up @@ -212,12 +220,21 @@ function actionArgs(
switch (action) {
case "merge":
return ["--status", "completed", "--squash", mergeMethod === "squash" ? "true" : "false"];
// Auto-complete is Azure's own name for it: the pull request stays active and Azure completes
// it once its policies pass. The squash choice is stored with it, as it is for a merge now.
case "enable-auto-merge":
return ["--auto-complete", "true", "--squash", mergeMethod === "squash" ? "true" : "false"];
case "disable-auto-merge":
return ["--auto-complete", "false"];
case "ready":
return ["--draft", "false"];
case "draft":
return ["--draft", "true"];
case "close":
return ["--status", "abandoned"];
// Never reached: this host does not declare the action, so nothing offers it.
case "update-branch":
return [];
case "reopen":
return ["--status", "active"];
}
Expand Down Expand Up @@ -481,6 +498,29 @@ export const make = Effect.gen(function* () {
],
})
.pipe(Effect.asVoid),

updatePullRequest: (input) =>
azure
.execute({
cwd: input.cwd,
args: [
"repos",
"pr",
"update",
...detectArgs,
"--id",
String(input.number),
// One argument rather than a flag and a value beside it: a description usually opens
// with a bullet, and az reads a dash in the next argv slot as a flag of its own.
// `--description` also takes several strings, and this keeps the whole text as one.
...(input.title === undefined ? [] : [`--title=${input.title}`]),
...(input.body === undefined ? [] : [`--description=${input.body}`]),
"--only-show-errors",
"--output",
"json",
],
})
.pipe(Effect.asVoid),
});
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,15 @@ describe("azure devops viewer permissions", () => {
// and an unknown permission is granted rather than guessed away. Azure refuses the ones it
// will not allow, at the moment they are taken, in words this could not have written.
expect(AZURE_DEVOPS_VIEWER_PERMISSIONS).toEqual({
actions: ["merge", "ready", "draft", "close", "reopen"],
actions: [
"merge",
"ready",
"draft",
"close",
"reopen",
"enable-auto-merge",
"disable-auto-merge",
],
// False because the host itself cannot post one, not because this viewer may not.
comment: false,
resolve: false,
Expand Down
Loading
Loading