scriptoriumadmin/fabro
1
0
Fork 0
mirror of https://github.com/fabro-sh/fabro.git synced 2026-09-11 22:53:00 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 3

refactor(install): tag GithubAppOwner with discriminated object shape

Browse source 
The install GitHub App manifest shape encoded owner as `"personal"` or
`"org:<slug>"` - a magic string parsed in install-app.tsx, built by
install-api.ts, and reparsed server-side. Replace with a tagged object
`{ kind: "personal" } | { kind: "org", slug }` in the OpenAPI spec, the
progenitor-generated Rust types, and the frontend.

Server-side, the internal `GitHubAppOwner` enum keeps its semantic
shape but gains a `TryFrom<GithubAppOwnerInput>` conversion and emits
the tagged JSON via `as_session_value`.

Frontend drops `buildGithubOwnerValue` in favor of
`buildInstallGithubAppOwner`, and the ready-screen renders the owner
through a small helper instead of string concatenation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Bryan Helmkamp 2026-04-19 13:48:31 -04:00
parent ad7fdc8d13
commit acb6b3f9d6
No known key found for this signature in database
6 changed files with 93 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
apps/fabro-web/app/install-api.test.ts
View file

@ -1,6 +1,6 @@
import { describe, expect, test } from "bun:test";
import { buildGithubOwnerValue, readInstallError } from "./install-api";
import { buildInstallGithubAppOwner, readInstallError } from "./install-api";
describe("readInstallError", () => {
test("prefers the structured install error payload", async () => {
@ -31,12 +31,15 @@ describe("readInstallError", () => {
});
});
describe("buildGithubOwnerValue", () => {
test("uses personal for personal app installs", () => {
expect(buildGithubOwnerValue("personal", "")).toBe("personal");
describe("buildInstallGithubAppOwner", () => {
test("uses the personal tag for personal app installs", () => {
expect(buildInstallGithubAppOwner("personal", "")).toEqual({ kind: "personal" });
});
test("formats organization owners with the expected prefix", () => {
expect(buildGithubOwnerValue("org", " acme ")).toBe("org:acme");
test("trims organization slugs and tags them with `org`", () => {
expect(buildInstallGithubAppOwner("org", " acme ")).toEqual({
kind: "org",
slug: "acme",
});
});
});

16
apps/fabro-web/app/install-api.ts
View file

@ -1,3 +1,7 @@
export type InstallGithubAppOwner =
| { kind: "personal" }
| { kind: "org"; slug: string };
export interface InstallSessionResponse {
completed_steps: string[];
llm:
@ -13,7 +17,7 @@ export interface InstallSessionResponse {
| {
strategy: string;
username?: string;
owner?: string;
owner?: InstallGithubAppOwner;
app_name?: string;
slug?: string;
allowed_username?: string;
@ -34,7 +38,7 @@ export interface InstallLlmProviderInput {
}
export interface InstallGithubAppManifestInput {
owner: string;
owner: InstallGithubAppOwner;
app_name: string;
allowed_username: string;
}
@ -92,13 +96,13 @@ export async function readInstallError(
return `${fallback} (${response.status})`;
}
export function buildGithubOwnerValue(
export function buildInstallGithubAppOwner(
ownerKind: "personal" | "org",
organizationSlug: string,
): string {
): InstallGithubAppOwner {
return ownerKind === "org"
? `org:${organizationSlug.trim()}`
: "personal";
? { kind: "org", slug: organizationSlug.trim() }
: { kind: "personal" };
}
export async function getInstallSession(token: string): Promise<InstallSessionResponse> {

20
apps/fabro-web/app/install-app.tsx
View file

@ -4,9 +4,10 @@ import { Link, Navigate, useLocation, useNavigate } from "react-router";
import {
type InstallFinishResponse,
type InstallGithubAppOwner,
type InstallLlmProviderInput,
type InstallSessionResponse,
buildGithubOwnerValue,
buildInstallGithubAppOwner,
createInstallGithubAppManifest,
finishInstall,
getInstallSession,
@ -116,10 +117,10 @@ export default function InstallApp() {
);
if (nextSession.github?.strategy === "app") {
setGithubStrategy("app");
const owner = nextSession.github.owner ?? "personal";
if (owner.startsWith("org:")) {
const owner = nextSession.github.owner ?? { kind: "personal" };
if (owner.kind === "org") {
setGithubOwnerKind("org");
setGithubOrganization(owner.slice(4));
setGithubOrganization(owner.slug);
} else {
setGithubOwnerKind("personal");
setGithubOrganization("");
@ -389,7 +390,7 @@ export default function InstallApp() {
}
const manifest = await createInstallGithubAppManifest(installToken, {
owner: buildGithubOwnerValue(githubOwnerKind, githubOrganization),
owner: buildInstallGithubAppOwner(githubOwnerKind, githubOrganization),
app_name: githubAppName.trim(),
allowed_username: githubAllowedUsername.trim(),
});
@ -970,7 +971,7 @@ function GithubAppDoneScreen({
</p>
</header>
<div className="grid gap-4 md:grid-cols-3">
<SummaryCard title="Owner" body={github.owner || "personal"} />
<SummaryCard title="Owner" body={describeGithubAppOwner(github.owner)} />
<SummaryCard title="App" body={github.slug || github.app_name || "GitHub App"} />
<SummaryCard
title="Allowed user"
@ -1071,6 +1072,13 @@ function describeGithubSummary(github: InstallSessionResponse["github"]): string
return github.username ? `Token for ${github.username}` : "Token configured";
}
function describeGithubAppOwner(
owner: InstallGithubAppOwner | undefined,
): string {
if (!owner || owner.kind === "personal") return "personal";
return owner.slug ? `org:${owner.slug}` : "org";
}
function submitGithubManifest(
formAction: string,
manifest: Record<string, unknown>,

19
docs/api-reference/fabro-api.yaml
View file

@ -2111,14 +2111,25 @@ components:
- allowed_username
properties:
owner:
type: string
description: >
Either `personal` or `org:<slug>`.
$ref: "#/components/schemas/InstallGithubAppOwner"
app_name:
type: string
allowed_username:
type: string
InstallGithubAppOwner:
description: Owner of the GitHub App being created during browser install.
type: object
required:
- kind
properties:
kind:
type: string
enum: [personal, org]
slug:
type: string
description: Required when `kind` is `org`; the organization slug.
InstallGithubAppManifestResponse:
description: Browser handoff payload for the GitHub App creation flow.
type: object
@ -2145,7 +2156,7 @@ components:
username:
type: string
owner:
type: string
$ref: "#/components/schemas/InstallGithubAppOwner"
app_name:
type: string
slug:

55
lib/crates/fabro-server/src/install.rs
View file

@ -227,11 +227,24 @@ struct GithubTokenTestInput {
#[derive(Clone, Debug, Deserialize)]
struct GithubAppManifestInput {
owner: String,
owner: GithubAppOwnerInput,
app_name: String,
allowed_username: String,
}
#[derive(Clone, Debug, Deserialize)]
struct GithubAppOwnerInput {
kind: GithubAppOwnerKind,
slug: Option<String>,
}
#[derive(Clone, Copy, Debug, Deserialize, PartialEq, Eq)]
#[serde(rename_all = "lowercase")]
enum GithubAppOwnerKind {
Personal,
Org,
}
#[derive(Clone, Debug, Deserialize)]
struct GithubAppRedirectQuery {
code: Option<String>,
@ -260,18 +273,6 @@ enum GitHubAppOwner {
}
impl GitHubAppOwner {
fn parse(raw: &str) -> anyhow::Result<Self> {
let value = raw.trim();
if value.eq_ignore_ascii_case("personal") {
return Ok(Self::Personal);
}
if let Some(org) = value.strip_prefix("org:") {
anyhow::ensure!(!org.trim().is_empty(), "organization owner cannot be empty");
return Ok(Self::Organization(org.trim().to_string()));
}
anyhow::bail!("owner must be 'personal' or 'org:<slug>'");
}
fn manifest_form_action(&self) -> String {
match self {
Self::Personal => "https://github.com/settings/apps/new".to_string(),
@ -281,10 +282,28 @@ impl GitHubAppOwner {
}
}
fn as_session_value(&self) -> String {
fn as_session_value(&self) -> serde_json::Value {
match self {
Self::Personal => "personal".to_string(),
Self::Organization(org) => format!("org:{org}"),
Self::Personal => serde_json::json!({ "kind": "personal" }),
Self::Organization(org) => serde_json::json!({ "kind": "org", "slug": org }),
}
}
}
impl TryFrom<GithubAppOwnerInput> for GitHubAppOwner {
type Error = String;
fn try_from(value: GithubAppOwnerInput) -> Result<Self, Self::Error> {
match value.kind {
GithubAppOwnerKind::Personal => Ok(Self::Personal),
GithubAppOwnerKind::Org => {
let slug = value.slug.unwrap_or_default();
let trimmed = slug.trim();
if trimmed.is_empty() {
return Err("organization owner requires a non-empty slug".to_string());
}
Ok(Self::Organization(trimmed.to_string()))
}
}
}
}
@ -605,10 +624,10 @@ async fn post_install_github_app_manifest(
}
observe_operator(&state, &headers);
let owner = match GitHubAppOwner::parse(&input.owner) {
let owner = match GitHubAppOwner::try_from(input.owner) {
Ok(owner) => owner,
Err(err) => {
return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, err.to_string());
return install_error_response(StatusCode::UNPROCESSABLE_ENTITY, err);
}
};
if input.app_name.trim().is_empty() {

14
lib/crates/fabro-server/tests/it/api/install.rs
View file

@ -197,7 +197,9 @@ async fn install_endpoints_reject_missing_and_wrong_tokens() {
(
"POST",
"/install/github/app/manifest",
Some(r#"{"owner":"personal","app_name":"Fabro","allowed_username":"octocat"}"#),
Some(
r#"{"owner":{"kind":"personal"},"app_name":"Fabro","allowed_username":"octocat"}"#,
),
),
("POST", "/install/finish", None),
];
@ -539,7 +541,7 @@ async fn github_app_manifest_round_trip_updates_install_session() {
.header("authorization", "Bearer test-install-token")
.header("content-type", "application/json")
.body(Body::from(
r#"{"owner":"personal","app_name":"Fabro Test","allowed_username":"octocat"}"#,
r#"{"owner":{"kind":"personal"},"app_name":"Fabro Test","allowed_username":"octocat"}"#,
))
.unwrap(),
)
@ -661,7 +663,7 @@ async fn github_app_manifest_rejects_retry_while_pending_and_preserves_prior_tok
.header("authorization", "Bearer test-install-token")
.header("content-type", "application/json")
.body(Body::from(
r#"{"owner":"personal","app_name":"Fabro Test","allowed_username":"octocat"}"#,
r#"{"owner":{"kind":"personal"},"app_name":"Fabro Test","allowed_username":"octocat"}"#,
))
.unwrap(),
)
@ -701,7 +703,7 @@ async fn github_app_manifest_rejects_retry_while_pending_and_preserves_prior_tok
.header("authorization", "Bearer test-install-token")
.header("content-type", "application/json")
.body(Body::from(
r#"{"owner":"personal","app_name":"Fabro Retry","allowed_username":"octocat"}"#,
r#"{"owner":{"kind":"personal"},"app_name":"Fabro Retry","allowed_username":"octocat"}"#,
))
.unwrap(),
)
@ -767,7 +769,7 @@ async fn github_app_redirect_rejects_invalid_or_missing_state_without_mutating_s
.header("authorization", "Bearer test-install-token")
.header("content-type", "application/json")
.body(Body::from(
r#"{"owner":"personal","app_name":"Fabro Test","allowed_username":"octocat"}"#,
r#"{"owner":{"kind":"personal"},"app_name":"Fabro Test","allowed_username":"octocat"}"#,
))
.unwrap(),
)
@ -907,7 +909,7 @@ async fn github_app_redirect_exchange_failure_returns_to_wizard_and_keeps_pendin
.header("authorization", "Bearer test-install-token")
.header("content-type", "application/json")
.body(Body::from(
r#"{"owner":"personal","app_name":"Fabro Test","allowed_username":"octocat"}"#,
r#"{"owner":{"kind":"personal"},"app_name":"Fabro Test","allowed_username":"octocat"}"#,
))
.unwrap(),
)