Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
427e8c0
fix: read every own-key for a dependency's source, count exports, bou…
devin-ai-integration[bot] Sep 25, 2026
5054723
fix: unreproducible dependency_overrides skip verification project-wide
devin-ai-integration[bot] Sep 28, 2026
64b824d
fix: read inline dependency mappings and every URI a directive carries
devin-ai-integration[bot] Sep 28, 2026
c90038d
fix: let a semicolon inside a quoted URI pass the directive regex
devin-ai-integration[bot] Sep 28, 2026
5a3be97
fix: consume escape pairs so an escaped quote does not end a URI literal
devin-ai-integration[bot] Sep 28, 2026
8837514
fix: send sdk: overrides on their own channel and name the wire field…
devin-ai-integration[bot] Sep 28, 2026
5e4b970
fix: do not let the SDK-name filter hide a forked package from verifi…
devin-ai-integration[bot] Sep 28, 2026
7922515
fix: skip package-importing classes when any unrepresentable entry ca…
devin-ai-integration[bot] Sep 28, 2026
220f1c8
fix: reassemble flow mappings that wrap onto following lines
devin-ai-integration[bot] Sep 28, 2026
578b32c
fix: write sdk: flutter entries for undeclared SDK imports; skip comp…
devin-ai-integration[bot] Sep 28, 2026
f452125
fix: raw-string import URIs, raiseable {version:} pins, doc updates
devin-ai-integration[bot] Sep 28, 2026
cdf06ef
fix: avoid ReDoS in directive pattern - ride bare-char alternative fo…
devin-ai-integration[bot] Sep 28, 2026
b2670ba
fix: consume quoted literals inside conditions; rewrite whole quoted …
devin-ai-integration[bot] Sep 28, 2026
43a1a02
fix: anchor dependency entries at the name line, not the flow map's c…
devin-ai-integration[bot] Sep 28, 2026
31e2192
fix: scan fixedCode for dependency discovery, matching the deployed c…
devin-ai-integration[bot] Sep 28, 2026
fc4d4ff
fix: match quoted version keys in rewriteVersionMember; report a fail…
devin-ai-integration[bot] Sep 28, 2026
7699c31
fix: consume non-directive string literals so keywords inside them ca…
devin-ai-integration[bot] Sep 28, 2026
9aebdf4
fix: search for the version member after the flow map's opening brace
devin-ai-integration[bot] Sep 28, 2026
6ebe6e9
Merge remote-tracking branch 'origin/milestone/stu-implement-updated-…
devin-ai-integration[bot] Sep 28, 2026
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
26 changes: 17 additions & 9 deletions DEPLOYMENT.md
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,26 @@ reported rather than silently dropped:
package would resolve different code, and a check against the wrong version
reports a result that does not describe what ships.

The second case is decided **per class**, from that class's own imports. A
`git:` dependency stops only the classes that import it; classes that do not are
still compiled. Deciding it once for the whole project meant one such entry left
an entire deploy uncompiled.
The second case poisons the whole resolution graph, so it is decided **at the
graph level**. A git or path dependency's own pubspec still constrains every
package it shares with a class's imports - even ones the class never names -
and only the project's `pubspec.lock`, which is never uploaded, records which
versions actually resolved. A check that cannot reproduce the graph cannot
distinguish "clean" from "clean under different code", so any class importing
a `package:` URI is reported unverified. A class using only `dart:` imports
resolves nothing through pub and is still compiled, and a class naming the
unreproducible entry directly gets a reason that says so.
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved

**Which packages the Flutter SDK supplies is read from the project's own
pubspec** - a dependency written in block form under `sdk:` is an SDK package by
definition. Nothing keeps a list of SDK package names, because a list goes stale
the moment the SDK ships one it has not heard of, and a stale list reads a
perfectly valid package as unreproducible. That is not hypothetical: a list
missing `flutter_web_plugins` refused whole deploys for projects that declared
it.
definition. A small name list still exists, but only for names the pubspec does
not declare at all: a package the code imports and the project lacks is written
as `sdk: flutter` when the SDK supplies it, so discovery never emits a hosted
constraint pub.dev cannot resolve. A name the list has not heard of degrades to
the ordinary hosted lookup, not a wrong answer. For declared dependencies the
list plays no part - the pubspec's own keys are the authority, because a stale
list read a perfectly valid `flutter_web_plugins` declaration as
unreproducible and refused whole deploys over it.

