Fix hosted review policy context

internal/serve/context.go

@@ -3,6 +3,8 @@ package serve
 import (
 	"github.com/marcus/td/internal/config"
 	"github.com/marcus/td/internal/db"
+	"github.com/marcus/td/internal/features"
+	"github.com/marcus/td/internal/reviewpolicy"
 	"github.com/marcus/td/internal/workdir"
 )
 
@@ -28,6 +30,18 @@ type HandlerConfig struct {
 	NotifyChange func()
 }
 
+// reviewPolicyModeFor resolves review policy consistently for local td serve
+// and remote td-sync handlers. The resolver accepts an empty BaseDir: it still
+// honors process-wide environment overrides and otherwise returns td's trusted
+// default. Skipping it for remote contexts silently selected strict mode and
+// made session-dependent actions such as Approve disappear from td-watch.
+func reviewPolicyModeFor(ctx HandlerContext) reviewpolicy.Mode {
+	if mode, err := features.ResolveReviewPolicyMode(ctx.BaseDir); err == nil {
+		return mode
+	}
+	return reviewpolicy.ModeStrict
+}
+
 // HandlerContext is the request-scoped dependency bundle that pure handler
 // functions receive in place of a `*Server` receiver. It lets td-sync mount the
 // same handlers against per-project DBs without standing up a `*serve.Server`.

internal/serve/handlers_reviews.go

@@ -11,7 +11,6 @@ import (
 	"unicode/utf8"
 
 	"github.com/marcus/td/internal/db"
-	"github.com/marcus/td/internal/features"
 	"github.com/marcus/td/internal/models"
 	"github.com/marcus/td/internal/reviewpolicy"
 )
@@ -153,12 +152,7 @@ func HandleRecordReview(ctx HandlerContext, w http.ResponseWriter, r *http.Reque
 		return
 	}
 
-	mode := reviewpolicy.ModeStrict
-	if ctx.BaseDir != "" {
-		if m, err := features.ResolveReviewPolicyMode(ctx.BaseDir); err == nil {
-			mode = m
-		}
-	}
+	mode := reviewPolicyModeFor(ctx)
 	// Delegated and trusted both support recording an approval without
 	// closing. Trusted is delegated plus the self-review escape hatch, so the
 	// close-using-recorded-approval path accepts these rows in both modes (see

internal/serve/handlers_reviews_test.go

@@ -1076,6 +1076,42 @@ func TestIntegration_CanApprove_AttributionUnlocks(t *testing.T) {
 	}
 }
 
+// Hosted td-sync handlers have no local BaseDir from which to read feature
+// flags. They must still use td's trusted default so an involved browser
+// session can reach Approve and provide the required review attestation.
+func TestAvailableTransitions_RemoteContextUsesTrustedDefault(t *testing.T) {
+	_, database, cleanup := setupIntegrationServer(t)
+	defer cleanup()
+
+	sessionID := "twu_mobile"
+	issueID := seedInReviewIssue(t, database, sessionID)
+	issue, err := database.GetIssue(issueID)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	actions := availableTransitionsFor(HandlerContext{
+		DB:        database,
+		SessionID: sessionID,
+		BaseDir:   "",
+	}, issue)
+
+	for _, action := range actions {
+		if action == "approve" {
+			return
+		}
+	}
+	t.Fatalf("remote trusted context should offer approve with attestation; transitions=%v", actions)
+}
+
+func TestReviewPolicyModeFor_RemoteEnvStrictOverridesDefault(t *testing.T) {
+	t.Setenv("TD_FEATURE_REVIEW_POLICY_MODE", "strict")
+	ctx := HandlerContext{BaseDir: ""}
+	if got := reviewPolicyModeFor(ctx); got != reviewpolicy.ModeStrict {
+		t.Fatalf("mode=%q want strict", got)
+	}
+}
+
 // TestIntegration_Approve_AuditParityWithCLI pins the audit behavior decided
 // for this epic, on the API side. The rule is about WHO RECORDED the row, not
 // which flag was used: an approval written by an implementation-involved

