Say which way a session echo missed, not always that Basecamp predates sessions

A server that echoed this session's id but a different label, or none,
was still told it predates agent session attribution, which its own echo
contradicts. Only a missing id says that now; an id for another session
and a label that came back wrong or not at all each say so.

Author: jeremy

internal/auth/agent_launch.go

@@ -135,14 +135,26 @@ func (l *agentLaunch) requireEcho(status int, body []byte) error {
 		return nil
 	}
 
+	// Say which way the echo missed. Only a server that echoed no id at
+	// all is one that predates sessions; one that echoed this id records
+	// them, and what it got wrong is the label.
 	msg := fmt.Sprintf("Basecamp minted a token but did not bind it to session %s, so it was discarded", l.id)
-	if echoed.ID == nil {
+	hint := "A session token needs a Basecamp that records agent sessions; without one, leave out --session-id and --session-label"
+	switch {
+	case echoed.ID == nil:
 		msg += ": this Basecamp predates agent session attribution"
-	} else {
-		msg += ": this Basecamp predates agent session attribution, or answered for a different session"
+	case !idMatches:
+		msg += ": Basecamp answered for a different session"
+		hint = "Run it again; if it keeps answering for another session, report it"
+	case echoed.Label == nil:
+		msg += ": Basecamp recorded the session but did not record the label"
+		hint = "Leave out --session-label to mint for the session alone"
+	default:
+		msg += ": Basecamp recorded a different label for the session"
+		hint = "Leave out --session-label to mint for the session alone"
 	}
 	e := output.ErrAPI(status, msg)
-	e.Hint = "A session token needs a Basecamp that records agent sessions; without one, leave out --session-id and --session-label"
+	e.Hint = hint
 	e.Cause = errLaunchNotRecorded
 	return e
 }

internal/auth/agent_launch_test.go

@@ -90,14 +90,25 @@ func TestSessionTokenWithoutLabelSendsNoLabel(t *testing.T) {
 
 // TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed: a Basecamp
 // that predates session attribution answers with an ordinary token. That
-// token is not bound to the session, so it is discarded, not printed.
+// token is not bound to the session, so it is discarded, not printed. The
+// message says which way the echo missed: a server that echoed the id
+// does record sessions, so it is not told it predates them.
 func TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed(t *testing.T) {
-	for name, body := range map[string]string{
-		"no echo":         `{"access_token":"unbound","token_type":"Bearer","expires_in":3600}`,
-		"other id":        `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"ffffffffffffffffffffffffffffffff","launch_label":"coworker@box"}`,
-		"other label":     `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `","launch_label":"someone-else"}`,
-		"label not given": `{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}`,
+	for name, tc := range map[string]struct{ body, says, never string }{
+		"no echo": {
+			`{"access_token":"unbound","token_type":"Bearer","expires_in":3600}`,
+			"predates agent session attribution", "different"},
+		"other id": {
+			`{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"ffffffffffffffffffffffffffffffff","launch_label":"coworker@box"}`,
+			"answered for a different session", "predates"},
+		"other label": {
+			`{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `","launch_label":"someone-else"}`,
+			"recorded a different label", "predates"},
+		"label not given": {
+			`{"access_token":"unbound","token_type":"Bearer","expires_in":3600,"launch_id":"` + testSessionID + `"}`,
+			"did not record the label", "predates"},
 	} {
+		body := tc.body
 		t.Run(name, func(t *testing.T) {
 			as := startDeviceAS(t)
 			as.token = func(int) (int, string) { return http.StatusOK, body }
@@ -109,7 +120,8 @@ func TestSessionTokenFromAServerThatIgnoresTheSessionFailsClosed(t *testing.T) {
 			assert.Empty(t, token)
 			assert.NotContains(t, err.Error(), "unbound", "the unbound token must not leak into the error")
 			assert.Contains(t, err.Error(), testSessionID)
-			assert.Contains(t, err.Error(), "predates agent session attribution")
+			assert.Contains(t, err.Error(), tc.says)
+			assert.NotContains(t, err.Error(), tc.never)
 
 			stored, loadErr := m.store.Load(key)
 			require.NoError(t, loadErr)