The manifest is sent as package names and version constraints, never as
pubspec.yaml text, and the runner builds the document itself - it runs
Expand Down
6 changes: 5 additions & 1 deletion app.js
Original file line number Diff line number Diff line change
Expand Up @@ -3127,7 +3127,11 @@ async function resolveProjectPubspec(apiClient, newDependencies = {}) {
if (merged.added.length > 0) {
console.log(
"Adding dependencies:",
merged.added.map((name) => `${name}: ${plan.additions[name] || "any"}`).join(", "),
merged.added.map((name) => {
const addition = plan.additions[name];
const display = addition?.sdk ? `sdk: ${addition.sdk}` : (addition || "any");
return `${name}: ${display}`;
}).join(", "),
plan.sdk.dartSdkFloor ? `(resolved for Dart ${plan.sdk.dartSdkFloor})` : "",
);
}
Expand Down
35 changes: 29 additions & 6 deletions cloud-run/ffai-runner/bin/server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import 'dart:io';
const maxClassesPerRequest = 20;
const maxCodeBytes = 500000;
const maxDependenciesPerRequest = 200;
// pub.dev's own ceiling on a package name.
const maxPackageNameLength = 64;

const analysisPackageName = 'ccc_custom_code_analysis';
const defaultSdkConstraint = '>=3.0.0 <4.0.0';
Expand Down Expand Up @@ -524,7 +526,8 @@ _VerificationRequest? _normalizeVerification(Object? value) {
value['dependencyOverrides'],
'dependencyOverrides',
),
sdkPackages: _normalizeSdkPackages(value['sdkPackages']),
sdkPackages: _normalizeSdkPackages(value['sdkPackages'], 'sdkPackages'),
sdkOverrides: _normalizeSdkPackages(value['sdkOverrides'], 'sdkOverrides'),
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved
sources: sources,
);
}
Expand All @@ -551,7 +554,7 @@ Map<String, String> _normalizeDependencyMap(Object? value, String field) {
final result = <String, String>{};
for (final entry in value.entries) {
final name = '${entry.key}';
if (!_packageNamePattern.hasMatch(name)) {
if (!_isValidPackageName(name)) {
throw FormatException('Invalid package name in verification.$field: $name.');
}
final constraint = '${entry.value}'.trim();
Expand All @@ -572,10 +575,16 @@ Map<String, String> _normalizeDependencyMap(Object? value, String field) {
/// `pub get` with an honest error, whereas a list refuses correct deploys the
/// moment it falls behind the SDK. The pattern is the load-bearing check - each
/// name becomes a key in the generated pubspec.
List<String> _normalizeSdkPackages(Object? value) {
List<String> _normalizeSdkPackages(Object? value, String field) {
if (value == null) return const <String>[];
if (value is! List) {
throw const FormatException('verification.sdkPackages must be an array.');
throw FormatException('verification.$field must be an array.');
}
// Bounded like the dependency maps: each entry expands into the generated
// pubspec, so an unbounded array is caller-controlled work before pub get
// ever sees it.
if (value.length > maxDependenciesPerRequest) {
throw FormatException('Too many entries in verification.$field.');
}
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved
if (value.length > maxDependenciesPerRequest) {
throw const FormatException('Too many entries in verification.sdkPackages.');
Expand All @@ -584,14 +593,17 @@ List<String> _normalizeSdkPackages(Object? value) {
final result = <String>{};
for (final raw in value) {
final name = '$raw';
if (!_packageNamePattern.hasMatch(name)) {
if (!_isValidPackageName(name)) {
throw FormatException('Invalid SDK package name: $name.');
}
result.add(name);
}
return result.toList();
}

bool _isValidPackageName(String name) =>
name.length <= maxPackageNameLength && _packageNamePattern.hasMatch(name);

String _validateConstraint(
String field,
String value, {
Expand Down Expand Up @@ -983,6 +995,7 @@ final class _VerificationRequest {
required this.dependencies,
required this.overrides,
required this.sdkPackages,
required this.sdkOverrides,
required this.sources,
}) : unavailableReason = null;

Expand All @@ -992,6 +1005,7 @@ final class _VerificationRequest {
dependencies = const <String, String>{},
overrides = const <String, String>{},
sdkPackages = const <String>[],
sdkOverrides = const <String>[],
sources = const <_VerificationSource>[];

/// Why this request cannot be compiled, or null when it can.
Expand All @@ -1001,6 +1015,7 @@ final class _VerificationRequest {
final Map<String, String> dependencies;
final Map<String, String> overrides;
final List<String> sdkPackages;
final List<String> sdkOverrides;
final List<_VerificationSource> sources;

/// Builds the pubspec the scratch package is compiled from.
Expand Down Expand Up @@ -1037,13 +1052,21 @@ final class _VerificationRequest {
lines.add(' $name: ${_yamlScalar(dependencies[name]!)}');
}

if (overrides.isNotEmpty) {
// SDK-sourced overrides belong in dependency_overrides, not in
// `dependencies` with sdkPackages: an override can share its name with a
// direct dependency, and emitting both under one map writes the key twice.
if (overrides.isNotEmpty || sdkOverrides.isNotEmpty) {
lines
..add('')
..add('dependency_overrides:');
for (final name in overrides.keys.toList()..sort()) {
lines.add(' $name: ${_yamlScalar(overrides[name]!)}');
}
for (final name in sdkOverrides.toList()..sort()) {
lines
..add(' $name:')
..add(' sdk: flutter');
}
}

return '${lines.join('\n')}\n';
Expand Down
179 changes: 90 additions & 89 deletions dist/assets/index-Df7z0XS6.js → dist/assets/index-CCswOqT6.js

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion dist/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -3313,7 +3313,7 @@
.send.is-busy .send-spin { animation: none; opacity: 0.6; }
}
</style>
<script type="module" crossorigin src="/assets/index-Df7z0XS6.js"></script>
<script type="module" crossorigin src="/assets/index-CCswOqT6.js"></script>
<link rel="stylesheet" crossorigin href="/assets/index-DcLSblVK.css">
</head>
<body>
Expand Down
4 changes: 3 additions & 1 deletion src/bundleDeployPlanner.js
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ export function buildBundleDeployPlan(bundle, options = {}) {
// the guess this replaced (`^1.0.0`) pinned every package to its first major.
const detectedDependencies = {};
selected.forEach((artifact) => {
extractPackageImports(artifact.code || "").forEach((name) => {
// The same expression that fills the file entry above - an import added by
// a review fix must be discovered like any other.
extractPackageImports(artifact.fixedCode || artifact.code || "").forEach((name) => {
if (!(name in declaredDependencies)) {
detectedDependencies[name] = "";
}
Expand Down
20 changes: 20 additions & 0 deletions src/bundleDeployPlanner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,26 @@ test("prefers artifact.fixedCode over artifact.code for deploy content", () => {
assert.equal(plan.fileEntries[0].content, "class WidgetA extends StatelessWidget {}");
});

test("discovers dependencies from fixedCode, the content actually deployed", () => {
// A review fix can add an import the original code never had; scanning only
// `code` would deploy the import while omitting its package.
const plan = buildBundleDeployPlan({
artifacts: [
{
id: "widget-a",
artifactType: "CustomWidget",
artifactName: "WidgetA",
fileName: "widget_a.dart",
code: "class WidgetA extends StatelessWidget {}",
fixedCode:
"import 'package:http/http.dart';\nclass WidgetA extends StatelessWidget {}",
},
],
});

assert.equal(plan.dependencies.http, "");
});

test("falls back to artifact.code when fixedCode is absent", () => {
const plan = buildBundleDeployPlan({
artifacts: [
Expand Down
10 changes: 10 additions & 0 deletions src/cloudRunRunner.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,16 @@ test("Cloud Run builds the analysis manifest itself rather than running caller-s
/dependency_overrides:/,
"the project's overrides must be reproduced, or resolution differs",
);
assert.match(
runnerSource,
/value\['sdkOverrides'\]/,
"sdk-sourced overrides must ride their own channel - merged into sdkPackages they land under dependencies: and duplicate a same-named scalar key",
);
assert.match(
runnerSource,
/for \(final name in sdkOverrides[\s\S]*?sdk: flutter/,
"an sdk: override must be emitted under dependency_overrides, preserving override precedence",
);
// The previous wire format sent pubspec text. It must degrade to
// "unavailable" rather than being executed, so a cached client neither keeps
// the vulnerability open nor has its deploys rejected mid-rollout.
Expand Down
51 changes: 39 additions & 12 deletions src/customCodeVerification.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,29 +111,42 @@ function collect(declared, into, sdkPackages) {
* @returns {{
* sdkConstraint: string,
* dependencies: Object<string, string>,
* overrides: Object<string, string>,
* dependencyOverrides: Object<string, string>,
* sdkPackages: string[],
* sdkOverrides: string[],
* availablePackages: Set<string>,
* unrepresentable: string[],
* unrepresentableOverrides: string[],
* }}
*/
export function buildAnalysisManifest(projectPubspecYaml) {
const sdkPackages = new Set();
const sdkOverrides = new Set();
const dependencies = {};
const overrides = {};
const dependencyOverrides = {};

const unrepresentable = collect(
// The two blocks are kept apart: a direct dependency only reaches a class
// that names it, but a dependency_overrides entry rewrites resolution for
// every package in the graph that depends on the overridden name, so the
// consequences of dropping one are much wider than dropping the other. SDK
// entries are kept apart too: the runner emits sdkPackages under
// `dependencies:` and sdkOverrides under `dependency_overrides:`, and an
// `sdk:` override sharing a name with a scalar dependency must land in the
// second list or the generated pubspec declares the name twice.
const unrepresentableDependencies = collect(
parseExistingDependencies(projectPubspecYaml),
dependencies,
sdkPackages,
);
unrepresentable.push(
...collect(
parseDependencyBlock(projectPubspecYaml, "dependency_overrides"),
overrides,
sdkPackages,
),
const unrepresentableOverrides = collect(
parseDependencyBlock(projectPubspecYaml, "dependency_overrides"),
dependencyOverrides,
sdkOverrides,
);
Comment thread
devin-ai-integration[bot] marked this conversation as resolved.
Resolved
const unrepresentable = [
...unrepresentableDependencies,
...unrepresentableOverrides,
];

const availablePackages = new Set([
...Object.keys(dependencies),
Expand All @@ -145,10 +158,12 @@ export function buildAnalysisManifest(projectPubspecYaml) {
return {
sdkConstraint: unquoteConstraint(sdk) || FALLBACK_SDK_CONSTRAINT,
dependencies,
overrides,
dependencyOverrides,
sdkPackages: [...sdkPackages].sort(),
sdkOverrides: [...sdkOverrides].sort(),
availablePackages,
unrepresentable,
unrepresentableOverrides,
};
}

Expand Down Expand Up @@ -199,13 +214,20 @@ export function planCustomCodeVerification(classes, projectPubspecYaml) {
const {
sdkConstraint,
dependencies,
overrides,
dependencyOverrides,
sdkPackages,
sdkOverrides,
availablePackages,
unrepresentable,
} = buildAnalysisManifest(projectPubspecYaml);

const manifest = { sdkConstraint, dependencies, overrides, sdkPackages };
const manifest = {
sdkConstraint,
dependencies,
dependencyOverrides,
sdkPackages,
sdkOverrides,
};
const sources = [];
const skipped = [];

Expand All @@ -222,6 +244,11 @@ export function planCustomCodeVerification(classes, projectPubspecYaml) {
for (const entry of classes) {
const { className, content } = entry;

// SDK names must reach the unrepresentable check: a project can declare
// `flutter` or `flutter_localizations` from git or hosted, and a name
// filtered out here would slip past that check and reach the analyzer as
// a missing-URI error, refusing the deploy instead of reporting the class
// unverified.
const importedPackages = extractPackageImports(content);

// A direct import of a dependency the manifest cannot express is this
Expand Down
Loading
Loading