diff --git a/backend/app/Exceptions/GitHubAuthenticationException.php b/backend/app/Exceptions/GitHubAuthenticationException.php new file mode 100644 index 0000000..ae71496 --- /dev/null +++ b/backend/app/Exceptions/GitHubAuthenticationException.php @@ -0,0 +1,20 @@ + '1', ]); - $files = collect($tree->json('tree', [])) + $treeJson = $tree->json(); + + if (! is_array($treeJson) || ! is_array($treeJson['tree'] ?? null)) { + throw GitHubMalformedResponseException::forRepository($repository, 'response body did not include a tree'); + } + + if ($treeJson['truncated'] ?? false) { + throw GitHubTreeTruncatedException::forRepository($repository); + } + + $files = collect($treeJson['tree']) ->filter(fn (array $node): bool => ($node['type'] ?? null) === 'blob') ->pluck('path') ->filter(fn (string $path): bool => Str::endsWith(Str::lower($path), '.md')) @@ -40,6 +57,14 @@ public function markdownFiles(Plugin $plugin, ?string $branch = null): array } /** + * Perform a GitHub API request and fail closed on every response that + * is not a trusted success. + * + * Every non-2xx/304 status, and every connection-level failure, is + * mapped to a stable {@see GitHubClientException} + * subtype so callers never mistake a GitHub-side failure for an empty + * or successful result. + * * @param array $query */ private function request(string $repository, string $uri, array $query = [], array $headers = []): Response @@ -51,19 +76,55 @@ private function request(string $repository, string $uri, array $query = [], arr 'X-GitHub-Api-Version' => '2022-11-28', ] + $headers)); - $response = $request->get($uri, $query); + try { + $response = $request->get($uri, $query); + } catch (ConnectionException) { + throw GitHubTransientErrorException::forRepository($repository, 'connection failure'); + } - if ($response->status() === 403) { - throw GitHubRateLimitException::forRepository($repository); + $status = $response->status(); + + if ($status === 304) { + return $response; } - if ($response->status() === 404) { + if ($status === 404) { throw GitHubRepositoryNotFoundException::forRepository($repository); } + if ($status === 429 || ($status === 403 && $this->isRateLimitSignal($response))) { + throw GitHubRateLimitException::forRepository($repository); + } + + if ($status === 401 || $status === 403) { + throw GitHubAuthenticationException::forRepository($repository, $status); + } + + if ($status === 409 || $status === 422) { + throw GitHubValidationException::forRepository($repository, $status); + } + + if ($status >= 500) { + throw GitHubTransientErrorException::forRepository($repository, "HTTP {$status}"); + } + + if (! $response->successful()) { + throw GitHubMalformedResponseException::forRepository($repository, "unexpected HTTP {$status}"); + } + return $response; } + /** + * GitHub signals primary rate limiting on a 403 response via the + * `X-RateLimit-Remaining` header. A 403 without that signal is an + * authentication/permission failure instead. + */ + private function isRateLimitSignal(Response $response): bool + { + return $response->header('X-RateLimit-Remaining') === '0'; + } + private function readMarkdownFile(string $repository, string $branch, string $path): GitHubMarkdownFile { $cacheKey = $this->etagCacheKey($repository, $branch, $path); @@ -90,12 +151,18 @@ private function readMarkdownFile(string $repository, string $branch, string $pa $encoding = $response->json('encoding'); if (! is_string($content) || $encoding !== 'base64') { - throw new RuntimeException("GitHub returned unsupported content encoding for [{$path}]."); + throw GitHubMalformedResponseException::forRepository($repository, "unsupported content encoding for [{$path}]"); + } + + $decoded = base64_decode(str_replace("\n", '', $content), true); + + if ($decoded === false) { + throw GitHubMalformedResponseException::forRepository($repository, "invalid base64 content for [{$path}]"); } return new GitHubMarkdownFile( path: $path, - content: base64_decode(str_replace("\n", '', $content), true) ?: '', + content: $decoded, etag: $nextEtag, ); } diff --git a/backend/app/Services/Ingestion/IngestionService.php b/backend/app/Services/Ingestion/IngestionService.php index e9129ac..9948e4d 100644 --- a/backend/app/Services/Ingestion/IngestionService.php +++ b/backend/app/Services/Ingestion/IngestionService.php @@ -7,8 +7,7 @@ use App\Data\ParsedDocument; use App\Events\CommandIndexUpdated; use App\Events\IngestionRunStatusChanged; -use App\Exceptions\GitHubRateLimitException; -use App\Exceptions\GitHubRepositoryNotFoundException; +use App\Exceptions\GitHubClientException; use App\Models\Category; use App\Models\Command; use App\Models\Document; @@ -84,10 +83,10 @@ public function run(PluginVersion $pluginVersion, ?IngestionRun $run = null): In try { $files = $this->github->markdownFiles($pluginVersion->plugin, $pluginVersion->git_ref ?: $pluginVersion->plugin->default_branch); - } catch (GitHubRateLimitException|GitHubRepositoryNotFoundException $exception) { + } catch (GitHubClientException $exception) { return $this->fail($run, $stats, [[ 'level' => 'error', - 'code' => Str::snake(class_basename($exception)), + 'code' => $exception->failureCode(), 'message' => $exception->getMessage(), ]]); } diff --git a/backend/tests/Feature/GitHubClientFailClosedTest.php b/backend/tests/Feature/GitHubClientFailClosedTest.php new file mode 100644 index 0000000..c0311a3 --- /dev/null +++ b/backend/tests/Feature/GitHubClientFailClosedTest.php @@ -0,0 +1,267 @@ + 'commandsphere/repro', + 'documentation_path' => 'docs', + 'default_branch' => 'main', + ]); +} + +it('fails closed with a stable, distinguishable code for 401', function (): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'Bad credentials'], 401)]); + + expect(fn () => (new HttpGitHubClient)->markdownFiles(githubReproPlugin())) + ->toThrow(GitHubAuthenticationException::class); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + } catch (GitHubClientException $exception) { + expect($exception->failureCode())->toBe('github_authentication_failed') + ->and($exception->getMessage())->not->toContain('Bearer') + ->and($exception->getMessage())->not->toContain('Authorization'); + } +}); + +it('treats 403 without a rate-limit signal as an authentication/permission failure', function (): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'Resource not accessible by integration'], 403)]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubAuthenticationException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubAuthenticationException::class) + ->and($exception->failureCode())->toBe('github_authentication_failed'); + } +}); + +it('treats 403 with a rate-limit signal as rate limiting', function (): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'Forbidden'], 403, ['X-RateLimit-Remaining' => '0'])]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubRateLimitException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubRateLimitException::class) + ->and($exception->failureCode())->toBe('git_hub_rate_limit_exception'); + } +}); + +it('treats 429 as rate limiting', function (): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'Too many requests'], 429)]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubRateLimitException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubRateLimitException::class) + ->and($exception->failureCode())->toBe('git_hub_rate_limit_exception'); + } +}); + +it('keeps repository/ref-not-found on 404 unchanged', function (): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'Not Found'], 404)]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubRepositoryNotFoundException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubRepositoryNotFoundException::class) + ->and($exception->failureCode())->toBe('git_hub_repository_not_found_exception'); + } +}); + +it('fails closed on 409 and 422 as validation failures', function (int $status): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'invalid'], $status)]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubValidationException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubValidationException::class) + ->and($exception->failureCode())->toBe('github_validation_failed'); + } +})->with([409, 422]); + +it('fails closed on 5xx responses as transient errors', function (int $status): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'server error'], $status)]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubTransientErrorException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubTransientErrorException::class) + ->and($exception->failureCode())->toBe('github_transient_error'); + } +})->with([500, 503]); + +it('fails closed on a connection failure as a transient error', function (): void { + Http::fake(['api.github.com/*' => fn () => throw new ConnectionException('Connection timed out')]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubTransientErrorException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubTransientErrorException::class) + ->and($exception->failureCode())->toBe('github_transient_error'); + } +}); + +it('fails closed on a 200 response with a non-JSON body', function (): void { + Http::fake(['api.github.com/*' => Http::response('not json at all', 200, ['Content-Type' => 'text/plain'])]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubMalformedResponseException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubMalformedResponseException::class) + ->and($exception->failureCode())->toBe('github_malformed_response'); + } +}); + +it('fails closed on a 200 response missing the tree key', function (): void { + Http::fake(['api.github.com/*' => Http::response(['sha' => 'abc'], 200)]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubMalformedResponseException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubMalformedResponseException::class) + ->and($exception->failureCode())->toBe('github_malformed_response'); + } +}); + +it('fails closed with a dedicated code when the tree is truncated', function (): void { + Http::fake(['api.github.com/*' => Http::response([ + 'tree' => [['path' => 'docs/a.md', 'type' => 'blob']], + 'truncated' => true, + ], 200)]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubTreeTruncatedException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubTreeTruncatedException::class) + ->and($exception->failureCode())->toBe('github_tree_truncated'); + } +}); + +it('fails closed on invalid base64 file content', function (): void { + Http::fake([ + 'api.github.com/repos/*/git/trees/*' => Http::response([ + 'tree' => [['path' => 'docs/a.md', 'type' => 'blob']], + 'truncated' => false, + ], 200), + 'api.github.com/repos/*/contents/*' => Http::response([ + 'content' => '###not-valid-base64###', + 'encoding' => 'base64', + ], 200), + ]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubMalformedResponseException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubMalformedResponseException::class) + ->and($exception->failureCode())->toBe('github_malformed_response'); + } +}); + +it('fails closed on an unsupported content encoding', function (): void { + Http::fake([ + 'api.github.com/repos/*/git/trees/*' => Http::response([ + 'tree' => [['path' => 'docs/a.md', 'type' => 'blob']], + 'truncated' => false, + ], 200), + 'api.github.com/repos/*/contents/*' => Http::response([ + 'content' => 'aGVsbG8=', + 'encoding' => 'utf-8', + ], 200), + ]); + + try { + (new HttpGitHubClient)->markdownFiles(githubReproPlugin()); + expect(false)->toBeTrue('Expected GitHubMalformedResponseException to be thrown.'); + } catch (GitHubClientException $exception) { + expect($exception)->toBeInstanceOf(GitHubMalformedResponseException::class) + ->and($exception->failureCode())->toBe('github_malformed_response'); + } +}); + +it('still honors a 304 response as unchanged content and keeps the cached ETag', function (): void { + Http::fake([ + 'api.github.com/repos/*/git/trees/*' => Http::sequence() + ->push(['tree' => [['path' => 'docs/a.md', 'type' => 'blob']], 'truncated' => false], 200) + ->push(['tree' => [['path' => 'docs/a.md', 'type' => 'blob']], 'truncated' => false], 200), + 'api.github.com/repos/*/contents/*' => Http::sequence() + ->push(['content' => base64_encode('hello'), 'encoding' => 'base64'], 200, ['ETag' => '"v1"']) + ->push(null, 304), + ]); + + $client = new HttpGitHubClient; + $plugin = githubReproPlugin(); + + $first = $client->markdownFiles($plugin); + expect($first)->toHaveCount(1) + ->and($first[0]->content)->toBe('hello') + ->and($first[0]->notModified)->toBeFalse(); + + $second = $client->markdownFiles($plugin); + expect($second)->toHaveCount(1) + ->and($second[0]->notModified)->toBeTrue() + ->and($second[0]->content)->toBeNull(); +}); + +it('ends the ingestion run as failed with the transient-error code instead of leaving it stuck running', function (): void { + Http::fake(['api.github.com/*' => Http::response(['message' => 'server error'], 503)]); + + $community = Community::factory()->create(); + $plugin = Plugin::factory()->create([ + 'community_id' => $community->id, + 'repository_url' => 'commandsphere/repro', + 'documentation_path' => 'docs', + 'default_branch' => 'main', + ]); + $pluginVersion = PluginVersion::factory()->latest()->create([ + 'plugin_id' => $plugin->id, + 'version' => '1.0.0', + 'git_ref' => 'main', + ]); + + app()->bind(GitHubClient::class, fn (): HttpGitHubClient => new HttpGitHubClient); + + Event::fake([CommandIndexUpdated::class, IngestionRunStatusChanged::class]); + config(['queue.default' => 'sync', 'scout.driver' => 'null']); + + $run = app(IngestionService::class)->run($pluginVersion); + + expect($run->status)->toBe('failed') + ->and($run->finished_at)->not->toBeNull() + ->and($run->log[0]['code'])->toBe('github_transient_error') + ->and(collect($run->log)->pluck('message')->implode(' ')) + ->not->toContain('Bearer'); +}); diff --git a/brain/audits/2026-06-13-system-audit.md b/brain/audits/2026-06-13-system-audit.md index 76d85d1..fab39ca 100644 --- a/brain/audits/2026-06-13-system-audit.md +++ b/brain/audits/2026-06-13-system-audit.md @@ -14,6 +14,8 @@ Remediation update: F-004 and F-007 were addressed in branch `fix/ingestion-run- Remediation update: F-005 was addressed in branch `fix/analytics-bounds`. The analytics endpoint now validates `days`, caps the period with `COMMANDSPHERE_ANALYTICS_MAX_DAYS`, and returns the accepted period in metadata. +Remediation update: F-006 was addressed in branch `fix/github-client-fail-closed`. The GitHub client now fails closed on every non-success status and every malformed payload, through the `GitHubClientException` hierarchy whose `failureCode()` persists a stable code in `IngestionRun.log`. `IngestionService::run()` catches the whole hierarchy, so no GitHub failure escapes the run and leaves it stuck in `running`. Taxonomy and scope trade-offs are recorded in ADR-27. + ## Executive Summary CommandSphere already has a stronger baseline than a typical portfolio project: scoped permissions, HMAC webhooks, private realtime channels, Markdown sanitization in the Angular viewer, rate limits on key public/operational endpoints, idempotent ingestion tests, and documented architecture decisions. @@ -170,6 +172,8 @@ Fix with the next analytics iteration. Severity: Medium +Status: Mitigated in `fix/github-client-fail-closed`. + Evidence: - `backend/app/Services/GitHub/HttpGitHubClient.php:45` diff --git a/brain/canonico/CURRENT_STATE.md b/brain/canonico/CURRENT_STATE.md index 2999dd8..c0adc13 100644 --- a/brain/canonico/CURRENT_STATE.md +++ b/brain/canonico/CURRENT_STATE.md @@ -15,6 +15,7 @@ The product is positioned as a documentation discovery platform for plugin ecosy - Community-scoped permissions using Spatie teams with `community_id`. - Plugin and plugin version domain model with documents, commands, categories, favorites, views, and ingestion runs. - GitHub Markdown ingestion with ETag support, idempotent upserts, command extraction, stale command reconciliation, warnings, and partial run handling. +- GitHub client fails closed: every non-success status and every malformed payload raises a `GitHubClientException` subtype, and `IngestionService::run()` ends the run as `failed` with a stable `failureCode()` in `IngestionRun.log`. A run can no longer finish `success` with zero documents because of an API error. - Search through Laravel Scout and Meilisearch with facets for community, plugin, and category. - Redis-backed discovery cache invalidated by a global version key after ingestion. - Favorites and command-view analytics with short-window dedupe. @@ -172,3 +173,15 @@ Latest analytics hardening phase: - `/api/v1/analytics/most-viewed` now validates `days` as integer `1..COMMANDSPHERE_ANALYTICS_MAX_DAYS`. - Analytics response metadata now includes `days` and `max_days`. - Local tests isolate command-view dedupe from Meilisearch when search indexing is not the behavior under test. + +Active GitHub client remediation branch: `fix/github-client-fail-closed`. + +Latest ingestion resilience phase: + +- F-006 GitHub client fail-closed taxonomy. +- `App\Exceptions\GitHubClientException` is the abstract base for every GitHub failure, exposing `failureCode()`. +- Categories and codes: `git_hub_rate_limit_exception`, `github_authentication_failed`, `git_hub_repository_not_found_exception`, `github_validation_failed`, `github_transient_error`, `github_malformed_response`, `github_tree_truncated`. +- A 403 is classified by the rate-limit header, not by status alone, so a permission denial is no longer reported as rate limiting. +- Transient failures match `>= 500` plus connection errors, not an enumerated status list. +- A truncated tree and an unreadable file both end the run as `failed`; neither degrades to `partial`. +- `IngestionService::fail()` signature and `IngestionRun.log` entry shape are unchanged, so downstream consumers read the taxonomy through the existing `code` key. diff --git a/brain/canonico/DECISIONS.md b/brain/canonico/DECISIONS.md index e3a36ee..74ee381 100644 --- a/brain/canonico/DECISIONS.md +++ b/brain/canonico/DECISIONS.md @@ -61,3 +61,16 @@ Consequences: - `critical` blocks the pipeline; `high` and `moderate` without a non-major fix are recorded as risk under ADR-24 and never reported as corrected. - An override that leaves the range declared by its dependent needs a recorded ADR stating the verified facts that justify it, as in ADR-26. - Toolchain major upgrades stay a deliberate, separately planned task. + +## BRAIN-007 - External Integrations Fail Closed With A Stable Failure Code + +Decision: A call to an external service either produces the data it promised or ends the operation explicitly. An unexpected status, an unparseable body, a payload missing a required field, and a response flagged as incomplete are all failures, never an empty successful result. Every failure category carries a stable code persisted in the operation record, under the `code` key that consumers already read. + +Consequences: + +- A run that ingested nothing must be distinguishable from a source that legitimately has nothing. `success` with zero documents is a defect, not a state. +- An exception type raised by an integration must be caught at the boundary that owns the operation record. An exception escaping that boundary leaves the record stuck in `running` with no `finished_at`, which is worse than a wrong status because no operator sees it end. +- A failure code, once asserted by a test, is a contract. It is not renamed for aesthetics; new categories get clean literals while legacy ones keep their shape, as in ADR-27. +- Transient categories are matched by range (`>= 500`), not by an enumerated list, so unseen statuses stay covered. +- Failure messages carry identifiers and status only. Tokens, authorization headers, and response bodies never reach a log or an exception message. +- Failing closed raises the cost of a transient outage. Retry belongs to the operator-facing surface, deliberately scoped, never smuggled into the client as silent recovery. diff --git a/brain/canonico/NEXT_ACTIONS.md b/brain/canonico/NEXT_ACTIONS.md index 7e5ce4f..96532aa 100644 --- a/brain/canonico/NEXT_ACTIONS.md +++ b/brain/canonico/NEXT_ACTIONS.md @@ -272,7 +272,6 @@ Risks: - Add Content Security Policy on Laravel and SSR Node responses. - Sanitize Markdown HTML server-side before storage or response. -- Harden GitHub client HTTP error taxonomy beyond 403/404. - Extract optional bearer-token user resolution shared by public discovery controllers. - Keyboard-first power-user UX for command palette actions. - Saved searches and team-level curated collections. diff --git a/brain/handoffs/2026-09-20-github-client-fail-closed.md b/brain/handoffs/2026-09-20-github-client-fail-closed.md new file mode 100644 index 0000000..7817225 --- /dev/null +++ b/brain/handoffs/2026-09-20-github-client-fail-closed.md @@ -0,0 +1,40 @@ +# Handoff - GitHub Client Fail-Closed + +Date: 2026-09-20 +Branch: `fix/github-client-fail-closed` + +## Scope + +Audited remediation item C2: close F-006. The GitHub client only mapped 403 and 404, so every other failure reached the pipeline as an ordinary response and produced a misleading outcome. + +## Implemented + +- `App\Exceptions\GitHubClientException`, abstract, exposing `failureCode()`. Six concrete subtypes plus the two pre-existing exceptions reparented onto it. +- `HttpGitHubClient` fails closed on every non-success status and validates the payload: non-JSON body, body without `tree`, `truncated: true`, invalid base64, and unsupported encoding all raise instead of degrading to an empty or partial result. +- `IngestionService::run()` catches the whole `GitHubClientException` hierarchy. This closed a second fail-open path that F-006 did not describe: any other exception previously escaped `run()` and left the `IngestionRun` stuck in `running`, with no `finished_at` and no log. +- Codes: `git_hub_rate_limit_exception`, `github_authentication_failed`, `git_hub_repository_not_found_exception`, `github_validation_failed`, `github_transient_error`, `github_malformed_response`, `github_tree_truncated`. Taxonomy and trade-offs in ADR-27. +- A 403 is classified by the rate-limit header; a permission denial no longer reports as rate limiting. +- `backend/tests/Feature/GitHubClientFailClosedTest.php`: one case per category, a 304/ETag regression, and an end-to-end case proving a transient failure ends the run as `failed` instead of hanging it. + +## Not changed + +`App\Contracts\GitHubClient`, `FixtureGitHubClient`, `IngestionService::fail()` signature, `IngestionRun.log` entry shape, and every pre-existing test expectation, including the literal `git_hub_rate_limit_exception` asserted in `IngestionPipelineTest`. No retry or backoff: that stays in Priority 8, and failing closed makes the operator-facing retry action there more necessary, not less. + +## Validation + +- Independent adversarial audit: APPROVED. The auditor rebuilt all 16 scenarios with its own `Http::fake` harness, ran `IngestionService::run()` end to end, and read `status` and `log` from the database. No error scenario finished `success`, none finished `success` with zero documents, and no two distinct categories shared a code. +- The auditor first verified that the container image matched the branch, by comparing `sha1sum` of all 73 `.php` files under `app/` and `tests/` inside and outside the container. The `backend` service has no bind mount, so gates run against the built image. +- 502, absent from the executor's own table, is covered: the implementation matches `>= 500` rather than a status list, confirmed empirically. +- Token leak check with a fictitious token across `IngestionRun.log`, `storage/logs`, exception messages, and the branch diff: no occurrence of the token, `Bearer`, or `Authorization`. +- Test count 33 to 50, no test removed, no `skip`, no weakened assertion. +- Gates on the four implementation commits: Pest 50 passed, Pint 122 files. + +## Gate trap found while closing + +The `backend` service has no bind mount, so `docker compose exec backend ./vendor/bin/pest` runs the code baked into the image, not the working tree. After the dead-helper cleanup commit `2f4b706` the image still contained the removed symbol while the disk did not, which means a gate run right after an edit can report success for code that was never executed. Rebuild with `docker compose up -d --build backend` before trusting a gate, and confirm the image matches the branch — the audit of this item did exactly that with `sha1sum` across `app/` and `tests/`. + +The final gates were re-run against a rebuilt image covering all six commits: Pest 50 passed (233 assertions), Pint 122 files. Assertion counts drift between runs (249, 239, 233 observed); the stable figure is the test count. + +## Open + +- Priority 8 retry controls now matter more: a momentary GitHub outage fails the whole run by design. diff --git a/docs/DECISIONS.md b/docs/DECISIONS.md index ca36056..3645ab7 100644 --- a/docs/DECISIONS.md +++ b/docs/DECISIONS.md @@ -301,3 +301,26 @@ Declarar em `frontend/package.json` os overrides `"pacote": "20.0.1"` e `"tar": ### Consequências `npm audit --audit-level=critical` volta a sair com código 0 sem afrouxar o gate, sem `npm audit fix --force` e sem upgrade de major. O custo é um ponto de manutenção: no upgrade da toolchain Angular os dois overrides devem ser reavaliados e removidos assim que a cadeia oficial trouxer `tar` 7.x. Vulnerabilidades `high` e `moderate` remanescentes seguem a política do ADR-24: registradas como risco, nunca declaradas corrigidas. + +## ADR-27 — Client GitHub fail-closed com taxonomia de códigos de falha + +### Contexto +`HttpGitHubClient::request()` mapeava apenas 403 para rate limit e 404 para repositório inexistente. Todo o restante — 401, 409, 422, 429, 5xx, falha de conexão — retornava uma `Response` normal, e `markdownFiles()` a consumia com `json('tree', [])`. O efeito era fail-open: corpo não-JSON, corpo sem `tree` ou erro de servidor viravam lista vazia, e a ingestão terminava `success` sem nenhum documento, indistinguível de um repositório legitimamente sem documentação. `base64_decode(...) ?: ''` transformava conteúdo inválido em documento vazio persistido em silêncio, e uma árvore com `truncated: true` era tratada como completa. Havia ainda uma segunda perna, não descrita na F-006: `IngestionService::run()` capturava apenas as duas exceções existentes, de modo que qualquer outra escapava do método e deixava o `IngestionRun` preso em `running`, sem `finished_at` e sem log. + +### Decisão +Introduzir a hierarquia `App\Exceptions\GitHubClientException`, abstrata, com o método `failureCode()` devolvendo um código estável e seguro para log. O client passa a falhar explicitamente em toda resposta que não seja sucesso e em todo payload que não satisfaça o formato esperado, e `IngestionService::run()` captura a hierarquia inteira, encerrando o run por `fail()`. As categorias e seus códigos: + +- rate limit → `git_hub_rate_limit_exception` (403 com sinal de rate limit, e 429) +- autenticação ou permissão → `github_authentication_failed` (401, e 403 sem sinal de rate limit) +- repositório ou ref inexistente → `git_hub_repository_not_found_exception` (404) +- conflito ou validação → `github_validation_failed` (409, 422) +- erro transitório → `github_transient_error` (qualquer status `>= 500`, falha de conexão, timeout) +- payload malformado → `github_malformed_response` (corpo não-JSON, corpo sem `tree`, base64 inválido, encoding não suportado) +- árvore truncada → `github_tree_truncated` + +Os dois códigos herdados mantêm o formato derivado do nome da classe porque há asserção literal em teste existente, e alterar expectativa de teste para acomodar estética seria degradar evidência. Códigos novos usam literais explícitos. A distinção entre 403 de rate limit e 403 de permissão é feita pelo cabeçalho de rate limit, não pelo status isolado. + +Decisões de escopo tomadas junto: árvore truncada encerra como `failed`, e não como `partial`, porque um inventário incompleto ingerido parcialmente acionaria a reconciliação do ADR-10 e apagaria comandos que apenas não vieram na resposta; a falha de um único arquivo também derruba o run inteiro, preservando o comportamento de abortar que já existia. Retry e backoff ficam fora, na Prioridade 8. + +### Consequências +Nenhuma resposta de erro do GitHub pode mais terminar como sucesso vazio, e todo run que falha carrega um código estável em `IngestionRun.log`, na chave `code` que já existia. A assinatura de `IngestionService::fail()` e a estrutura das entradas de `log` não mudaram, então consumidores a jusante — telemetria e correlação de eventos — leem a taxonomia sem migração. O teste de transitório usa `>= 500` em vez de lista de status, de modo que qualquer 5xx futuro é coberto sem alteração. O caminho 304/ETag do ADR-11 retorna antes de qualquer verificação de erro e segue inalterado: o run permanece `success` e registra `document_not_modified`. O trade-off é rigidez deliberada — uma indisponibilidade momentânea do GitHub agora reprova o run inteiro em vez de ingerir o que deu, e é exatamente por isso que a ação de retry da Prioridade 8 se torna mais necessária. Mensagens de exceção carregam apenas repositório, caminho e status; nunca o token, o cabeçalho `Authorization` ou o corpo da resposta.