internal/serve/handlers_transitions.go

@@ -12,7 +12,6 @@ import (
 	"unicode/utf8"
 
 	"github.com/marcus/td/internal/db"
-	"github.com/marcus/td/internal/features"
 	"github.com/marcus/td/internal/models"
 	"github.com/marcus/td/internal/reviewpolicy"
 	"github.com/marcus/td/internal/workflow"
@@ -115,12 +114,7 @@ func serveReviewerDecision(ctx HandlerContext, issue *models.Issue, selfReviewAc
 // --reviewed-by attestation. Kept as a separate entry point so the many
 // existing callers that never attribute stay unchanged.
 func serveReviewerDecisionAttributed(ctx HandlerContext, issue *models.Issue, selfReviewAcknowledged bool, attributedTo string) reviewpolicy.ReviewerEligibility {
-	mode := reviewpolicy.ModeStrict
-	if ctx.BaseDir != "" {
-		if m, err := features.ResolveReviewPolicyMode(ctx.BaseDir); err == nil {
-			mode = m
-		}
-	}
+	mode := reviewPolicyModeFor(ctx)
 
 	isCreator := issue != nil && issue.CreatorSession != "" && issue.CreatorSession == ctx.SessionID
 	isImplementer := issue != nil && issue.ImplementerSession != "" && issue.ImplementerSession == ctx.SessionID
@@ -168,12 +162,7 @@ func serveCloseDecision(ctx HandlerContext, issue *models.Issue, selfReviewAckno
 // path, close eligibility re-runs the trusted reviewer predicate, so dropping
 // the attribution here rejects an approval the reviewer check just allowed.
 func serveCloseDecisionAttributed(ctx HandlerContext, issue *models.Issue, selfReviewAcknowledged bool, attributedTo string) reviewpolicy.CloseEligibility {
-	mode := reviewpolicy.ModeStrict
-	if ctx.BaseDir != "" {
-		if m, err := features.ResolveReviewPolicyMode(ctx.BaseDir); err == nil {
-			mode = m
-		}
-	}
+	mode := reviewPolicyModeFor(ctx)
 
 	isCreator := issue != nil && issue.CreatorSession != "" && issue.CreatorSession == ctx.SessionID
 	isImplementer := issue != nil && issue.ImplementerSession != "" && issue.ImplementerSession == ctx.SessionID
@@ -243,12 +232,7 @@ func canApprove(ctx HandlerContext, issue *models.Issue) bool {
 	// Mode-C (delegated): an active approval lets an eligible closer finish the
 	// close even when they are not the reviewer of record. Mirrors
 	// handledCloseAfterReview.
-	mode := reviewpolicy.ModeStrict
-	if ctx.BaseDir != "" {
-		if m, err := features.ResolveReviewPolicyMode(ctx.BaseDir); err == nil {
-			mode = m
-		}
-	}
+	mode := reviewPolicyModeFor(ctx)
 	if mode == reviewpolicy.ModeDelegated || mode == reviewpolicy.ModeTrusted {
 		if active, err := ctx.DB.GetActiveApprovalReview(issue.ID); err == nil && active != nil {
 			if serveCloseDecision(ctx, issue, false).Allowed {
@@ -810,12 +794,7 @@ func handledCloseAfterReview(ctx HandlerContext, w http.ResponseWriter, r *http.
 	if issue.Status != models.StatusInReview {
 		return false
 	}
-	mode := reviewpolicy.ModeStrict
-	if ctx.BaseDir != "" {
-		if m, err := features.ResolveReviewPolicyMode(ctx.BaseDir); err == nil {
-			mode = m
-		}
-	}
+	mode := reviewPolicyModeFor(ctx)
 	// Delegated and trusted both close on a recorded approval. Trusted is
 	// delegated plus the self-review escape hatch (reviewpolicy.evaluateCloseTrusted
 	// Case 1 is identical to delegated's), so withholding this path left the