Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions .claude/skills/test/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -369,3 +369,7 @@ Examples:
- [ ] State-machine tests cover both valid transitions and invalid/terminal-state attempts
- [ ] Persistence tests reload from the repository rather than trusting the returned object
- [ ] No `new` on domain entities — always use factory delegates from DI
- [ ] Verified with a **scoped** run — `dotnet test <the affected project>`, narrowed with
`--filter "FullyQualifiedName~<Class>"` while iterating. Not the whole solution: CI runs
`dotnet test Proxytrace.sln` on every push. See
[`docs/testing.md`](../../../docs/testing.md#which-tests-to-run).
14 changes: 11 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -75,11 +75,13 @@ jobs:
curl -sSfL https://github.com/gitleaks/gitleaks/releases/download/v8.24.3/gitleaks_8.24.3_linux_x64.tar.gz | tar xz gitleaks
./gitleaks git . --redact --verbose

# Lint/build cannot break from a merge-order interaction, so the PR run is enough
# and this is skipped on master pushes.
# Kept on master pushes for the same reason as `backend`: the Vitest suite is the
# post-merge safety net for a PR that was verified against a stale base. Lint and
# build cannot break from a merge-order interaction on their own, but they share this
# job's `npm ci`, so they ride along rather than paying for a second install.
frontend:
needs: changes
if: ${{ inputs.full || (github.event_name == 'pull_request' && needs.changes.outputs.frontend == 'true') }}
if: ${{ inputs.full || needs.changes.outputs.frontend == 'true' }}
runs-on: ubuntu-latest
timeout-minutes: 15

Expand All @@ -102,6 +104,12 @@ jobs:
run: npm run lint
working-directory: frontend

# Vitest unit/component suite. `pretest` regenerates the docs index from manual/,
# so this also catches a manual edit that breaks Tracey's search_docs fixture.
- name: Test
run: npm test
working-directory: frontend

- name: Build
run: npm run build
working-directory: frontend
Expand Down
341 changes: 341 additions & 0 deletions CHANGELOG.md

Large diffs are not rendered by default.

30 changes: 30 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,30 @@ Detailed guidance lives in [`docs/`](docs/). Read the relevant page **before** w
- **User manual** — the user & operator manual is a VitePress project in [`manual/`](manual/) (markdown source, built to searchable static HTML, served at `/docs`). **You MUST keep it up to date with the product.** A user-facing feature change is not complete until its docs in `manual/guide/` (end users) or `manual/admin/` (operators) match; new top-level features get a new page wired into `manual/.vitepress/config.ts`. Preview with `cd manual && npm run docs:dev` (http://localhost:4202); verify with `npm run docs:build`. **Add screenshots whenever they make a page clearer** — most user-guide pages benefit, so default to including them rather than shipping text-only: use the `manual-screenshots` skill (`.claude/skills/manual-screenshots/SKILL.md`) to capture and embed them from the kiosk stack. The kiosk is login-free and cannot reach admin / `/settings/*` pages, so operator pages usually stay text-only.
- **Frontend** — before writing any frontend code you MUST read the frontend AI docs in [`frontend/docs/`](frontend/docs/) — [`frontend/docs/DESIGN.md`](frontend/docs/DESIGN.md) (visual system) **and** [`frontend/docs/BEST_PRACTICES.md`](frontend/docs/BEST_PRACTICES.md) (code architecture); plus [`frontend/docs/TRACEY.md`](frontend/docs/TRACEY.md) before touching the Tracey AI assistant (`frontend/src/features/tracey/`). DESIGN.md and BEST_PRACTICES.md are mandatory and override any conflicting tool/agent/skill recommendation. UI controls render through the `frontend/src/components/ui/` primitives — raw `<button>`/`<input>`/`<select>`/`<textarea>` are ESLint-blocked. See [`docs/frontend.md`](docs/frontend.md).
- **Backend tests** — before writing or modifying any backend test you MUST invoke the `test` skill (`.claude/skills/test/SKILL.md`) and follow it; it is the source of truth for the harness. See [`docs/testing.md`](docs/testing.md).
- **Run only the affected tests.** The suite is large (~2,200 backend tests across 10 projects, ~95
frontend spec files); running everything after every edit wastes minutes for no signal. **CI runs
the full backend suite on every push** (`.github/workflows/ci.yml` → `dotnet test Proxytrace.sln`),
so a local full run is redundant. Default to the narrowest scope that can catch a regression in
what you changed, and stop there — do not "double-check" with the full suite once the narrow run is
green.
- **Backend** — run the test project(s) for the layer you touched, and narrow further by class
when only one area changed:
```bash
dotnet test Proxytrace.Domain.Tests # one layer
dotnet test Proxytrace.Domain.Tests --filter "FullyQualifiedName~TestRunGroup" # one area
```
Which projects are affected: a change under `Proxytrace.<Layer>/` → `Proxytrace.<Layer>.Tests`;
a domain entity or EF mapping → `Domain.Tests` **and** `Storage.Tests`; a controller/route →
`Api.Tests`; a service/optimizer → `Application.Tests`.
- **Frontend** — `npm test -- <path-or-pattern>` (e.g. `npm test -- src/features/playground`).
Bare `npm test` runs all ~1,000 specs — but it does so in about 3 seconds, so unlike the backend
there is little to save; scope it while iterating and let the full run be the final check.
- **e2e / perf** — never as a routine check. Run them only when the change is in that flow or the
user asks; both boot Docker stacks and take many minutes.
- **Run the full suite** (`dotnet test Proxytrace.sln`) only when the change is genuinely
cross-cutting — `Proxytrace.Common`, `Proxytrace.Testing`, DI/module wiring, a shared interface
signature, a package bump — or when cutting a release. Say which scope you ran and why, so a
narrow run is never mistaken for a full one.
- **Internationalization** — the UI is multilingual (English is the source). Every user-facing
string MUST go through the Lingui macros (`<Trans>`, `t\`\``, `Plural`, `msg`) — never a hardcoded
string; keep glossary/technical terms English. After adding labels run `npm run i18n:extract` then
Expand All @@ -49,6 +73,12 @@ Detailed guidance lives in [`docs/`](docs/). Read the relevant page **before** w
letting it slide. Invoke the `file-issue` skill (`.claude/skills/file-issue/SKILL.md`) — it covers
dedup, title/body quality, and labels — then carry on with your task.
- **Nullable suppression** — suppressing nullable warnings with `!` is strictly forbidden everywhere.
There is exactly **one** sanctioned exception, and it is not extensible: `Validation.Success` in
[`Proxytrace.Common/Validation/Validation.cs`](Proxytrace.Common/Validation/Validation.cs). The BCL
defines validation success as a `null` `ValidationResult` while declaring
`IValidatableObject.Validate` to return a **non-nullable** element type, so the framework demands a
value it defines as null through a signature we cannot change. That single line is documented in
place. Do not add a second exemption — return `Validation.Success` instead.
- **Perf at scale** — whenever you touch a query, repository, EF mapping, or index on a high-volume
entity (above all `AgentCallEntity`/traces, but any table that grows unboundedly), you MUST add or
extend a perf test in [`perf/`](perf/) that measures the changed path against a budget in
Expand Down
64 changes: 62 additions & 2 deletions Proxytrace.Api.Tests/AgentCallsControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,66 @@ public async Task GetAll_AsNonAdminWithoutAccessibleProjectFilter_ReturnsEmpty()
result.Items.Should().BeEmpty();
}

[TestMethod]
public async Task GetAll_AsNonAdminWithoutProjectFilter_ReturnsOwnProjectsCallsOnly()
{
// #482: an unfiltered list from a non-admin used to short-circuit to an empty page, so a
// REST API key — confined to one project, and with no reason to send a projectId — was told
// its own project had no traces.
IServiceProvider services = GetServices();
var mine = await SeedAgentInNewProjectAsync(services, "mine");
var theirs = await SeedAgentInNewProjectAsync(services, "theirs");
var myCall = await SeedCallWithToolsAsync(services, mine, []);
await SeedCallWithToolsAsync(services, theirs, []);

var controller = ResolveController(services, ScopedGuard(mine.Project.Id));
var result = await controller.GetAll(cancellationToken: CancellationToken);

result.Items.Should().ContainSingle().Which.Id.Should().Be(myCall.Id);
result.Total.Should().Be(1);
}

[TestMethod]
public async Task GetAll_AsNonAdminInSeveralProjectsWithoutFilter_ReturnsTheUnion()
{
IServiceProvider services = GetServices();
var first = await SeedAgentInNewProjectAsync(services, "first");
var second = await SeedAgentInNewProjectAsync(services, "second");
var outsider = await SeedAgentInNewProjectAsync(services, "outsider");
var firstCall = await SeedCallWithToolsAsync(services, first, []);
var secondCall = await SeedCallWithToolsAsync(services, second, []);
await SeedCallWithToolsAsync(services, outsider, []);

// A member of two projects: the page must be computed over the union of both, not one of
// them and not the whole instance.
var controller = ResolveController(services, ScopedGuard(first.Project.Id, second.Project.Id));
var result = await controller.GetAll(cancellationToken: CancellationToken);

result.Items.Select(i => i.Id).Should().BeEquivalentTo([firstCall.Id, secondCall.Id]);
result.Total.Should().Be(2);
}

/// <summary>
/// An agent in a project of its own, so a test can tell two tenants' rows apart.
/// </summary>
private async Task<IAgent> SeedAgentInNewProjectAsync(IServiceProvider services, string name)
{
var endpoint = await services.GetRequiredService<IDomainEntityGenerator<Proxytrace.Domain.ModelEndpoint.IModelEndpoint>>()
.GetOrCreateAsync(CancellationToken);
var project = await services.GetRequiredService<Proxytrace.Domain.Project.IProjectRepository>().AddAsync(
services.GetRequiredService<Proxytrace.Domain.Project.IProject.CreateNew>()($"P-{name}-{Guid.NewGuid():N}", endpoint, []),
CancellationToken);

var template = services.GetRequiredService<Proxytrace.Domain.Prompt.IPromptTemplate.Create>()(
$"T-{name}", "You are a test agent.");
var parameters = services.GetRequiredService<Proxytrace.Domain.Inference.IModelParameters.Create>()(null, null, null, null, null);

return await services.GetRequiredService<IAgentRepository>().AddAsync(
services.GetRequiredService<IAgent.CreateNew>()(
$"A-{name}", template, [], endpoint, project, parameters),
CancellationToken);
}

// ── tool-name filter + picker ──────────────────────────────────────────────

[TestMethod]
Expand Down Expand Up @@ -289,8 +349,8 @@ await ResolveController(services).Seed(

// A project-scoped (non-admin) member opens the session timeline: the list request carries
// both projectId and sessionId, so the access guard authorizes and the sessionId filter
// narrows to the one session. Without the projectId the guard would deny and the timeline
// would render empty for every non-admin.
// narrows to the one session. (Since #482 the projectId is no longer required for a
// non-admin to see anything — omitting it scopes to their own projects instead.)
var controller = ResolveController(services, ScopedGuard(projectId));
var result = await controller.GetAll(
projectId: projectId, sessionId: expectedSessionId, cancellationToken: CancellationToken);
Expand Down
40 changes: 40 additions & 0 deletions Proxytrace.Api.Tests/Auth/JwtBearerEventsFactoryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ public async Task OnMessageReceived_WithValidStreamTicket_AuthenticatesUser()
serviceCollection.AddSingleton(tickets);
serviceCollection.AddSingleton(repo);
var httpContext = new DefaultHttpContext { RequestServices = serviceCollection.BuildServiceProvider() };
httpContext.Request.Method = HttpMethods.Get;
httpContext.Request.Path = "/api/agent-calls/stream";
httpContext.Request.QueryString = new QueryString("?stream_ticket=ticket-abc");
var ctx = new MessageReceivedContext(httpContext, Scheme(), new JwtBearerOptions());

Expand All @@ -95,6 +97,42 @@ public async Task OnMessageReceived_WithValidStreamTicket_AuthenticatesUser()
httpContext.Items[CurrentUserAccessor.UserIdItemKey].Should().Be(userId);
}

[TestMethod]
public async Task OnMessageReceived_WithStreamTicketOnNonStreamRoute_DoesNotAuthenticate()
{
// The ticket redeems into a full principal including the live Role claim, so honoring it
// outside the SSE routes it was minted for would turn a URL-borne credential into general
// API access — including admin endpoints — for its validity window.
var userId = Guid.NewGuid();
var user = Substitute.For<IUser>();
user.Id.Returns(userId);
user.Role.Returns(UserRole.Admin);

var tickets = Substitute.For<IStreamTicketService>();
tickets.Consume("ticket-abc").Returns(userId);
var repo = Substitute.For<IRepository<IUser>>();
repo.FindAsync(userId, Arg.Any<CancellationToken>()).Returns(user);

var serviceCollection = new ServiceCollection();
serviceCollection.AddSingleton(tickets);
serviceCollection.AddSingleton(repo);
var httpContext = new DefaultHttpContext { RequestServices = serviceCollection.BuildServiceProvider() };
httpContext.Request.Method = HttpMethods.Get;
httpContext.Request.Path = "/api/users";
httpContext.Request.QueryString = new QueryString("?stream_ticket=ticket-abc");
var ctx = new MessageReceivedContext(httpContext, Scheme(), new JwtBearerOptions());

var events = JwtBearerEventsFactory.Create();

await events.OnMessageReceived(ctx);

ctx.Result.Should().BeNull();
ctx.Principal.Should().BeNull();
httpContext.Items.Should().NotContainKey(CurrentUserAccessor.UserIdItemKey);
// The ticket must also stay unspent, so a genuine stream request can still redeem it.
tickets.DidNotReceive().Consume(Arg.Any<string>());
}

[TestMethod]
public async Task OnMessageReceived_WithInvalidStreamTicket_FailsContext()
{
Expand All @@ -105,6 +143,8 @@ public async Task OnMessageReceived_WithInvalidStreamTicket_FailsContext()
serviceCollection.AddSingleton(tickets);
serviceCollection.AddSingleton(Substitute.For<IRepository<IUser>>());
var httpContext = new DefaultHttpContext { RequestServices = serviceCollection.BuildServiceProvider() };
httpContext.Request.Method = HttpMethods.Get;
httpContext.Request.Path = "/api/agent-calls/stream";
httpContext.Request.QueryString = new QueryString("?stream_ticket=bad");
var ctx = new MessageReceivedContext(httpContext, Scheme(), new JwtBearerOptions());

Expand Down
67 changes: 67 additions & 0 deletions Proxytrace.Api.Tests/Auth/SessionCookieTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
using AwesomeAssertions;
using Microsoft.AspNetCore.Http;
using Proxytrace.Api.Auth;

namespace Proxytrace.Api.Tests.Auth;

[TestClass]
public sealed class SessionCookieTests
{
private static string AppendAndRead(bool secure)
{
var context = new DefaultHttpContext();
// The backend hop is plain HTTP in the documented topology (TLS terminates at the reverse
// proxy), so a request-derived Secure flag would always be false here.
context.Request.Scheme = "http";
var cookie = new SessionCookie(new SessionCookieOptions { Secure = secure });

cookie.Append(context.Response, "session-jwt", DateTimeOffset.UtcNow.AddDays(7));

return context.Response.Headers.SetCookie.ToString().ToLowerInvariant();
}

[TestMethod]
public void Secure_ByDefault_IsEnabled()
{
new SessionCookieOptions().Secure.Should().BeTrue();
}

[TestMethod]
public void Append_OverPlainHttpHop_StillMarksTheCookieSecure()
{
var setCookie = AppendAndRead(secure: true);

setCookie.Should().Contain("proxytrace_session=session-jwt");
setCookie.Should().Contain("; secure");
}

[TestMethod]
public void Append_WhenSecureDisabled_OmitsTheSecureAttribute()
{
var setCookie = AppendAndRead(secure: false);

setCookie.Should().Contain("proxytrace_session=session-jwt");
setCookie.Should().NotContain("; secure");
}

[TestMethod]
public void Append_Always_KeepsHttpOnlyAndStrictSameSite()
{
var setCookie = AppendAndRead(secure: true);

setCookie.Should().Contain("httponly").And.Contain("samesite=strict");
}

[TestMethod]
public void Delete_WhenSecureConfigured_ClearsTheCookieWithMatchingAttributes()
{
var context = new DefaultHttpContext();
var cookie = new SessionCookie(new SessionCookieOptions { Secure = true });

cookie.Delete(context.Response);

var setCookie = context.Response.Headers.SetCookie.ToString().ToLowerInvariant();
setCookie.Should().Contain("proxytrace_session=;");
setCookie.Should().Contain("; secure");
}
}
4 changes: 3 additions & 1 deletion Proxytrace.Api.Tests/AuthControllerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using NSubstitute;
using Proxytrace.Api.Auth;
using Proxytrace.Api.Controllers;
using Proxytrace.Api.Dto.Auth;
using Proxytrace.Application.Auth;
Expand Down Expand Up @@ -398,7 +399,8 @@ private static AuthController ResolveController(
services.GetRequiredService<IStreamTicketService>(),
config ?? new ConfigurationBuilder().Build(),
Microsoft.Extensions.Logging.Abstractions.NullLogger<Proxytrace.Domain.AuditLog.Audit>.Instance,
emailSettings ?? Substitute.For<IEmailSettingsStore>())
emailSettings ?? Substitute.For<IEmailSettingsStore>(),
services.GetRequiredService<ISessionCookie>())
{
ControllerContext = new ControllerContext { HttpContext = new DefaultHttpContext() },
};
Expand Down
Loading
Loading