Skip to content

Commit 813fcfc

Browse files
committed
test(server): pin the JSON-path placement and assert "not charged" as data (BUG-2793)
Codex round 3, on the tests. Two ways they could pass without proving what their names say. 1. The JSON refusal test sends VALID json, so a gate placed after the decode would still return 403 and it would stay green. The bundle test covers the gzip half of the placement claim; nothing covered the JSON half, and the code comment claims the gate sits above EITHER body read. Added an at-limit case with an undecodable body: reaching 403 rather than a decode error is only possible if nothing read the body first. 2. The no-resolved-user test asserted only a 201. A regression that quietly attributed the import to the at-limit fixture user would also return 201 and pass. It now asserts the fact instead of inferring it — the user's workspace count is unchanged across the request, and the created workspace has no owner. The mutation matrix now separates the two placements, which is the point of having both tests: - gate below the Content-Type dispatch (still above the decode) -> only the BUNDLE test fails. - gate below the JSON decode -> the bundle test AND the new JSON test fail. Neither mutation is caught by the original refusal test, which is what "passes for the wrong reason" looked like here. Gates: `go test ./...` under Postgres 17 EXIT=0; gofmt clean; `make lint` 0 issues. BUG-2793 Claude-Session: https://claude.ai/code/session_011T365kP1N9V88y15HxL4YN
1 parent 6c40d1d commit 813fcfc

1 file changed

Lines changed: 49 additions & 1 deletion

File tree

internal/server/handlers_workspace_import_plan_limit_test.go

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,30 @@ func TestImportWorkspace_EnforcesThePlanLimitOnTheBundlePathToo(t *testing.T) {
121121
}
122122
}
123123

124+
// TestImportWorkspace_EnforcesThePlanLimitBeforeReadingTheJSONBody is the
125+
// JSON-path half of the placement claim, and the refusal test above cannot
126+
// make it: that one sends VALID JSON, so a gate placed after the decode would
127+
// still return 403 and it would stay green (codex round 3).
128+
//
129+
// The code comment claims the gate sits above EITHER body read. The bundle
130+
// test proves it for gzip. This proves it for JSON, by sending a body that
131+
// cannot be decoded: reaching 403 rather than a decode error is only possible
132+
// if nothing read the body first.
133+
func TestImportWorkspace_EnforcesThePlanLimitBeforeReadingTheJSONBody(t *testing.T) {
134+
atLimit := store.DefaultFreeLimits.Workspaces
135+
srv, user := importLimitFixture(t, atLimit)
136+
137+
rr := importRequest(t, srv, user, "application/json", []byte("{this is not json"))
138+
139+
if rr.Code != http.StatusForbidden {
140+
t.Fatalf("at-limit import with an undecodable body returned %d, want 403 — the gate is "+
141+
"running after the JSON decode: %s", rr.Code, rr.Body.String())
142+
}
143+
if b := rr.Body.String(); !strings.Contains(b, "plan_limit_exceeded") {
144+
t.Errorf("refused for the wrong reason — want the plan-limit code, got: %s", b)
145+
}
146+
}
147+
124148
// TestImportWorkspace_UnderTheLimitIsNotRefused is the control. Without it, a
125149
// gate that refused every import — or one wired to the wrong feature key —
126150
// passes both tests above while breaking the feature outright.
@@ -163,12 +187,36 @@ func TestImportWorkspace_SelfHostedIsUnaffected(t *testing.T) {
163187
// here — only that the guard does what create's does when no user resolves.
164188
// Whether these doors should mint unowned workspaces at all is BUG-2809.
165189
func TestImportWorkspace_NoResolvedUserIsNotCharged(t *testing.T) {
166-
srv, _ := importLimitFixture(t, store.DefaultFreeLimits.Workspaces)
190+
srv, user := importLimitFixture(t, store.DefaultFreeLimits.Workspaces)
191+
192+
before, err := srv.store.CheckUserLimit(user.ID, "workspaces")
193+
if err != nil {
194+
t.Fatalf("read the user's limit: %v", err)
195+
}
167196

168197
rr := importRequest(t, srv, nil, "application/json", exportBody(t))
169198

170199
if rr.Code != http.StatusCreated {
171200
t.Fatalf("import with no resolved user returned %d, want 201 — there is nobody to charge: %s",
172201
rr.Code, rr.Body.String())
173202
}
203+
204+
// "Not charged" asserted as a FACT about the data, not inferred from a
205+
// status code (codex round 3). A regression that quietly attributed the
206+
// import to the at-limit fixture user would return 201 too, and pass on
207+
// the check above alone.
208+
after, err := srv.store.CheckUserLimit(user.ID, "workspaces")
209+
if err != nil {
210+
t.Fatalf("re-check the user's limit: %v", err)
211+
}
212+
if after.Current != before.Current {
213+
t.Errorf("the fixture user's workspace count moved %d -> %d; an import with no resolved "+
214+
"user was attributed to them", before.Current, after.Current)
215+
}
216+
217+
var created models.Workspace
218+
parseJSON(t, rr, &created)
219+
if created.OwnerID != "" {
220+
t.Errorf("workspace created with owner %q by a caller with no resolved user", created.OwnerID)
221+
}
174222
}

0 commit comments

Comments
 (0)